From 5be23b793e5941f8dd5b875038198713a628086c Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 17 Nov 2016 09:43:11 +1100 Subject: [PATCH 1/3] Added test to ensure categories are remoed when a repo is disabled. This will help diagnose, test, and prevent regressions of #806. --- .../fdroid/fdroid/data/AppProviderTest.java | 6 ++++- .../fdroid/data/CategoryProviderTest.java | 23 +++++++++++++++---- .../fdroid/fdroid/data/ProviderUriTests.java | 1 - 3 files changed, 23 insertions(+), 7 deletions(-) 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 00553448e..844e81dd9 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java @@ -275,10 +275,14 @@ public class AppProviderTest extends FDroidProviderTest { } public static App insertApp(ShadowContentResolver contentResolver, Context context, String id, String name, ContentValues additionalValues) { + return insertApp(contentResolver, context, id, name, additionalValues, 1); + } + + public static App insertApp(ShadowContentResolver contentResolver, Context context, String id, String name, ContentValues additionalValues, long repoId) { ContentValues values = new ContentValues(); values.put(Cols.Package.PACKAGE_NAME, id); - values.put(Cols.REPO_ID, 1); + values.put(Cols.REPO_ID, repoId); values.put(Cols.NAME, name); // Required fields (NOT NULL in the database). 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 ba6bcd48a..f093a7c89 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/CategoryProviderTest.java @@ -8,6 +8,7 @@ import android.net.Uri; import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.R; import org.fdroid.fdroid.data.Schema.AppMetadataTable.Cols; +import org.fdroid.fdroid.mock.MockRepo; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -107,9 +108,11 @@ public class CategoryProviderTest extends FDroidProviderTest { @Test public void testCategoriesMultiple() { - insertAppWithCategory("com.rock.dog", "Rock-Dog", "Mineral,Animal"); - insertAppWithCategory("com.dog.rock.apple", "Dog-Rock-Apple", "Animal,Mineral,Vegetable"); - insertAppWithCategory("com.banana.apple", "Banana", "Vegetable,Vegetable"); + long mainRepo = 1; + + insertAppWithCategory("com.rock.dog", "Rock-Dog", "Mineral,Animal", mainRepo); + 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[] { @@ -123,9 +126,11 @@ public class CategoryProviderTest extends FDroidProviderTest { }; assertContainsOnly(categories, expected); + int additionalRepo = 2; + insertAppWithCategory("com.example.game", "Game", "Running,Shooting,Jumping,Bleh,Sneh,Pleh,Blah,Test category," + - "The quick brown fox jumps over the lazy dog,With apostrophe's"); + "The quick brown fox jumps over the lazy dog,With apostrophe's", additionalRepo); List categoriesLonger = CategoryProvider.Helper.categories(context); String[] expectedLonger = new String[] { @@ -150,11 +155,19 @@ public class CategoryProviderTest extends FDroidProviderTest { }; assertContainsOnly(categoriesLonger, expectedLonger); + + RepoProvider.Helper.purgeApps(context, new MockRepo(additionalRepo)); + List categoriesAfterPurge = CategoryProvider.Helper.categories(context); + assertContainsOnly(categoriesAfterPurge, expected); } private void insertAppWithCategory(String id, String name, String categories) { + insertAppWithCategory(id, name, categories, 1); + } + + private void insertAppWithCategory(String id, String name, String categories, long repoId) { ContentValues values = new ContentValues(1); values.put(Cols.ForWriting.Categories.CATEGORIES, categories); - AppProviderTest.insertApp(contentResolver, context, id, name, values); + AppProviderTest.insertApp(contentResolver, context, id, name, values, repoId); } } diff --git a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java index 788c3aa31..2aa284e9d 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java @@ -88,7 +88,6 @@ public class ProviderUriTests { assertValidUri(resolver, AppProvider.getSearchUri("/"), "content://org.fdroid.fdroid.data.AppProvider/search/%2F", projection); assertValidUri(resolver, AppProvider.getSearchUri(""), "content://org.fdroid.fdroid.data.AppProvider", projection); assertValidUri(resolver, AppProvider.getSearchUri(null), "content://org.fdroid.fdroid.data.AppProvider", projection); - assertValidUri(resolver, AppProvider.getNoApksUri(), "content://org.fdroid.fdroid.data.AppProvider/noApks", projection); assertValidUri(resolver, AppProvider.getInstalledUri(), "content://org.fdroid.fdroid.data.AppProvider/installed", projection); assertValidUri(resolver, AppProvider.getCanUpdateUri(), "content://org.fdroid.fdroid.data.AppProvider/canUpdate", projection); From 99ad9752c86e3859871ed1c355219fd614d4a263 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 17 Nov 2016 09:45:40 +1100 Subject: [PATCH 2/3] Use repo foreign key from app rather than apk table. Since recently, app metadata now knows which repo it comes from. As such, we no longer need to ask the apk table for this info. --- app/src/main/java/org/fdroid/fdroid/data/AppProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2745d75c8..5bb05f96e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -562,7 +562,7 @@ public class AppProvider extends FDroidProvider { } private AppQuerySelection queryRepo(long repoId) { - final String selection = getApkTableName() + "." + ApkTable.Cols.REPO_ID + " = ? "; + final String selection = getTableName() + "." + Cols.REPO_ID + " = ? "; final String[] args = {String.valueOf(repoId)}; return new AppQuerySelection(selection, args); } From 18885a66c2bcc4924e9aa9d7bf7c4fc9630cc4d9 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 17 Nov 2016 09:46:23 +1100 Subject: [PATCH 3/3] Ensure app-category join table is cleared out properly upon disabling repo. There are certain things we can leave in the database even when they are not being used. The criteria for this is: * Could it be used again in the future? * Can it be excluded from queries easily while it is unused? Examples are entries in the package table, and entries in the category table. This fixes a problem where entries in the category-app join table stayed in the database, causing categories to be considered as "in use" when really there were no apps in those categories. These rows need to be removed, because when new apps are added again in the future, they will have different primary keys. These different primary keys mean that the rows in the category-app table will never be useful again, and thus should be removed. Fixes #806. --- .../org/fdroid/fdroid/data/AppProvider.java | 32 +++++++------------ .../org/fdroid/fdroid/data/RepoProvider.java | 4 +-- 2 files changed, 13 insertions(+), 23 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 5bb05f96e..3022fdfcb 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -369,7 +369,6 @@ public class AppProvider extends FDroidProvider { private static final String PATH_SEARCH_INSTALLED = "searchInstalled"; private static final String PATH_SEARCH_CAN_UPDATE = "searchCanUpdate"; private static final String PATH_SEARCH_REPO = "searchRepo"; - private static final String PATH_NO_APKS = "noApks"; protected static final String PATH_APPS = "apps"; protected static final String PATH_SPECIFIC_APP = "app"; private static final String PATH_RECENTLY_UPDATED = "recentlyUpdated"; @@ -383,8 +382,7 @@ public class AppProvider extends FDroidProvider { private static final int CAN_UPDATE = CODE_SINGLE + 1; private static final int INSTALLED = CAN_UPDATE + 1; private static final int SEARCH = INSTALLED + 1; - private static final int NO_APKS = SEARCH + 1; - private static final int RECENTLY_UPDATED = NO_APKS + 1; + private static final int RECENTLY_UPDATED = SEARCH + 1; private static final int NEWLY_ADDED = RECENTLY_UPDATED + 1; private static final int CATEGORY = NEWLY_ADDED + 1; private static final int CALC_SUGGESTED_APKS = CATEGORY + 1; @@ -408,7 +406,6 @@ public class AppProvider extends FDroidProvider { MATCHER.addURI(getAuthority(), PATH_REPO + "/#", REPO); MATCHER.addURI(getAuthority(), PATH_CAN_UPDATE, CAN_UPDATE); MATCHER.addURI(getAuthority(), PATH_INSTALLED, INSTALLED); - MATCHER.addURI(getAuthority(), PATH_NO_APKS, NO_APKS); MATCHER.addURI(getAuthority(), PATH_HIGHEST_PRIORITY + "/*", HIGHEST_PRIORITY); MATCHER.addURI(getAuthority(), PATH_SPECIFIC_APP + "/#/*", CODE_SINGLE); MATCHER.addURI(getAuthority(), PATH_CALC_PREFERRED_METADATA, CALC_PREFERRED_METADATA); @@ -437,10 +434,6 @@ public class AppProvider extends FDroidProvider { .build(); } - public static Uri getNoApksUri() { - return Uri.withAppendedPath(getContentUri(), PATH_NO_APKS); - } - public static Uri getInstalledUri() { return Uri.withAppendedPath(getContentUri(), PATH_INSTALLED); } @@ -688,13 +681,6 @@ public class AppProvider extends FDroidProvider { return new AppQuerySelection(selection, args); } - private AppQuerySelection queryNoApks() { - final String apk = getApkTableName(); - final String app = getTableName(); - String selection = "(SELECT COUNT(*) FROM " + apk + " WHERE " + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + ") = 0"; - return new AppQuerySelection(selection); - } - static AppQuerySelection queryPackageNames(String packageNames, String packageNameField) { String[] args = packageNames.split(","); String selection = packageNameField + " IN (" + generateQuestionMarksForInClause(args.length) + ")"; @@ -770,10 +756,6 @@ public class AppProvider extends FDroidProvider { repoIsKnown = true; break; - case NO_APKS: - selection = selection.add(queryNoApks()); - break; - case CATEGORY: selection = selection.add(queryCategory(uri.getLastPathSegment())); includeSwap = false; @@ -833,11 +815,19 @@ public class AppProvider extends FDroidProvider { @Override public int delete(Uri uri, String where, String[] whereArgs) { - if (MATCHER.match(uri) != NO_APKS) { + if (MATCHER.match(uri) != REPO) { throw new UnsupportedOperationException("Delete not supported for " + uri + "."); } - AppQuerySelection selection = new AppQuerySelection(where, whereArgs).add(queryNoApks()); + long repoId = Long.parseLong(uri.getLastPathSegment()); + + final String catJoin = getCatJoinTableName(); + final String app = getTableName(); + String query = "DELETE FROM " + catJoin + " WHERE " + CatJoinTable.Cols.APP_METADATA_ID + " IN " + + "(SELECT " + Cols.ROW_ID + " FROM " + app + " WHERE " + app + "." + Cols.REPO_ID + " = ?)"; + db().execSQL(query, new String[] {String.valueOf(repoId)}); + + AppQuerySelection selection = new AppQuerySelection(where, whereArgs).add(queryRepo(repoId)); return db().delete(getTableName(), selection.getSelection(), selection.getArgs()); } diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java index e5a6b5347..e8491f00b 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoProvider.java @@ -229,9 +229,9 @@ public class RepoProvider extends FDroidProvider { int apkCount = resolver.delete(apkUri, null, null); Utils.debugLog(TAG, "Removed " + apkCount + " apks from repo " + repo.name); - Uri appUri = AppProvider.getNoApksUri(); + Uri appUri = AppProvider.getRepoUri(repo); int appCount = resolver.delete(appUri, null, null); - Utils.debugLog(TAG, "Removed " + appCount + " apps with no apks."); + Utils.debugLog(TAG, "Removed " + appCount + " apps from repo " + repo.address + "."); AppProvider.Helper.recalculatePreferredMetadata(context); }