From 3ec64d6d82b965305757e630ba5eed7a70902e2f Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 8 Aug 2016 22:13:09 +1000 Subject: [PATCH] Finalise tests for repo priorities + app metadata --- .../org/fdroid/fdroid/data/AppProvider.java | 61 ++++++++++++++++--- .../fdroid/fdroid/data/PackageProvider.java | 5 +- .../java/org/fdroid/fdroid/data/Schema.java | 2 +- .../org/fdroid/fdroid/Issue763MultiRepo.java | 1 + .../updater/ProperMultiRepoUpdaterTest.java | 57 ++++++++++++++--- 5 files changed, 106 insertions(+), 20 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 b1a02f932..2d84d1d84 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -122,7 +122,8 @@ public class AppProvider extends FDroidProvider { } public static App findHighestPriorityMetadata(ContentResolver resolver, String packageName) { - throw new UnsupportedOperationException("TODO: Pull back the metadata with the highest priority for packageName"); + final Uri uri = getHighestPriorityMetadataUri(packageName); + return cursorToApp(resolver.query(uri, Cols.ALL, null, null, null)); } public static App findByPackageName(ContentResolver resolver, String packageName, long repoId) { @@ -266,8 +267,8 @@ public class AppProvider extends FDroidProvider { return pkg + " JOIN " + app + " ON (" + app + "." + Cols.PACKAGE_ID + " = " + pkg + "." + PackageTable.Cols.ROW_ID + ") " + - " LEFT JOIN " + apk + " ON (" + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + ") " + - " LEFT JOIN " + repo + " ON (" + apk + "." + ApkTable.Cols.REPO_ID + " = " + repo + "." + RepoTable.Cols._ID + ") "; + " JOIN " + repo + " ON (" + app + "." + Cols.REPO_ID + " = " + repo + "." + RepoTable.Cols._ID + ") " + + " LEFT JOIN " + apk + " ON (" + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + ") "; } @Override @@ -420,6 +421,7 @@ public class AppProvider extends FDroidProvider { private static final String PATH_CATEGORY = "category"; private static final String PATH_CALC_APP_DETAILS_FROM_INDEX = "calcDetailsFromIndex"; private static final String PATH_REPO = "repo"; + private static final String PATH_HIGHEST_PRIORITY = "highestPriority"; private static final int CAN_UPDATE = CODE_SINGLE + 1; private static final int INSTALLED = CAN_UPDATE + 1; @@ -433,6 +435,7 @@ public class AppProvider extends FDroidProvider { private static final int SEARCH_REPO = REPO + 1; private static final int SEARCH_INSTALLED = SEARCH_REPO + 1; private static final int SEARCH_CAN_UPDATE = SEARCH_INSTALLED + 1; + private static final int HIGHEST_PRIORITY = SEARCH_CAN_UPDATE + 1; static { MATCHER.addURI(getAuthority(), null, CODE_LIST); @@ -448,6 +451,7 @@ public class AppProvider extends FDroidProvider { 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_APP + "/#/*", CODE_SINGLE); } @@ -510,6 +514,13 @@ public class AppProvider extends FDroidProvider { .build(); } + private static Uri getHighestPriorityMetadataUri(String packageName) { + return getContentUri().buildUpon() + .appendPath(PATH_HIGHEST_PRIORITY) + .appendPath(packageName) + .build(); + } + public static Uri getContentUri(String packageName) { return Uri.withAppendedPath(getContentUri(), packageName); } @@ -649,10 +660,9 @@ public class AppProvider extends FDroidProvider { } protected AppQuerySelection querySingle(String packageName, long repoId) { - final String selection = PackageTable.NAME + "." + PackageTable.Cols.PACKAGE_NAME + " = ? " + - " AND " + getTableName() + "." + Cols.REPO_ID + " = ? "; - final String[] args = {packageName, Long.toString(repoId)}; - return new AppQuerySelection(selection, args); + final String selection = getTableName() + "." + Cols.REPO_ID + " = ? "; + final String[] args = {Long.toString(repoId)}; + return new AppQuerySelection(selection, args).add(queryPackageName(packageName)); } /** @@ -683,6 +693,23 @@ public class AppProvider extends FDroidProvider { return new AppQuerySelection(selection, args); } + private AppQuerySelection queryHighestPriority() { + final String highestPriority = + "SELECT MIN(r." + RepoTable.Cols.PRIORITY + ") " + + "FROM " + RepoTable.NAME + " AS r " + + "JOIN " + AppMetadataTable.NAME + " AS m ON (m." + Cols.REPO_ID + " = r." + RepoTable.Cols._ID + ") " + + "WHERE m." + Cols.PACKAGE_ID + " = " + getTableName() + "." + Cols.PACKAGE_ID; + + final String selection = RepoTable.NAME + "." + RepoTable.Cols.PRIORITY + " = (" + highestPriority + ")"; + return new AppQuerySelection(selection); + } + + private AppQuerySelection queryPackageName(String packageName) { + final String selection = PackageTable.NAME + "." + PackageTable.Cols.PACKAGE_NAME + " = ? "; + final String[] args = {packageName}; + return new AppQuerySelection(selection, args); + } + private AppQuerySelection queryRecentlyUpdated() { final String app = getTableName(); final String lastUpdated = app + "." + Cols.LAST_UPDATED; @@ -729,6 +756,14 @@ public class AppProvider extends FDroidProvider { // Queries which are for the main list of apps should not include swap apps. boolean includeSwap = true; + // It is usually the case that we ask for app(s) for which we don't care what repo is + // responsible for providing them. In that case, we need to populate the metadata with + // that form the repo with the highest priority. + // Whenever we know which repo it is coming from, then it is important that we don't + // delegate to the repo with the highest priority, but rather the specific repo we are + // querying from. + boolean repoIsKnown = false; + switch (MATCHER.match(uri)) { case CODE_LIST: includeSwap = false; @@ -739,6 +774,7 @@ public class AppProvider extends FDroidProvider { long repoId = Long.parseLong(pathParts.get(1)); String packageName = pathParts.get(2); selection = selection.add(querySingle(packageName, repoId)); + repoIsKnown = true; break; case CAN_UPDATE: @@ -748,6 +784,7 @@ public class AppProvider extends FDroidProvider { case REPO: selection = selection.add(queryRepo(Long.parseLong(uri.getLastPathSegment()))); + repoIsKnown = true; break; case INSTALLED: @@ -774,6 +811,7 @@ public class AppProvider extends FDroidProvider { selection = selection .add(querySearch(uri.getPathSegments().get(2))) .add(queryRepo(Long.parseLong(uri.getPathSegments().get(1)))); + repoIsKnown = true; break; case NO_APKS: @@ -797,11 +835,20 @@ public class AppProvider extends FDroidProvider { includeSwap = false; break; + case HIGHEST_PRIORITY: + selection = selection.add(queryPackageName(uri.getLastPathSegment())); + includeSwap = false; + break; + default: Log.e(TAG, "Invalid URI for app content provider: " + uri); throw new UnsupportedOperationException("Invalid URI for app content provider: " + uri); } + if (!repoIsKnown) { + selection = selection.add(queryHighestPriority()); + } + return runQuery(uri, selection, projection, includeSwap, sortOrder); } diff --git a/app/src/main/java/org/fdroid/fdroid/data/PackageProvider.java b/app/src/main/java/org/fdroid/fdroid/data/PackageProvider.java index 9c3d6942c..5667cb5f9 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/PackageProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/PackageProvider.java @@ -29,7 +29,8 @@ public class PackageProvider extends FDroidProvider { } public static long getPackageId(Context context, String packageName) { - Cursor cursor = context.getContentResolver().query(getPackageUri(packageName), Cols.ALL, null, null, null); + String[] projection = new String[] {Cols.ROW_ID}; + Cursor cursor = context.getContentResolver().query(getPackageUri(packageName), projection, null, null, null); if (cursor == null) { return 0; } @@ -39,7 +40,7 @@ public class PackageProvider extends FDroidProvider { return 0; } else { cursor.moveToFirst(); - return cursor.getLong(cursor.getColumnIndexOrThrow(Cols.PACKAGE_NAME)); + return cursor.getLong(cursor.getColumnIndexOrThrow(Cols.ROW_ID)); } } finally { cursor.close(); diff --git a/app/src/main/java/org/fdroid/fdroid/data/Schema.java b/app/src/main/java/org/fdroid/fdroid/data/Schema.java index eee638a18..5b1f1779f 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -115,7 +115,7 @@ public interface Schema { * @see Cols#ALL_COLS */ String[] ALL = { - _ID, ROW_ID, IS_COMPATIBLE, NAME, SUMMARY, ICON, DESCRIPTION, + _ID, ROW_ID, REPO_ID, IS_COMPATIBLE, NAME, SUMMARY, ICON, DESCRIPTION, LICENSE, AUTHOR, EMAIL, WEB_URL, TRACKER_URL, SOURCE_URL, CHANGELOG_URL, DONATE_URL, BITCOIN_ADDR, LITECOIN_ADDR, FLATTR_ID, UPSTREAM_VERSION_NAME, UPSTREAM_VERSION_CODE, ADDED, LAST_UPDATED, diff --git a/app/src/test/java/org/fdroid/fdroid/Issue763MultiRepo.java b/app/src/test/java/org/fdroid/fdroid/Issue763MultiRepo.java index 8df75aedb..c236a5ad2 100644 --- a/app/src/test/java/org/fdroid/fdroid/Issue763MultiRepo.java +++ b/app/src/test/java/org/fdroid/fdroid/Issue763MultiRepo.java @@ -7,6 +7,7 @@ import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.data.Schema; +import org.fdroid.fdroid.updater.MultiRepoUpdaterTest; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; 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 68ed05c9b..2878aff2e 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -1,6 +1,7 @@ package org.fdroid.fdroid.updater; +import android.content.ContentValues; import android.support.annotation.StringDef; import android.util.Log; @@ -13,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.RepoTable.Cols; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricGradleTestRunner; @@ -20,7 +22,9 @@ import org.robolectric.annotation.Config; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -56,13 +60,24 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertConflictingRepo(); } + private Map allApps() { + List apps = AppProvider.Helper.all(context.getContentResolver()); + Map appsIndexedByPackageName = new HashMap<>(apps.size()); + for (App app : apps) { + appsIndexedByPackageName.put(app.packageName, app); + } + return appsIndexedByPackageName; + } + @Test - public void conflictingMetadataTakesPriority() throws RepoUpdater.UpdateException { + public void metadataWithRepoPriority() throws RepoUpdater.UpdateException { updateConflicting(); updateMain(); updateArchive(); - assertEquals(1, RepoProvider.Helper.findByAddress(context, REPO_CONFLICTING_URI).priority); + Repo conflictingRepo = RepoProvider.Helper.findByAddress(context, REPO_CONFLICTING_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); @@ -70,22 +85,44 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertMainArchiveRepoMetadata(); assertConflictingRepo(); - App a2048 = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "com.uberspot.a2048"); - assert2048Metadata(a2048, "Conflicting"); + assertRepoTakesPriority("Conflicting"); + + // 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); + + assertRepoTakesPriority("Normal"); + } + + private void assertRepoTakesPriority(@RepoIdentifier String higherPriority) { + Map allApps = allApps(); + + // Provided by both the "Main" and "Conflicting" repo, so need to fetch metdata from the + // repo with the higher "Conflicting" repo has a higher priority. + App adAway = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "org.adaway"); + assertAdAwayMetadata(adAway, higherPriority); + assertAdAwayMetadata(allApps.get("org.adaway"), higherPriority); - // This is only provided by the "Conflicting" repo. - App calendar = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "org.dgtale.icsimport"); - assertCalendarMetadata(calendar, "Conflicting"); // This is only provided by the "Main" or "Archive" repo. Both the main and archive repo both // pull their metadata from the same build recipe in fdroidserver. The only difference is that // the archive repository contains .apks from further back, but their metadata is the same. - App adAway = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "org.adaway"); - assertAdAwayMetadata(adAway, "Normal"); + App a2048 = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "com.uberspot.a2048"); + assert2048Metadata(a2048, "Normal"); + assert2048Metadata(allApps.get("com.uberspot.a2048"), "Normal"); + + // This is only provided by the "Conflicting" repo. + App calendar = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "org.dgtale.icsimport"); + assertCalendarMetadata(calendar, "Conflicting"); + assertCalendarMetadata(allApps.get("org.dgtale.icsimport"), "Conflicting"); // This is only provided by the "Main" repo. App adb = AppProvider.Helper.findHighestPriorityMetadata(context.getContentResolver(), "siir.es.adbWireless"); - assertAdAwayMetadata(adb, "Normal"); + assertAdbMetadata(adb, "Normal"); + assertAdbMetadata(allApps.get("siir.es.adbWireless"), "Normal"); } @Test