From c6efdbb20c17fffea7cd8c012772e125c02a5dec Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Tue, 4 Apr 2017 16:26:42 +1000 Subject: [PATCH 1/2] Repos added later should take higher priority. The database still treats repos with a _low_ number as _low_ priority. This means it sounds weird when you say "Repo with priority 1 is the least important", but other than that, everything works as expected now. Technically we could recreate the query to update the repo metadata within DBHelper, but that is difficult because it is sort of build into the content providers. Unfortunately, we are unable to access content providers from the DBHelper. In the future if we are able to migrate away from content providers to a more dumb data access layer, then we could reuse the query to update the metadata priorities in the DBHelper. However that is a tomorrow problem. --- .../java/org/fdroid/fdroid/data/AppProvider.java | 2 +- .../main/java/org/fdroid/fdroid/data/DBHelper.java | 12 +++++++++++- 2 files changed, 12 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 c4cf76521..df1474543 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -960,7 +960,7 @@ public class AppProvider extends FDroidProvider { final String app = getTableName(); final String highestPriority = - "SELECT MIN(r." + RepoTable.Cols.PRIORITY + ") " + + "SELECT MAX(r." + RepoTable.Cols.PRIORITY + ") " + "FROM " + RepoTable.NAME + " AS r " + "JOIN " + getTableName() + " AS m ON (m." + Cols.REPO_ID + " = r." + RepoTable.Cols._ID + ") " + "WHERE m." + Cols.PACKAGE_ID + " = " + "metadata." + Cols.PACKAGE_ID; 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 079972d5b..0dd87abb9 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -190,7 +190,7 @@ class DBHelper extends SQLiteOpenHelper { + InstalledAppTable.Cols.HASH + " TEXT NOT NULL" + " );"; - protected static final int DB_VERSION = 67; + protected static final int DB_VERSION = 68; private final Context context; @@ -272,6 +272,16 @@ class DBHelper extends SQLiteOpenHelper { addCategoryTables(db, oldVersion); addIndexV1Fields(db, oldVersion); addIndexV1AppFields(db, oldVersion); + recalculatePreferredMetadata(db, oldVersion); + } + + private void recalculatePreferredMetadata(SQLiteDatabase db, int oldVersion) { + if (oldVersion >= 68) { + return; + } + + Log.i(TAG, "Previously, the repository metadata was being interpreted backwards. Need to force a repo refresh to fix this."); + resetTransient(db); } private void addIndexV1AppFields(SQLiteDatabase db, int oldVersion) { From 1aea1c930217323768d262257cf7c51f5c77c4a5 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 5 Apr 2017 09:09:51 +1000 Subject: [PATCH 2/2] Updated tests to work with flipped repo priorities. --- .../fdroid/data/CategoryProviderTest.java | 33 ++++++++++++++----- .../updater/ProperMultiRepoUpdaterTest.java | 16 ++++----- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java index 72b5dfbe0..986491b42 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java @@ -44,8 +44,7 @@ public class CategoryProviderTest extends FDroidProviderTest { insertAppWithCategory("com.dog.rock.apple", "Dog-Rock-Apple", "Animal,Mineral,Vegetable", mainRepo); insertAppWithCategory("com.banana.apple", "Banana", "Vegetable,Vegetable", mainRepo); - List categories = CategoryProvider.Helper.categories(context); - String[] expected = new String[] { + String[] expectedFDroid = new String[] { context.getResources().getString(R.string.category_Whats_New), context.getResources().getString(R.string.category_Recently_Updated), context.getResources().getString(R.string.category_All), @@ -56,13 +55,8 @@ public class CategoryProviderTest extends FDroidProviderTest { "Vegetable", "Writing", }; - assertContainsOnly(categories, expected); - insertAppWithCategory("info.guardianproject.notepadbot", "NoteCipher", "Office,GuardianProject", gpRepo); - assertContainsOnly(CategoryProvider.Helper.categories(context), expected); - - RepoProvider.Helper.purgeApps(context, new MockRepo(mainRepo)); - String[] expectedGp = new String[] { + String[] expectedGP = new String[] { context.getResources().getString(R.string.category_Whats_New), context.getResources().getString(R.string.category_Recently_Updated), context.getResources().getString(R.string.category_All), @@ -70,8 +64,29 @@ public class CategoryProviderTest extends FDroidProviderTest { "GuardianProject", "Office", }; + + // We overwrite "Security" + "Writing" with "GuardianProject" + "Office" + String[] expectedBoth = new String[] { + context.getResources().getString(R.string.category_Whats_New), + context.getResources().getString(R.string.category_Recently_Updated), + context.getResources().getString(R.string.category_All), + + "Animal", + "Mineral", + "Vegetable", + + "GuardianProject", + "Office", + }; + + assertContainsOnly(CategoryProvider.Helper.categories(context), expectedFDroid); + + insertAppWithCategory("info.guardianproject.notepadbot", "NoteCipher", "Office,GuardianProject", gpRepo); + assertContainsOnly(CategoryProvider.Helper.categories(context), expectedBoth); + + RepoProvider.Helper.purgeApps(context, new MockRepo(mainRepo)); List categoriesAfterPurge = CategoryProvider.Helper.categories(context); - assertContainsOnly(categoriesAfterPurge, expectedGp); + assertContainsOnly(categoriesAfterPurge, expectedGP); } @Test 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 578391376..db9e3267e 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -95,15 +95,15 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { @Test public void metadataWithRepoPriority() throws RepoUpdater.UpdateException { - updateConflicting(); updateMain(); updateArchive(); + updateConflicting(); - Repo conflictingRepo = RepoProvider.Helper.findByAddress(context, REPO_CONFLICTING_URI); + Repo mainRepo = RepoProvider.Helper.findByAddress(context, REPO_MAIN_URI); - assertEquals(1, conflictingRepo.priority); - assertEquals(2, RepoProvider.Helper.findByAddress(context, REPO_MAIN_URI).priority); - assertEquals(3, RepoProvider.Helper.findByAddress(context, REPO_ARCHIVE_URI).priority); + assertEquals(1, mainRepo.priority); + assertEquals(2, RepoProvider.Helper.findByAddress(context, REPO_ARCHIVE_URI).priority); + assertEquals(3, RepoProvider.Helper.findByAddress(context, REPO_CONFLICTING_URI).priority); assertMainRepo(); assertMainArchiveRepoMetadata(); @@ -114,9 +114,9 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { // Make the conflicting repo less important than the main repo. ContentValues values = new ContentValues(1); values.put(Cols.PRIORITY, 5); - RepoProvider.Helper.update(context, conflictingRepo, values); - Repo updatedConflictingRepo = RepoProvider.Helper.findByAddress(context, REPO_CONFLICTING_URI); - assertEquals(5, updatedConflictingRepo.priority); + RepoProvider.Helper.update(context, mainRepo, values); + Repo updatedMainRepo = RepoProvider.Helper.findByAddress(context, REPO_MAIN_URI); + assertEquals(5, updatedMainRepo.priority); assertRepoTakesPriority("Normal"); }