diff --git a/app/src/main/java/org/fdroid/fdroid/AppDetails.java b/app/src/main/java/org/fdroid/fdroid/AppDetails.java index ccb9c4edc..5ab62ded9 100644 --- a/app/src/main/java/org/fdroid/fdroid/AppDetails.java +++ b/app/src/main/java/org/fdroid/fdroid/AppDetails.java @@ -425,7 +425,7 @@ public class AppDetails extends AppCompatActivity { // register observer to know when install status changes myAppObserver = new AppObserver(new Handler()); getContentResolver().registerContentObserver( - AppProvider.getAppUri(app), + AppProvider.getHighestPriorityMetadataUri(app.packageName), true, myAppObserver); } 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 1f5c257e8..611072bf9 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -46,6 +46,13 @@ public class App extends ValueObject implements Comparable, Parcelable { public String packageName = "unknown"; public String name = "Unknown"; + + /** + * This is primarily for the purpose of saving app metadata when parsing an index.xml file. + * At most other times, we don't particularly care which repo an {@link App} object came from. + * It is pretty much transparent, because the metadata will be populated from the repo with + * the highest priority. The UI doesn't care normally _which_ repo provided the metadata. + */ public long repoId; public String summary = "Unknown application"; public String icon; 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 2d84d1d84..299875313 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -28,6 +28,23 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +/** + * Each app has a bunch of metadata that it associates with a package name (such as org.fdroid.fdroid). + * Multiple repositories can host the same package, and provide different metadata for that app. + * + * As such, it is usually the case that you are interested in an {@link App} which has its metadata + * provided by "the repo with the best priority", rather than "specific repo X". This is important + * when asking for an apk, whereby the preferable way is likely using: + * + * * {@link AppProvider.Helper#findHighestPriorityMetadata(ContentResolver, String)} + * + * rather than: + * + * * {@link AppProvider.Helper#findSpecificApp(ContentResolver, String, long, String[])} + * + * The same can be said of retrieving a list of {@link App} objects, where the metadata for each app + * in the result set should be populated from the repository with the best priority. + */ public class AppProvider extends FDroidProvider { private static final String TAG = "AppProvider"; @@ -126,13 +143,15 @@ public class AppProvider extends FDroidProvider { return cursorToApp(resolver.query(uri, Cols.ALL, null, null, null)); } - public static App findByPackageName(ContentResolver resolver, String packageName, long repoId) { - return findByPackageName(resolver, packageName, repoId, Cols.ALL); - } - - public static App findByPackageName(ContentResolver resolver, String packageName, long repoId, - String[] projection) { - final Uri uri = getAppUri(packageName, repoId); + /** + * Returns an {@link App} with metadata provided by a specific {@code repoId}. Keep in mind + * that most of the time we don't care which repo provides the metadata for a particular app, + * as long as it is the repo with the best priority. In those cases, you should instead use + * {@link AppProvider.Helper#findHighestPriorityMetadata(ContentResolver, String)}. + */ + public static App findSpecificApp(ContentResolver resolver, String packageName, long repoId, + String[] projection) { + final Uri uri = getSpecificAppUri(packageName, repoId); return cursorToApp(resolver.query(uri, projection, null, null, null)); } @@ -501,11 +520,12 @@ public class AppProvider extends FDroidProvider { return getContentUri(app.packageName); } - public static Uri getAppUri(App app) { - return getAppUri(app.packageName, app.repoId); - } - - public static Uri getAppUri(String packageName, long repoId) { + /** + * @see AppProvider.Helper#findSpecificApp(ContentResolver, String, long, String[]) for details + * of why you should usually prefer {@link AppProvider#getHighestPriorityMetadataUri(String)} to + * this method. + */ + public static Uri getSpecificAppUri(String packageName, long repoId) { return getContentUri() .buildUpon() .appendPath(PATH_APP) @@ -514,7 +534,7 @@ public class AppProvider extends FDroidProvider { .build(); } - private static Uri getHighestPriorityMetadataUri(String packageName) { + public static Uri getHighestPriorityMetadataUri(String packageName) { return getContentUri().buildUpon() .appendPath(PATH_HIGHEST_PRIORITY) .appendPath(packageName) @@ -669,7 +689,7 @@ public class AppProvider extends FDroidProvider { * Same as {@link AppProvider#querySingle(String, long)} except it is used for the purpose * of an UPDATE query rather than a SELECT query. This means that it must use a subquery to get * the {@link Cols.Package#PACKAGE_ID} rather than the join which is already in place for that - * table. + * table. The reason is because UPDATE queries cannot include joins in SQLite. */ protected AppQuerySelection querySingleForUpdate(String packageName, long repoId) { final String selection = Cols.PACKAGE_ID + " = (" + getPackageIdFromPackageNameQuery() + @@ -693,6 +713,10 @@ public class AppProvider extends FDroidProvider { return new AppQuerySelection(selection, args); } + /** + * Ensures that for each app metadata row with the same package name, only the one from the repo + * with the best priority is represented in the result set. + */ private AppQuerySelection queryHighestPriority() { final String highestPriority = "SELECT MIN(r." + RepoTable.Cols.PRIORITY + ") " + @@ -895,34 +919,17 @@ public class AppProvider extends FDroidProvider { if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); } - return getAppUri(values.getAsString(PackageTable.Cols.PACKAGE_NAME), values.getAsLong(Cols.REPO_ID)); + return getSpecificAppUri(values.getAsString(PackageTable.Cols.PACKAGE_NAME), values.getAsLong(Cols.REPO_ID)); } @Override public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { - QuerySelection query = new QuerySelection(where, whereArgs); - switch (MATCHER.match(uri)) { - - case CALC_APP_DETAILS_FROM_INDEX: - updateAppDetails(); - return 0; - - case CODE_SINGLE: - List pathParts = uri.getPathSegments(); - long repoId = Long.parseLong(pathParts.get(1)); - String packageName = pathParts.get(2); - query = query.add(querySingleForUpdate(packageName, repoId)); - break; - - default: - throw new UnsupportedOperationException("Update not supported for " + uri + "."); - + if (MATCHER.match(uri) != CALC_APP_DETAILS_FROM_INDEX) { + throw new UnsupportedOperationException("Update not supported for " + uri + "."); } - int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); - if (!isApplyingBatch()) { - getContext().getContentResolver().notifyChange(uri, null); - } - return count; + + updateAppDetails(); + return 0; } protected void updateAppDetails() { diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java index 6ddee8a9e..98247bbeb 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -226,7 +226,7 @@ public class RepoPersister { */ private boolean isAppInDatabase(App app) { String[] fields = {Schema.AppMetadataTable.Cols.Package.PACKAGE_NAME}; - App found = AppProvider.Helper.findByPackageName(context.getContentResolver(), app.packageName, repo.id, fields); + App found = AppProvider.Helper.findSpecificApp(context.getContentResolver(), app.packageName, repo.id, fields); return found != null; } diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java index 9927fc28d..47896dbb1 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -142,7 +142,7 @@ public class TempAppProvider extends AppProvider { int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); if (!isApplyingBatch()) { - getContext().getContentResolver().notifyChange(uri, null); + getContext().getContentResolver().notifyChange(getHighestPriorityMetadataUri(packageName), null); } return count; } diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java index d14f831f4..325ffc630 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -319,7 +319,7 @@ public class SwapAppsView extends ListView implements // implemented on API-16, so leaving like this for now. getActivity().getContentResolver().unregisterContentObserver(appObserver); getActivity().getContentResolver().registerContentObserver( - AppProvider.getAppUri(this.app), true, appObserver); + AppProvider.getSpecificAppUri(this.app.packageName, this.app.repoId), true, appObserver); } resetView(); } diff --git a/app/src/test/java/org/fdroid/fdroid/Assert.java b/app/src/test/java/org/fdroid/fdroid/Assert.java index adbd30db4..ba7085050 100644 --- a/app/src/test/java/org/fdroid/fdroid/Assert.java +++ b/app/src/test/java/org/fdroid/fdroid/Assert.java @@ -198,14 +198,14 @@ public class Assert { Uri uri = AppProvider.getContentUri(); context.getContentResolver().insert(uri, values); - return AppProvider.Helper.findByPackageName(context.getContentResolver(), packageName, 1); + return AppProvider.Helper.findSpecificApp(context.getContentResolver(), packageName, 1, AppMetadataTable.Cols.ALL); } private static App ensureApp(Context context, String packageName) { - App app = AppProvider.Helper.findByPackageName(context.getContentResolver(), packageName, 1); + App app = AppProvider.Helper.findSpecificApp(context.getContentResolver(), packageName, 1, AppMetadataTable.Cols.ALL); if (app == null) { insertApp(context, packageName, packageName); - app = AppProvider.Helper.findByPackageName(context.getContentResolver(), packageName, 1); + app = AppProvider.Helper.findSpecificApp(context.getContentResolver(), packageName, 1, AppMetadataTable.Cols.ALL); } assertNotNull(app); return app; 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 b0ccd5e6e..73b8cd1da 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/AppProviderTest.java @@ -68,7 +68,7 @@ public class AppProviderTest extends FDroidProviderTest { @Test public void testCantFindApp() { - assertNull(AppProvider.Helper.findByPackageName(context.getContentResolver(), "com.example.doesnt-exist", 1)); + assertNull(AppProvider.Helper.findSpecificApp(context.getContentResolver(), "com.example.doesnt-exist", 1, Cols.ALL)); } @Test @@ -111,14 +111,14 @@ public class AppProviderTest extends FDroidProviderTest { ContentResolver r = context.getContentResolver(); // Can't "update", although can "install"... - App notInstalled = AppProvider.Helper.findByPackageName(r, "not installed", 1); + App notInstalled = AppProvider.Helper.findSpecificApp(r, "not installed", 1, Cols.ALL); assertFalse(notInstalled.canAndWantToUpdate(context)); - App installedOnlyOneVersionAvailable = AppProvider.Helper.findByPackageName(r, "installed, only one version available", 1); - App installedAlreadyLatestNoIgnore = AppProvider.Helper.findByPackageName(r, "installed, already latest, no ignore", 1); - App installedAlreadyLatestIgnoreAll = AppProvider.Helper.findByPackageName(r, "installed, already latest, ignore all", 1); - App installedAlreadyLatestIgnoreLatest = AppProvider.Helper.findByPackageName(r, "installed, already latest, ignore latest", 1); - App installedAlreadyLatestIgnoreOld = AppProvider.Helper.findByPackageName(r, "installed, already latest, ignore old", 1); + 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); assertFalse(installedOnlyOneVersionAvailable.canAndWantToUpdate(context)); assertFalse(installedAlreadyLatestNoIgnore.canAndWantToUpdate(context)); @@ -126,10 +126,10 @@ public class AppProviderTest extends FDroidProviderTest { assertFalse(installedAlreadyLatestIgnoreLatest.canAndWantToUpdate(context)); assertFalse(installedAlreadyLatestIgnoreOld.canAndWantToUpdate(context)); - App installedOldNoIgnore = AppProvider.Helper.findByPackageName(r, "installed, old version, no ignore", 1); - App installedOldIgnoreAll = AppProvider.Helper.findByPackageName(r, "installed, old version, ignore all", 1); - App installedOldIgnoreLatest = AppProvider.Helper.findByPackageName(r, "installed, old version, ignore latest", 1); - App installedOldIgnoreNewerNotLatest = AppProvider.Helper.findByPackageName(r, "installed, old version, ignore newer, but not latest", 1); + 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)); assertFalse(installedOldIgnoreAll.canAndWantToUpdate(context)); @@ -239,7 +239,7 @@ public class AppProviderTest extends FDroidProviderTest { assertEquals("org.fdroid.fdroid", app.packageName); assertEquals("F-Droid", app.name); - App otherApp = AppProvider.Helper.findByPackageName(context.getContentResolver(), "org.fdroid.fdroid", 1); + App otherApp = AppProvider.Helper.findSpecificApp(context.getContentResolver(), "org.fdroid.fdroid", 1, Cols.ALL); assertNotNull(otherApp); assertEquals("org.fdroid.fdroid", otherApp.packageName); assertEquals("F-Droid", otherApp.name); @@ -371,6 +371,6 @@ public class AppProviderTest extends FDroidProviderTest { Uri uri = AppProvider.getContentUri(); contentResolver.insert(uri, values); - return AppProvider.Helper.findByPackageName(context.getContentResolver(), id, 1); + return AppProvider.Helper.findSpecificApp(context.getContentResolver(), id, 1, Cols.ALL); } } 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 5f4d445bb..cda22e331 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java @@ -96,7 +96,7 @@ public class ProviderUriTests { app.repoId = 1; app.packageName = "org.fdroid.fdroid"; - assertValidUri(resolver, AppProvider.getAppUri(app), "content://org.fdroid.fdroid.data.AppProvider/app/1/org.fdroid.fdroid", projection); + assertValidUri(resolver, AppProvider.getSpecificAppUri(app.packageName, app.repoId), "content://org.fdroid.fdroid.data.AppProvider/app/1/org.fdroid.fdroid", projection); } @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 2878aff2e..a031289c0 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -14,6 +14,7 @@ import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.Schema; +import org.fdroid.fdroid.data.Schema.AppMetadataTable; import org.fdroid.fdroid.data.Schema.RepoTable.Cols; import org.junit.Test; import org.junit.runner.RunWith; @@ -236,7 +237,7 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { } private void assert2048Metadata(Repo repo, @RepoIdentifier String id) { - App a2048 = AppProvider.Helper.findByPackageName(context.getContentResolver(), "com.uberspot.a2048", repo.getId()); + App a2048 = AppProvider.Helper.findSpecificApp(context.getContentResolver(), "com.uberspot.a2048", repo.getId(), AppMetadataTable.Cols.ALL); assert2048Metadata(a2048, id); } @@ -255,7 +256,7 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { } private void assertAdAwayMetadata(Repo repo, @RepoIdentifier String id) { - App adaway = AppProvider.Helper.findByPackageName(context.getContentResolver(), "org.adaway", repo.getId()); + App adaway = AppProvider.Helper.findSpecificApp(context.getContentResolver(), "org.adaway", repo.getId(), AppMetadataTable.Cols.ALL); assertAdAwayMetadata(adaway, id); } @@ -274,7 +275,7 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { } private void assertAdbMetadata(Repo repo, @RepoIdentifier String id) { - App adb = AppProvider.Helper.findByPackageName(context.getContentResolver(), "siir.es.adbWireless", repo.getId()); + App adb = AppProvider.Helper.findSpecificApp(context.getContentResolver(), "siir.es.adbWireless", repo.getId(), AppMetadataTable.Cols.ALL); assertAdbMetadata(adb, id); } @@ -290,7 +291,7 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { } private void assertCalendarMetadata(Repo repo, @RepoIdentifier String id) { - App calendar = AppProvider.Helper.findByPackageName(context.getContentResolver(), "org.dgtale.icsimport", repo.getId()); + App calendar = AppProvider.Helper.findSpecificApp(context.getContentResolver(), "org.dgtale.icsimport", repo.getId(), AppMetadataTable.Cols.ALL); assertCalendarMetadata(calendar, id); }