From eded748ab82f14cc09c84f48aba6473b2a8ac9ad Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 3 Apr 2014 00:32:56 +1100 Subject: [PATCH] Fixed the suggestedVersion calculation, now done via SQL. The archive repo was getting updated after the regular repo. In these situations, we didn't have every single app/apk in memory in order to calculate the suggested version. As a result, F-Droid ended up choosing a suggested version from the archived versions, when terhere was actually a newer version in the database. This change does all of the calculations in two database queries now. Although the implementation of the query is not hackey, they way I get to the code in order to execute the query is a bit hacky, so most of the implementation is private. --- src/org/fdroid/fdroid/UpdateService.java | 53 +------- src/org/fdroid/fdroid/data/AppProvider.java | 133 +++++++++++++++++++- 2 files changed, 133 insertions(+), 53 deletions(-) diff --git a/src/org/fdroid/fdroid/UpdateService.java b/src/org/fdroid/fdroid/UpdateService.java index 405793f48..7231b99a6 100644 --- a/src/org/fdroid/fdroid/UpdateService.java +++ b/src/org/fdroid/fdroid/UpdateService.java @@ -311,7 +311,6 @@ public class UpdateService extends IntentService implements ProgressListener { calcCompatibilityFlags(this, apksToUpdate, appsToUpdate); calcIconUrls(this, apksToUpdate, appsToUpdate, repos); - calcCurrentApk(apksToUpdate, appsToUpdate); // Need to do this BEFORE updating the apks, otherwise when it continually // calls "get apks for repo X" then it will be getting the newly created apks @@ -322,6 +321,7 @@ public class UpdateService extends IntentService implements ProgressListener { updateOrInsertApks(apksToUpdate, totalInsertsUpdates, listOfAppsToUpdate.size()); removeApksFromRepos(disabledRepos); removeAppsWithoutApks(); + AppProvider.Helper.calcSuggestedVersionsForAll(this); notifyContentProviders(); if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, false)) { @@ -374,57 +374,6 @@ public class UpdateService extends IntentService implements ProgressListener { } } - /** - * Get the current version - this will be one of the Apks from 'apks'. - * Can return null if there are no available versions. - * This should be the 'current' version, as in the most recent stable - * one, that most users would want by default. It might not be the - * most recent, if for example there are betas etc. - */ - private static void calcCurrentApk(List apks, Map apps ) { - for ( App app : apps.values() ) { - List apksForApp = new ArrayList(); - for (Apk apk : apks) { - if (apk.id.equals(app.id)) { - apksForApp.add(apk); - } - } - calcCurrentApkForApp(app, apksForApp); - } - } - - private static void calcCurrentApkForApp(App app, List apksForApp) { - Apk latestApk = null; - // Try and return the real current version first. It will find the - // closest version smaller than the upstreamVercode, being the same - // vercode if it exists. - if (app.upstreamVercode > 0) { - int latestcode = -1; - for (Apk apk : apksForApp) { - if ((!app.compatible || apk.compatible) - && apk.vercode <= app.upstreamVercode - && apk.vercode > latestcode) { - latestApk = apk; - latestcode = apk.vercode; - } - } - } else { - // If the current version was not set we return the most recent apk. - int latestCode = -1; - for (Apk apk : apksForApp) { - if ((!app.compatible || apk.compatible) - && apk.vercode > latestCode) { - latestApk = apk; - latestCode = apk.vercode; - } - } - } - - if (latestApk != null) { - app.suggestedVercode = latestApk.vercode; - } - } - private static void calcIconUrls(Context context, List apks, Map apps, List repos) { String iconsDir = Utils.getIconsDir(context); diff --git a/src/org/fdroid/fdroid/data/AppProvider.java b/src/org/fdroid/fdroid/data/AppProvider.java index 95176fcdb..a76046bea 100644 --- a/src/org/fdroid/fdroid/data/AppProvider.java +++ b/src/org/fdroid/fdroid/data/AppProvider.java @@ -120,8 +120,19 @@ public class AppProvider extends FDroidProvider { return app; } - public static void deleteAppsWithNoApks(ContentResolver resolver) { + /* + * I wasn't quite sure on the best way to execute arbitrary queries using the same DBHelper as the + * content provider class, so I've hidden the implementation of this (by making it private) in case + * I find a better way in the future. + */ + public static void calcSuggestedVersionsForAll(Context context) { + Uri fromUpstream = calcSuggestedVersionFromUpstream(); + context.getContentResolver().update(fromUpstream, null, null, null); + + Uri fromLatest = calcSuggestedVersionFromLatest(); + context.getContentResolver().update(fromLatest, null, null, null); } + } public interface DataColumns { @@ -235,6 +246,8 @@ public class AppProvider extends FDroidProvider { private static final String PATH_NEWLY_ADDED = "newlyAdded"; private static final String PATH_CATEGORY = "category"; private static final String PATH_IGNORED = "ignored"; + private static final String PATH_CALC_SUGGESTED_FROM_UPSTREAM = "calcSuggestedFromUpstream"; + private static final String PATH_CALC_SUGGESTED_FROM_LATEST = "calcSuggestedFromLatest"; private static final int CAN_UPDATE = CODE_SINGLE + 1; private static final int INSTALLED = CAN_UPDATE + 1; @@ -245,9 +258,13 @@ public class AppProvider extends FDroidProvider { private static final int NEWLY_ADDED = RECENTLY_UPDATED + 1; private static final int CATEGORY = NEWLY_ADDED + 1; private static final int IGNORED = CATEGORY + 1; + private static final int CALC_SUGGESTED_FROM_UPSTREAM = IGNORED + 1; + private static final int CALC_SUGGESTED_FROM_LATEST = CALC_SUGGESTED_FROM_UPSTREAM + 1; static { matcher.addURI(getAuthority(), null, CODE_LIST); + matcher.addURI(getAuthority(), PATH_CALC_SUGGESTED_FROM_UPSTREAM, CALC_SUGGESTED_FROM_UPSTREAM); + matcher.addURI(getAuthority(), PATH_CALC_SUGGESTED_FROM_LATEST, CALC_SUGGESTED_FROM_LATEST); matcher.addURI(getAuthority(), PATH_IGNORED, IGNORED); matcher.addURI(getAuthority(), PATH_RECENTLY_UPDATED, RECENTLY_UPDATED); matcher.addURI(getAuthority(), PATH_NEWLY_ADDED, NEWLY_ADDED); @@ -276,6 +293,14 @@ public class AppProvider extends FDroidProvider { return Uri.withAppendedPath(getContentUri(), PATH_IGNORED); } + private static Uri calcSuggestedVersionFromUpstream() { + return Uri.withAppendedPath(getContentUri(), PATH_CALC_SUGGESTED_FROM_UPSTREAM); + } + + private static Uri calcSuggestedVersionFromLatest() { + return Uri.withAppendedPath(getContentUri(), PATH_CALC_SUGGESTED_FROM_LATEST); + } + public static Uri getCategoryUri(String category) { return getContentUri().buildUpon() .appendPath(PATH_CATEGORY) @@ -550,6 +575,14 @@ public class AppProvider extends FDroidProvider { QuerySelection query = new QuerySelection(where, whereArgs); switch (matcher.match(uri)) { + case CALC_SUGGESTED_FROM_LATEST: + setSuggestedFromLatest(); + return 0; + + case CALC_SUGGESTED_FROM_UPSTREAM: + setSuggestedFromUpstream(); + return 0; + case CODE_SINGLE: query = query.add(querySingle(uri.getLastPathSegment())); break; @@ -565,4 +598,102 @@ public class AppProvider extends FDroidProvider { return count; } + /** + * Look at the upstream version of each app, our goal is to find the apk + * with the closest version code to that, without going over. + * If the app is not compatible at all (i.e. no versions were compatible) + * then we take the highest, otherwise we take the highest compatible version. + * + * Replaces the existing Java code: + * + * if (app.upstreamVercode > 0) { + * int latestcode = -1; + * for (Apk apk : apksForApp) { + * if ((!app.compatible || apk.compatible) + * && apk.vercode <= app.upstreamVercode + * && apk.vercode > latestcode) { + * latestApk = apk; + * latestcode = apk.vercode; + * } + * } + * } + * + * And it can be read a little easier like this (without the string concats): + * + * UPDATE fdroid_app + * SET suggestedVercode = ( + * SELECT MAX(fdroid_apk.vercode) + * FROM fdroid_apk + * WHERE + * fdroid_app.id = fdroid_apk.id AND + * fdroid_apk.vercode <= fdroid_app.upstreamVercode AND + * ( fdroid_app.compatible = 0 OR fdroid_apk.compatible = 1 ) + * ) + * WHERE upstreamVercode > 0 + */ + private void setSuggestedFromUpstream() { + + final String apk = DBHelper.TABLE_APK; + final String app = DBHelper.TABLE_APP; + + String updateSql = + "UPDATE " + app + + " SET suggestedVercode = ( " + + " SELECT MAX( " + apk + ".vercode ) " + + " FROM " + apk + + " WHERE " + + app + ".id = " + apk + ".id AND " + + apk + ".vercode <= " + app + ".upstreamVercode AND " + + " ( " + app + ".compatible = 0 OR " + apk + ".compatible = 1 ) ) " + + " WHERE upstreamVercode > 0 "; + + write().execSQL(updateSql); + } + + /** + * For all apps that don't specify an upstream version code, we take the + * latest apk in the repo. If the app is not compatible at all (i.e. no versions + * were compatible) then we take the highest, otherwise we take the highest + * compatible version. + * + * Replaces the existing Java code: + * + * for (Apk apk : apksForApp) { + * if ((!app.compatible || apk.compatible) + * && apk.vercode > latestCode) { + * latestApk = apk; + * latestCode = apk.vercode; + * } + * } + * + * And it can be read a little easier like this (without the string concats): + * + * UPDATE fdroid_app + * SET suggestedVercode = ( + * SELECT MAX(fdroid_apk.vercode) + * FROM fdroid_apk + * WHERE + * fdroid_app.id = fdroid_apk.id AND + * ( fdroid_app.compatible = 0 OR fdroid_apk.compatible = 1 ) + * ) + * WHERE upstreamVercode = 0 OR upstreamVercode IS NULL; + */ + private void setSuggestedFromLatest() { + + final String apk = DBHelper.TABLE_APK; + final String app = DBHelper.TABLE_APP; + + String updateSql = + "UPDATE " + app + + " SET suggestedVercode = ( " + + " SELECT MAX( " + apk + ".vercode ) " + + " FROM " + apk + + " WHERE " + + app + ".id = " + apk + ".id AND " + + " ( " + app + ".compatible = 0 OR " + apk + ".compatible = 1 ) ) " + + " WHERE upstreamVercode = 0 OR upstreamVercode IS NULL "; + + write().execSQL(updateSql); + } + }