From 2733081b3a75110e1c7846aba72350ed8d72b413 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Thu, 23 Jun 2016 11:30:01 +1000 Subject: [PATCH] Change join from `app` to `apk` table to auto increment integers rather than Android package name strings. This is important for the ability to refactor the database for better performance in the future. See #511 for details. For those interested in the details, here is a query plan of selecting the "All" category of apps before this commit: * `SCAN TABLE fdroid_app USING INDEX app_id` * `SEARCH TABLE fdroid_apk USING INDEX apk_id (id=?)` * `SEARCH TABLE fdroid_repo USING INTEGER PRIMARY KEY (rowid=?)` * `SEARCH TABLE fdroid_installedApp AS installed USING INDEX sqlite_autoindex_fdroid_installedApp_1 (appId=?)` * `SEARCH TABLE fdroid_apk AS suggestedApk USING INDEX sqlite_autoindex_fdroid_apk_1 (id=? AND vercode=?)` * `USE TEMP B-TREE FOR ORDER BY` And here is a query plan of afterwards: * `SCAN TABLE fdroid_app` * `SEARCH TABLE fdroid_apk USING INDEX apk_appId (appId=?)` * `SEARCH TABLE fdroid_repo USING INTEGER PRIMARY KEY (rowid=?)` * `SEARCH TABLE fdroid_installedApp AS installed USING INDEX sqlite_autoindex_fdroid_installedApp_1 (appId=?)` * `SEARCH TABLE fdroid_apk AS suggestedApk USING INDEX apk_appId (appId=?)` * `USE TEMP B-TREE FOR ORDER BY` The things of note are: * `SCAN TABLE` doesn't use an index, which means that it is really using the rowid index. Shouldn't behave much differently. * The second item now uses an integer primary key index rather than a package name index. Should increase search speed marginally which was the goal of this commit. As more apks exist, the speed improvement will also increase. --- .../main/java/org/fdroid/fdroid/data/Apk.java | 13 ++- .../org/fdroid/fdroid/data/ApkProvider.java | 97 ++++++++++++++----- .../main/java/org/fdroid/fdroid/data/App.java | 11 +++ .../org/fdroid/fdroid/data/AppProvider.java | 51 ++++------ .../java/org/fdroid/fdroid/data/DBHelper.java | 25 ++++- .../org/fdroid/fdroid/data/RepoPersister.java | 38 +++++++- .../java/org/fdroid/fdroid/data/Schema.java | 28 ++++-- .../fdroid/fdroid/data/TempApkProvider.java | 7 +- .../fdroid/fdroid/data/TempAppProvider.java | 37 ++++++- .../test/java/org/fdroid/fdroid/Assert.java | 42 ++++++-- .../java/org/fdroid/fdroid/TestUtils.java | 25 +++++ .../fdroid/fdroid/data/ApkProviderTest.java | 49 +++++----- .../fdroid/data/FDroidProviderTest.java | 9 +- .../fdroid/fdroid/data/ProviderUriTests.java | 22 ++++- 14 files changed, 340 insertions(+), 114 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/Apk.java b/app/src/main/java/org/fdroid/fdroid/data/Apk.java index bb9d66df7..745050bd4 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java @@ -61,6 +61,11 @@ public class Apk extends ValueObject implements Comparable { public String repoAddress; public String[] incompatibleReasons; + /** + * The numeric primary key of the App table, which is used to join apks. + */ + public long appId; + public Apk() { } @@ -74,6 +79,9 @@ public class Apk extends ValueObject implements Comparable { for (int i = 0; i < cursor.getColumnCount(); i++) { switch (cursor.getColumnName(i)) { + case Cols.APP_ID: + appId = cursor.getLong(i); + break; case Cols.HASH: hash = cursor.getString(i); break; @@ -131,10 +139,10 @@ public class Apk extends ValueObject implements Comparable { case Cols.VERSION_CODE: versionCode = cursor.getInt(i); break; - case Cols.REPO_VERSION: + case Cols.Repo.VERSION: repoVersion = cursor.getInt(i); break; - case Cols.REPO_ADDRESS: + case Cols.Repo.ADDRESS: repoAddress = cursor.getString(i); break; } @@ -192,6 +200,7 @@ public class Apk extends ValueObject implements Comparable { public ContentValues toContentValues() { ContentValues values = new ContentValues(); + values.put(Cols.APP_ID, appId); values.put(Cols.PACKAGE_NAME, packageName); values.put(Cols.VERSION_NAME, versionName); values.put(Cols.VERSION_CODE, versionCode); diff --git a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java index cbb265a74..d777866a7 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java @@ -11,6 +11,7 @@ import android.util.Log; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Schema.ApkTable; import org.fdroid.fdroid.data.Schema.ApkTable.Cols; +import org.fdroid.fdroid.data.Schema.AppTable; import org.fdroid.fdroid.data.Schema.RepoTable; import java.util.ArrayList; @@ -135,7 +136,7 @@ public class ApkProvider extends FDroidProvider { String packageName, String[] projection) { ContentResolver resolver = context.getContentResolver(); final Uri uri = getAppUri(packageName); - final String sort = Cols.VERSION_CODE + " DESC"; + final String sort = "apk." + Cols.VERSION_CODE + " DESC"; Cursor cursor = resolver.query(uri, projection, null, null, sort); return cursorToList(cursor); } @@ -213,10 +214,12 @@ public class ApkProvider extends FDroidProvider { private static final UriMatcher MATCHER = new UriMatcher(-1); private static final Map REPO_FIELDS = new HashMap<>(); + private static final Map APP_FIELDS = new HashMap<>(); static { - REPO_FIELDS.put(Cols.REPO_VERSION, RepoTable.Cols.VERSION); - REPO_FIELDS.put(Cols.REPO_ADDRESS, RepoTable.Cols.ADDRESS); + REPO_FIELDS.put(Cols.Repo.VERSION, RepoTable.Cols.VERSION); + REPO_FIELDS.put(Cols.Repo.ADDRESS, RepoTable.Cols.ADDRESS); + APP_FIELDS.put(Cols.App.PACKAGE_NAME, AppTable.Cols.PACKAGE_NAME); MATCHER.addURI(getAuthority(), PATH_REPO + "/#", CODE_REPO); MATCHER.addURI(getAuthority(), PATH_APK + "/#/*", CODE_SINGLE); @@ -323,6 +326,10 @@ public class ApkProvider extends FDroidProvider { return ApkTable.NAME; } + protected String getAppTableName() { + return AppTable.NAME; + } + @Override protected String getProviderName() { return PROVIDER_NAME; @@ -333,30 +340,40 @@ public class ApkProvider extends FDroidProvider { return MATCHER; } - private static class Query extends QueryBuilder { + private class Query extends QueryBuilder { private boolean repoTableRequired; @Override protected String getRequiredTables() { - return ApkTable.NAME + " AS apk"; + final String apk = getTableName(); + final String app = getAppTableName(); + + return apk + " AS apk " + + " LEFT JOIN " + app + " AS app ON (app." + AppTable.Cols.ROW_ID + " = apk." + Cols.APP_ID + ")"; } @Override public void addField(String field) { - if (REPO_FIELDS.containsKey(field)) { + if (APP_FIELDS.containsKey(field)) { + addAppField(APP_FIELDS.get(field), field); + } else if (REPO_FIELDS.containsKey(field)) { addRepoField(REPO_FIELDS.get(field), field); } else if (field.equals(Cols._ID)) { appendField("rowid", "apk", "_id"); } else if (field.equals(Cols._COUNT)) { appendField("COUNT(*) AS " + Cols._COUNT); } else if (field.equals(Cols._COUNT_DISTINCT)) { - appendField("COUNT(DISTINCT apk." + Cols.PACKAGE_NAME + ") AS " + Cols._COUNT_DISTINCT); + appendField("COUNT(DISTINCT apk." + Cols.APP_ID + ") AS " + Cols._COUNT_DISTINCT); } else { appendField(field, "apk"); } } + private void addAppField(String field, String alias) { + appendField(field, "app", alias); + } + private void addRepoField(String field, String alias) { if (!repoTableRequired) { repoTableRequired = true; @@ -368,13 +385,23 @@ public class ApkProvider extends FDroidProvider { } private QuerySelection queryApp(String packageName) { - final String selection = Cols.PACKAGE_NAME + " = ? "; + return queryApp(packageName, true); + } + + private QuerySelection queryApp(String packageName, boolean includeTableAlias) { + String alias = includeTableAlias ? "apk." : ""; + final String selection = alias + Cols.APP_ID + " = (" + getAppIdFromPackageNameQuery() + ")"; final String[] args = {packageName}; return new QuerySelection(selection, args); } private QuerySelection querySingle(Uri uri) { - final String selection = Cols.VERSION_CODE + " = ? and " + Cols.PACKAGE_NAME + " = ? "; + return querySingle(uri, true); + } + + private QuerySelection querySingle(Uri uri, boolean includeAlias) { + String alias = includeAlias ? "apk." : ""; + final String selection = " " + alias + Cols.VERSION_CODE + " = ? and " + alias + Cols.PACKAGE_NAME + " = ? "; final String[] args = { // First (0th) path segment is the word "apk", // and we are not interested in it. @@ -385,22 +412,32 @@ public class ApkProvider extends FDroidProvider { } protected QuerySelection queryRepo(long repoId) { - final String selection = Cols.REPO_ID + " = ? "; + return queryRepo(repoId, true); + } + + protected QuerySelection queryRepo(long repoId, boolean includeAlias) { + String alias = includeAlias ? "apk." : ""; + final String selection = alias + Cols.REPO_ID + " = ? "; final String[] args = {Long.toString(repoId)}; return new QuerySelection(selection, args); } private QuerySelection queryRepoApps(long repoId, String packageNames) { - return queryRepo(repoId).add(AppProvider.queryApps(packageNames, Cols.PACKAGE_NAME)); + return queryRepo(repoId).add(AppProvider.queryApps(packageNames, "app." + AppTable.Cols.PACKAGE_NAME)); } protected QuerySelection queryApks(String apkKeys) { + return queryApks(apkKeys, true); + } + + protected QuerySelection queryApks(String apkKeys, boolean includeAlias) { final String[] apkDetails = apkKeys.split(","); if (apkDetails.length > MAX_APKS_TO_QUERY) { throw new IllegalArgumentException( "Cannot query more than " + MAX_APKS_TO_QUERY + ". " + "You tried to query " + apkDetails.length); } + String alias = includeAlias ? "apk." : ""; final String[] args = new String[apkDetails.length * 2]; StringBuilder sb = new StringBuilder(); for (int i = 0; i < apkDetails.length; i++) { @@ -412,11 +449,23 @@ public class ApkProvider extends FDroidProvider { if (i != 0) { sb.append(" OR "); } - sb.append(" ( " + Cols.PACKAGE_NAME + " = ? AND " + Cols.VERSION_CODE + " = ? ) "); + sb.append(" ( ") + .append(alias) + .append(Cols.APP_ID) + .append(" = (") + .append(getAppIdFromPackageNameQuery()) + .append(") AND ") + .append(alias) + .append(Cols.VERSION_CODE) + .append(" = ? ) "); } return new QuerySelection(sb.toString(), args); } + private String getAppIdFromPackageNameQuery() { + return "SELECT " + AppTable.Cols.ROW_ID + " FROM " + getAppTableName() + " WHERE " + AppTable.Cols.PACKAGE_NAME + " = ?"; + } + @Override public Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder) { @@ -464,13 +513,17 @@ public class ApkProvider extends FDroidProvider { return cursor; } - private static void removeRepoFields(ContentValues values) { + private static void removeFieldsFromOtherTables(ContentValues values) { for (Map.Entry repoField : REPO_FIELDS.entrySet()) { final String field = repoField.getKey(); if (values.containsKey(field)) { - Utils.debugLog(TAG, "Cannot insert/update '" + field + "' field " + - "on apk table, as it belongs to the repo table. " + - "This field will be ignored."); + values.remove(field); + } + } + + for (Map.Entry appField : APP_FIELDS.entrySet()) { + final String field = appField.getKey(); + if (values.containsKey(field)) { values.remove(field); } } @@ -478,7 +531,7 @@ public class ApkProvider extends FDroidProvider { @Override public Uri insert(Uri uri, ContentValues values) { - removeRepoFields(values); + removeFieldsFromOtherTables(values); validateFields(Cols.ALL, values); db().insertOrThrow(getTableName(), null, values); if (!isApplyingBatch()) { @@ -498,15 +551,15 @@ public class ApkProvider extends FDroidProvider { switch (MATCHER.match(uri)) { case CODE_REPO: - query = query.add(queryRepo(Long.parseLong(uri.getLastPathSegment()))); + query = query.add(queryRepo(Long.parseLong(uri.getLastPathSegment()), false)); break; case CODE_APP: - query = query.add(queryApp(uri.getLastPathSegment())); + query = query.add(queryApp(uri.getLastPathSegment(), false)); break; case CODE_APKS: - query = query.add(queryApks(uri.getLastPathSegment())); + query = query.add(queryApks(uri.getLastPathSegment(), false)); break; // TODO: Add tests for this. @@ -542,10 +595,10 @@ public class ApkProvider extends FDroidProvider { protected int performUpdateUnchecked(Uri uri, ContentValues values, String where, String[] whereArgs) { validateFields(Cols.ALL, values); - removeRepoFields(values); + removeFieldsFromOtherTables(values); QuerySelection query = new QuerySelection(where, whereArgs); - query = query.add(querySingle(uri)); + query = query.add(querySingle(uri, false)); int numRows = db().update(getTableName(), values, query.getSelection(), query.getArgs()); if (!isApplyingBatch()) { 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 c9971f2b1..a071393b5 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -130,6 +130,8 @@ public class App extends ValueObject implements Comparable { public String installedSig; + private long id; + public static String getIconName(String packageName, int versionCode) { return packageName + "_" + versionCode + ".png"; } @@ -153,6 +155,9 @@ public class App extends ValueObject implements Comparable { for (int i = 0; i < cursor.getColumnCount(); i++) { String n = cursor.getColumnName(i); switch (n) { + case Cols.ROW_ID: + id = cursor.getLong(i); + break; case Cols.IS_COMPATIBLE: compatible = cursor.getInt(i) == 1; break; @@ -440,6 +445,8 @@ public class App extends ValueObject implements Comparable { public ContentValues toContentValues() { final ContentValues values = new ContentValues(); + // Intentionally don't put "ROW_ID" in here, because we don't ever want to change that + // primary key generated by sqlite. values.put(Cols.PACKAGE_NAME, packageName); values.put(Cols.NAME, name); values.put(Cols.SUMMARY, summary); @@ -549,4 +556,8 @@ public class App extends ValueObject implements Comparable { } return new int[]{minSdkVersion, targetSdkVersion, maxSdkVersion}; } + + public long getId() { + return id; + } } 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 f16999bc8..1eab64e1b 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -64,7 +64,7 @@ public class AppProvider extends FDroidProvider { return cursorToList(cursor); } - private static List cursorToList(Cursor cursor) { + static List cursorToList(Cursor cursor) { int knownAppCount = cursor != null ? cursor.getCount() : 0; List apps = new ArrayList<>(knownAppCount); if (cursor != null) { @@ -153,7 +153,6 @@ public class AppProvider extends FDroidProvider { final Uri fromUpstream = calcAppDetailsFromIndexUri(); context.getContentResolver().update(fromUpstream, null, null, null); } - } /** @@ -184,7 +183,7 @@ public class AppProvider extends FDroidProvider { * method from this class will return an instance of this class, that is aware of * the install apps table. */ - private static class AppQuerySelection extends QuerySelection { + protected static class AppQuerySelection extends QuerySelection { private boolean naturalJoinToInstalled; @@ -247,7 +246,7 @@ public class AppProvider extends FDroidProvider { final String repo = RepoTable.NAME; return app + - " LEFT JOIN " + apk + " ON (" + apk + "." + ApkTable.Cols.PACKAGE_NAME + " = " + app + "." + Cols.PACKAGE_NAME + ") " + + " 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 + ") "; } @@ -259,7 +258,7 @@ public class AppProvider extends FDroidProvider { @Override protected String groupBy() { // If the count field has been requested, then we want to group all rows together. - return countFieldAppended ? null : getTableName() + "." + Cols.PACKAGE_NAME; + return countFieldAppended ? null : getTableName() + "." + Cols.ROW_ID; } public void addSelection(AppQuerySelection selection) { @@ -320,7 +319,7 @@ public class AppProvider extends FDroidProvider { private void appendCountField() { countFieldAppended = true; - appendField("COUNT( DISTINCT " + getTableName() + "." + Cols.PACKAGE_NAME + " ) AS " + Cols._COUNT); + appendField("COUNT( DISTINCT " + getTableName() + "." + Cols.ROW_ID + " ) AS " + Cols._COUNT); } private void addSuggestedApkVersionField() { @@ -335,7 +334,7 @@ public class AppProvider extends FDroidProvider { leftJoin( getApkTableName(), "suggestedApk", - getTableName() + "." + Cols.SUGGESTED_VERSION_CODE + " = suggestedApk." + ApkTable.Cols.VERSION_CODE + " AND " + getTableName() + "." + Cols.PACKAGE_NAME + " = suggestedApk." + ApkTable.Cols.PACKAGE_NAME); + getTableName() + "." + Cols.SUGGESTED_VERSION_CODE + " = suggestedApk." + ApkTable.Cols.VERSION_CODE + " AND " + getTableName() + "." + Cols.ROW_ID + " = suggestedApk." + ApkTable.Cols.APP_ID); } appendField(fieldName, "suggestedApk", alias); } @@ -378,7 +377,7 @@ public class AppProvider extends FDroidProvider { 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"; - private static final String PATH_APPS = "apps"; + protected static final String PATH_APPS = "apps"; private static final String PATH_RECENTLY_UPDATED = "recentlyUpdated"; private static final String PATH_NEWLY_ADDED = "newlyAdded"; private static final String PATH_CATEGORY = "category"; @@ -390,8 +389,7 @@ public class AppProvider extends FDroidProvider { 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 APPS = NO_APKS + 1; - private static final int RECENTLY_UPDATED = APPS + 1; + private static final int RECENTLY_UPDATED = NO_APKS + 1; private static final int NEWLY_ADDED = RECENTLY_UPDATED + 1; private static final int CATEGORY = NEWLY_ADDED + 1; private static final int IGNORED = CATEGORY + 1; @@ -416,7 +414,6 @@ 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_APPS + "/*", APPS); MATCHER.addURI(getAuthority(), "*", CODE_SINGLE); } @@ -466,20 +463,6 @@ public class AppProvider extends FDroidProvider { .build(); } - public static Uri getContentUri(List apps) { - StringBuilder builder = new StringBuilder(); - for (int i = 0; i < apps.size(); i++) { - if (i != 0) { - builder.append(','); - } - builder.append(apps.get(i).packageName); - } - return getContentUri().buildUpon() - .appendPath(PATH_APPS) - .appendPath(builder.toString()) - .build(); - } - public static Uri getContentUri(App app) { return getContentUri(app.packageName); } @@ -684,10 +667,6 @@ public class AppProvider extends FDroidProvider { return new AppQuerySelection(selection, args); } - private AppQuerySelection queryApps(String packageNames) { - return queryApps(packageNames, getTableName() + "." + Cols.PACKAGE_NAME); - } - @Override public Cursor query(Uri uri, String[] projection, String customSelection, String[] selectionArgs, String sortOrder) { AppQuerySelection selection = new AppQuerySelection(customSelection, selectionArgs); @@ -742,10 +721,6 @@ public class AppProvider extends FDroidProvider { selection = selection.add(queryNoApks()); break; - case APPS: - selection = selection.add(queryApps(uri.getLastPathSegment())); - break; - case IGNORED: selection = selection.add(queryIgnored()); break; @@ -772,6 +747,14 @@ public class AppProvider extends FDroidProvider { throw new UnsupportedOperationException("Invalid URI for app content provider: " + uri); } + return runQuery(uri, selection, projection, includeSwap, sortOrder); + } + + /** + * Helper method used by both the genuine {@link AppProvider} and the temporary version used + * by the repo updater ({@link TempAppProvider}). + */ + protected Cursor runQuery(Uri uri, AppQuerySelection selection, String[] projection, boolean includeSwap, String sortOrder) { if (!includeSwap) { selection = selection.add(queryExcludeSwap()); } @@ -973,7 +956,7 @@ public class AppProvider extends FDroidProvider { apk + " JOIN " + repo + " ON (" + repo + "." + RepoTable.Cols._ID + " = " + apk + "." + ApkTable.Cols.REPO_ID + ") " + " WHERE " + - app + "." + Cols.PACKAGE_NAME + " = " + apk + "." + ApkTable.Cols.PACKAGE_NAME + " AND " + + app + "." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + " AND " + apk + "." + ApkTable.Cols.VERSION_CODE + " = " + app + "." + Cols.SUGGESTED_VERSION_CODE; return "UPDATE " + app + " SET " 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 3b682064e..c688ee01a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -47,6 +47,7 @@ class DBHelper extends SQLiteOpenHelper { private static final String CREATE_TABLE_APK = "CREATE TABLE " + ApkTable.NAME + " ( " + ApkTable.Cols.PACKAGE_NAME + " text not null, " + + ApkTable.Cols.APP_ID + " text not null, " + ApkTable.Cols.VERSION_NAME + " text not null, " + ApkTable.Cols.REPO_ID + " integer not null, " + ApkTable.Cols.HASH + " text not null, " @@ -114,7 +115,7 @@ class DBHelper extends SQLiteOpenHelper { + " );"; private static final String DROP_TABLE_INSTALLED_APP = "DROP TABLE " + InstalledAppTable.NAME + ";"; - private static final int DB_VERSION = 57; + private static final int DB_VERSION = 58; private final Context context; @@ -315,8 +316,29 @@ class DBHelper extends SQLiteOpenHelper { requireTimestampInRepos(db, oldVersion); recreateInstalledAppTable(db, oldVersion); addTargetSdkVersionToApk(db, oldVersion); + migrateAppPrimaryKeyToRowId(db, oldVersion); } + private void migrateAppPrimaryKeyToRowId(SQLiteDatabase db, int oldVersion) { + if (oldVersion < 58) { + final String alter = "ALTER TABLE " + ApkTable.NAME + " ADD COLUMN appId NUMERIC"; + Log.i(TAG, "Adding appId foreign key to fdroid_apk."); + Utils.debugLog(TAG, alter); + db.execSQL(alter); + + final String update = + "UPDATE " + ApkTable.NAME + " SET appId = ( " + + "SELECT app.rowid " + + "FROM " + ApkTable.NAME + " AS app " + + "WHERE " + ApkTable.NAME + ".id = app.id " + + ")"; + Log.i(TAG, "Updating foreign key from fdroid_apk to fdroid_app to use numeric foreign key."); + Utils.debugLog(TAG, update); + db.execSQL(update); + } + } + + /** * Migrate repo list to new structure. (No way to change primary * key in sqlite - table must be recreated). @@ -570,6 +592,7 @@ class DBHelper extends SQLiteOpenHelper { db.execSQL("create index app_id on " + AppTable.NAME + " (" + AppTable.Cols.PACKAGE_NAME + ");"); db.execSQL(CREATE_TABLE_APK); db.execSQL("create index apk_vercode on " + ApkTable.NAME + " (" + ApkTable.Cols.VERSION_CODE + ");"); + db.execSQL("create index apk_appId on " + ApkTable.NAME + " (" + ApkTable.Cols.APP_ID + ");"); db.execSQL("create index apk_id on " + ApkTable.NAME + " (" + AppTable.Cols.PACKAGE_NAME + ");"); } 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 ff34ffe53..98fdd849a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -98,16 +98,19 @@ public class RepoPersister { if (apksToSave.size() > 0 || appsToSave.size() > 0) { Utils.debugLog(TAG, "Flushing details of up to " + MAX_APP_BUFFER + " apps and their packages to the database."); - flushAppsToDbInBatch(); - flushApksToDbInBatch(); + Map appIds = flushAppsToDbInBatch(); + flushApksToDbInBatch(appIds); apksToSave.clear(); appsToSave.clear(); } } - private void flushApksToDbInBatch() throws RepoUpdater.UpdateException { + private void flushApksToDbInBatch(Map appIds) throws RepoUpdater.UpdateException { List apksToSaveList = new ArrayList<>(); for (Map.Entry> entries : apksToSave.entrySet()) { + for (Apk apk : entries.getValue()) { + apk.appId = appIds.get(apk.packageName); + } apksToSaveList.addAll(entries.getValue()); } @@ -127,16 +130,43 @@ public class RepoPersister { } } - private void flushAppsToDbInBatch() throws RepoUpdater.UpdateException { + /** + * Will first insert new or update existing rows in the database for each {@link RepoPersister#appsToSave}. + * Then, will query the database for the ID + packageName for each of these apps, so that they + * can be returned and the relevant apks can be joined to the app table correctly. + */ + private Map flushAppsToDbInBatch() throws RepoUpdater.UpdateException { ArrayList appOperations = insertOrUpdateApps(appsToSave); try { context.getContentResolver().applyBatch(TempAppProvider.getAuthority(), appOperations); + return getIdsForPackages(appsToSave); } catch (RemoteException | OperationApplicationException e) { throw new RepoUpdater.UpdateException(repo, "An internal error occurred while updating the database", e); } } + /** + * Although this might seem counter intuitive - receiving a list of apps, then querying the + * database again for info about these apps, it is required because the apps came from the + * repo metadata, but we are really interested in their IDs from the database. These IDs only + * exist in SQLite and not the repo metadata. + */ + private Map getIdsForPackages(List apps) { + List packageNames = new ArrayList<>(appsToSave.size()); + for (App app : apps) { + packageNames.add(app.packageName); + } + String[] projection = {Schema.AppTable.Cols.ROW_ID, Schema.AppTable.Cols.PACKAGE_NAME}; + List fromDb = TempAppProvider.Helper.findByPackageNames(context, packageNames, projection); + + Map ids = new HashMap<>(fromDb.size()); + for (App app : fromDb) { + ids.put(app.packageName, app.getId()); + } + return ids; + } + /** * Depending on whether the {@link App}s have been added to the database previously, this * will queue up an update or an insert {@link ContentProviderOperation} for each app. 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 0d6571794..d9c12d7f0 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -14,7 +14,12 @@ public interface Schema { String NAME = "fdroid_app"; interface Cols { - String _ID = "rowid as _id"; // Required for CursorLoaders + /** + * Same as the primary key {@link Cols#ROW_ID}, except aliased as "_id" instead + * of "rowid". Required for {@link android.content.CursorLoader}s. + */ + String _ID = "rowid as _id"; + String ROW_ID = "rowid"; String _COUNT = "_count"; String IS_COMPATIBLE = "compatible"; String PACKAGE_NAME = "id"; @@ -57,7 +62,7 @@ public interface Schema { } String[] ALL = { - _ID, IS_COMPATIBLE, PACKAGE_NAME, NAME, SUMMARY, ICON, DESCRIPTION, + _ID, ROW_ID, IS_COMPATIBLE, PACKAGE_NAME, 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, @@ -82,6 +87,10 @@ public interface Schema { interface Cols extends BaseColumns { String _COUNT_DISTINCT = "countDistinct"; + /** + * Foreign key to the {@link AppTable}. + */ + String APP_ID = "appId"; String PACKAGE_NAME = "id"; String VERSION_NAME = "version"; String REPO_ID = "repo"; @@ -101,14 +110,21 @@ public interface Schema { String ADDED_DATE = "added"; String IS_COMPATIBLE = "compatible"; String INCOMPATIBLE_REASONS = "incompatibleReasons"; - String REPO_VERSION = "repoVersion"; - String REPO_ADDRESS = "repoAddress"; + + interface Repo { + String VERSION = "repoVersion"; + String ADDRESS = "repoAddress"; + } + + interface App { + String PACKAGE_NAME = "appPackageName"; + } String[] ALL = { - _ID, PACKAGE_NAME, VERSION_NAME, REPO_ID, HASH, VERSION_CODE, NAME, + _ID, APP_ID, PACKAGE_NAME, VERSION_NAME, REPO_ID, HASH, VERSION_CODE, NAME, SIZE, SIGNATURE, SOURCE_NAME, MIN_SDK_VERSION, TARGET_SDK_VERSION, MAX_SDK_VERSION, PERMISSIONS, FEATURES, NATIVE_CODE, HASH_TYPE, ADDED_DATE, - IS_COMPATIBLE, REPO_VERSION, REPO_ADDRESS, INCOMPATIBLE_REASONS, + IS_COMPATIBLE, Repo.VERSION, Repo.ADDRESS, INCOMPATIBLE_REASONS, }; } } diff --git a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java index 0033c353e..3f0335b69 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -39,6 +39,11 @@ public class TempApkProvider extends ApkProvider { return TABLE_TEMP_APK; } + @Override + protected String getAppTableName() { + return TempAppProvider.TABLE_TEMP_APP; + } + public static String getAuthority() { return AUTHORITY + "." + PROVIDER_NAME; } @@ -111,7 +116,7 @@ public class TempApkProvider extends ApkProvider { switch (MATCHER.match(uri)) { case CODE_REPO_APK: List pathSegments = uri.getPathSegments(); - query = query.add(queryRepo(Long.parseLong(pathSegments.get(1)))).add(queryApks(pathSegments.get(2))); + query = query.add(queryRepo(Long.parseLong(pathSegments.get(1)), false)).add(queryApks(pathSegments.get(2), false)); break; default: 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 3290dfb88..e3907c480 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -3,9 +3,13 @@ package org.fdroid.fdroid.data; import android.content.ContentValues; import android.content.Context; import android.content.UriMatcher; +import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteException; import android.net.Uri; +import android.text.TextUtils; + +import java.util.List; import org.fdroid.fdroid.data.Schema.ApkTable; import org.fdroid.fdroid.data.Schema.AppTable; @@ -22,19 +26,21 @@ public class TempAppProvider extends AppProvider { private static final String PROVIDER_NAME = "TempAppProvider"; - private static final String TABLE_TEMP_APP = "temp_" + AppTable.NAME; + static final String TABLE_TEMP_APP = "temp_" + AppTable.NAME; private static final String PATH_INIT = "init"; private static final String PATH_COMMIT = "commit"; private static final int CODE_INIT = 10000; private static final int CODE_COMMIT = CODE_INIT + 1; + private static final int APPS = CODE_COMMIT + 1; private static final UriMatcher MATCHER = new UriMatcher(-1); static { MATCHER.addURI(getAuthority(), PATH_INIT, CODE_INIT); MATCHER.addURI(getAuthority(), PATH_COMMIT, CODE_COMMIT); + MATCHER.addURI(getAuthority(), PATH_APPS + "/*", APPS); MATCHER.addURI(getAuthority(), "*", CODE_SINGLE); } @@ -55,6 +61,17 @@ public class TempAppProvider extends AppProvider { return Uri.withAppendedPath(getContentUri(), app.packageName); } + public static Uri getAppsUri(List apps) { + return getContentUri().buildUpon() + .appendPath(PATH_APPS) + .appendPath(TextUtils.join(",", apps)) + .build(); + } + + private AppQuerySelection queryApps(String packageNames) { + return queryApps(packageNames, getTableName() + ".id"); + } + public static class Helper { /** @@ -67,6 +84,12 @@ public class TempAppProvider extends AppProvider { TempApkProvider.Helper.init(context); } + public static List findByPackageNames(Context context, List packageNames, String[] projection) { + Uri uri = getAppsUri(packageNames); + Cursor cursor = context.getContentResolver().query(uri, projection, null, null, null); + return AppProvider.Helper.cursorToList(cursor); + } + /** * Saves data from the temp table to the apk table, by removing _EVERYTHING_ from the real * apk table and inserting all of the records from here. The temporary table is then removed. @@ -116,6 +139,18 @@ public class TempAppProvider extends AppProvider { return count; } + @Override + public Cursor query(Uri uri, String[] projection, String customSelection, String[] selectionArgs, String sortOrder) { + AppQuerySelection selection = new AppQuerySelection(customSelection, selectionArgs); + switch (MATCHER.match(uri)) { + case APPS: + selection = selection.add(queryApps(uri.getLastPathSegment())); + break; + } + + return super.runQuery(uri, selection, projection, true, sortOrder); + } + private void ensureTempTableDetached(SQLiteDatabase db) { try { db.execSQL("DETACH DATABASE " + DB); diff --git a/app/src/test/java/org/fdroid/fdroid/Assert.java b/app/src/test/java/org/fdroid/fdroid/Assert.java index cb6354b1f..97d3dff7c 100644 --- a/app/src/test/java/org/fdroid/fdroid/Assert.java +++ b/app/src/test/java/org/fdroid/fdroid/Assert.java @@ -1,12 +1,14 @@ package org.fdroid.fdroid; import android.content.ContentValues; +import android.content.Context; import android.database.Cursor; import android.net.Uri; import junit.framework.AssertionFailedError; import org.fdroid.fdroid.data.ApkProvider; +import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.InstalledAppProvider; import org.fdroid.fdroid.data.Schema.ApkTable; @@ -174,14 +176,14 @@ public class Assert { cursor.close(); } - public static void insertApp(ShadowContentResolver resolver, String appId, String name) { - insertApp(resolver, appId, name, new ContentValues()); + public static App insertApp(Context context, String packageName, String name) { + return insertApp(context, packageName, name, new ContentValues()); } - public static void insertApp(ShadowContentResolver resolver, String id, String name, ContentValues additionalValues) { + public static App insertApp(Context context, String packageName, String name, ContentValues additionalValues) { ContentValues values = new ContentValues(); - values.put(AppTable.Cols.PACKAGE_NAME, id); + values.put(AppTable.Cols.PACKAGE_NAME, packageName); values.put(AppTable.Cols.NAME, name); // Required fields (NOT NULL in the database). @@ -196,18 +198,38 @@ public class Assert { Uri uri = AppProvider.getContentUri(); - resolver.insert(uri, values); + context.getContentResolver().insert(uri, values); + return AppProvider.Helper.findByPackageName(context.getContentResolver(), packageName); } - public static Uri insertApk(ShadowContentResolver resolver, String id, int versionCode) { - return insertApk(resolver, id, versionCode, new ContentValues()); + private static App ensureApp(Context context, String packageName) { + App app = AppProvider.Helper.findByPackageName(context.getContentResolver(), packageName); + if (app == null) { + insertApp(context, packageName, packageName); + app = AppProvider.Helper.findByPackageName(context.getContentResolver(), packageName); + } + assertNotNull(app); + return app; } - public static Uri insertApk(ShadowContentResolver resolver, String id, int versionCode, ContentValues additionalValues) { + public static Uri insertApk(Context context, String packageName, int versionCode) { + return insertApk(context, ensureApp(context, packageName), versionCode); + } + + public static Uri insertApk(Context context, String packageName, int versionCode, ContentValues additionalValues) { + return insertApk(context, ensureApp(context, packageName), versionCode, additionalValues); + } + + public static Uri insertApk(Context context, App app, int versionCode) { + return insertApk(context, app, versionCode, new ContentValues()); + } + + public static Uri insertApk(Context context, App app, int versionCode, ContentValues additionalValues) { ContentValues values = new ContentValues(); - values.put(ApkTable.Cols.PACKAGE_NAME, id); + values.put(ApkTable.Cols.APP_ID, app.getId()); + values.put(ApkTable.Cols.PACKAGE_NAME, app.packageName); values.put(ApkTable.Cols.VERSION_CODE, versionCode); // Required fields (NOT NULL in the database). @@ -222,7 +244,7 @@ public class Assert { Uri uri = ApkProvider.getContentUri(); - return resolver.insert(uri, values); + return context.getContentResolver().insert(uri, values); } } diff --git a/app/src/test/java/org/fdroid/fdroid/TestUtils.java b/app/src/test/java/org/fdroid/fdroid/TestUtils.java index 2ced7a025..2cc05800c 100644 --- a/app/src/test/java/org/fdroid/fdroid/TestUtils.java +++ b/app/src/test/java/org/fdroid/fdroid/TestUtils.java @@ -1,5 +1,12 @@ package org.fdroid.fdroid; +import android.content.ContentResolver; +import android.content.ContextWrapper; + +import org.mockito.AdditionalAnswers; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.shadows.ShadowContentResolver; + import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -8,6 +15,7 @@ import java.io.OutputStream; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; public class TestUtils { @@ -37,4 +45,21 @@ public class TestUtils { return tempFile; } + /** + * The way that Robolectric has to implement shadows for Android classes such as {@link android.content.ContentProvider} + * is by using a special annotation that means the classes will implement the correct methods at runtime. + * However this means that the shadow of a content provider does not actually extend + * {@link android.content.ContentProvider}. As such, we need to do some special mocking using + * Mockito in order to provide a {@link ContextWrapper} which is able to return a proper + * content resolver that delegates to the Robolectric shadow object. + */ + public static ContextWrapper createContextWithContentResolver(ShadowContentResolver contentResolver) { + final ContentResolver resolver = mock(ContentResolver.class, AdditionalAnswers.delegatesTo(contentResolver)); + return new ContextWrapper(RuntimeEnvironment.application.getApplicationContext()) { + @Override + public ContentResolver getContentResolver() { + return resolver; + } + }; + } } diff --git a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java index 089706aab..20d9b609c 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -25,6 +25,7 @@ import java.util.List; import static org.fdroid.fdroid.Assert.assertCantDelete; import static org.fdroid.fdroid.Assert.assertContainsOnly; import static org.fdroid.fdroid.Assert.assertResultCount; +import static org.fdroid.fdroid.Assert.insertApp; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -38,9 +39,11 @@ public class ApkProviderTest extends FDroidProviderTest { @Test public void testAppApks() { + App fdroidApp = insertApp(context, "org.fdroid.fdroid", "F-Droid"); + App exampleApp = insertApp(context, "com.example", "Example"); for (int i = 1; i <= 10; i++) { - Assert.insertApk(contentResolver, "org.fdroid.fdroid", i); - Assert.insertApk(contentResolver, "com.example", i); + Assert.insertApk(context, fdroidApp, i); + Assert.insertApk(context, exampleApp, i); } assertTotalApkCount(20); @@ -188,7 +191,7 @@ public class ApkProviderTest extends FDroidProviderTest { Apk apk = new MockApk("org.fdroid.fdroid", 13); // Insert a new record... - Uri newUri = Assert.insertApk(contentResolver, apk.packageName, apk.versionCode); + Uri newUri = Assert.insertApk(context, apk.packageName, apk.versionCode); assertEquals(ApkProvider.getContentUri(apk).toString(), newUri.toString()); cursor = queryAllApks(); assertNotNull(cursor); @@ -206,7 +209,7 @@ public class ApkProviderTest extends FDroidProviderTest { @Test(expected = IllegalArgumentException.class) public void testCursorMustMoveToFirst() { - Assert.insertApk(contentResolver, "org.example.test", 12); + Assert.insertApk(context, "org.example.test", 12); Cursor cursor = queryAllApks(); new Apk(cursor); } @@ -216,7 +219,7 @@ public class ApkProviderTest extends FDroidProviderTest { String[] projectionCount = new String[] {Cols._COUNT}; for (int i = 0; i < 13; i++) { - Assert.insertApk(contentResolver, "com.example", i); + Assert.insertApk(context, "com.example", i); } Uri all = ApkProvider.getContentUri(); @@ -261,7 +264,7 @@ public class ApkProviderTest extends FDroidProviderTest { public void assertInvalidExtraField(String field) { ContentValues invalidRepo = new ContentValues(); invalidRepo.put(field, "Test data"); - Assert.insertApk(contentResolver, "org.fdroid.fdroid", 10, invalidRepo); + Assert.insertApk(context, "org.fdroid.fdroid", 10, invalidRepo); } @Test @@ -271,10 +274,10 @@ public class ApkProviderTest extends FDroidProviderTest { ContentValues values = new ContentValues(); values.put(Cols.REPO_ID, 10); - values.put(Cols.REPO_ADDRESS, "http://example.com"); - values.put(Cols.REPO_VERSION, 3); + values.put(Cols.Repo.ADDRESS, "http://example.com"); + values.put(Cols.Repo.VERSION, 3); values.put(Cols.FEATURES, "Some features"); - Uri uri = Assert.insertApk(contentResolver, "com.example.com", 1, values); + Uri uri = Assert.insertApk(context, "com.example.com", 1, values); assertResultCount(1, queryAllApks()); @@ -301,18 +304,18 @@ public class ApkProviderTest extends FDroidProviderTest { public void testKnownApks() { for (int i = 0; i < 7; i++) { - Assert.insertApk(contentResolver, "org.fdroid.fdroid", i); + Assert.insertApk(context, "org.fdroid.fdroid", i); } for (int i = 0; i < 9; i++) { - Assert.insertApk(contentResolver, "org.example", i); + Assert.insertApk(context, "org.example", i); } for (int i = 0; i < 3; i++) { - Assert.insertApk(contentResolver, "com.example", i); + Assert.insertApk(context, "com.example", i); } - Assert.insertApk(contentResolver, "com.apk.thingo", 1); + Assert.insertApk(context, "com.apk.thingo", 1); Apk[] known = { new MockApk("org.fdroid.fdroid", 1), @@ -359,18 +362,18 @@ public class ApkProviderTest extends FDroidProviderTest { public void testFindByApp() { for (int i = 0; i < 7; i++) { - Assert.insertApk(contentResolver, "org.fdroid.fdroid", i); + Assert.insertApk(context, "org.fdroid.fdroid", i); } for (int i = 0; i < 9; i++) { - Assert.insertApk(contentResolver, "org.example", i); + Assert.insertApk(context, "org.example", i); } for (int i = 0; i < 3; i++) { - Assert.insertApk(contentResolver, "com.example", i); + Assert.insertApk(context, "com.example", i); } - Assert.insertApk(contentResolver, "com.apk.thingo", 1); + Assert.insertApk(context, "com.apk.thingo", 1); assertTotalApkCount(7 + 9 + 3 + 1); @@ -394,7 +397,7 @@ public class ApkProviderTest extends FDroidProviderTest { @Test public void testUpdate() { - Uri apkUri = Assert.insertApk(contentResolver, "com.example", 10); + Uri apkUri = Assert.insertApk(context, "com.example", 10); String[] allFields = Cols.ALL; Cursor cursor = contentResolver.query(apkUri, allFields, null, null, null); @@ -453,18 +456,20 @@ public class ApkProviderTest extends FDroidProviderTest { // the Helper.find() method doesn't stumble upon the app we are interested // in by shear dumb luck... for (int i = 0; i < 10; i++) { - Assert.insertApk(contentResolver, "org.fdroid.apk." + i, i); + Assert.insertApk(context, "org.fdroid.apk." + i, i); } + App exampleApp = insertApp(context, "com.example", "Example"); + ContentValues values = new ContentValues(); values.put(Cols.VERSION_NAME, "v1.1"); values.put(Cols.HASH, "xxxxyyyy"); values.put(Cols.HASH_TYPE, "a hash type"); - Assert.insertApk(contentResolver, "com.example", 11, values); + Assert.insertApk(context, exampleApp, 11, values); // ...and a few more for good measure... for (int i = 15; i < 20; i++) { - Assert.insertApk(contentResolver, "com.other.thing." + i, i); + Assert.insertApk(context, "com.other.thing." + i, i); } Apk apk = ApkProvider.Helper.find(context, "com.example", 11); @@ -540,7 +545,7 @@ public class ApkProviderTest extends FDroidProviderTest { protected Apk insertApkForRepo(String id, int versionCode, long repoId) { ContentValues additionalValues = new ContentValues(); additionalValues.put(Cols.REPO_ID, repoId); - Uri uri = Assert.insertApk(contentResolver, id, versionCode, additionalValues); + Uri uri = Assert.insertApk(context, id, versionCode, additionalValues); return ApkProvider.Helper.get(context, uri); } } diff --git a/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java b/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java index adddf695c..713992d4b 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java @@ -3,6 +3,7 @@ package org.fdroid.fdroid.data; import android.content.ContentResolver; import android.content.ContextWrapper; +import org.fdroid.fdroid.TestUtils; import org.junit.After; import org.junit.Before; import org.mockito.AdditionalAnswers; @@ -20,13 +21,7 @@ public abstract class FDroidProviderTest { @Before public final void setupBase() { contentResolver = Shadows.shadowOf(RuntimeEnvironment.application.getContentResolver()); - final ContentResolver resolver = mock(ContentResolver.class, AdditionalAnswers.delegatesTo(contentResolver)); - context = new ContextWrapper(RuntimeEnvironment.application.getApplicationContext()) { - @Override - public ContentResolver getContentResolver() { - return resolver; - } - }; + context = TestUtils.createContextWithContentResolver(contentResolver); ShadowContentResolver.registerProvider(AppProvider.getAuthority(), new AppProvider()); } 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 d8b8f0599..945b5f646 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java @@ -1,6 +1,7 @@ package org.fdroid.fdroid.data; import org.fdroid.fdroid.BuildConfig; +import org.fdroid.fdroid.TestUtils; import org.fdroid.fdroid.data.Schema.InstalledAppTable; import org.fdroid.fdroid.mock.MockApk; import org.junit.After; @@ -93,14 +94,27 @@ public class ProviderUriTests { App app = new App(); app.packageName = "org.fdroid.fdroid"; - List apps = new ArrayList<>(1); - apps.add(app); - assertValidUri(resolver, AppProvider.getContentUri(app), "content://org.fdroid.fdroid.data.AppProvider/org.fdroid.fdroid", projection); - assertValidUri(resolver, AppProvider.getContentUri(apps), "content://org.fdroid.fdroid.data.AppProvider/apps/org.fdroid.fdroid", projection); assertValidUri(resolver, AppProvider.getContentUri("org.fdroid.fdroid"), "content://org.fdroid.fdroid.data.AppProvider/org.fdroid.fdroid", projection); } + @Test + public void validTempAppProviderUris() { + ShadowContentResolver.registerProvider(TempAppProvider.getAuthority(), new TempAppProvider()); + String[] projection = new String[]{Schema.AppTable.Cols._ID}; + + // Required so that the `assertValidUri` calls below will indeed have a real temp_fdroid_app + // table to query. + TempAppProvider.Helper.init(TestUtils.createContextWithContentResolver(resolver)); + + List packageNames = new ArrayList<>(2); + packageNames.add("org.fdroid.fdroid"); + packageNames.add("com.example.com"); + + assertValidUri(resolver, TempAppProvider.getAppsUri(packageNames), "content://org.fdroid.fdroid.data.TempAppProvider/apps/org.fdroid.fdroid%2Ccom.example.com", projection); + assertValidUri(resolver, TempAppProvider.getContentUri(), "content://org.fdroid.fdroid.data.TempAppProvider", projection); + } + @Test public void invalidApkProviderUris() { ShadowContentResolver.registerProvider(ApkProvider.getAuthority(), new ApkProvider());