From b1a1c68ad7cab9306762bdb30722e4c75a041224 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 28 Apr 2017 09:39:24 +1000 Subject: [PATCH 1/3] Improved tests to catch incorrect suggested versions with multi repos. This is the bug outlined in #974. This commit just adds a test which catches the problem, but does not fix it yet. --- .../updater/ProperMultiRepoUpdaterTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java index db9e3267e..c2baf6369 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -11,6 +11,7 @@ import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; +import org.fdroid.fdroid.data.InstalledAppTestUtils; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.Schema; @@ -68,6 +69,9 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertEmpty(); updateMain(); assertMainRepo(); + + // Even though there is a version 54 in the repo, version 53 is marked as the current version. + assertCanUpdate("org.adaway", 49, 53); } @Test @@ -75,6 +79,8 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertEmpty(); updateArchive(); assertMainArchiveRepoMetadata(); + + assertCanUpdate("org.adaway", 49, 51); } @Test @@ -82,6 +88,8 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertEmpty(); updateConflicting(); assertConflictingRepo(); + + assertCanUpdate("org.adaway", 49, 53); } private Map allApps() { @@ -228,6 +236,17 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertMainRepo(repos); assertMainArchiveRepoMetadata(repos); assertConflictingRepo(repos); + + // Even though there is a version 54 in the repo, version 53 is marked as the current version. + assertCanUpdate("org.adaway", 49, 53); + } + + private void assertCanUpdate(String packageName, int installedVersion, int expectedUpdateVersion) { + InstalledAppTestUtils.install(context, packageName, installedVersion, "v" + installedVersion); + List appsToUpdate = AppProvider.Helper.findCanUpdate(context, AppMetadataTable.Cols.ALL); + assertEquals(1, appsToUpdate.size()); + assertEquals(installedVersion, appsToUpdate.get(0).installedVersionCode); + assertEquals(expectedUpdateVersion, appsToUpdate.get(0).suggestedVersionCode); } private void assertMainRepo() { From 294e1d2821e4aa4cff39f0739cfec1561e588971 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 28 Apr 2017 10:14:45 +1000 Subject: [PATCH 2/3] Simulate armeabi as the architecture during multi-repo test This allows us to test "installing" Adaway, which has a native code dependency that the default Robolectric setup doesn't support (defaults to armeabi-v7a). --- .../updater/ProperMultiRepoUpdaterTest.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java index c2baf6369..f8f55dc0d 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -21,6 +21,9 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.shadows.ShadowSystemProperties; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -31,7 +34,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -@Config(constants = BuildConfig.class, sdk = 24) +@Config(constants = BuildConfig.class, sdk = 24, shadows = ProperMultiRepoUpdaterTest.ArmSystemProperties.class) @RunWith(RobolectricTestRunner.class) public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { private static final String TAG = "ProperMultiRepoSupport"; @@ -405,4 +408,23 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertCalendarMetadata(repo, "Conflicting"); } + /** + * Allows us to customize the result of Build.SUPPORTED_ABIS. + * In these tests, we want to "install" and check for updates of Adaway, but that depends + * on the armeabi, x86, or mips architectures, whereas the {@link ShadowSystemProperties} + * only returns armeabi-v7a by default. + * Based on https://groups.google.com/d/msg/robolectric/l_W2EbOek6s/O-GTce8jBQAJ. + */ + @Implements(className = "android.os.SystemProperties") + public static class ArmSystemProperties extends ShadowSystemProperties { + @Implementation + @SuppressWarnings("unused") + public static String get(String key) { + if ("ro.product.cpu.abilist".equals(key)) { + return "armeabi"; + } + return ShadowSystemProperties.get(key); + } + } + } From 6c08e054f55463bb759272f9c1e1b119d0444c25 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Fri, 28 Apr 2017 10:50:27 +1000 Subject: [PATCH 3/3] Calc suggested versioncode properly, regardless of repo priorities. There was a bug where the repo with the highest priority would be responsible for specifying the suggested version code. When doing so, it would only select from the list of apks available in that repo. This improves the calculation so that when any given repos app gets a suggested version code assigned, it selects from _all_ available apks, not just those from the repository in question. Fixes #974. --- .../org/fdroid/fdroid/data/AppProvider.java | 28 +++++++++++++++++-- 1 file changed, 26 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 60056b62f..c7cf1fd04 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -957,12 +957,13 @@ 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 "); + String updateSql = "UPDATE " + app + " SET " + Cols.SUGGESTED_VERSION_CODE + " = ( " + " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " + " FROM " + apk + " WHERE " + - app + "." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + " AND " + + joinToApksRegardlessOfRepo() + " AND " + restrictToStable + " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " + " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 "; @@ -970,6 +971,29 @@ 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. @@ -989,7 +1013,7 @@ public class AppProvider extends FDroidProvider { " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " + " FROM " + apk + " WHERE " + - app + "." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + " AND " + + joinToApksRegardlessOfRepo() + " 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 ";