From aae0a57dfee4f11a65fac9645c57fd7fc25a8a79 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 9 Jun 2017 13:47:46 +1000
Subject: [PATCH 01/12] Improve performance of suggested version calculation.

The history of this is that #974 identified a problem, which was fixed
in !497. That MR added test coverage for the bug.

However, the fix for it actually added a huge performance hit, on the
order of 30 seconds or so when calculating the suggested version.

This fixes that performance problem by removing the need for a sub
query. The end goal is to take the following query:

```
UPDATE app
SET suggestedVersion = (
  SELECT MAX(apk.version)
  FROM apk
  WHERE ...
)
```

and the `WHERE` clause needs to somehow join the outer `app` table with
the inner `apk` table, such that the repo in question does not matter.
It can't just join directly from `apk.appId -> app.rowid`, because the
`app` is specific to a given repository, but we want to select the
`MAX(apk.version)` from every related apk, regardless of repo.

This commit solves it by joining the inner `apk` table onto an
intermediate `app` table, which is used purely so that we can select
apks where their `packageId` is the same as the `packageId` of the app
being updated.
---
 .../org/fdroid/fdroid/data/AppProvider.java   | 39 +++++++------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
index 92f06faae..a0e90ed5f 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -953,6 +953,8 @@ public class AppProvider extends FDroidProvider {
      * 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.
+     *
+     * @see #updateSuggestedFromLatest()
      */
     private void updateSuggestedFromUpstream() {
         Utils.debugLog(TAG, "Calculating suggested versions for all apps which specify an upstream version code.");
@@ -963,12 +965,19 @@ public class AppProvider extends FDroidProvider {
         final boolean unstableUpdates = Preferences.get().getUnstableUpdates();
         String restrictToStable = unstableUpdates ? "" : (apk + "." + ApkTable.Cols.VERSION_CODE + " <= " + app + "." + Cols.UPSTREAM_VERSION_CODE + " AND ");
 
+        // The join onto `appForThisApk` is to ensure that the MAX(apk.versionCode) is chosen from
+        // all apps regardless of repo. If we joined directly onto the outer `app` table we are
+        // in the process of updating, then it would be limited to only apks from the same repo.
+        // By adding the extra join, and then joining based on the packageId of this inner app table
+        // and the app table we are updating, we take into account all apks for this app.
+
         String updateSql =
                 "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " +
                 " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " +
                 " FROM " + apk +
+                "   JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " +
                 " WHERE " +
-                    joinToApksRegardlessOfRepo() + " AND " +
+                    app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " +
                     restrictToStable +
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 ";
@@ -976,29 +985,6 @@ public class AppProvider extends FDroidProvider {
         db().execSQL(updateSql);
     }
 
-    /**
-     * Ensure that when we select a list of {@link ApkTable} rows for which to calculate the
-     * {@link Cols#SUGGESTED_VERSION_CODE}, that we select all apks belonging to the same package,
-     * regardless of which repo they come from. We can't just join {@link ApkTable} onto the
-     * {@link AppMetadataTable}, because the {@link AppMetadataTable} table is specific to a repo.
-     *
-     * This is required so that apps always have the highest possible
-     * {@link Cols#SUGGESTED_VERSION_CODE}, regardless of the repository priorities. Without this,
-     * then each {@link AppMetadataTable} row will have a different {@link Cols#SUGGESTED_VERSION_CODE}
-     * depending on which repo it came from. With this, each {@link AppMetadataTable} row has the
-     * same {@link Cols#SUGGESTED_VERSION_CODE}, even if that version is from a different repo.
-     */
-    private String joinToApksRegardlessOfRepo() {
-        final String apk = getApkTableName();
-        final String app = getTableName();
-
-        return app + "." + Cols.PACKAGE_ID + " = (" +
-                    " SELECT innerAppName." + Cols.PACKAGE_ID +
-                    " FROM " + app + " as innerAppName " +
-                    " WHERE innerAppName." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID +
-                ") ";
-    }
-
     /**
      * We set each app's suggested version to the latest available that is
      * compatible, or the latest available if none are compatible.
@@ -1006,6 +992,8 @@ public class AppProvider extends FDroidProvider {
      * If the suggested version is null, it means that we could not figure it
      * out from the upstream vercode. In such a case, fall back to the simpler
      * algorithm as if upstreamVercode was 0.
+     *
+     * @see #updateSuggestedFromUpstream()
      */
     private void updateSuggestedFromLatest() {
         Utils.debugLog(TAG, "Calculating suggested versions for all apps which don't specify an upstream version code.");
@@ -1017,8 +1005,9 @@ public class AppProvider extends FDroidProvider {
                 "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " +
                 " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " +
                 " FROM " + apk +
+                "   JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " +
                 " WHERE " +
-                    joinToApksRegardlessOfRepo() + " AND " +
+                    app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " +
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE COALESCE(" + Cols.UPSTREAM_VERSION_CODE + ", 0) = 0 OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL ";
 

From 44ffaa37d6c0defc4f251d8c2821eb615b7a73a1 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 8 Jun 2017 10:11:02 +1000
Subject: [PATCH 02/12] Add suggested version tests.

Some are failing as expected, but should define the correct behaviour.

Still needs support for multi-repo tests.
---
 .../java/org/fdroid/fdroid/Preferences.java   |   4 +
 .../test/java/org/fdroid/fdroid/Assert.java   |   9 +-
 .../fdroid/data/InstalledAppTestUtils.java    |  12 ++
 .../fdroid/data/SuggestedVersionTest.java     | 141 ++++++++++++++++++
 4 files changed, 164 insertions(+), 2 deletions(-)
 create mode 100644 app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java

diff --git a/app/src/main/java/org/fdroid/fdroid/Preferences.java b/app/src/main/java/org/fdroid/fdroid/Preferences.java
index d71bf60f8..fa4079a77 100644
--- a/app/src/main/java/org/fdroid/fdroid/Preferences.java
+++ b/app/src/main/java/org/fdroid/fdroid/Preferences.java
@@ -195,6 +195,10 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
         return preferences.getBoolean(PREF_UNSTABLE_UPDATES, DEFAULT_UNSTABLE_UPDATES);
     }
 
+    public void setUnstableUpdates(boolean value) {
+        preferences.edit().putBoolean(PREF_UNSTABLE_UPDATES, value).apply();
+    }
+
     public boolean isKeepingInstallHistory() {
         return preferences.getBoolean(PREF_KEEP_INSTALL_HISTORY, DEFAULT_KEEP_INSTALL_HISTORY);
     }
diff --git a/app/src/test/java/org/fdroid/fdroid/Assert.java b/app/src/test/java/org/fdroid/fdroid/Assert.java
index c0ebe5e3f..741be9814 100644
--- a/app/src/test/java/org/fdroid/fdroid/Assert.java
+++ b/app/src/test/java/org/fdroid/fdroid/Assert.java
@@ -196,11 +196,16 @@ public class Assert {
 
         values.putAll(additionalValues);
 
+        // Don't hard code to 1, let consumers override it in additionalValues then ask for it back.
+        int repoId = values.getAsInteger(AppMetadataTable.Cols.REPO_ID);
+
         Uri uri = AppProvider.getContentUri();
 
         context.getContentResolver().insert(uri, values);
-        return AppProvider.Helper.findSpecificApp(context.getContentResolver(), packageName, 1,
-                AppMetadataTable.Cols.ALL);
+        App app = AppProvider.Helper.findSpecificApp(context.getContentResolver(), packageName,
+                repoId, AppMetadataTable.Cols.ALL);
+        assertNotNull(app);
+        return app;
     }
 
     public static App ensureApp(Context context, String packageName) {
diff --git a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java
index 332adc687..9a9eec115 100644
--- a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java
+++ b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java
@@ -3,6 +3,8 @@ package org.fdroid.fdroid.data;
 import android.content.Context;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
+import android.content.pm.Signature;
+import android.support.annotation.Nullable;
 
 public class InstalledAppTestUtils {
 
@@ -13,12 +15,22 @@ public class InstalledAppTestUtils {
     public static void install(Context context,
                                String packageName,
                                int versionCode, String versionName) {
+        install(context, packageName, versionCode, versionName, null);
+    }
+
+    public static void install(Context context,
+                               String packageName,
+                               int versionCode, String versionName,
+                               @Nullable String signature) {
         PackageInfo info = new PackageInfo();
         info.packageName = packageName;
         info.versionCode = versionCode;
         info.versionName = versionName;
         info.applicationInfo = new ApplicationInfo();
         info.applicationInfo.publicSourceDir = "/tmp/mock-location";
+        if (signature != null) {
+            info.signatures = new Signature[]{new Signature(signature)};
+        }
         String hashType = "sha256";
         String hash = "00112233445566778899aabbccddeeff";
         InstalledAppProviderService.insertAppIntoDb(context, info, hashType, hash);
diff --git a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
new file mode 100644
index 000000000..ca96d7a66
--- /dev/null
+++ b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
@@ -0,0 +1,141 @@
+package org.fdroid.fdroid.data;
+
+import android.app.Application;
+import android.content.ContentValues;
+import android.content.Context;
+
+import org.fdroid.fdroid.Assert;
+import org.fdroid.fdroid.BuildConfig;
+import org.fdroid.fdroid.Hasher;
+import org.fdroid.fdroid.Preferences;
+import org.fdroid.fdroid.TestUtils;
+import org.fdroid.fdroid.data.Schema.AppMetadataTable.Cols;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+import java.security.NoSuchAlgorithmException;
+
+import static org.junit.Assert.assertEquals;
+
+@Config(constants = BuildConfig.class, application = Application.class, sdk = 24)
+@RunWith(RobolectricTestRunner.class)
+public class SuggestedVersionTest extends FDroidProviderTest {
+
+    private static final String FDROID_CERT = "308202ed308201d5a003020102020426ffa009300d06092a864886f70d01010b05003027310b300906035504061302444531183016060355040a130f4e4f47415050532050726f6a656374301e170d3132313030363132303533325a170d3337303933303132303533325a3027310b300906035504061302444531183016060355040a130f4e4f47415050532050726f6a65637430820122300d06092a864886f70d01010105000382010f003082010a02820101009a8d2a5336b0eaaad89ce447828c7753b157459b79e3215dc962ca48f58c2cd7650df67d2dd7bda0880c682791f32b35c504e43e77b43c3e4e541f86e35a8293a54fb46e6b16af54d3a4eda458f1a7c8bc1b7479861ca7043337180e40079d9cdccb7e051ada9b6c88c9ec635541e2ebf0842521c3024c826f6fd6db6fd117c74e859d5af4db04448965ab5469b71ce719939a06ef30580f50febf96c474a7d265bb63f86a822ff7b643de6b76e966a18553c2858416cf3309dd24278374bdd82b4404ef6f7f122cec93859351fc6e5ea947e3ceb9d67374fe970e593e5cd05c905e1d24f5a5484f4aadef766e498adf64f7cf04bddd602ae8137b6eea40722d0203010001a321301f301d0603551d0e04160414110b7aa9ebc840b20399f69a431f4dba6ac42a64300d06092a864886f70d01010b0500038201010007c32ad893349cf86952fb5a49cfdc9b13f5e3c800aece77b2e7e0e9c83e34052f140f357ec7e6f4b432dc1ed542218a14835acd2df2deea7efd3fd5e8f1c34e1fb39ec6a427c6e6f4178b609b369040ac1f8844b789f3694dc640de06e44b247afed11637173f36f5886170fafd74954049858c6096308fc93c1bc4dd5685fa7a1f982a422f2a3b36baa8c9500474cf2af91c39cbec1bc898d10194d368aa5e91f1137ec115087c31962d8f76cd120d28c249cf76f4c70f5baa08c70a7234ce4123be080cee789477401965cfe537b924ef36747e8caca62dfefdd1a6288dcb1c4fd2aaa6131a7ad254e9742022cfd597d2ca5c660ce9e41ff537e5a4041e37";
+    private static final String UPSTREAM_CERT = "308204e1308202c9a0030201020204483450fa300d06092a864886f70d01010b050030213110300e060355040b1307462d44726f6964310d300b06035504031304736f7661301e170d3136303832333133333131365a170d3434303130393133333131365a30213110300e060355040b1307462d44726f6964310d300b06035504031304736f766130820222300d06092a864886f70d01010105000382020f003082020a0282020100dfdcd120f3ab224999dddf4ea33ea588d295e4d7130bef48c143e9d76e5c0e0e9e5d45e64208e35feebc79a83f08939dd6a343b7d1e2179930a105a1249ccd36d88ff3feffc6e4dc53dae0163a7876dd45ecc1ddb0adf5099aa56c1a84b52affcd45d0711ffa4de864f35ac0333ebe61ea8673eeda35a88f6af678cc4d0f80b089338ac8f2a8279a64195c611d19445cab3fd1a020afed9bd739bb95142fb2c00a8f847db5ef3325c814f8eb741bacf86ed3907bfe6e4564d2de5895df0c263824e0b75407589bae2d3a4666c13b92102d8781a8ee9bb4a5a1a78c4a9c21efdaf5584da42e84418b28f5a81d0456a3dc5b420991801e6b21e38c99bbe018a5b2d690894a114bc860d35601416aa4dc52216aff8a288d4775cddf8b72d45fd2f87303a8e9c0d67e442530be28eaf139894337266e0b33d57f949256ab32083bcc545bc18a83c9ab8247c12aea037e2b68dee31c734cb1f04f241d3b94caa3a2b258ffaf8e6eae9fbbe029a934dc0a0859c5f120334812693a1c09352340a39f2a678dbc1afa2a978bfee43afefcb7e224a58af2f3d647e5745db59061236b8af6fcfd93b3602f9e456978534f3a7851e800071bf56da80401c81d91c45f82568373af0576b1cc5eef9b85654124b6319770be3cdba3fbebe3715e8918fb6c8966624f3d0e815effac3d2ee06dd34ab9c693218b2c7c06ba99d6b74d4f17b8c3cb0203010001a321301f301d0603551d0e04160414d62bee9f3798509546acc62eb1de14b08b954d4f300d06092a864886f70d01010b05000382020100743f7c5692085895f9d1fffad390fb4202c15f123ed094df259185960fd6dadf66cb19851070f180297bba4e6996a4434616573b375cfee94fee73a4505a7ec29136b7e6c22e6436290e3686fe4379d4e3140ec6a08e70cfd3ed5b634a5eb5136efaaabf5f38e0432d3d79568a556970b8cfba2972f5d23a3856d8a981b9e9bbbbb88f35e708bde9cbc5f681cbd974085b9da28911296fe2579fa64bbe9fa0b93475a7a8db051080b0c5fade0d1c018e7858cd4cbe95145b0620e2f632cbe0f8af9cbf22e2fdaa72245ae31b0877b07181cc69dd2df74454251d8de58d25e76354abe7eb690f22e59b08795a8f2c98c578e0599503d9085927634072c82c9f82abd50fd12b8fd1a9d1954eb5cc0b4cfb5796b5aaec0356643b4a65a368442d92ef94edd3ac6a2b7fe3571b8cf9f462729228aab023ef9183f73792f5379633ccac51079177d604c6bc1873ada6f07d8da6d68c897e88a5fa5d63fdb8df820f46090e0716e7562dd3c140ba279a65b996f60addb0abe29d4bf2f5abe89480771d492307b926d91f02f341b2148502903c43d40f3c6c86a811d060711f0698b384acdcc0add44eb54e42962d3d041accc715afd49407715adc09350cb55e8d9281a3b0b6b5fcd91726eede9b7c8b13afdebb2c2b377629595f1096ba62fb14946dbac5f3c5f0b4e5b712e7acc7dcf6c46cdc5e6d6dfdeee55a0c92c2d70f080ac6";
+
+    private static final String FDROID_SIG;
+    private static final String UPSTREAM_SIG;
+
+    static {
+        // Some code requires the full certificate (e.g. when we mock PackageInfo to give to the
+        // installed app provider), while others requires the hashed certificate (e.g. inserting
+        // into the apk provider directly, without the need to mock anything).
+        try {
+            FDROID_SIG = new Hasher("MD5", FDROID_CERT.getBytes()).getHash();
+            UPSTREAM_SIG = new Hasher("MD5", UPSTREAM_CERT.getBytes()).getHash();
+        } catch (NoSuchAlgorithmException e) {
+            throw new IllegalStateException(e);
+        }
+    }
+
+    @Before
+    public void setup() {
+        TestUtils.registerContentProvider(AppProvider.getAuthority(), AppProvider.class);
+        Preferences.setup(context);
+    }
+
+    @After
+    public void tearDown() {
+        Preferences.clearSingletonForTesting();
+    }
+
+    @Test
+    public void singleRepoSingleSig() {
+        App singleApp = insertApp(context, "single.app", "Single App (with beta)", 2, "https://beta.simple.repo");
+        insertApk(context, singleApp, 1, FDROID_SIG);
+        insertApk(context, singleApp, 2, FDROID_SIG);
+        insertApk(context, singleApp, 3, FDROID_SIG);
+        AppProvider.Helper.calcSuggestedApks(context);
+
+        App found2 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        assertEquals(2, found2.suggestedVersionCode);
+
+        // By enabling unstable updates, the "upstreamVersionCode" should get ignored, and we should
+        // suggest the latest version (3).
+        Preferences.get().setUnstableUpdates(true);
+        AppProvider.Helper.calcSuggestedApks(context);
+        App found3 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        assertEquals(3, found3.suggestedVersionCode);
+    }
+
+    /**
+     * TODO: Failing for two reasons:
+     *  * We don't support suggested versioncode with multi-sig.
+     *  * More importantly, we don't even support multi-sig due to the [appId, vercode, repo] primary key.
+     */
+    @Test
+    public void singleRepoMultiSig() {
+        App singleApp = insertApp(context, "single.app", "Single App", 4, "https://simple.repo");
+        insertApk(context, singleApp, 1, FDROID_SIG);
+        insertApk(context, singleApp, 2, FDROID_SIG);
+        insertApk(context, singleApp, 3, FDROID_SIG);
+        insertApk(context, singleApp, 4, UPSTREAM_SIG);
+        insertApk(context, singleApp, 5, UPSTREAM_SIG);
+        AppProvider.Helper.calcSuggestedApks(context);
+
+        // Given we aren't installed yet, we don't care which signature.
+        // Just get as close to upstreamVersionCode as possible.
+        App suggestUpstream4 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        assertEquals(4, suggestUpstream4.suggestedVersionCode);
+
+        // Now install v1 with the f-droid signature. In response, we should only suggest
+        // apps with that sig in the future. That is, version 4 from upstream is not considered.
+        InstalledAppTestUtils.install(context, "single.app", 1, "v1", FDROID_CERT);
+        AppProvider.Helper.calcSuggestedApks(context);
+        App suggestFDroid3 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        assertEquals(3, suggestFDroid3.suggestedVersionCode);
+
+        // This adds the "upstreamVersionCode" version of the app, but signed by f-droid.
+        insertApk(context, singleApp, 4, FDROID_SIG);
+        insertApk(context, singleApp, 5, FDROID_SIG);
+        AppProvider.Helper.calcSuggestedApks(context);
+        App suggestFDroid4 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        assertEquals(4, suggestFDroid4.suggestedVersionCode);
+
+        // Version 5 from F-Droid is not the "upstreamVersionCode", but with beta updates it should
+        // still become the suggested version now.
+        Preferences.get().setUnstableUpdates(true);
+        AppProvider.Helper.calcSuggestedApks(context);
+        App suggestFDroid5 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        assertEquals(5, suggestFDroid5.suggestedVersionCode);
+    }
+
+    private void insertApk(Context context, App app, int versionCode, String signature) {
+        ContentValues values = new ContentValues();
+        values.put(Schema.ApkTable.Cols.SIGNATURE, signature);
+        Assert.insertApk(context, app, versionCode, values);
+    }
+
+    private App insertApp(Context context, String packageName, String appName, int upstreamVersionCode, String repoUrl) {
+        Repo repo = ensureRepo(context, repoUrl);
+        ContentValues values = new ContentValues();
+        values.put(Cols.REPO_ID, repo.getId());
+        values.put(Cols.UPSTREAM_VERSION_CODE, upstreamVersionCode);
+        return Assert.insertApp(context, packageName, appName, values);
+    }
+
+    private Repo ensureRepo(Context context, String repoUrl) {
+        Repo existing = RepoProvider.Helper.findByAddress(context, repoUrl);
+        if (existing != null) {
+            return existing;
+        }
+
+        return RepoProviderTest.insertRepo(context, repoUrl, "", "", "");
+    }
+
+}

From b95a330ccfd6f24fc4c262dbd26b7dc564e50c84 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 8 Jun 2017 13:20:26 +1000
Subject: [PATCH 03/12] Restrict suggested versions to those with the same sig
 as installed.

Only if there is actually a version installed.
---
 .../org/fdroid/fdroid/data/AppProvider.java   | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
index a0e90ed5f..d7d9f2b34 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -977,6 +977,7 @@ public class AppProvider extends FDroidProvider {
                 " FROM " + apk +
                 "   JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " +
                 " WHERE " +
+                    restrictToSameSigIfInstalled(app, apk) + " AND " +
                     app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " +
                     restrictToStable +
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " +
@@ -1007,6 +1008,7 @@ public class AppProvider extends FDroidProvider {
                 " FROM " + apk +
                 "   JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " +
                 " WHERE " +
+                    restrictToSameSigIfInstalled(app, apk) + " AND " +
                     app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " +
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE COALESCE(" + Cols.UPSTREAM_VERSION_CODE + ", 0) = 0 OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL ";
@@ -1014,6 +1016,30 @@ public class AppProvider extends FDroidProvider {
         db().execSQL(updateSql);
     }
 
+    /**
+     * Limits results for an apk query. If the app in question is installed, then will limit apk
+     * results to those matching the same signature as the installed one. Otherwise, allows all apks
+     * to be returned.
+     */
+    private static String restrictToSameSigIfInstalled(String appTable, String apkTable) {
+        String installedSig =
+                "(SELECT installed." + InstalledAppTable.Cols.SIGNATURE +
+                        " FROM " + InstalledAppTable.NAME + " AS installed " +
+                        " JOIN " + PackageTable.NAME + " AS pkg ON " +
+                        "   (pkg." + PackageTable.Cols.ROW_ID + " = " + appTable + "." + Cols.PACKAGE_ID + " AND " +
+                        "    installed." + InstalledAppTable.Cols.PACKAGE_NAME + " = pkg." + PackageTable.Cols.PACKAGE_NAME + ") " +
+                        ")";
+
+        // If the installed sig is not null, then the apk signature will need to match that.
+        // If the installed sig IS null, then it will check whether the apk sig matches the apk sig
+        // (i.e. it will always return the app).
+        // This would be better writen as: `installedSig IS NULL OR installedSig = apk.sig`,
+        // however that would require a separate sub query for each `installedSig` which is more
+        // expensive. This is a less expressive way to write the same thing.
+        return apkTable + "." + ApkTable.Cols.SIGNATURE + " = " +
+                "COALESCE(" + installedSig + ", " + apkTable + "." + ApkTable.Cols.SIGNATURE + ")";
+    }
+
     private void updateIconUrls() {
         final String appTable = getTableName();
         final String apkTable = getApkTableName();

From aa472ba764e850f738498751ff7ca9acfbac7a86 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 8 Jun 2017 13:31:47 +1000
Subject: [PATCH 04/12] Drop the composite vercode + repo primary key from apk
 table

We expect repos to serve multiple apks with the same version code but
different signing certificates in the future.
---
 .../java/org/fdroid/fdroid/data/DBHelper.java   | 17 ++++++++++++++---
 .../fdroid/data/SuggestedVersionTest.java       |  5 -----
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
index 5a8852668..6bca3d3d8 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
@@ -108,8 +108,7 @@ class DBHelper extends SQLiteOpenHelper {
             + ApkTable.Cols.ADDED_DATE + " string, "
             + ApkTable.Cols.IS_COMPATIBLE + " int not null, "
             + ApkTable.Cols.INCOMPATIBLE_REASONS + " text, "
-            + ApkTable.Cols.ANTI_FEATURES + " string, "
-            + "PRIMARY KEY (" + ApkTable.Cols.APP_ID + ", " + ApkTable.Cols.VERSION_CODE + ", " + ApkTable.Cols.REPO_ID + ")"
+            + ApkTable.Cols.ANTI_FEATURES + " string"
             + ");";
 
     static final String CREATE_TABLE_APP_METADATA = "CREATE TABLE " + AppMetadataTable.NAME
@@ -193,7 +192,7 @@ class DBHelper extends SQLiteOpenHelper {
             + InstalledAppTable.Cols.HASH + " TEXT NOT NULL"
             + " );";
 
-    protected static final int DB_VERSION = 69;
+    protected static final int DB_VERSION = 70;
 
     private final Context context;
 
@@ -277,6 +276,18 @@ class DBHelper extends SQLiteOpenHelper {
         addIndexV1AppFields(db, oldVersion);
         recalculatePreferredMetadata(db, oldVersion);
         addWhatsNewAndVideo(db, oldVersion);
+        dropApkPrimaryKey(db, oldVersion);
+    }
+
+    private void dropApkPrimaryKey(SQLiteDatabase db, int oldVersion) {
+        if (oldVersion >= 70) {
+            return;
+        }
+
+        // versionCode + repo is no longer a valid primary key given a repo can have multiple apks
+        // with the same versionCode, signed by different certificates.
+        Log.i(TAG, "Dropping composite primary key on apk table in favour of sqlite's rowid");
+        resetTransient(db);
     }
 
     private void addWhatsNewAndVideo(SQLiteDatabase db, int oldVersion) {
diff --git a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
index ca96d7a66..f9afb61c6 100644
--- a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
@@ -73,11 +73,6 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         assertEquals(3, found3.suggestedVersionCode);
     }
 
-    /**
-     * TODO: Failing for two reasons:
-     *  * We don't support suggested versioncode with multi-sig.
-     *  * More importantly, we don't even support multi-sig due to the [appId, vercode, repo] primary key.
-     */
     @Test
     public void singleRepoMultiSig() {
         App singleApp = insertApp(context, "single.app", "Single App", 4, "https://simple.repo");

From 82eb50c2fe11ab120b888e4df6090b0882fb844c Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Thu, 8 Jun 2017 15:02:35 +1000
Subject: [PATCH 05/12] Add test for multi-repo multi-sig apps

---
 .../fdroid/data/SuggestedVersionTest.java     | 94 +++++++++++++++++--
 1 file changed, 85 insertions(+), 9 deletions(-)

diff --git a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
index f9afb61c6..55c029541 100644
--- a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java
@@ -25,11 +25,13 @@ import static org.junit.Assert.assertEquals;
 @RunWith(RobolectricTestRunner.class)
 public class SuggestedVersionTest extends FDroidProviderTest {
 
-    private static final String FDROID_CERT = "308202ed308201d5a003020102020426ffa009300d06092a864886f70d01010b05003027310b300906035504061302444531183016060355040a130f4e4f47415050532050726f6a656374301e170d3132313030363132303533325a170d3337303933303132303533325a3027310b300906035504061302444531183016060355040a130f4e4f47415050532050726f6a65637430820122300d06092a864886f70d01010105000382010f003082010a02820101009a8d2a5336b0eaaad89ce447828c7753b157459b79e3215dc962ca48f58c2cd7650df67d2dd7bda0880c682791f32b35c504e43e77b43c3e4e541f86e35a8293a54fb46e6b16af54d3a4eda458f1a7c8bc1b7479861ca7043337180e40079d9cdccb7e051ada9b6c88c9ec635541e2ebf0842521c3024c826f6fd6db6fd117c74e859d5af4db04448965ab5469b71ce719939a06ef30580f50febf96c474a7d265bb63f86a822ff7b643de6b76e966a18553c2858416cf3309dd24278374bdd82b4404ef6f7f122cec93859351fc6e5ea947e3ceb9d67374fe970e593e5cd05c905e1d24f5a5484f4aadef766e498adf64f7cf04bddd602ae8137b6eea40722d0203010001a321301f301d0603551d0e04160414110b7aa9ebc840b20399f69a431f4dba6ac42a64300d06092a864886f70d01010b0500038201010007c32ad893349cf86952fb5a49cfdc9b13f5e3c800aece77b2e7e0e9c83e34052f140f357ec7e6f4b432dc1ed542218a14835acd2df2deea7efd3fd5e8f1c34e1fb39ec6a427c6e6f4178b609b369040ac1f8844b789f3694dc640de06e44b247afed11637173f36f5886170fafd74954049858c6096308fc93c1bc4dd5685fa7a1f982a422f2a3b36baa8c9500474cf2af91c39cbec1bc898d10194d368aa5e91f1137ec115087c31962d8f76cd120d28c249cf76f4c70f5baa08c70a7234ce4123be080cee789477401965cfe537b924ef36747e8caca62dfefdd1a6288dcb1c4fd2aaa6131a7ad254e9742022cfd597d2ca5c660ce9e41ff537e5a4041e37";
-    private static final String UPSTREAM_CERT = "308204e1308202c9a0030201020204483450fa300d06092a864886f70d01010b050030213110300e060355040b1307462d44726f6964310d300b06035504031304736f7661301e170d3136303832333133333131365a170d3434303130393133333131365a30213110300e060355040b1307462d44726f6964310d300b06035504031304736f766130820222300d06092a864886f70d01010105000382020f003082020a0282020100dfdcd120f3ab224999dddf4ea33ea588d295e4d7130bef48c143e9d76e5c0e0e9e5d45e64208e35feebc79a83f08939dd6a343b7d1e2179930a105a1249ccd36d88ff3feffc6e4dc53dae0163a7876dd45ecc1ddb0adf5099aa56c1a84b52affcd45d0711ffa4de864f35ac0333ebe61ea8673eeda35a88f6af678cc4d0f80b089338ac8f2a8279a64195c611d19445cab3fd1a020afed9bd739bb95142fb2c00a8f847db5ef3325c814f8eb741bacf86ed3907bfe6e4564d2de5895df0c263824e0b75407589bae2d3a4666c13b92102d8781a8ee9bb4a5a1a78c4a9c21efdaf5584da42e84418b28f5a81d0456a3dc5b420991801e6b21e38c99bbe018a5b2d690894a114bc860d35601416aa4dc52216aff8a288d4775cddf8b72d45fd2f87303a8e9c0d67e442530be28eaf139894337266e0b33d57f949256ab32083bcc545bc18a83c9ab8247c12aea037e2b68dee31c734cb1f04f241d3b94caa3a2b258ffaf8e6eae9fbbe029a934dc0a0859c5f120334812693a1c09352340a39f2a678dbc1afa2a978bfee43afefcb7e224a58af2f3d647e5745db59061236b8af6fcfd93b3602f9e456978534f3a7851e800071bf56da80401c81d91c45f82568373af0576b1cc5eef9b85654124b6319770be3cdba3fbebe3715e8918fb6c8966624f3d0e815effac3d2ee06dd34ab9c693218b2c7c06ba99d6b74d4f17b8c3cb0203010001a321301f301d0603551d0e04160414d62bee9f3798509546acc62eb1de14b08b954d4f300d06092a864886f70d01010b05000382020100743f7c5692085895f9d1fffad390fb4202c15f123ed094df259185960fd6dadf66cb19851070f180297bba4e6996a4434616573b375cfee94fee73a4505a7ec29136b7e6c22e6436290e3686fe4379d4e3140ec6a08e70cfd3ed5b634a5eb5136efaaabf5f38e0432d3d79568a556970b8cfba2972f5d23a3856d8a981b9e9bbbbb88f35e708bde9cbc5f681cbd974085b9da28911296fe2579fa64bbe9fa0b93475a7a8db051080b0c5fade0d1c018e7858cd4cbe95145b0620e2f632cbe0f8af9cbf22e2fdaa72245ae31b0877b07181cc69dd2df74454251d8de58d25e76354abe7eb690f22e59b08795a8f2c98c578e0599503d9085927634072c82c9f82abd50fd12b8fd1a9d1954eb5cc0b4cfb5796b5aaec0356643b4a65a368442d92ef94edd3ac6a2b7fe3571b8cf9f462729228aab023ef9183f73792f5379633ccac51079177d604c6bc1873ada6f07d8da6d68c897e88a5fa5d63fdb8df820f46090e0716e7562dd3c140ba279a65b996f60addb0abe29d4bf2f5abe89480771d492307b926d91f02f341b2148502903c43d40f3c6c86a811d060711f0698b384acdcc0add44eb54e42962d3d041accc715afd49407715adc09350cb55e8d9281a3b0b6b5fcd91726eede9b7c8b13afdebb2c2b377629595f1096ba62fb14946dbac5f3c5f0b4e5b712e7acc7dcf6c46cdc5e6d6dfdeee55a0c92c2d70f080ac6";
+    private static final String FDROID_CERT = "308202ed308201d5a003020102020426ffa009300d06092a864886f70d01010b05003027310b300906035504061302444531183016060355040a130f4e4f47415050532050726f6a656374301e170d3132313030363132303533325a170d3337303933303132303533325a3027310b300906035504061302444531183016060355040a130f4e4f47415050532050726f6a65637430820122300d06092a864886f70d01010105000382010f003082010a02820101009a8d2a5336b0eaaad89ce447828c7753b157459b79e3215dc962ca48f58c2cd7650df67d2dd7bda0880c682791f32b35c504e43e77b43c3e4e541f86e35a8293a54fb46e6b16af54d3a4eda458f1a7c8bc1b7479861ca7043337180e40079d9cdccb7e051ada9b6c88c9ec635541e2ebf0842521c3024c826f6fd6db6fd117c74e859d5af4db04448965ab5469b71ce719939a06ef30580f50febf96c474a7d265bb63f86a822ff7b643de6b76e966a18553c2858416cf3309dd24278374bdd82b4404ef6f7f122cec93859351fc6e5ea947e3ceb9d67374fe970e593e5cd05c905e1d24f5a5484f4aadef766e498adf64f7cf04bddd602ae8137b6eea40722d0203010001a321301f301d0603551d0e04160414110b7aa9ebc840b20399f69a431f4dba6ac42a64300d06092a864886f70d01010b0500038201010007c32ad893349cf86952fb5a49cfdc9b13f5e3c800aece77b2e7e0e9c83e34052f140f357ec7e6f4b432dc1ed542218a14835acd2df2deea7efd3fd5e8f1c34e1fb39ec6a427c6e6f4178b609b369040ac1f8844b789f3694dc640de06e44b247afed11637173f36f5886170fafd74954049858c6096308fc93c1bc4dd5685fa7a1f982a422f2a3b36baa8c9500474cf2af91c39cbec1bc898d10194d368aa5e91f1137ec115087c31962d8f76cd120d28c249cf76f4c70f5baa08c70a7234ce4123be080cee789477401965cfe537b924ef36747e8caca62dfefdd1a6288dcb1c4fd2aaa6131a7ad254e9742022cfd597d2ca5c660ce9e41ff537e5a4041e37"; // NOCHECKSTYLE LineLength
+    private static final String UPSTREAM_CERT = "308204e1308202c9a0030201020204483450fa300d06092a864886f70d01010b050030213110300e060355040b1307462d44726f6964310d300b06035504031304736f7661301e170d3136303832333133333131365a170d3434303130393133333131365a30213110300e060355040b1307462d44726f6964310d300b06035504031304736f766130820222300d06092a864886f70d01010105000382020f003082020a0282020100dfdcd120f3ab224999dddf4ea33ea588d295e4d7130bef48c143e9d76e5c0e0e9e5d45e64208e35feebc79a83f08939dd6a343b7d1e2179930a105a1249ccd36d88ff3feffc6e4dc53dae0163a7876dd45ecc1ddb0adf5099aa56c1a84b52affcd45d0711ffa4de864f35ac0333ebe61ea8673eeda35a88f6af678cc4d0f80b089338ac8f2a8279a64195c611d19445cab3fd1a020afed9bd739bb95142fb2c00a8f847db5ef3325c814f8eb741bacf86ed3907bfe6e4564d2de5895df0c263824e0b75407589bae2d3a4666c13b92102d8781a8ee9bb4a5a1a78c4a9c21efdaf5584da42e84418b28f5a81d0456a3dc5b420991801e6b21e38c99bbe018a5b2d690894a114bc860d35601416aa4dc52216aff8a288d4775cddf8b72d45fd2f87303a8e9c0d67e442530be28eaf139894337266e0b33d57f949256ab32083bcc545bc18a83c9ab8247c12aea037e2b68dee31c734cb1f04f241d3b94caa3a2b258ffaf8e6eae9fbbe029a934dc0a0859c5f120334812693a1c09352340a39f2a678dbc1afa2a978bfee43afefcb7e224a58af2f3d647e5745db59061236b8af6fcfd93b3602f9e456978534f3a7851e800071bf56da80401c81d91c45f82568373af0576b1cc5eef9b85654124b6319770be3cdba3fbebe3715e8918fb6c8966624f3d0e815effac3d2ee06dd34ab9c693218b2c7c06ba99d6b74d4f17b8c3cb0203010001a321301f301d0603551d0e04160414d62bee9f3798509546acc62eb1de14b08b954d4f300d06092a864886f70d01010b05000382020100743f7c5692085895f9d1fffad390fb4202c15f123ed094df259185960fd6dadf66cb19851070f180297bba4e6996a4434616573b375cfee94fee73a4505a7ec29136b7e6c22e6436290e3686fe4379d4e3140ec6a08e70cfd3ed5b634a5eb5136efaaabf5f38e0432d3d79568a556970b8cfba2972f5d23a3856d8a981b9e9bbbbb88f35e708bde9cbc5f681cbd974085b9da28911296fe2579fa64bbe9fa0b93475a7a8db051080b0c5fade0d1c018e7858cd4cbe95145b0620e2f632cbe0f8af9cbf22e2fdaa72245ae31b0877b07181cc69dd2df74454251d8de58d25e76354abe7eb690f22e59b08795a8f2c98c578e0599503d9085927634072c82c9f82abd50fd12b8fd1a9d1954eb5cc0b4cfb5796b5aaec0356643b4a65a368442d92ef94edd3ac6a2b7fe3571b8cf9f462729228aab023ef9183f73792f5379633ccac51079177d604c6bc1873ada6f07d8da6d68c897e88a5fa5d63fdb8df820f46090e0716e7562dd3c140ba279a65b996f60addb0abe29d4bf2f5abe89480771d492307b926d91f02f341b2148502903c43d40f3c6c86a811d060711f0698b384acdcc0add44eb54e42962d3d041accc715afd49407715adc09350cb55e8d9281a3b0b6b5fcd91726eede9b7c8b13afdebb2c2b377629595f1096ba62fb14946dbac5f3c5f0b4e5b712e7acc7dcf6c46cdc5e6d6dfdeee55a0c92c2d70f080ac6"; // NOCHECKSTYLE LineLength
+    private static final String THIRD_PARTY_CERT = "308204e1308202c9a0030201020204483450fa300d06092a864886f70d01010b050030213110300e060355040b130abcdeabcde012340123400b06035504031304736f7661301e170d3136303832333133333131365a170d3434303130393133333131365a30213110300e060355040b1307462d44726f6964310d300b06035504031304736f766130820222300d06092a864886f70d01010105000382020f003082020a0282020100dfdcd120f3ab224999dddf4ea33ea588d295e4d7130bef48c143e9d76e5c0e0e9e5d45e64208e35feebc79a83f08939dd6a343b7d1e2179930a105a1249ccd36d88ff3feffc6e4dc53dae0163a7876dd45ecc1ddb0adf5099aa56c1a84b52affcd45d0711ffa4de864f35ac0333ebe61ea8673eeda35a88f6af678cc4d0f80b089338ac8f2a8279a64195c611d19445cab3fd1a020afed9bd739bb95142fb2c00a8f847db5ef3325c814f8eb741bacf86ed3907bfe6e4564d2de5895df0c263824e0b75407589bae2d3a4666c13b92102d8781a8ee9bb4a5a1a78c4a9c21efdaf5584da42e84418b28f5a81d0456a3dc5b420991801e6b21e38c99bbe018a5b2d690894a114bc860d35601416aa4dc52216aff8a288d4775cddf8b72d45fd2f87303a8e9c0d67e442530be28eaf139894337266e0b33d57f949256ab32083bcc545bc18a83c9ab8247c12aea037e2b68dee31c734cb1f04f241d3b94caa3a2b258ffaf8e6eae9fbbe029a934dc0a0859c5f120334812693a1c09352340a39f2a678dbc1afa2a978bfee43afefcb7e224a58af2f3d647e5745db59061236b8af6fcfd93b3602f9e456978534f3a7851e800071bf56da80401c81d91c45f82568373af0576b1cc5eef9b85654124b6319770be3cdba3fbebe3715e8918fb6c8966624f3d0e815effac3d2ee06dd34ab9c693218b2c7c06ba99d6b74d4f17b8c3cb0203010001a321301f301d0603551d0e04160414d62bee9f3798509546acc62eb1de14b08b954d4f300d06092a864886f70d01010b05000382020100743f7c5692085895f9d1fffad390fb4202c15f123ed094df259185960fd6dadf66cb19851070f180297bba4e6996a4434616573b375cfee94fee73a4505a7ec29136b7e6c22e6436290e3686fe4379d4e3140ec6a08e70cfd3ed5b634a5eb5136efaaabf5f38e0432d3d79568a556970b8cfba2972f5d23a3856d8a981b9e9bbbbb88f35e708bde9cbc5f681cbd974085b9da28911296fe2579fa64bbe9fa0b93475a7a8db051080b0c5fade0d1c018e7858cd4cbe95145b0620e2f632cbe0f8af9cbf22e2fdaa72245ae31b0877b07181cc69dd2df74454251d8de58d25e76354abe7eb690f22e59b08795a8f2c98c578e0599503d9085927634072c82c9f82abd50fd12b8fd1a9d1954eb5cc0b4cfb5796b5aaec0356643b4a65a368442d92ef94edd3ac6a2b7fe3571b8cf9f462729228aab023ef9183f73792f5379633ccac51079177d604c6bc1873ada6f07d8da6d68c897e88a5fa5d63fdb8df820f46090e0716e7562dd3c140ba279a65b996f60addb0abe29d4bf2f5abe89480771d492307b926d91f02f341b2148502903c43d40f3c6c86a811d060711f0698b384acdcc0add44eb54e42962d3d041accc715afd49407715adc09350cb55e8d9281a3b0b6b5fcd91726eede9b7c8b13afdebb2c2b377629595f1096ba62fb14946dbac5f3c5f0b4e5b712e7acc7dcf6c46cdc5e6d6dfdeee55a0c92c2d70f080ac6"; // NOCHECKSTYLE LineLength
 
     private static final String FDROID_SIG;
     private static final String UPSTREAM_SIG;
+    private static final String THIRD_PARTY_SIG;
 
     static {
         // Some code requires the full certificate (e.g. when we mock PackageInfo to give to the
@@ -38,6 +40,7 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         try {
             FDROID_SIG = new Hasher("MD5", FDROID_CERT.getBytes()).getHash();
             UPSTREAM_SIG = new Hasher("MD5", UPSTREAM_CERT.getBytes()).getHash();
+            THIRD_PARTY_SIG = new Hasher("MD5", THIRD_PARTY_CERT.getBytes()).getHash();
         } catch (NoSuchAlgorithmException e) {
             throw new IllegalStateException(e);
         }
@@ -62,19 +65,26 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         insertApk(context, singleApp, 3, FDROID_SIG);
         AppProvider.Helper.calcSuggestedApks(context);
 
-        App found2 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        App found2 = findApp(singleApp);
         assertEquals(2, found2.suggestedVersionCode);
 
         // By enabling unstable updates, the "upstreamVersionCode" should get ignored, and we should
         // suggest the latest version (3).
         Preferences.get().setUnstableUpdates(true);
         AppProvider.Helper.calcSuggestedApks(context);
-        App found3 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        App found3 = findApp(singleApp);
         assertEquals(3, found3.suggestedVersionCode);
     }
 
+    private App findApp(App app) {
+        return AppProvider.Helper.findSpecificApp(context.getContentResolver(), app.packageName, app.repoId);
+    }
+
     @Test
     public void singleRepoMultiSig() {
+        App unrelatedApp = insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo");
+        insertApk(context, unrelatedApp, 3, FDROID_SIG);
+
         App singleApp = insertApp(context, "single.app", "Single App", 4, "https://simple.repo");
         insertApk(context, singleApp, 1, FDROID_SIG);
         insertApk(context, singleApp, 2, FDROID_SIG);
@@ -85,38 +95,104 @@ public class SuggestedVersionTest extends FDroidProviderTest {
 
         // Given we aren't installed yet, we don't care which signature.
         // Just get as close to upstreamVersionCode as possible.
-        App suggestUpstream4 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        App suggestUpstream4 = findApp(singleApp);
         assertEquals(4, suggestUpstream4.suggestedVersionCode);
 
         // Now install v1 with the f-droid signature. In response, we should only suggest
         // apps with that sig in the future. That is, version 4 from upstream is not considered.
         InstalledAppTestUtils.install(context, "single.app", 1, "v1", FDROID_CERT);
         AppProvider.Helper.calcSuggestedApks(context);
-        App suggestFDroid3 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        App suggestFDroid3 = findApp(singleApp);
         assertEquals(3, suggestFDroid3.suggestedVersionCode);
 
         // This adds the "upstreamVersionCode" version of the app, but signed by f-droid.
         insertApk(context, singleApp, 4, FDROID_SIG);
         insertApk(context, singleApp, 5, FDROID_SIG);
         AppProvider.Helper.calcSuggestedApks(context);
-        App suggestFDroid4 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        App suggestFDroid4 = findApp(singleApp);
         assertEquals(4, suggestFDroid4.suggestedVersionCode);
 
         // Version 5 from F-Droid is not the "upstreamVersionCode", but with beta updates it should
         // still become the suggested version now.
         Preferences.get().setUnstableUpdates(true);
         AppProvider.Helper.calcSuggestedApks(context);
-        App suggestFDroid5 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), singleApp.packageName, singleApp.repoId);
+        App suggestFDroid5 = findApp(singleApp);
         assertEquals(5, suggestFDroid5.suggestedVersionCode);
     }
 
+    private void recalculateMetadata() {
+        AppProvider.Helper.calcSuggestedApks(context);
+        AppProvider.Helper.recalculatePreferredMetadata(context);
+    }
+
+    private App highestPriorityApp(String packageName) {
+        return AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), packageName);
+    }
+
+    @Test
+    public void multiRepoMultiSig() {
+        App unrelatedApp = insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo");
+        insertApk(context, unrelatedApp, 3, FDROID_SIG);
+
+        App mainApp = insertApp(context, "single.app", "Single App (Main repo)", 4, "https://main.repo");
+        App thirdPartyApp = insertApp(context, "single.app", "Single App (3rd party)", 4, "https://3rd-party.repo");
+
+        insertApk(context, mainApp, 1, FDROID_SIG);
+        insertApk(context, mainApp, 2, FDROID_SIG);
+        insertApk(context, mainApp, 3, FDROID_SIG);
+        insertApk(context, mainApp, 4, UPSTREAM_SIG);
+        insertApk(context, mainApp, 5, UPSTREAM_SIG);
+
+        insertApk(context, thirdPartyApp, 3, THIRD_PARTY_SIG);
+        insertApk(context, thirdPartyApp, 4, THIRD_PARTY_SIG);
+        insertApk(context, thirdPartyApp, 5, THIRD_PARTY_SIG);
+        insertApk(context, thirdPartyApp, 6, THIRD_PARTY_SIG);
+
+        recalculateMetadata();
+
+        // Given we aren't installed yet, we don't care which signature or even which repo.
+        // Just get as close to upstreamVersionCode as possible.
+        App suggestAnyVersion4 = highestPriorityApp("single.app");
+        assertEquals(4, suggestAnyVersion4.suggestedVersionCode);
+
+        // Now install v1 with the f-droid signature. In response, we should only suggest
+        // apps with that sig in the future. That is, version 4 from upstream is not considered.
+        InstalledAppTestUtils.install(context, "single.app", 1, "v1", FDROID_CERT);
+        recalculateMetadata();
+        App suggestFDroid3 = highestPriorityApp("single.app");
+        assertEquals(3, suggestFDroid3.suggestedVersionCode);
+
+        // This adds the "upstreamVersionCode" version of the app, but signed by f-droid.
+        insertApk(context, mainApp, 4, FDROID_SIG);
+        insertApk(context, mainApp, 5, FDROID_SIG);
+        recalculateMetadata();
+        App suggestFDroid4 = highestPriorityApp("single.app");
+        assertEquals(4, suggestFDroid4.suggestedVersionCode);
+
+        // Uninstalling the F-Droid build and installing v3 of the third party means we can now go
+        // back to suggesting version 4.
+        InstalledAppProviderService.deleteAppFromDb(context, "single.app");
+        InstalledAppTestUtils.install(context, "single.app", 3, "v3", THIRD_PARTY_CERT);
+        recalculateMetadata();
+        suggestAnyVersion4 = highestPriorityApp("single.app");
+        assertEquals(4, suggestAnyVersion4.suggestedVersionCode);
+
+        // Version 6 from the 3rd party repo is not the "upstreamVersionCode", but with beta updates
+        // it should still become the suggested version now.
+        Preferences.get().setUnstableUpdates(true);
+        recalculateMetadata();
+        App suggest3rdParty6 = highestPriorityApp("single.app");
+        assertEquals(6, suggest3rdParty6.suggestedVersionCode);
+    }
+
     private void insertApk(Context context, App app, int versionCode, String signature) {
         ContentValues values = new ContentValues();
         values.put(Schema.ApkTable.Cols.SIGNATURE, signature);
         Assert.insertApk(context, app, versionCode, values);
     }
 
-    private App insertApp(Context context, String packageName, String appName, int upstreamVersionCode, String repoUrl) {
+    private App insertApp(Context context, String packageName, String appName, int upstreamVersionCode,
+                          String repoUrl) {
         Repo repo = ensureRepo(context, repoUrl);
         ContentValues values = new ContentValues();
         values.put(Cols.REPO_ID, repo.getId());

From d0444dafcac0f0fa5180d5c2ba8517bb05378ac4 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 9 Jun 2017 09:03:56 +1000
Subject: [PATCH 06/12] Clarify comments in response to CR.

---
 .../org/fdroid/fdroid/data/AppProvider.java   | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
index d7d9f2b34..4c5c3b2f7 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -953,6 +953,8 @@ public class AppProvider extends FDroidProvider {
      * 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.
+     * If the app is installed, then all apks signed by a different certificate are
+     * ignored for the purpose of this calculation.
      *
      * @see #updateSuggestedFromLatest()
      */
@@ -1024,18 +1026,20 @@ public class AppProvider extends FDroidProvider {
     private static String restrictToSameSigIfInstalled(String appTable, String apkTable) {
         String installedSig =
                 "(SELECT installed." + InstalledAppTable.Cols.SIGNATURE +
-                        " FROM " + InstalledAppTable.NAME + " AS installed " +
-                        " JOIN " + PackageTable.NAME + " AS pkg ON " +
-                        "   (pkg." + PackageTable.Cols.ROW_ID + " = " + appTable + "." + Cols.PACKAGE_ID + " AND " +
-                        "    installed." + InstalledAppTable.Cols.PACKAGE_NAME + " = pkg." + PackageTable.Cols.PACKAGE_NAME + ") " +
-                        ")";
+                " FROM " + InstalledAppTable.NAME + " AS installed " +
+                " JOIN " + PackageTable.NAME + " AS pkg ON " +
+                "   (pkg." + PackageTable.Cols.ROW_ID + " = " + appTable + "." + Cols.PACKAGE_ID + " AND " +
+                "    installed." + InstalledAppTable.Cols.PACKAGE_NAME + " = pkg." + PackageTable.Cols.PACKAGE_NAME + ") " +
+                ")";
 
-        // If the installed sig is not null, then the apk signature will need to match that.
-        // If the installed sig IS null, then it will check whether the apk sig matches the apk sig
-        // (i.e. it will always return the app).
-        // This would be better writen as: `installedSig IS NULL OR installedSig = apk.sig`,
+        // Ideally, the check below would actually be written as:
+        //   `installedSig IS NULL OR installedSig = apk.sig`
         // however that would require a separate sub query for each `installedSig` which is more
-        // expensive. This is a less expressive way to write the same thing.
+        // expensive. Using a COALESCE is a less expressive way to write the same thing with only
+        // a single subquery.
+        // Also note that the `installedSig IS NULL` is not because there is a `NULL` entry in the
+        // installed table (this is impossible), but rather because the subselect above returned
+        // zero rows.
         return apkTable + "." + ApkTable.Cols.SIGNATURE + " = " +
                 "COALESCE(" + installedSig + ", " + apkTable + "." + ApkTable.Cols.SIGNATURE + ")";
     }

From b729f4dc84a3e671af5b85c4dfbf004f78f6195a Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 9 Jun 2017 09:37:46 +1000
Subject: [PATCH 07/12] Add slow query logging to updateSuggestedFrom* methods.

Produces the following output:

D  Explain:
D    SCAN TABLE fdroid_app
D    EXECUTE CORRELATED SCALAR SUBQUERY 0
D    SEARCH TABLE fdroid_apk USING INDEX apk_vercode
D    EXECUTE CORRELATED SCALAR SUBQUERY 1
D    SEARCH TABLE fdroid_app AS innerAppName USING INTEGER PRIMARY KEY (rowid=?)
D    EXECUTE CORRELATED SCALAR SUBQUERY 2
D    SEARCH TABLE fdroid_package AS pkg USING INTEGER PRIMARY KEY (rowid=?)
D    SEARCH TABLE fdroid_installedApp AS installed USING INDEX sqlite_autoindex_fdroid_installedApp_1 (appId=?)

There are two possibilities here, one is the number of correlated sub
queries (three seems a bit excessive). Alterantively, it could be the
fact that one of the inner queries is using a string index (appId=?)
instead of an integer primary key.
---
 .../org/fdroid/fdroid/data/AppProvider.java    |  4 ++--
 .../org/fdroid/fdroid/data/LoggingQuery.java   | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
index 4c5c3b2f7..5ea9e1a6e 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -985,7 +985,7 @@ public class AppProvider extends FDroidProvider {
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 ";
 
-        db().execSQL(updateSql);
+        LoggingQuery.execSQL(db(), updateSql);
     }
 
     /**
@@ -1015,7 +1015,7 @@ public class AppProvider extends FDroidProvider {
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE COALESCE(" + Cols.UPSTREAM_VERSION_CODE + ", 0) = 0 OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL ";
 
-        db().execSQL(updateSql);
+        LoggingQuery.execSQL(db(), updateSql);
     }
 
     /**
diff --git a/app/src/main/java/org/fdroid/fdroid/data/LoggingQuery.java b/app/src/main/java/org/fdroid/fdroid/data/LoggingQuery.java
index 9eb40417b..db6731307 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/LoggingQuery.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/LoggingQuery.java
@@ -66,6 +66,20 @@ final class LoggingQuery {
         return db.rawQuery(query, queryArgs);
     }
 
+    private void execSQLInternal() {
+        if (BuildConfig.DEBUG) {
+            long startTime = System.currentTimeMillis();
+            db.execSQL(query);
+            long queryDuration = System.currentTimeMillis() - startTime;
+
+            if (queryDuration >= SLOW_QUERY_DURATION) {
+                logSlowQuery(queryDuration);
+            }
+        } else {
+            db.execSQL(query);
+        }
+    }
+
     /**
      * Log the query and its duration to the console. In addition, execute an "EXPLAIN QUERY PLAN"
      * for the query in question so that the query can be diagnosed (https://sqlite.org/eqp.html)
@@ -116,4 +130,8 @@ final class LoggingQuery {
     public static Cursor query(SQLiteDatabase db, String query, String[] queryBuilderArgs) {
         return new LoggingQuery(db, query, queryBuilderArgs).rawQuery();
     }
+
+    public static void execSQL(SQLiteDatabase db, String sql) {
+        new LoggingQuery(db, sql, null).execSQLInternal();
+    }
 }

From bb96cdeff96e92550b23943d60cc996629c5a8c3 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 9 Jun 2017 12:05:03 +1000
Subject: [PATCH 08/12] Refactor join between installed apps and packages.

---
 .../org/fdroid/fdroid/data/AppProvider.java   | 10 ++--
 .../java/org/fdroid/fdroid/data/DBHelper.java | 34 +++++++++--
 .../org/fdroid/fdroid/data/InstalledApp.java  |  2 +-
 .../fdroid/data/InstalledAppProvider.java     | 57 +++++++++++++++++--
 .../data/InstalledAppProviderService.java     |  2 +-
 .../java/org/fdroid/fdroid/data/Schema.java   |  8 ++-
 .../fdroid/views/swap/SelectAppsView.java     |  6 +-
 .../test/java/org/fdroid/fdroid/Assert.java   |  4 +-
 .../fdroid/data/InstalledAppProviderTest.java | 10 ++--
 9 files changed, 104 insertions(+), 29 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
index 5ea9e1a6e..47733d7f2 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -250,7 +250,7 @@ public class AppProvider extends FDroidProvider {
                 join(
                         InstalledAppTable.NAME,
                         "installed",
-                        "installed." + InstalledAppTable.Cols.PACKAGE_NAME + " = " + PackageTable.NAME + "." + PackageTable.Cols.PACKAGE_NAME);
+                        "installed." + InstalledAppTable.Cols.PACKAGE_ID + " = " + PackageTable.NAME + "." + PackageTable.Cols.ROW_ID);
                 requiresInstalledTable = true;
             }
         }
@@ -270,7 +270,7 @@ public class AppProvider extends FDroidProvider {
                 leftJoin(
                         InstalledAppTable.NAME,
                         "installed",
-                        "installed." + InstalledAppTable.Cols.PACKAGE_NAME + " = " + PackageTable.NAME + "." + PackageTable.Cols.PACKAGE_NAME);
+                        "installed." + InstalledAppTable.Cols.PACKAGE_ID + " = " + PackageTable.NAME + "." + PackageTable.Cols.ROW_ID);
                 requiresInstalledTable = true;
             }
         }
@@ -959,8 +959,9 @@ public class AppProvider extends FDroidProvider {
      * @see #updateSuggestedFromLatest()
      */
     private void updateSuggestedFromUpstream() {
-        Utils.debugLog(TAG, "Calculating suggested versions for all apps which specify an upstream version code.");
+        Utils.debugLog(TAG, "Calculating suggested versions for all NON-INSTALLED apps which specify an upstream version code.");
 
+        Utils.Profiler profiler = new Utils.Profiler("UpdateSuggestedApks");
         final String apk = getApkTableName();
         final String app = getTableName();
 
@@ -986,6 +987,7 @@ public class AppProvider extends FDroidProvider {
                 " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 ";
 
         LoggingQuery.execSQL(db(), updateSql);
+        profiler.log("Done");
     }
 
     /**
@@ -1029,7 +1031,7 @@ public class AppProvider extends FDroidProvider {
                 " FROM " + InstalledAppTable.NAME + " AS installed " +
                 " JOIN " + PackageTable.NAME + " AS pkg ON " +
                 "   (pkg." + PackageTable.Cols.ROW_ID + " = " + appTable + "." + Cols.PACKAGE_ID + " AND " +
-                "    installed." + InstalledAppTable.Cols.PACKAGE_NAME + " = pkg." + PackageTable.Cols.PACKAGE_NAME + ") " +
+                "    installed." + InstalledAppTable.Cols.PACKAGE_ID + " = pkg." + PackageTable.Cols.ROW_ID + ") " +
                 ")";
 
         // Ideally, the check below would actually be written as:
diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
index 6bca3d3d8..555d39f90 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
@@ -182,7 +182,7 @@ class DBHelper extends SQLiteOpenHelper {
 
     private static final String CREATE_TABLE_INSTALLED_APP = "CREATE TABLE " + InstalledAppTable.NAME
             + " ( "
-            + InstalledAppTable.Cols.PACKAGE_NAME + " TEXT NOT NULL PRIMARY KEY, "
+            + InstalledAppTable.Cols.PACKAGE_ID + " INT NOT NULL UNIQUE, "
             + InstalledAppTable.Cols.VERSION_CODE + " INT NOT NULL, "
             + InstalledAppTable.Cols.VERSION_NAME + " TEXT NOT NULL, "
             + InstalledAppTable.Cols.APPLICATION_LABEL + " TEXT NOT NULL, "
@@ -192,7 +192,7 @@ class DBHelper extends SQLiteOpenHelper {
             + InstalledAppTable.Cols.HASH + " TEXT NOT NULL"
             + " );";
 
-    protected static final int DB_VERSION = 70;
+    protected static final int DB_VERSION = 71;
 
     private final Context context;
 
@@ -277,6 +277,28 @@ class DBHelper extends SQLiteOpenHelper {
         recalculatePreferredMetadata(db, oldVersion);
         addWhatsNewAndVideo(db, oldVersion);
         dropApkPrimaryKey(db, oldVersion);
+        addIntegerPrimaryKeyToInstalledApps(db, oldVersion);
+    }
+
+    private void addIntegerPrimaryKeyToInstalledApps(SQLiteDatabase db, int oldVersion) {
+        if (oldVersion >= 71) {
+            return;
+        }
+
+        Log.i(TAG, "Replacing primary key on installed app table with integer for performance.");
+
+        db.beginTransaction();
+        try {
+            if (tableExists(db, Schema.InstalledAppTable.NAME)) {
+                db.execSQL("DROP TABLE " + Schema.InstalledAppTable.NAME);
+            }
+
+            db.execSQL(CREATE_TABLE_INSTALLED_APP);
+            ensureIndexes(db);
+            db.setTransactionSuccessful();
+        } finally {
+            db.endTransaction();
+        }
     }
 
     private void dropApkPrimaryKey(SQLiteDatabase db, int oldVersion) {
@@ -1061,9 +1083,11 @@ class DBHelper extends SQLiteOpenHelper {
                     AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");");
         }
 
-        Utils.debugLog(TAG, "Ensuring indexes exist for " + InstalledAppTable.NAME);
-        db.execSQL("CREATE INDEX IF NOT EXISTS installedApp_appId_vercode on " + InstalledAppTable.NAME + " (" +
-                InstalledAppTable.Cols.PACKAGE_NAME + ", " + InstalledAppTable.Cols.VERSION_CODE + ");");
+        if (columnExists(db, InstalledAppTable.NAME, InstalledAppTable.Cols.PACKAGE_ID)) {
+            Utils.debugLog(TAG, "Ensuring indexes exist for " + InstalledAppTable.NAME);
+            db.execSQL("CREATE INDEX IF NOT EXISTS installedApp_packageId_vercode on " + InstalledAppTable.NAME + " (" +
+                    InstalledAppTable.Cols.PACKAGE_ID + ", " + InstalledAppTable.Cols.VERSION_CODE + ");");
+        }
 
         Utils.debugLog(TAG, "Ensuring indexes exist for " + RepoTable.NAME);
         db.execSQL("CREATE INDEX IF NOT EXISTS repo_id_isSwap on " + RepoTable.NAME + " (" +
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledApp.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledApp.java
index ec9aec9fd..20edcd367 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledApp.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledApp.java
@@ -24,7 +24,7 @@ public class InstalledApp extends ValueObject {
                 case Schema.InstalledAppTable.Cols._ID:
                     id = cursor.getLong(i);
                     break;
-                case Schema.InstalledAppTable.Cols.PACKAGE_NAME:
+                case Schema.InstalledAppTable.Cols.Package.NAME:
                     packageName = cursor.getString(i);
                     break;
                 case Schema.InstalledAppTable.Cols.VERSION_CODE:
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
index f7ee7ec66..5b30b2549 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProvider.java
@@ -10,6 +10,7 @@ import android.content.res.Resources;
 import android.database.Cursor;
 import android.net.Uri;
 import android.support.annotation.Nullable;
+import android.text.TextUtils;
 import android.util.Log;
 import org.fdroid.fdroid.R;
 import org.fdroid.fdroid.Utils;
@@ -41,7 +42,7 @@ public class InstalledAppProvider extends FDroidProvider {
                     cursor.moveToFirst();
                     while (!cursor.isAfterLast()) {
                         cachedInfo.put(
-                                cursor.getString(cursor.getColumnIndex(Cols.PACKAGE_NAME)),
+                                cursor.getString(cursor.getColumnIndex(Cols.Package.NAME)),
                                 cursor.getLong(cursor.getColumnIndex(Cols.LAST_UPDATE_TIME))
                         );
                         cursor.moveToNext();
@@ -136,7 +137,17 @@ public class InstalledAppProvider extends FDroidProvider {
     }
 
     private QuerySelection queryApp(String packageName) {
-        return new QuerySelection(Cols.PACKAGE_NAME + " = ?", new String[]{packageName});
+        return new QuerySelection(Cols.Package.NAME + " = ?", new String[]{packageName});
+    }
+
+    private QuerySelection queryAppSubQuery(String packageName) {
+        String pkg = Schema.PackageTable.NAME;
+        String subQuery = "(" +
+                " SELECT " + pkg + "." + Schema.PackageTable.Cols.ROW_ID +
+                " FROM " + pkg +
+                " WHERE " + pkg + "." + Schema.PackageTable.Cols.PACKAGE_NAME + " = ?)";
+        String query = Cols.PACKAGE_ID + " = " + subQuery;
+        return new QuerySelection(query, new String[]{packageName});
     }
 
     private QuerySelection querySearch(String query) {
@@ -144,6 +155,26 @@ public class InstalledAppProvider extends FDroidProvider {
                 new String[]{"%" + query + "%"});
     }
 
+    private static class QueryBuilder extends org.fdroid.fdroid.data.QueryBuilder {
+        @Override
+        protected String getRequiredTables() {
+            String pkg = Schema.PackageTable.NAME;
+            String installed = InstalledAppTable.NAME;
+            return installed + " JOIN " + pkg +
+                    " ON (" + pkg + "." + Schema.PackageTable.Cols.ROW_ID + " = " +
+                    installed + "." + Cols.PACKAGE_ID + ")";
+        }
+
+        @Override
+        public void addField(String field) {
+            if (TextUtils.equals(field, Cols.Package.NAME)) {
+                appendField(Schema.PackageTable.Cols.PACKAGE_NAME, Schema.PackageTable.NAME, field);
+            } else {
+                appendField(field, InstalledAppTable.NAME);
+            }
+        }
+    }
+
     @Override
     public Cursor query(Uri uri, String[] projection,
                         String customSelection, String[] selectionArgs, String sortOrder) {
@@ -170,8 +201,15 @@ public class InstalledAppProvider extends FDroidProvider {
                 throw new UnsupportedOperationException(message);
         }
 
-        Cursor cursor = db().query(getTableName(), projection,
-                selection.getSelection(), selection.getArgs(), null, null, sortOrder);
+        QueryBuilder query = new QueryBuilder();
+        query.addFields(projection);
+        if (projection.length == 0) {
+            query.addField(Cols._ID);
+        }
+        query.addSelection(selection);
+        query.addOrderBy(sortOrder);
+
+        Cursor cursor = db().rawQuery(query.toString(), selection.getArgs());
         cursor.setNotificationUri(getContext().getContentResolver(), uri);
         return cursor;
     }
@@ -184,7 +222,7 @@ public class InstalledAppProvider extends FDroidProvider {
         }
 
         QuerySelection query = new QuerySelection(where, whereArgs);
-        query = query.add(queryApp(uri.getLastPathSegment()));
+        query = query.add(queryAppSubQuery(uri.getLastPathSegment()));
 
         return db().delete(getTableName(), query.getSelection(), query.getArgs());
     }
@@ -196,9 +234,16 @@ public class InstalledAppProvider extends FDroidProvider {
             throw new UnsupportedOperationException("Insert not supported for " + uri + ".");
         }
 
+        if (values.containsKey(Cols.Package.NAME)) {
+            String packageName = values.getAsString(Cols.Package.NAME);
+            long packageId = PackageProvider.Helper.ensureExists(getContext(), packageName);
+            values.remove(Cols.Package.NAME);
+            values.put(Cols.PACKAGE_ID, packageId);
+        }
+
         verifyVersionNameNotNull(values);
         db().replaceOrThrow(getTableName(), null, values);
-        return getAppUri(values.getAsString(Cols.PACKAGE_NAME));
+        return getAppUri(values.getAsString(Cols.Package.NAME));
     }
 
     /**
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
index aa71c8bc1..d319b4033 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
@@ -288,7 +288,7 @@ public class InstalledAppProviderService extends IntentService {
     static void insertAppIntoDb(Context context, PackageInfo packageInfo, String hashType, String hash) {
         Uri uri = InstalledAppProvider.getContentUri();
         ContentValues contentValues = new ContentValues();
-        contentValues.put(InstalledAppTable.Cols.PACKAGE_NAME, packageInfo.packageName);
+        contentValues.put(InstalledAppTable.Cols.Package.NAME, packageInfo.packageName);
         contentValues.put(InstalledAppTable.Cols.VERSION_CODE, packageInfo.versionCode);
         contentValues.put(InstalledAppTable.Cols.VERSION_NAME, packageInfo.versionName);
         contentValues.put(InstalledAppTable.Cols.APPLICATION_LABEL,
diff --git a/app/src/main/java/org/fdroid/fdroid/data/Schema.java b/app/src/main/java/org/fdroid/fdroid/data/Schema.java
index 7ae47d8c5..cb02a85d9 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java
@@ -329,7 +329,7 @@ public interface Schema {
 
         interface Cols {
             String _ID = "rowid as _id"; // Required for CursorLoaders
-            String PACKAGE_NAME = "appId";
+            String PACKAGE_ID = "packageId";
             String VERSION_CODE = "versionCode";
             String VERSION_NAME = "versionName";
             String APPLICATION_LABEL = "applicationLabel";
@@ -338,8 +338,12 @@ public interface Schema {
             String HASH_TYPE = "hashType";
             String HASH = "hash";
 
+            interface Package {
+                String NAME = "packageName";
+            }
+
             String[] ALL = {
-                    _ID, PACKAGE_NAME, VERSION_CODE, VERSION_NAME, APPLICATION_LABEL,
+                    _ID, PACKAGE_ID, Package.NAME, VERSION_CODE, VERSION_NAME, APPLICATION_LABEL,
                     SIGNATURE, LAST_UPDATE_TIME, HASH_TYPE, HASH,
             };
         }
diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SelectAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SelectAppsView.java
index 77daa73bd..5f0d7a496 100644
--- a/app/src/main/java/org/fdroid/fdroid/views/swap/SelectAppsView.java
+++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SelectAppsView.java
@@ -142,7 +142,7 @@ public class SelectAppsView extends ListView implements
 
     private void toggleAppSelected(int position) {
         Cursor c = (Cursor) adapter.getItem(position);
-        String packageName = c.getString(c.getColumnIndex(InstalledAppTable.Cols.PACKAGE_NAME));
+        String packageName = c.getString(c.getColumnIndex(InstalledAppTable.Cols.Package.NAME));
         if (getState().hasSelectedPackage(packageName)) {
             getState().deselectPackage(packageName);
             adapter.updateCheckedIndicatorView(position, false);
@@ -176,7 +176,7 @@ public class SelectAppsView extends ListView implements
 
         for (int i = 0; i < getCount(); i++) {
             Cursor c = (Cursor) getItemAtPosition(i);
-            String packageName = c.getString(c.getColumnIndex(InstalledAppTable.Cols.PACKAGE_NAME));
+            String packageName = c.getString(c.getColumnIndex(InstalledAppTable.Cols.Package.NAME));
             getState().ensureFDroidSelected();
             for (String selected : getState().getAppsToSwap()) {
                 if (TextUtils.equals(packageName, selected)) {
@@ -257,7 +257,7 @@ public class SelectAppsView extends ListView implements
             TextView labelView = (TextView) view.findViewById(R.id.application_label);
             ImageView iconView = (ImageView) view.findViewById(android.R.id.icon);
 
-            String packageName = cursor.getString(cursor.getColumnIndex(InstalledAppTable.Cols.PACKAGE_NAME));
+            String packageName = cursor.getString(cursor.getColumnIndex(InstalledAppTable.Cols.Package.NAME));
             String appLabel = cursor.getString(cursor.getColumnIndex(InstalledAppTable.Cols.APPLICATION_LABEL));
 
             Drawable icon;
diff --git a/app/src/test/java/org/fdroid/fdroid/Assert.java b/app/src/test/java/org/fdroid/fdroid/Assert.java
index 741be9814..a9db602e7 100644
--- a/app/src/test/java/org/fdroid/fdroid/Assert.java
+++ b/app/src/test/java/org/fdroid/fdroid/Assert.java
@@ -158,7 +158,7 @@ public class Assert {
         Uri uri = InstalledAppProvider.getAppUri(appId);
 
         String[] projection = {
-                InstalledAppTable.Cols.PACKAGE_NAME,
+                InstalledAppTable.Cols.Package.NAME,
                 InstalledAppTable.Cols.VERSION_CODE,
                 InstalledAppTable.Cols.VERSION_NAME,
                 InstalledAppTable.Cols.APPLICATION_LABEL,
@@ -171,7 +171,7 @@ public class Assert {
 
         cursor.moveToFirst();
 
-        assertEquals(appId, cursor.getString(cursor.getColumnIndex(InstalledAppTable.Cols.PACKAGE_NAME)));
+        assertEquals(appId, cursor.getString(cursor.getColumnIndex(InstalledAppTable.Cols.Package.NAME)));
         assertEquals(versionCode, cursor.getInt(cursor.getColumnIndex(InstalledAppTable.Cols.VERSION_CODE)));
         assertEquals(versionName, cursor.getString(cursor.getColumnIndex(InstalledAppTable.Cols.VERSION_NAME)));
         cursor.close();
diff --git a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java
index 23872b54d..cd5a3e38a 100644
--- a/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/data/InstalledAppProviderTest.java
@@ -37,7 +37,7 @@ public class InstalledAppProviderTest extends FDroidProviderTest {
         assertEquals(foundBefore.size(), 0);
 
         ContentValues values = new ContentValues();
-        values.put(Cols.PACKAGE_NAME, "org.example.test-app");
+        values.put(Cols.Package.NAME, "org.example.test-app");
         values.put(Cols.APPLICATION_LABEL, "Test App");
         values.put(Cols.VERSION_CODE, 1021);
         values.put(Cols.VERSION_NAME, "Longhorn");
@@ -56,7 +56,7 @@ public class InstalledAppProviderTest extends FDroidProviderTest {
         assertEquals(cursor.getCount(), 1);
 
         cursor.moveToFirst();
-        assertEquals("org.example.test-app", cursor.getString(cursor.getColumnIndex(Cols.PACKAGE_NAME)));
+        assertEquals("org.example.test-app", cursor.getString(cursor.getColumnIndex(Cols.Package.NAME)));
         assertEquals("Test App", cursor.getString(cursor.getColumnIndex(Cols.APPLICATION_LABEL)));
         assertEquals(1021, cursor.getInt(cursor.getColumnIndex(Cols.VERSION_CODE)));
         assertEquals("Longhorn", cursor.getString(cursor.getColumnIndex(Cols.VERSION_NAME)));
@@ -125,7 +125,7 @@ public class InstalledAppProviderTest extends FDroidProviderTest {
         Uri uri = InstalledAppProvider.getAppUri(packageName);
 
         String[] projection = {
-                Cols.PACKAGE_NAME,
+                Cols.Package.NAME,
                 Cols.LAST_UPDATE_TIME,
         };
 
@@ -133,7 +133,7 @@ public class InstalledAppProviderTest extends FDroidProviderTest {
         assertNotNull(cursor);
         assertEquals("App \"" + packageName + "\" not installed", 1, cursor.getCount());
         cursor.moveToFirst();
-        assertEquals(packageName, cursor.getString(cursor.getColumnIndex(Cols.PACKAGE_NAME)));
+        assertEquals(packageName, cursor.getString(cursor.getColumnIndex(Cols.Package.NAME)));
         long lastUpdateTime = cursor.getLong(cursor.getColumnIndex(Cols.LAST_UPDATE_TIME));
         assertTrue(lastUpdateTime > 0);
         assertTrue(lastUpdateTime < System.currentTimeMillis());
@@ -170,7 +170,7 @@ public class InstalledAppProviderTest extends FDroidProviderTest {
     private ContentValues createContentValues(String appId, int versionCode, String versionNumber) {
         ContentValues values = new ContentValues(3);
         if (appId != null) {
-            values.put(Cols.PACKAGE_NAME, appId);
+            values.put(Cols.Package.NAME, appId);
         }
         values.put(Cols.APPLICATION_LABEL, "Mock app: " + appId);
         values.put(Cols.VERSION_CODE, versionCode);

From de149cf589c449551a824d85aeb8c467cb06a0b4 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 9 Jun 2017 14:46:05 +1000
Subject: [PATCH 09/12] Remove subselect and use better index.

The main problem is that we were using an index on fdroid_apk.vercode,
when it should have been using an index on fdroid_apk.appId. There are
thousands of apks which would match based on vercode, but only two or
three which match based on appId. This improves performance of the
calculate-suggested-vercode query from 25,000ms to 100ms.
---
 .../org/fdroid/fdroid/data/AppProvider.java   | 44 ++++++-------------
 .../fdroid/fdroid/data/TempApkProvider.java   |  2 +-
 2 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
index 47733d7f2..b803bc95f 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java
@@ -961,9 +961,9 @@ public class AppProvider extends FDroidProvider {
     private void updateSuggestedFromUpstream() {
         Utils.debugLog(TAG, "Calculating suggested versions for all NON-INSTALLED apps which specify an upstream version code.");
 
-        Utils.Profiler profiler = new Utils.Profiler("UpdateSuggestedApks");
         final String apk = getApkTableName();
         final String app = getTableName();
+        final String installed = InstalledAppTable.NAME;
 
         final boolean unstableUpdates = Preferences.get().getUnstableUpdates();
         String restrictToStable = unstableUpdates ? "" : (apk + "." + ApkTable.Cols.VERSION_CODE + " <= " + app + "." + Cols.UPSTREAM_VERSION_CODE + " AND ");
@@ -974,20 +974,28 @@ public class AppProvider extends FDroidProvider {
         // By adding the extra join, and then joining based on the packageId of this inner app table
         // and the app table we are updating, we take into account all apks for this app.
 
+        // The check apk.sig = COALESCE(installed.sig, apk.sig) would ideally be better written as:
+        //   `installedSig IS NULL OR installedSig = apk.sig`
+        // however that would require a separate sub query for each `installedSig` which is more
+        // expensive. Using a COALESCE is a less expressive way to write the same thing with only
+        // a single subquery.
+        // Also note that the `installedSig IS NULL` is not because there is a `NULL` entry in the
+        // installed table (this is impossible), but rather because the subselect above returned
+        // zero rows.
         String updateSql =
                 "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " +
                 " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " +
                 " FROM " + apk +
                 "   JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " +
+                        "   LEFT JOIN " + installed + " ON (" + installed + "." + InstalledAppTable.Cols.PACKAGE_ID + " = " + app + "." + Cols.PACKAGE_ID + ") " +
                 " WHERE " +
-                    restrictToSameSigIfInstalled(app, apk) + " AND " +
                     app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " +
+                    apk + "." + ApkTable.Cols.SIGNATURE + " = COALESCE(" + installed + "." + InstalledAppTable.Cols.SIGNATURE + ", " + apk + "." + ApkTable.Cols.SIGNATURE + ") AND " +
                     restrictToStable +
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 ";
 
         LoggingQuery.execSQL(db(), updateSql);
-        profiler.log("Done");
     }
 
     /**
@@ -1005,47 +1013,23 @@ public class AppProvider extends FDroidProvider {
 
         final String apk = getApkTableName();
         final String app = getTableName();
+        final String installed = InstalledAppTable.NAME;
 
         String updateSql =
                 "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " +
                 " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " +
                 " FROM " + apk +
                 "   JOIN " + app + " AS appForThisApk ON (appForThisApk." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + ") " +
+                "   LEFT JOIN " + installed + " ON (" + installed + "." + InstalledAppTable.Cols.PACKAGE_ID + " = " + app + "." + Cols.PACKAGE_ID + ") " +
                 " WHERE " +
-                    restrictToSameSigIfInstalled(app, apk) + " AND " +
                     app + "." + Cols.PACKAGE_ID + " = appForThisApk." + Cols.PACKAGE_ID + " AND " +
+                    apk + "." + ApkTable.Cols.SIGNATURE + " = COALESCE(" + installed + "." + InstalledAppTable.Cols.SIGNATURE + ", " + apk + "." + ApkTable.Cols.SIGNATURE + ") AND " +
                     " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " +
                 " WHERE COALESCE(" + Cols.UPSTREAM_VERSION_CODE + ", 0) = 0 OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL ";
 
         LoggingQuery.execSQL(db(), updateSql);
     }
 
-    /**
-     * Limits results for an apk query. If the app in question is installed, then will limit apk
-     * results to those matching the same signature as the installed one. Otherwise, allows all apks
-     * to be returned.
-     */
-    private static String restrictToSameSigIfInstalled(String appTable, String apkTable) {
-        String installedSig =
-                "(SELECT installed." + InstalledAppTable.Cols.SIGNATURE +
-                " FROM " + InstalledAppTable.NAME + " AS installed " +
-                " JOIN " + PackageTable.NAME + " AS pkg ON " +
-                "   (pkg." + PackageTable.Cols.ROW_ID + " = " + appTable + "." + Cols.PACKAGE_ID + " AND " +
-                "    installed." + InstalledAppTable.Cols.PACKAGE_ID + " = pkg." + PackageTable.Cols.ROW_ID + ") " +
-                ")";
-
-        // Ideally, the check below would actually be written as:
-        //   `installedSig IS NULL OR installedSig = apk.sig`
-        // however that would require a separate sub query for each `installedSig` which is more
-        // expensive. Using a COALESCE is a less expressive way to write the same thing with only
-        // a single subquery.
-        // Also note that the `installedSig IS NULL` is not because there is a `NULL` entry in the
-        // installed table (this is impossible), but rather because the subselect above returned
-        // zero rows.
-        return apkTable + "." + ApkTable.Cols.SIGNATURE + " = " +
-                "COALESCE(" + installedSig + ", " + apkTable + "." + ApkTable.Cols.SIGNATURE + ")";
-    }
-
     private void updateIconUrls() {
         final String appTable = getTableName();
         final String apkTable = getApkTableName();
diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java
index d5a688022..412688121 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java
@@ -129,7 +129,7 @@ public class TempApkProvider extends ApkProvider {
         final String memoryDbName = TempAppProvider.DB;
         db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(Schema.ApkTable.NAME, memoryDbName + "." + getTableName()));
         db.execSQL(TempAppProvider.copyData(Schema.ApkTable.Cols.ALL_COLS, Schema.ApkTable.NAME, memoryDbName + "." + getTableName()));
-        db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_vercode on " + getTableName() + " (" + ApkTable.Cols.VERSION_CODE + ");");
+        db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + ApkTable.Cols.APP_ID + ");");
         db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + ApkTable.Cols.IS_COMPATIBLE + ");");
     }
 

From f7d9be9cd5834dd5992786a27ed12aff69a9065a Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Tue, 13 Jun 2017 08:15:52 +1000
Subject: [PATCH 10/12] Remove System.out.print's. Replace with Logs where
 appropriate.

Some were removed and left removed if they were run during tests,
because the tests are supposed to be automated and the noise they added
would not have helped diagnose a failure.

Also removed the dead code around "uses-feature" which will never
get implemented, especially as it is in the XML index.
---
 .../org/fdroid/fdroid/compat/FileCompatTest.java     |  7 +++++--
 .../org/fdroid/fdroid/net/HttpDownloaderTest.java    |  5 -----
 .../main/java/org/fdroid/fdroid/RepoXMLHandler.java  |  3 ---
 .../fdroid/fdroid/updater/IndexV1UpdaterTest.java    | 12 ++++--------
 4 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/app/src/androidTest/java/org/fdroid/fdroid/compat/FileCompatTest.java b/app/src/androidTest/java/org/fdroid/fdroid/compat/FileCompatTest.java
index 18a9a1367..e56971a15 100644
--- a/app/src/androidTest/java/org/fdroid/fdroid/compat/FileCompatTest.java
+++ b/app/src/androidTest/java/org/fdroid/fdroid/compat/FileCompatTest.java
@@ -6,6 +6,7 @@ import android.os.Build;
 import android.os.Environment;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.runner.AndroidJUnit4;
+import android.util.Log;
 
 import org.fdroid.fdroid.AssetUtils;
 import org.fdroid.fdroid.data.SanitizedFile;
@@ -30,6 +31,8 @@ import static org.junit.Assume.assumeTrue;
 @RunWith(AndroidJUnit4.class)
 public class FileCompatTest {
 
+    private static final String TAG = "FileCompatTest";
+
     private SanitizedFile sourceFile;
     private SanitizedFile destFile;
 
@@ -47,11 +50,11 @@ public class FileCompatTest {
     @After
     public void tearDown() {
         if (!sourceFile.delete()) {
-            System.out.println("Can't delete " + sourceFile.getAbsolutePath() + ".");
+            Log.w(TAG, "Can't delete " + sourceFile.getAbsolutePath() + ".");
         }
 
         if (!destFile.delete()) {
-            System.out.println("Can't delete " + destFile.getAbsolutePath() + ".");
+            Log.w(TAG, "Can't delete " + destFile.getAbsolutePath() + ".");
         }
     }
 
diff --git a/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java b/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java
index 08f218716..944304c55 100644
--- a/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java
+++ b/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java
@@ -46,16 +46,12 @@ public class HttpDownloaderTest {
         final CountDownLatch latch = new CountDownLatch(1);
         String urlString = "https://f-droid.org/repo/index.jar";
         receivedProgress = false;
-        System.out.println("downloadUninterruptedTestWithProgress: " + urlString);
-        receivedProgress = false;
         URL url = new URL(urlString);
         File destFile = File.createTempFile("dl-", "");
         final HttpDownloader httpDownloader = new HttpDownloader(url, destFile);
         httpDownloader.setListener(new ProgressListener() {
             @Override
             public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
-                System.out.println("DownloaderProgressListener.sendProgress "
-                        + sourceUrl + " " + bytesRead + " / " + totalBytes);
                 receivedProgress = true;
             }
         });
@@ -118,7 +114,6 @@ public class HttpDownloaderTest {
         httpDownloader.setListener(new ProgressListener() {
             @Override
             public void onProgress(URL sourceUrl, int bytesRead, int totalBytes) {
-                System.out.println("DownloaderProgressListener.sendProgress " + bytesRead + " / " + totalBytes);
                 receivedProgress = true;
                 latch.countDown();
             }
diff --git a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java
index df9b625d1..8a51c1697 100644
--- a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java
+++ b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java
@@ -379,9 +379,6 @@ public class RepoXMLHandler extends DefaultHandler {
             } else {
                 removeRequestedPermission(attributes.getValue("name"));
             }
-        } else if ("uses-feature".equals(localName) && curapk != null) {
-            System.out.println("TODO startElement " + uri + " " + localName + " " + qName);
-            // TODO
         }
         curchars.setLength(0);
     }
diff --git a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
index 8447be3c2..d44c63292 100644
--- a/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java
@@ -1,6 +1,7 @@
 package org.fdroid.fdroid.updater;
 
 import android.support.annotation.NonNull;
+import android.text.TextUtils;
 import android.util.Log;
 import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonParser;
@@ -177,7 +178,6 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
                 Repo.PUSH_REQUEST_ACCEPT_ALWAYS);
         indexV0Details.apps.size();
 
-        System.out.println("total apps: " + apps.length + " " + indexV0Details.apps.size());
         assertEquals(indexV0Details.apps.size(), apps.length);
         assertEquals(apps.length, packages.size());
 
@@ -336,13 +336,10 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
             fields.remove(field);
         }
         if (fields.size() > 0) {
-            System.out.print(instance.getClass() + " has fields not setup for Jackson: ");
-            for (String field : fields) {
-                System.out.print("\"" + field + "\", ");
-            }
-            System.out.println("\nRead class javadoc for more info.");
+            String sb = String.valueOf(instance.getClass()) + " has fields not setup for Jackson: " +
+                    TextUtils.join(", ", fields) + "\nRead class javadoc for more info.";
+            fail(sb);
         }
-        assertEquals(0, fields.size());
     }
 
     @Test
@@ -408,7 +405,6 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
     }
 
     private Repo parseRepo(ObjectMapper mapper, JsonParser parser) throws IOException {
-        System.out.println("parseRepo ");
         parser.nextToken();
         parser.nextToken();
         ObjectReader repoReader = mapper.readerFor(Repo.class);

From 655a30c3093b832ff565e3d2c81a49e199fdfe02 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Tue, 13 Jun 2017 08:43:47 +1000
Subject: [PATCH 11/12] Use integer instead of boolean.

There is some magic conversions going on so that booleans get
converted into integers, but they are only on Android. Under
robolectric, it throws a class cast exception instead.
---
 app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java  | 2 +-
 .../test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java
index ce19186dd..ad1d84816 100644
--- a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java
+++ b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java
@@ -278,7 +278,7 @@ public class SwapService extends Service {
             if (!TextUtils.isEmpty(fingerprint)) {
                 values.put(Schema.RepoTable.Cols.FINGERPRINT, peer.getFingerprint());
             }
-            values.put(Schema.RepoTable.Cols.IN_USE, true);
+            values.put(Schema.RepoTable.Cols.IN_USE, 1);
             values.put(Schema.RepoTable.Cols.IS_SWAP, true);
             Uri uri = RepoProvider.Helper.insert(this, values);
             repo = RepoProvider.Helper.get(this, uri);
diff --git a/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java b/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java
index 22bab5077..8358613e0 100644
--- a/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java
+++ b/app/src/test/java/org/fdroid/fdroid/updater/Issue763MultiRepo.java
@@ -38,7 +38,7 @@ public class Issue763MultiRepo extends MultiRepoUpdaterTest {
 
     public void setEnabled(Repo repo, boolean enabled) {
         ContentValues values = new ContentValues(1);
-        values.put(Schema.RepoTable.Cols.IN_USE, enabled);
+        values.put(Schema.RepoTable.Cols.IN_USE, enabled ? 1 : 0);
         RepoProvider.Helper.update(context, repo, values);
     }
 

From b092d5240320a550607c778874db9f50b8121e2f Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Tue, 13 Jun 2017 09:16:53 +1000
Subject: [PATCH 12/12] Find better way to detach DB which doesn't pollute test
 output

---
 .../java/org/fdroid/fdroid/data/TempAppProvider.java  | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java
index 3c2ed7d2b..79d61817c 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java
@@ -209,11 +209,14 @@ public class TempAppProvider extends AppProvider {
 
     private void ensureTempTableDetached(SQLiteDatabase db) {
         try {
+            // Ideally we'd ask SQLite if the temp table is attached, but that is not possible.
+            // Instead, we resort to hackery:
+            // If the first statement does not throw an exception, then the temp db is attached and the second
+            // statement will detach the database.
+            db.rawQuery("SELECT * FROM " + DB + "." + getTableName() + " WHERE 0", null);
             db.execSQL("DETACH DATABASE " + DB);
-        } catch (SQLiteException e) {
-            // We expect that most of the time the database will not exist unless an error occurred
-            // midway through the last update, The resulting exception is:
-            //   android.database.sqlite.SQLiteException: no such database: temp_update_db (code 1)
+        } catch (SQLiteException ignored) {
+
         }
     }