Fixed assumption that repo updates have all apps available.

Previously, I accidentally made the repo updater presume that it
had access to all apps in a big fat list. This meant that I was iterating
over that list, performing calculations, etc, rather than actually
querying the entire database.

The solution was to bundled all update-service related processing to one
process in AppProvider (I didn't want to have to pull every single app/apk
out of the database in order to perform the update, because that will become
more and more burdensom as the repo grows).

There is a method calcDetailsFromIndex() in the AppProvider.Helper class.
It now does three things:
 * updates compatibility flag.
 * updates suggested version (outstanding issue is documented in gitlab issue #1)
 * updates iconsUrl (fixed in this commit)

Icons from old repos will just have icons in an "icons" dir
in the same folder as the index.jar. New repos have density
specific icon dirs (e.g. "icons-240") which depend on the
device F-Droid is running on.

src/org/fdroid/fdroid/UpdateService.java

@@ -43,7 +43,6 @@ public class UpdateService extends IntentService implements ProgressListener {
     public static final String RESULT_MESSAGE = "msg";
     public static final String RESULT_EVENT   = "event";
 
-
     public static final int STATUS_COMPLETE_WITH_CHANGES = 0;
     public static final int STATUS_COMPLETE_AND_SAME     = 1;
     public static final int STATUS_ERROR                 = 2;
@@ -309,11 +308,11 @@ public class UpdateService extends IntentService implements ProgressListener {
                 List<App> listOfAppsToUpdate = new ArrayList<App>();
                 listOfAppsToUpdate.addAll(appsToUpdate.values());
 
-                calcCompatibilityFlags(this, apksToUpdate, appsToUpdate);
+                calcApkCompatibilityFlags(this, apksToUpdate);
-                calcIconUrls(this, apksToUpdate, appsToUpdate, repos);
 
                 // 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
+                // calls "get existing apks for repo X" then it will be getting the newly
+                // created apks, rather than those from the fresh, juicy index we just processed.
                 removeApksNoLongerInRepo(apksToUpdate, updatedRepos);
 
                 int totalInsertsUpdates = listOfAppsToUpdate.size() + apksToUpdate.size();
@@ -321,7 +320,13 @@ public class UpdateService extends IntentService implements ProgressListener {
                 updateOrInsertApks(apksToUpdate, totalInsertsUpdates, listOfAppsToUpdate.size());
                 removeApksFromRepos(disabledRepos);
                 removeAppsWithoutApks();
-                AppProvider.Helper.calcSuggestedVersionsForAll(this);
+
+                // This will sort out the icon urls, compatibility flags. and suggested version
+                // for each app. It used to happen here in Java code, but was moved to SQL when
+                // it became apparant we don't always have enough info (depending on which repos
+                // were updated).
+                AppProvider.Helper.calcDetailsFromIndex(this);
+
                 notifyContentProviders();
 
                 if (prefs.getBoolean(Preferences.PREF_UPD_NOTIFY, false)) {
@@ -358,8 +363,13 @@ public class UpdateService extends IntentService implements ProgressListener {
         getContentResolver().notifyChange(ApkProvider.getContentUri(), null);
     }
 
-    private static void calcCompatibilityFlags(Context context, List<Apk> apks,
+    /**
-                                              Map<String, App> apps) {
+     * This cannot be offloaded to the database (as we did with the query which
+     * updates apps, depending on whether their apks are compatible or not).
+     * The reason is that we need to interact with the CompatibilityChecker
+     * in order to see if, and why an apk is not compatible.
+     */
+    private static void calcApkCompatibilityFlags(Context context, List<Apk> apks) {
         CompatibilityChecker checker = new CompatibilityChecker(context);
         for (Apk apk : apks) {
             List<String> reasons = checker.getIncompatibleReasons(apk);
@@ -369,42 +379,6 @@ public class UpdateService extends IntentService implements ProgressListener {
             } else {
                 apk.compatible = true;
                 apk.incompatible_reasons = null;
-                apps.get(apk.id).compatible = true;
-            }
-        }
-    }
-
-    private static void calcIconUrls(Context context, List<Apk> apks,
-                                     Map<String, App> apps, List<Repo> repos) {
-        String iconsDir = Utils.getIconsDir(context);
-        Log.d("FDroid", "Density-specific icons dir is " + iconsDir);
-        for (App app : apps.values()) {
-            if (app.iconUrl == null && app.icon != null) {
-                calcIconUrl(iconsDir, app, apks, repos);
-            }
-        }
-    }
-
-    private static void calcIconUrl(String iconsDir, App app,
-                                    List<Apk> allApks, List<Repo> repos) {
-        List<Apk> apksForApp = new ArrayList<Apk>();
-        for (Apk apk : allApks) {
-            if (apk.id.equals(app.id)) {
-                apksForApp.add(apk);
-            }
-        }
-
-        Collections.sort(apksForApp);
-        for (int i = apksForApp.size() - 1; i >= 0; i --) {
-            Apk apk = apksForApp.get(i);
-            for (Repo repo : repos) {
-                if (repo.getId() != apk.repo) continue;
-                if (repo.version >= Repo.VERSION_DENSITY_SPECIFIC_ICONS) {
-                    app.iconUrl = repo.address + iconsDir + app.icon;
-                } else {
-                    app.iconUrl = repo.address + "/icons/" + app.icon;
-                }
-                return;
             }
         }
     }

src/org/fdroid/fdroid/data/AppProvider.java

@@ -7,7 +7,6 @@ import android.net.Uri;
 import android.util.Log;
 import org.fdroid.fdroid.Preferences;
 import org.fdroid.fdroid.R;
-import org.fdroid.fdroid.UpdateService;
 import org.fdroid.fdroid.Utils;
 
 import java.util.*;
@@ -125,12 +124,9 @@ public class AppProvider extends FDroidProvider {
          * 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) {
+        public static void calcDetailsFromIndex(Context context) {
-            Uri fromUpstream = calcSuggestedVersionFromUpstream();
+            Uri fromUpstream = calcAppDetailsFromIndexUri();
             context.getContentResolver().update(fromUpstream, null, null, null);
-
-            Uri fromLatest = calcSuggestedVersionFromLatest();
-            context.getContentResolver().update(fromLatest, null, null, null);
         }
 
     }
@@ -246,8 +242,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 String PATH_CALC_APP_DETAILS_FROM_INDEX = "calcDetailsFromIndex";
 
     private static final int CAN_UPDATE       = CODE_SINGLE + 1;
     private static final int INSTALLED        = CAN_UPDATE + 1;
@@ -258,13 +254,12 @@ 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;
+    private static final int CALC_APP_DETAILS_FROM_INDEX = IGNORED + 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_APP_DETAILS_FROM_INDEX, CALC_APP_DETAILS_FROM_INDEX);
-        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);
@@ -293,12 +288,8 @@ public class AppProvider extends FDroidProvider {
         return Uri.withAppendedPath(getContentUri(), PATH_IGNORED);
     }
 
-    private static Uri calcSuggestedVersionFromUpstream() {
+    private static Uri calcAppDetailsFromIndexUri() {
-        return Uri.withAppendedPath(getContentUri(), PATH_CALC_SUGGESTED_FROM_UPSTREAM);
+        return Uri.withAppendedPath(getContentUri(), PATH_CALC_APP_DETAILS_FROM_INDEX);
-    }
-
-    private static Uri calcSuggestedVersionFromLatest() {
-        return Uri.withAppendedPath(getContentUri(), PATH_CALC_SUGGESTED_FROM_LATEST);
     }
 
     public static Uri getCategoryUri(String category) {
@@ -575,12 +566,8 @@ public class AppProvider extends FDroidProvider {
         QuerySelection query = new QuerySelection(where, whereArgs);
         switch (matcher.match(uri)) {
 
-            case CALC_SUGGESTED_FROM_LATEST:
+            case CALC_APP_DETAILS_FROM_INDEX:
-                setSuggestedFromLatest();
+                updateAppDetails();
-                return 0;
-
-            case CALC_SUGGESTED_FROM_UPSTREAM:
-                setSuggestedFromUpstream();
                 return 0;
 
             case CODE_SINGLE:
@@ -598,6 +585,13 @@ public class AppProvider extends FDroidProvider {
         return count;
     }
 
+    private void updateAppDetails() {
+        updateCompatibleFlags();
+        updateSuggestedFromLatest();
+        updateSuggestedFromUpstream();
+        updateIconUrls();
+    }
+
     /**
      * 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.
@@ -631,7 +625,9 @@ public class AppProvider extends FDroidProvider {
      *   )
      *   WHERE upstreamVercode > 0
      */
-    private void setSuggestedFromUpstream() {
+    private void updateSuggestedFromUpstream() {
+
+        Log.d("FDroid", "Calculating suggested versions for all apps which specify an upstream version code.");
 
         final String apk = DBHelper.TABLE_APK;
         final String app = DBHelper.TABLE_APP;
@@ -650,6 +646,33 @@ public class AppProvider extends FDroidProvider {
         write().execSQL(updateSql);
     }
 
+    /**
+     * For each app, we want to set the isCompatible flag to 1 if any of the apks we know
+     * about are compatible, and 0 otherwise.
+     *
+     * Here is the SQL query without all of the concatenations (hopefully it's a bit easier to read):
+     *
+     *  UPDATE fdroid_app SET compatible = (
+     *      SELECT TOTAL( fdroid_apk.compatible ) > 0
+     *      FROM fdroid_apk
+     *      WHERE fdroid_apk.id = fdroid_app.id );
+     */
+    private void updateCompatibleFlags() {
+
+        Log.d("FDroid", "Calculating whether apps are compatible, based on whether any of their apks are compatible");
+
+        final String apk = DBHelper.TABLE_APK;
+        final String app = DBHelper.TABLE_APP;
+
+        String updateSql =
+            "UPDATE " + app + " SET compatible = ( " +
+                " SELECT TOTAL( " + apk + ".compatible ) > 0 " +
+                " FROM " + apk +
+                " WHERE " + apk + ".id = " + app + ".id );";
+
+        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
@@ -678,7 +701,9 @@ public class AppProvider extends FDroidProvider {
      *  )
      *  WHERE upstreamVercode = 0 OR upstreamVercode IS NULL;
      */
-    private void setSuggestedFromLatest() {
+    private void updateSuggestedFromLatest() {
+
+        Log.d("FDroid", "Calculating suggested versions for all apps which don't specify an upstream version code.");
 
         final String apk = DBHelper.TABLE_APK;
         final String app = DBHelper.TABLE_APP;
@@ -696,4 +721,61 @@ public class AppProvider extends FDroidProvider {
         write().execSQL(updateSql);
     }
 
+    private void updateIconUrls() {
+
+        Log.d("FDroid", "Updating icon paths for apps belonging to repos with version >= " + Repo.VERSION_DENSITY_SPECIFIC_ICONS);
+        String iconsDir = Utils.getIconsDir(getContext());
+        String repoVersion = Integer.toString(Repo.VERSION_DENSITY_SPECIFIC_ICONS);
+        String query = getIconUpdateQuery();
+        String[] params = { iconsDir, repoVersion };
+        write().execSQL(query, params);
+    }
+
+    /**
+     * Returns a query which requires two parameters to be bound. These are (in order):
+     *  1) The repo version that introduced density specific icons
+     *  2) The dir to density specific icons for the current device.
+     */
+    private String getIconUpdateQuery() {
+
+        final String apk = DBHelper.TABLE_APK;
+        final String app = DBHelper.TABLE_APP;
+        final String repo = DBHelper.TABLE_REPO;
+
+        return
+            " UPDATE " + app + " SET iconUrl = ( " +
+                " SELECT " +
+
+                    // Concatenate (using the "||" operator) the address, the icons directory (bound to the ? as the
+                    // second parameter when executing the query) and the icon path.
+                    " ( " +
+                        repo + ".address " +
+                        " || " +
+
+                        // If the repo has the relevant version, then use a more intelligent icons dir,
+                        // otherwise revert to '/icons/'
+                        " CASE WHEN " + repo + ".version >= ? THEN ? ELSE '/icons/' END " +
+
+                        " || " +
+                        app + ".icon " +
+                    ") " +
+                " FROM " +
+                    apk +
+                    " JOIN " + repo + " ON (" + repo + "._id = " + apk + ".repo) " +
+                " WHERE " +
+                    app + ".id = " + apk + ".id AND " +
+                    apk + ".vercode = ( " +
+
+                        // We only want the latest apk here. Ideally, we should instead join
+                        // onto apk.suggestedVercode, but as per https://gitlab.com/fdroid/fdroidclient/issues/1
+                        // there may be some situations where suggestedVercode isn't set.
+                        // TODO: If we can guarantee that suggestedVercode is set, then join onto that instead.
+                        // This will save from doing a futher sub query for each app.
+                        " SELECT MAX(inner_apk.vercode)  " +
+                        " FROM fdroid_apk as inner_apk " +
+                        " WHERE inner_apk.id = fdroid_apk.id ) " +
+                    " AND fdroid_apk.repo = fdroid_repo._id " +
+            " ) ";
+    }
+
 }