From 0730d3c67685af0d29e977b9b258a9b45940065e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Sat, 2 Sep 2017 22:23:51 +0200 Subject: [PATCH] set App.preferredSigner when using index v0 #1086 This was an oversight when we added this functionality, though there was a related TODO. 41f85f3c9df934daba0ee0d60c4c01bb071fa6e7 --- .../org/fdroid/fdroid/RepoXMLHandler.java | 4 ++++ .../main/java/org/fdroid/fdroid/data/App.java | 12 +++++----- .../java/org/fdroid/fdroid/TestUtils.java | 7 ++---- .../fdroid/fdroid/data/AppProviderTest.java | 19 ++++++++------- .../fdroid/data/SuggestedVersionTest.java | 24 ++++++++++++------- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java index 8a51c1697..0b1fc1a9b 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java @@ -138,6 +138,10 @@ public class RepoXMLHandler extends DefaultHandler { break; case ApkTable.Cols.SIGNATURE: curapk.sig = str; + // the first APK in the list provides the preferred signature + if (curapp.preferredSigner == null) { + curapp.preferredSigner = str; + } break; case ApkTable.Cols.SOURCE_NAME: curapk.srcname = str; diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 49cc65a02..16e2925a1 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -15,6 +15,7 @@ import android.os.Environment; import android.os.LocaleList; import android.os.Parcel; import android.os.Parcelable; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.text.TextUtils; import android.util.Log; @@ -101,6 +102,7 @@ public class App extends ValueObject implements Comparable, Parcelable { @JsonIgnore private AppPrefs prefs; @JsonIgnore + @NonNull public String preferredSigner; @JacksonInject("repoId") @@ -159,6 +161,7 @@ public class App extends ValueObject implements Comparable, Parcelable { * The index-v1 metadata uses the term `suggestedVersionCode` but we need that * value to end up in the `upstreamVersionCode` property here. These variables * need to be renamed across the whole F-Droid ecosystem to make sense. + * * @see issue #1063 */ @JsonProperty("suggestedVersionCode") @@ -210,7 +213,7 @@ public class App extends ValueObject implements Comparable, Parcelable { } @Override - public int compareTo(App app) { + public int compareTo(@NonNull App app) { return name.compareToIgnoreCase(app.name); } @@ -1137,17 +1140,14 @@ public class App extends ValueObject implements Comparable, Parcelable { * However, if the app is installed, then we override this and instead want to only encourage * the user to try and install versions with that signature (because thats all the OS will let * them do). - * TODO: I don't think preferredSigner should ever be null, because if an app has apks then - * we should have chosen the first and used that. If so, then we should change to @NonNull and - * throw an exception if it is null. */ - @Nullable + @NonNull public String getMostAppropriateSignature() { if (!TextUtils.isEmpty(installedSig)) { return installedSig; } else if (!TextUtils.isEmpty(preferredSigner)) { return preferredSigner; } - return null; + throw new IllegalStateException("Most Appropriate Signature not found!"); } } diff --git a/app/src/test/java/org/fdroid/fdroid/TestUtils.java b/app/src/test/java/org/fdroid/fdroid/TestUtils.java index 29db4acd7..dee0ec9c5 100644 --- a/app/src/test/java/org/fdroid/fdroid/TestUtils.java +++ b/app/src/test/java/org/fdroid/fdroid/TestUtils.java @@ -95,12 +95,9 @@ public class TestUtils { } public static App insertApp(Context context, String packageName, String appName, int upstreamVersionCode, - String repoUrl) { + String repoUrl, String preferredSigner) { Repo repo = ensureRepo(context, repoUrl); - ContentValues values = new ContentValues(); - values.put(Schema.AppMetadataTable.Cols.REPO_ID, repo.getId()); - values.put(Schema.AppMetadataTable.Cols.UPSTREAM_VERSION_CODE, upstreamVersionCode); - return Assert.insertApp(context, packageName, appName, values); + return insertApp(context, packageName, appName, upstreamVersionCode, repo, preferredSigner); } public static App insertApp(Context context, String packageName, String appName, int upstreamVersionCode, diff --git a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java index 9152ded6a..f6c00c306 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java @@ -6,7 +6,6 @@ import android.content.ContentValues; import android.content.Context; import android.database.Cursor; import android.net.Uri; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.TestUtils; @@ -135,11 +134,11 @@ public class AppProviderTest extends FDroidProviderTest { assertResultCount(contentResolver, 2, AppProvider.getCanUpdateUri(), PROJ); assertResultCount(contentResolver, 9, AppProvider.getInstalledUri(), PROJ); - App installedOnlyOneVersionAvailable = AppProvider.Helper.findSpecificApp(r, "installed, only one version available", 1, Cols.ALL); - App installedAlreadyLatestNoIgnore = AppProvider.Helper.findSpecificApp(r, "installed, already latest, no ignore", 1, Cols.ALL); - App installedAlreadyLatestIgnoreAll = AppProvider.Helper.findSpecificApp(r, "installed, already latest, ignore all", 1, Cols.ALL); + App installedOnlyOneVersionAvailable = AppProvider.Helper.findSpecificApp(r, "installed, only one version available", 1, Cols.ALL); + App installedAlreadyLatestNoIgnore = AppProvider.Helper.findSpecificApp(r, "installed, already latest, no ignore", 1, Cols.ALL); + App installedAlreadyLatestIgnoreAll = AppProvider.Helper.findSpecificApp(r, "installed, already latest, ignore all", 1, Cols.ALL); App installedAlreadyLatestIgnoreLatest = AppProvider.Helper.findSpecificApp(r, "installed, already latest, ignore latest", 1, Cols.ALL); - App installedAlreadyLatestIgnoreOld = AppProvider.Helper.findSpecificApp(r, "installed, already latest, ignore old", 1, Cols.ALL); + App installedAlreadyLatestIgnoreOld = AppProvider.Helper.findSpecificApp(r, "installed, already latest, ignore old", 1, Cols.ALL); assertFalse(installedOnlyOneVersionAvailable.canAndWantToUpdate(context)); assertFalse(installedAlreadyLatestNoIgnore.canAndWantToUpdate(context)); @@ -147,9 +146,9 @@ public class AppProviderTest extends FDroidProviderTest { assertFalse(installedAlreadyLatestIgnoreLatest.canAndWantToUpdate(context)); assertFalse(installedAlreadyLatestIgnoreOld.canAndWantToUpdate(context)); - App installedOldNoIgnore = AppProvider.Helper.findSpecificApp(r, "installed, old version, no ignore", 1, Cols.ALL); - App installedOldIgnoreAll = AppProvider.Helper.findSpecificApp(r, "installed, old version, ignore all", 1, Cols.ALL); - App installedOldIgnoreLatest = AppProvider.Helper.findSpecificApp(r, "installed, old version, ignore latest", 1, Cols.ALL); + App installedOldNoIgnore = AppProvider.Helper.findSpecificApp(r, "installed, old version, no ignore", 1, Cols.ALL); + App installedOldIgnoreAll = AppProvider.Helper.findSpecificApp(r, "installed, old version, ignore all", 1, Cols.ALL); + App installedOldIgnoreLatest = AppProvider.Helper.findSpecificApp(r, "installed, old version, ignore latest", 1, Cols.ALL); App installedOldIgnoreNewerNotLatest = AppProvider.Helper.findSpecificApp(r, "installed, old version, ignore newer, but not latest", 1, Cols.ALL); assertTrue(installedOldNoIgnore.canAndWantToUpdate(context)); @@ -280,7 +279,7 @@ public class AppProviderTest extends FDroidProviderTest { } private Cursor queryAllApps() { - String[] projection = new String[] { + String[] projection = new String[]{ Cols._ID, Cols.NAME, Cols.Package.PACKAGE_NAME, @@ -314,6 +313,8 @@ public class AppProviderTest extends FDroidProviderTest { values.put(Cols.LICENSE, "GPL?"); values.put(Cols.IS_COMPATIBLE, 1); + values.put(Cols.PREFERRED_SIGNER, "eaa1d713b9c2a0475234a86d6539f910"); + values.putAll(additionalValues); Uri uri = AppProvider.getContentUri(); 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 447bd3301..4157de23b 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/SuggestedVersionTest.java @@ -1,7 +1,6 @@ package org.fdroid.fdroid.data; import android.app.Application; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.TestUtils; @@ -44,7 +43,7 @@ public class SuggestedVersionTest extends FDroidProviderTest { @Test public void singleRepoSingleSig() { App singleApp = TestUtils.insertApp( - context, "single.app", "Single App (with beta)", 2, "https://beta.simple.repo"); + context, "single.app", "Single App (with beta)", 2, "https://beta.simple.repo", TestUtils.FDROID_SIG); TestUtils.insertApk(context, singleApp, 1, TestUtils.FDROID_SIG); TestUtils.insertApk(context, singleApp, 2, TestUtils.FDROID_SIG); TestUtils.insertApk(context, singleApp, 3, TestUtils.FDROID_SIG); @@ -59,10 +58,12 @@ public class SuggestedVersionTest extends FDroidProviderTest { @Test public void singleRepoMultiSig() { - App unrelatedApp = TestUtils.insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo"); + App unrelatedApp = TestUtils.insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo", + TestUtils.FDROID_SIG); TestUtils.insertApk(context, unrelatedApp, 3, TestUtils.FDROID_SIG); - App singleApp = TestUtils.insertApp(context, "single.app", "Single App", 4, "https://simple.repo"); + App singleApp = TestUtils.insertApp(context, "single.app", "Single App", 4, "https://simple.repo", + TestUtils.UPSTREAM_SIG); TestUtils.insertApk(context, singleApp, 1, TestUtils.FDROID_SIG); TestUtils.insertApk(context, singleApp, 2, TestUtils.FDROID_SIG); TestUtils.insertApk(context, singleApp, 3, TestUtils.FDROID_SIG); @@ -93,12 +94,15 @@ public class SuggestedVersionTest extends FDroidProviderTest { @Test public void multiRepoMultiSig() { - App unrelatedApp = TestUtils.insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo"); + App unrelatedApp = TestUtils.insertApp(context, "noisy.app", "Noisy App", 3, "https://simple.repo", + TestUtils.FDROID_SIG); TestUtils.insertApk(context, unrelatedApp, 3, TestUtils.FDROID_SIG); - App mainApp = TestUtils.insertApp(context, "single.app", "Single App (Main repo)", 4, "https://main.repo"); + App mainApp = TestUtils.insertApp(context, "single.app", "Single App (Main repo)", 4, "https://main.repo", + TestUtils.FDROID_SIG); App thirdPartyApp = TestUtils.insertApp( - context, "single.app", "Single App (3rd party)", 4, "https://3rd-party.repo"); + context, "single.app", "Single App (3rd party)", 4, "https://3rd-party.repo", + TestUtils.THIRD_PARTY_SIG); TestUtils.insertApk(context, mainApp, 1, TestUtils.FDROID_SIG); TestUtils.insertApk(context, mainApp, 2, TestUtils.FDROID_SIG); @@ -147,7 +151,8 @@ public class SuggestedVersionTest extends FDroidProviderTest { @Test public void dontSuggestUpstreamVersions() { // By setting the "upstreamVersionCode" to 0, we are letting F-Droid choose the highest compatible version. - App mainApp = TestUtils.insertApp(context, "single.app", "Single App (Main repo)", 0, "https://main.repo"); + App mainApp = TestUtils.insertApp(context, "single.app", "Single App (Main repo)", 0, "https://main.repo", + TestUtils.UPSTREAM_SIG); TestUtils.insertApk(context, mainApp, 1, TestUtils.FDROID_SIG); TestUtils.insertApk(context, mainApp, 2, TestUtils.FDROID_SIG); @@ -180,6 +185,7 @@ public class SuggestedVersionTest extends FDroidProviderTest { /** * Same as {@link #assertSuggested(String, int, String, int)} except only for non installed apps. + * * @see #assertSuggested(String, int, String, int) */ private void assertSuggested(String packageName, int suggestedVersion) { @@ -189,7 +195,7 @@ public class SuggestedVersionTest extends FDroidProviderTest { /** * Checks that the app exists, that its suggested version code is correct, and that the apk which is "suggested" * has the correct signature. - * + *

* If {@param installedSig} is null then {@param installedVersion} is ignored and the signature of the suggested * apk is not checked. */