Refactor ApkProvider.getApkFromAnyRepo to getSuggestedApk(Apk).

This is really the intention of the method, given it used to accept
a version code and a package name. Now it optionally accepts a sig
also. If present, it will restrict the query to apks with that sig.

Also added to the multi-sig tests to ensure this method takes it into
consideration.

app/src/main/java/org/fdroid/fdroid/AppDetails2.java

@@ -729,13 +729,13 @@ public class AppDetails2 extends AppCompatActivity implements ShareChooserDialog
 
     @Override
     public void installApk() {
-        Apk apkToInstall = ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode);
+        Apk apkToInstall = ApkProvider.Helper.findSuggestedApk(this, app);
         installApk(apkToInstall);
     }
 
     @Override
     public void upgradeApk() {
-        Apk apkToInstall = ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode);
+        Apk apkToInstall = ApkProvider.Helper.findSuggestedApk(this, app);
         installApk(apkToInstall);
     }
 

app/src/main/java/org/fdroid/fdroid/UpdateService.java

@@ -499,7 +499,7 @@ public class UpdateService extends IntentService {
             cursor.moveToFirst();
             for (int i = 0; i < cursor.getCount(); i++) {
                 App app = new App(cursor);
-                Apk apk = ApkProvider.Helper.findApkFromAnyRepo(context, app.packageName, app.suggestedVersionCode);
+                Apk apk = ApkProvider.Helper.findSuggestedApk(context, app);
                 InstallManagerService.queue(context, app, apk);
                 cursor.moveToNext();
             }
@@ -514,7 +514,7 @@ public class UpdateService extends IntentService {
             for (int i = 0; i < hasUpdates.getCount(); i++) {
                 App app = new App(hasUpdates);
                 hasUpdates.moveToNext();
-                apksToUpdate.add(ApkProvider.Helper.findApkFromAnyRepo(this, app.packageName, app.suggestedVersionCode));
+                apksToUpdate.add(ApkProvider.Helper.findSuggestedApk(this, app));
             }
             appUpdateStatusManager.addApks(apksToUpdate, AppUpdateStatusManager.Status.UpdateAvailable);
         }

app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java

@@ -7,6 +7,7 @@ import android.content.UriMatcher;
 import android.database.Cursor;
 import android.net.Uri;
 import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.util.Log;
 
 import org.fdroid.fdroid.data.Schema.ApkTable;
@@ -74,8 +75,16 @@ public class ApkProvider extends FDroidProvider {
             return resolver.delete(uri, null, null);
         }
 
+        public static Apk findSuggestedApk(Context context, App app) {
+            return findApkFromAnyRepo(context, app.packageName, app.suggestedVersionCode, app.installedSig);
+        }
+
         public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode) {
-            return findApkFromAnyRepo(context, packageName, versionCode, Cols.ALL);
+            return findApkFromAnyRepo(context, packageName, versionCode, null, Cols.ALL);
+        }
+
+        public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode, String signature) {
+            return findApkFromAnyRepo(context, packageName, versionCode, signature, Cols.ALL);
         }
 
         /**
@@ -89,9 +98,9 @@ public class ApkProvider extends FDroidProvider {
             return cursorToList(cursor);
         }
 
-        public static Apk findApkFromAnyRepo(Context context,
+        public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode,
-                                             String packageName, int versionCode, String[] projection) {
+                                             @Nullable String signature, String[] projection) {
-            final Uri uri = getApkFromAnyRepoUri(packageName, versionCode);
+            final Uri uri = getApkFromAnyRepoUri(packageName, versionCode, signature);
             return findByUri(context, uri, projection);
         }
 
@@ -113,8 +122,7 @@ public class ApkProvider extends FDroidProvider {
             return findByPackageName(context, packageName, Cols.ALL);
         }
 
-        public static List<Apk> findByPackageName(Context context,
+        public static List<Apk> findByPackageName(Context context, String packageName, String[] projection) {
-                                                  String packageName, String[] projection) {
             ContentResolver resolver = context.getContentResolver();
             final Uri uri = getAppUri(packageName);
             final String sort = "apk." + Cols.VERSION_CODE + " DESC";
@@ -218,6 +226,7 @@ public class ApkProvider extends FDroidProvider {
         PACKAGE_FIELDS.put(Cols.Package.PACKAGE_NAME, PackageTable.Cols.PACKAGE_NAME);
 
         MATCHER.addURI(getAuthority(), PATH_REPO + "/#", CODE_REPO);
+        MATCHER.addURI(getAuthority(), PATH_APK_FROM_ANY_REPO + "/#/*/*", CODE_APK_FROM_ANY_REPO);
         MATCHER.addURI(getAuthority(), PATH_APK_FROM_ANY_REPO + "/#/*", CODE_APK_FROM_ANY_REPO);
         MATCHER.addURI(getAuthority(), PATH_APK_FROM_REPO + "/#/#", CODE_APK_FROM_REPO);
         MATCHER.addURI(getAuthority(), PATH_APKS + "/*", CODE_APKS);
@@ -260,16 +269,21 @@ public class ApkProvider extends FDroidProvider {
     }
 
     public static Uri getApkFromAnyRepoUri(Apk apk) {
-        return getApkFromAnyRepoUri(apk.packageName, apk.versionCode);
+        return getApkFromAnyRepoUri(apk.packageName, apk.versionCode, null);
     }
 
-    public static Uri getApkFromAnyRepoUri(String packageName, int versionCode) {
+    public static Uri getApkFromAnyRepoUri(String packageName, int versionCode, @Nullable String signature) {
-        return getContentUri()
+        Uri.Builder builder = getContentUri()
                 .buildUpon()
                 .appendPath(PATH_APK_FROM_ANY_REPO)
                 .appendPath(Integer.toString(versionCode))
-            .appendPath(packageName)
+                .appendPath(packageName);
-            .build();
+
+        if (signature != null) {
+            builder.appendPath(signature);
+        }
+
+        return builder.build();
     }
 
     public static Uri getContentUriForApps(Repo repo, List<App> apps) {
@@ -395,20 +409,20 @@ public class ApkProvider extends FDroidProvider {
     private QuerySelection querySingleFromAnyRepo(Uri uri, boolean includeAlias) {
         String alias = includeAlias ? "apk." : "";
 
-        // TODO: Technically multiple repositories can provide the apk with this version code.
+        String selection =
-        //       Therefore, in the very near future we'll need to change from calculating a
+                alias + Cols.VERSION_CODE + " = ? AND " +
-        //       "suggested version code" to a "suggested apk" and join directly onto the apk table.
+                alias + Cols.APP_ID + " IN (" + getMetadataIdFromPackageNameQuery() + ")";
-        //       This way, we can take into account both repo priorities and signing keys of any
+
-        //       already installed apks to ensure that the best version is suggested to the user.
+        List<String> pathSegments = uri.getPathSegments();
-        //       At this point, we may pull back the "wrong" apk in weird edge cases, but the user
+        List<String> args = new ArrayList<>(3);
-        //       wont be tricked into installing it, as it will (likely) have a different signing key.
+        args.add(pathSegments.get(1)); // First (0th) path segment is the word "apk" and we are not interested in it.
-        final String selection = alias + Cols.VERSION_CODE + " = ? and " + alias + Cols.APP_ID + " IN (" + getMetadataIdFromPackageNameQuery() + ")";
+        args.add(pathSegments.get(2));
-        final String[] args = {
+
-            // First (0th) path segment is the word "apk",
+        if (pathSegments.size() >= 4) {
-            // and we are not interested in it.
+            selection += " AND " + alias + Cols.SIGNATURE + " = ? ";
-            uri.getPathSegments().get(1),
+            args.add(pathSegments.get(3));
-            uri.getPathSegments().get(2),
+        }
-        };
+
         return new QuerySelection(selection, args);
     }
 

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

@@ -1048,7 +1048,7 @@ public class AppProvider extends FDroidProvider {
     }
 
     /**
-     * Returns a query which requires two parameters to be bound. These are (in order):
+     * Returns a query which requires two parameters to be bdeatound. These are (in order):
      *  1) The repo version that introduced density specific icons
      *  2) The dir to density specific icons for the current device.
      */

app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java

@@ -535,7 +535,7 @@ public class AppListItemController extends RecyclerView.ViewHolder {
                 Installer installer = InstallerFactory.create(activity, currentStatus.apk);
                 installer.installPackage(Uri.parse(apkFilePath.toURI().toString()), Uri.parse(currentStatus.apk.getUrl()));
             } else {
-                final Apk suggestedApk = ApkProvider.Helper.findApkFromAnyRepo(activity, currentApp.packageName, currentApp.suggestedVersionCode);
+                final Apk suggestedApk = ApkProvider.Helper.findSuggestedApk(activity, currentApp);
                 InstallManagerService.queue(activity, currentApp, suggestedApk);
             }
         }

app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java

@@ -68,7 +68,7 @@ public class ApkProviderTest extends FDroidProviderTest {
         Apk apk = new MockApk("org.fdroid.fdroid", 10);
 
         assertCantDelete(contentResolver, ApkProvider.getContentUri());
-        assertCantDelete(contentResolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 10));
+        assertCantDelete(contentResolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 10, null));
         assertCantDelete(contentResolver, ApkProvider.getApkFromAnyRepoUri(apk));
         assertCantDelete(contentResolver, Uri.withAppendedPath(ApkProvider.getContentUri(), "some-random-path"));
     }
@@ -432,7 +432,7 @@ public class ApkProviderTest extends FDroidProviderTest {
             Cols.HASH,
         };
 
-        Apk apkLessFields = ApkProvider.Helper.findApkFromAnyRepo(context, "com.example", 11, projection);
+        Apk apkLessFields = ApkProvider.Helper.findApkFromAnyRepo(context, "com.example", 11, null, projection);
 
         assertNotNull(apkLessFields);
 

app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java

@@ -141,7 +141,7 @@ public class ProviderUriTests {
         assertValidUri(resolver, ApkProvider.getAppUri("org.fdroid.fdroid"), "content://org.fdroid.fdroid.data.ApkProvider/app/org.fdroid.fdroid", projection);
         assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri(new MockApk("org.fdroid.fdroid", 100)), "content://org.fdroid.fdroid.data.ApkProvider/apk-any-repo/100/org.fdroid.fdroid", projection);
         assertValidUri(resolver, ApkProvider.getContentUri(apks), projection);
-        assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 100), "content://org.fdroid.fdroid.data.ApkProvider/apk-any-repo/100/org.fdroid.fdroid", projection);
+        assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 100, null), "content://org.fdroid.fdroid.data.ApkProvider/apk-any-repo/100/org.fdroid.fdroid", projection);
         assertValidUri(resolver, ApkProvider.getRepoUri(1000), "content://org.fdroid.fdroid.data.ApkProvider/repo/1000", projection);
     }
 

app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java

@@ -20,6 +20,8 @@ import org.robolectric.annotation.Config;
 import java.security.NoSuchAlgorithmException;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 @Config(constants = BuildConfig.class, application = Application.class, sdk = 24)
 @RunWith(RobolectricTestRunner.class)
@@ -68,18 +70,68 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         App found2 = findApp(singleApp);
         assertEquals(2, found2.suggestedVersionCode);
 
+        Apk found2Apk = findApk(found2);
+        assertEquals(2, found2Apk.versionCode);
+        assertEquals(FDROID_SIG, found2Apk.sig);
+
         // 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 = findApp(singleApp);
         assertEquals(3, found3.suggestedVersionCode);
+
+        Apk found3Apk = findApk(found3);
+        assertEquals(3, found3Apk.versionCode);
+        assertEquals(FDROID_SIG, found3Apk.sig);
     }
 
     private App findApp(App app) {
         return AppProvider.Helper.findSpecificApp(context.getContentResolver(), app.packageName, app.repoId);
     }
 
+    private Apk findApk(App app) {
+        return ApkProvider.Helper.findSuggestedApk(context, app);
+    }
+
+    @Test
+    public void suggestedApkQuery() {
+        App singleApp = insertApp(context, "single.app", "Single App", 0, "https://simple.repo");
+        insertApk(context, singleApp, 1, FDROID_SIG);
+        insertApk(context, singleApp, 1, UPSTREAM_SIG);
+        insertApk(context, singleApp, 2, FDROID_SIG);
+        insertApk(context, singleApp, 2, UPSTREAM_SIG);
+        insertApk(context, singleApp, 3, FDROID_SIG);
+        insertApk(context, singleApp, 3, UPSTREAM_SIG);
+        insertApk(context, singleApp, 3, THIRD_PARTY_SIG);
+        insertApk(context, singleApp, 4, FDROID_SIG);
+        insertApk(context, singleApp, 4, UPSTREAM_SIG);
+        insertApk(context, singleApp, 4, THIRD_PARTY_SIG);
+        insertApk(context, singleApp, 5, FDROID_SIG);
+        insertApk(context, singleApp, 5, UPSTREAM_SIG);
+        AppProvider.Helper.calcSuggestedApks(context);
+
+        App notInstalled = findApp(singleApp);
+        Apk suggestedApkForNotInstalled = findApk(notInstalled);
+        assertNull(notInstalled.installedSig);
+
+        // It could be either of these, I think it is actually non-deterministic as to which is chosen.
+        // TODO: Make it deterministic based on repo priority.
+        assertTrue(FDROID_SIG.equals(suggestedApkForNotInstalled.sig) ||
+                UPSTREAM_SIG.equals(suggestedApkForNotInstalled.sig));
+        assertEquals(5, suggestedApkForNotInstalled.versionCode);
+
+        InstalledAppTestUtils.install(context, "single.app", 3, "v1", THIRD_PARTY_CERT);
+        AppProvider.Helper.calcSuggestedApks(context);
+
+        App installed = findApp(singleApp);
+        Apk suggestedApkForInstalled = findApk(installed);
+        assertEquals(THIRD_PARTY_SIG, installed.installedSig);
+        assertEquals(4, installed.suggestedVersionCode);
+        assertEquals(THIRD_PARTY_SIG, suggestedApkForInstalled.sig);
+        assertEquals(4, suggestedApkForInstalled.versionCode);
+    }
+
     @Test
     public void singleRepoMultiSig() {
         App unrelatedApp = insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo");
@@ -98,6 +150,10 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         App suggestUpstream4 = findApp(singleApp);
         assertEquals(4, suggestUpstream4.suggestedVersionCode);
 
+        Apk suggestedUpstream4Apk = findApk(suggestUpstream4);
+        assertEquals(4, suggestedUpstream4Apk.versionCode);
+        assertEquals(UPSTREAM_SIG, suggestedUpstream4Apk.sig);
+
         // 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);
@@ -105,6 +161,10 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         App suggestFDroid3 = findApp(singleApp);
         assertEquals(3, suggestFDroid3.suggestedVersionCode);
 
+        Apk suggestedFDroid3Apk = findApk(suggestFDroid3);
+        assertEquals(3, suggestedFDroid3Apk.versionCode);
+        assertEquals(FDROID_SIG, suggestedFDroid3Apk.sig);
+
         // 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);
@@ -112,12 +172,20 @@ public class SuggestedVersionTest extends FDroidProviderTest {
         App suggestFDroid4 = findApp(singleApp);
         assertEquals(4, suggestFDroid4.suggestedVersionCode);
 
+        Apk suggestedFDroid4Apk = findApk(suggestFDroid4);
+        assertEquals(4, suggestedFDroid4Apk.versionCode);
+        assertEquals(FDROID_SIG, suggestedFDroid4Apk.sig);
+
         // 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 = findApp(singleApp);
         assertEquals(5, suggestFDroid5.suggestedVersionCode);
+
+        Apk suggestedFDroid5Apk = findApk(suggestFDroid5);
+        assertEquals(5, suggestedFDroid5Apk.versionCode);
+        assertEquals(FDROID_SIG, suggestedFDroid5Apk.sig);
     }
 
     private void recalculateMetadata() {