diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java index e14344b2c..bf12dad1a 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java @@ -139,7 +139,7 @@ public class IndexV1Updater extends RepoUpdater { * and {@link Apk} instances. This uses {@link RepoPersister} to add the apps * and packages to the database in {@link RepoPersister#saveToDb(App, List)} * to write the {@link Repo}, and commit the whole thing in - * {@link RepoPersister#commit(ContentValues)}. One confusing thing about this + * {@link RepoPersister#commit(ContentValues, long)}. One confusing thing about this * whole process is that {@link RepoPersister} needs to first create and entry * in the database, then fetch the ID from the database to populate * {@link Repo#id}. That has to happen first, then the rest of the {@code Repo} @@ -265,7 +265,7 @@ public class IndexV1Updater extends RepoUpdater { if (repo.mirrors != null && repo.mirrors.length > 0) { contentValues.put(Schema.RepoTable.Cols.MIRRORS, Utils.serializeCommaSeparatedString(repo.mirrors)); } - repoPersister.commit(contentValues); + repoPersister.commit(contentValues, repo.getId()); profiler.log("Persited to database."); diff --git a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java index f77548e33..b76475c03 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoUpdater.java @@ -280,7 +280,7 @@ public class RepoUpdater { private void commitToDb() throws UpdateException { Log.i(TAG, "Repo signature verified, saving app metadata to database."); notifyCommittingToDb(); - persister.commit(repoDetailsToSave); + persister.commit(repoDetailsToSave, repo.getId()); } private void assertSigningCertFromXmlCorrect() throws SigningException { 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 d3f3e8ddc..41988a378 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java @@ -94,17 +94,6 @@ public class ApkProvider extends FDroidProvider { return findApkFromAnyRepo(context, packageName, versionCode, signature, Cols.ALL); } - /** - * Find all apks for a particular app, but limit it to those originating from the - * specified repo. - */ - public static List findByUri(Context context, Repo repo, List apps, String[] projection) { - ContentResolver resolver = context.getContentResolver(); - final Uri uri = getContentUriForApps(repo, apps); - Cursor cursor = resolver.query(uri, projection, null, null, null); - return cursorToList(cursor); - } - public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode, @Nullable String signature, String[] projection) { final Uri uri = getApkFromAnyRepoUri(packageName, versionCode, signature); @@ -137,36 +126,6 @@ public class ApkProvider extends FDroidProvider { return cursorToList(cursor); } - /** - * Returns apks in the database, which have the same packageName and version as - * one of the apks in the "apks" argument. - */ - public static List knownApks(Context context, List apks, String[] fields) { - if (apks.isEmpty()) { - return new ArrayList<>(); - } - - List knownApks = new ArrayList<>(); - if (apks.size() > ApkProvider.MAX_APKS_TO_QUERY) { - int middle = apks.size() / 2; - List apks1 = apks.subList(0, middle); - List apks2 = apks.subList(middle, apks.size()); - knownApks.addAll(knownApks(context, apks1, fields)); - knownApks.addAll(knownApks(context, apks2, fields)); - } else { - knownApks.addAll(knownApksSafe(context, apks, fields)); - } - return knownApks; - - } - - private static List knownApksSafe(final Context context, final List apks, final String[] fields) { - ContentResolver resolver = context.getContentResolver(); - final Uri uri = getContentUri(apks); - Cursor cursor = resolver.query(uri, fields, null, null, null); - return cursorToList(cursor); - } - public static List findByRepo(Context context, Repo repo, String[] fields) { ContentResolver resolver = context.getContentResolver(); final Uri uri = getRepoUri(repo.getId()); @@ -206,9 +165,7 @@ public class ApkProvider extends FDroidProvider { private static final int CODE_PACKAGE = CODE_SINGLE + 1; private static final int CODE_REPO = CODE_PACKAGE + 1; private static final int CODE_APKS = CODE_REPO + 1; - private static final int CODE_REPO_APPS = CODE_APKS + 1; - protected static final int CODE_REPO_APK = CODE_REPO_APPS + 1; - private static final int CODE_APK_ROW_ID = CODE_REPO_APK + 1; + private static final int CODE_APK_ROW_ID = CODE_APKS + 1; static final int CODE_APK_FROM_ANY_REPO = CODE_APK_ROW_ID + 1; static final int CODE_APK_FROM_REPO = CODE_APK_FROM_ANY_REPO + 1; @@ -218,8 +175,6 @@ public class ApkProvider extends FDroidProvider { private static final String PATH_APKS = "apks"; private static final String PATH_APP = "app"; private static final String PATH_REPO = "repo"; - private static final String PATH_REPO_APPS = "repo-apps"; - protected static final String PATH_REPO_APK = "repo-apk"; private static final String PATH_APK_ROW_ID = "apk-rowId"; private static final UriMatcher MATCHER = new UriMatcher(-1); @@ -238,8 +193,6 @@ public class ApkProvider extends FDroidProvider { MATCHER.addURI(getAuthority(), PATH_APK_FROM_REPO + "/#/#", CODE_APK_FROM_REPO); MATCHER.addURI(getAuthority(), PATH_APKS + "/*", CODE_APKS); MATCHER.addURI(getAuthority(), PATH_APP + "/*", CODE_PACKAGE); - MATCHER.addURI(getAuthority(), PATH_REPO_APPS + "/#/*", CODE_REPO_APPS); - MATCHER.addURI(getAuthority(), PATH_REPO_APK + "/#/*", CODE_REPO_APK); MATCHER.addURI(getAuthority(), PATH_APK_ROW_ID + "/#", CODE_APK_ROW_ID); MATCHER.addURI(getAuthority(), null, CODE_LIST); } @@ -293,51 +246,6 @@ public class ApkProvider extends FDroidProvider { return builder.build(); } - public static Uri getContentUriForApps(Repo repo, List apps) { - return getContentUri() - .buildUpon() - .appendPath(PATH_REPO_APPS) - .appendPath(Long.toString(repo.id)) - .appendPath(buildAppString(apps)) - .build(); - } - - /** - * Intentionally left protected because it will break if apks is larger than - * {@link org.fdroid.fdroid.data.ApkProvider#MAX_APKS_TO_QUERY}. Instead of using - * this directly, think about using - * {@link ApkProvider.Helper#knownApks(android.content.Context, java.util.List, String[])} - */ - static Uri getContentUri(List apks) { - return getContentUri().buildUpon() - .appendPath(PATH_APKS) - .appendPath(buildApkString(apks)) - .build(); - } - - protected static String buildApkString(List apks) { - StringBuilder builder = new StringBuilder(); - for (int i = 0; i < apks.size(); i++) { - if (i != 0) { - builder.append(','); - } - final Apk apk = apks.get(i); - builder.append(apk.appId).append(':').append(apk.versionCode); - } - return builder.toString(); - } - - private static String buildAppString(List apks) { - StringBuilder builder = new StringBuilder(); - for (int i = 0; i < apks.size(); i++) { - if (i != 0) { - builder.append(','); - } - builder.append(apks.get(i).packageName); - } - return builder.toString(); - } - @Override protected String getTableName() { return ApkTable.NAME; @@ -470,10 +378,6 @@ public class ApkProvider extends FDroidProvider { return new QuerySelection(selection, args); } - private QuerySelection queryRepoApps(long repoId, String packageNames) { - return queryRepo(repoId).add(AppProvider.queryPackageNames(packageNames, "pkg." + PackageTable.Cols.PACKAGE_NAME)); - } - protected QuerySelection queryApks(String apkKeys) { return queryApks(apkKeys, true); } @@ -547,11 +451,6 @@ public class ApkProvider extends FDroidProvider { query = query.add(queryRepo(Long.parseLong(uri.getLastPathSegment()))); break; - case CODE_REPO_APPS: - List pathSegments = uri.getPathSegments(); - query = query.add(queryRepoApps(Long.parseLong(pathSegments.get(1)), pathSegments.get(2))); - break; - default: Log.e(TAG, "Invalid URI for apk content provider: " + uri); throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); @@ -611,12 +510,6 @@ public class ApkProvider extends FDroidProvider { query = query.add(queryApks(uri.getLastPathSegment(), false)); break; - // TODO: Add tests for this. - case CODE_REPO_APK: - List pathSegments = uri.getPathSegments(); - query = query.add(queryRepo(Long.parseLong(pathSegments.get(1)))).add(queryApks(pathSegments.get(2))); - break; - default: Log.e(TAG, "Invalid URI for apk content provider: " + uri); throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); @@ -633,10 +526,7 @@ public class ApkProvider extends FDroidProvider { if (MATCHER.match(uri) != CODE_APK_FROM_REPO) { throw new UnsupportedOperationException("Cannot update anything other than a single apk."); } - return performUpdateUnchecked(uri, values, where, whereArgs); - } - protected int performUpdateUnchecked(Uri uri, ContentValues values, String where, String[] whereArgs) { validateFields(Cols.ALL, values); removeFieldsFromOtherTables(values); 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 36426fc0d..4b7f74c1b 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -24,8 +24,10 @@ import org.fdroid.fdroid.data.Schema.RepoTable; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; /** diff --git a/app/src/main/java/org/fdroid/fdroid/data/CategoryProvider.java b/app/src/main/java/org/fdroid/fdroid/data/CategoryProvider.java index 6810fdd48..3997d43dd 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/CategoryProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/CategoryProvider.java @@ -12,13 +12,39 @@ import org.fdroid.fdroid.data.Schema.CategoryTable; import org.fdroid.fdroid.data.Schema.CategoryTable.Cols; import org.fdroid.fdroid.data.Schema.PackageTable; +import java.util.HashMap; +import java.util.Map; + public class CategoryProvider extends FDroidProvider { public static final class Helper { private Helper() { } + /** + * During repo updates, each app needs to know the ID of each category it belongs to. + * This results in lots of database lookups, usually at least one for each app, sometimes more. + * To improve performance, this caches the association between categories and their database IDs. + * + * It can stay around for the entire F-Droid process, even across multiple repo updates, as we + * don't actually remove data from the categories table. + */ + private static final Map KNOWN_CATEGORIES = new HashMap<>(); + + /** + * Used by tests to clear that the "Category -> ID" cache (used to prevent excessive disk reads). + */ + static void clearCategoryIdCache() { + KNOWN_CATEGORIES.clear(); + } + public static long ensureExists(Context context, String category) { + // Check our in-memory cache to potentially prevent a trip to the database (and hence disk). + String lowerCaseCategory = category.toLowerCase(); + if (KNOWN_CATEGORIES.containsKey(lowerCaseCategory)) { + return KNOWN_CATEGORIES.get(lowerCaseCategory); + } + long id = getCategoryId(context, category); if (id <= 0) { ContentValues values = new ContentValues(1); @@ -26,10 +52,13 @@ public class CategoryProvider extends FDroidProvider { Uri uri = context.getContentResolver().insert(getContentUri(), values); id = Long.parseLong(uri.getLastPathSegment()); } + + KNOWN_CATEGORIES.put(lowerCaseCategory, id); + return id; } - public static long getCategoryId(Context context, String category) { + private static long getCategoryId(Context context, String category) { String[] projection = new String[]{Cols.ROW_ID}; Cursor cursor = context.getContentResolver().query(getCategoryUri(category), projection, null, null, null); 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 abe30a51d..f278de932 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -7,7 +7,6 @@ import android.content.OperationApplicationException; import android.net.Uri; import android.os.RemoteException; import android.support.annotation.NonNull; -import android.support.annotation.Nullable; import org.fdroid.fdroid.CompatibilityChecker; import org.fdroid.fdroid.RepoUpdater; @@ -18,7 +17,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -@SuppressWarnings("LineLength") public class RepoPersister { private static final String TAG = "RepoPersister"; @@ -66,9 +64,9 @@ public class RepoPersister { } } - public void commit(ContentValues repoDetailsToSave) throws RepoUpdater.UpdateException { + public void commit(ContentValues repoDetailsToSave, long repoIdToCommit) throws RepoUpdater.UpdateException { flushBufferToDb(); - TempAppProvider.Helper.commitAppsAndApks(context); + TempAppProvider.Helper.commitAppsAndApks(context, repoIdToCommit); RepoProvider.Helper.update(context, repo, repoDetailsToSave); } @@ -79,12 +77,12 @@ public class RepoPersister { // the index was signed with until we've finished reading it - and we don't // want to put stuff in the real database until we are sure it is from a // trusted source. It also helps performance as it is done via an in-memory database. - TempAppProvider.Helper.init(context); + TempAppProvider.Helper.init(context, repo.getId()); hasBeenInitialized = true; } 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."); + Utils.debugLog(TAG, "Flushing details of up to " + MAX_APP_BUFFER + " apps/packages to the database."); Map appIds = flushAppsToDbInBatch(); flushApksToDbInBatch(appIds); apksToSave.clear(); @@ -103,12 +101,7 @@ public class RepoPersister { calcApkCompatibilityFlags(apksToSaveList); - ArrayList apkOperations = new ArrayList<>(); - ContentProviderOperation clearOrphans = deleteOrphanedApks(appsToSave, apksToSave); - if (clearOrphans != null) { - apkOperations.add(clearOrphans); - } - apkOperations.addAll(insertOrUpdateApks(apksToSaveList)); + ArrayList apkOperations = insertApks(apksToSaveList); try { context.getContentResolver().applyBatch(TempApkProvider.getAuthority(), apkOperations); @@ -123,7 +116,7 @@ public class RepoPersister { * 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); + ArrayList appOperations = insertApps(appsToSave); try { context.getContentResolver().applyBatch(TempAppProvider.getAuthority(), appOperations); @@ -144,7 +137,12 @@ public class RepoPersister { for (App app : apps) { packageNames.add(app.packageName); } - String[] projection = {Schema.AppMetadataTable.Cols.ROW_ID, Schema.AppMetadataTable.Cols.Package.PACKAGE_NAME}; + + String[] projection = { + Schema.AppMetadataTable.Cols.ROW_ID, + Schema.AppMetadataTable.Cols.Package.PACKAGE_NAME, + }; + List fromDb = TempAppProvider.Helper.findByPackageNames(context, packageNames, repo.id, projection); Map ids = new HashMap<>(fromDb.size()); @@ -154,138 +152,27 @@ public class RepoPersister { 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. - */ - private ArrayList insertOrUpdateApps(List apps) { + private ArrayList insertApps(List apps) { ArrayList operations = new ArrayList<>(apps.size()); for (App app : apps) { - if (isAppInDatabase(app)) { - operations.add(updateExistingApp(app)); - } else { - operations.add(insertNewApp(app)); - } + ContentValues values = app.toContentValues(); + Uri uri = TempAppProvider.getContentUri(); + operations.add(ContentProviderOperation.newInsert(uri).withValues(values).build()); } return operations; } - /** - * Depending on whether the .apks have been added to the database previously, this - * will queue up an update or an insert {@link ContentProviderOperation} for each package. - */ - private ArrayList insertOrUpdateApks(List packages) { - String[] projection = new String[]{ - Schema.ApkTable.Cols.Package.PACKAGE_NAME, - Schema.ApkTable.Cols.VERSION_CODE, - Schema.ApkTable.Cols.REPO_ID, - Schema.ApkTable.Cols.APP_ID, - }; - List existingApks = ApkProvider.Helper.knownApks(context, packages, projection); + private ArrayList insertApks(List packages) { ArrayList operations = new ArrayList<>(packages.size()); for (Apk apk : packages) { - boolean exists = false; - for (Apk existing : existingApks) { - if (existing.repoId == apk.repoId && existing.packageName.equals(apk.packageName) && existing.versionCode == apk.versionCode) { - exists = true; - break; - } - } - - if (exists) { - operations.add(updateExistingApk(apk)); - } else { - operations.add(insertNewApk(apk)); - } + ContentValues values = apk.toContentValues(); + Uri uri = TempApkProvider.getContentUri(); + operations.add(ContentProviderOperation.newInsert(uri).withValues(values).build()); } return operations; } - /** - * Creates an update {@link ContentProviderOperation} for the {@link App} in question. - * Does not do any checks to see if the app already exists or not. - */ - private ContentProviderOperation updateExistingApp(App app) { - Uri uri = TempAppProvider.getSpecificTempAppUri(app.packageName, app.repoId); - return ContentProviderOperation.newUpdate(uri).withValues(app.toContentValues()).build(); - } - - /** - * Creates an insert {@link ContentProviderOperation} for the {@link App} in question. - * Does not do any checks to see if the app already exists or not. - */ - private ContentProviderOperation insertNewApp(App app) { - ContentValues values = app.toContentValues(); - Uri uri = TempAppProvider.getContentUri(); - return ContentProviderOperation.newInsert(uri).withValues(values).build(); - } - - /** - * Looks in the database to see which apps we already know about. Only - * returns ids of apps that are in the database if they are in the "apps" - * array. - */ - private boolean isAppInDatabase(App app) { - String[] fields = {Schema.AppMetadataTable.Cols.Package.PACKAGE_NAME}; - App found = AppProvider.Helper.findSpecificApp(context.getContentResolver(), app.packageName, repo.id, fields); - return found != null; - } - - /** - * Creates an update {@link ContentProviderOperation} for the {@link Apk} in question. - * Does not do any checks to see if the apk already exists or not. - */ - private ContentProviderOperation updateExistingApk(final Apk apk) { - Uri uri = TempApkProvider.getApkUri(apk); - ContentValues values = apk.toContentValues(); - return ContentProviderOperation.newUpdate(uri).withValues(values).build(); - } - - /** - * Creates an insert {@link ContentProviderOperation} for the {@link Apk} in question. - * Does not do any checks to see if the apk already exists or not. - */ - private ContentProviderOperation insertNewApk(final Apk apk) { - ContentValues values = apk.toContentValues(); - Uri uri = TempApkProvider.getContentUri(); - return ContentProviderOperation.newInsert(uri).withValues(values).build(); - } - - /** - * Finds all apks from the repo we are currently updating, that belong to the specified app, - * and delete them as they are no longer provided by that repo. - */ - @Nullable - private ContentProviderOperation deleteOrphanedApks(List apps, Map> packages) { - String[] projection = new String[]{Schema.ApkTable.Cols.Package.PACKAGE_NAME, Schema.ApkTable.Cols.VERSION_CODE}; - List existing = ApkProvider.Helper.findByUri(context, repo, apps, projection); - List toDelete = new ArrayList<>(); - - for (Apk existingApk : existing) { - boolean shouldStay = false; - - if (packages.containsKey(existingApk.packageName)) { - for (Apk newApk : packages.get(existingApk.packageName)) { - if (newApk.versionCode == existingApk.versionCode) { - shouldStay = true; - break; - } - } - } - - if (!shouldStay) { - toDelete.add(existingApk); - } - } - - if (toDelete.size() == 0) { - return null; - } - Uri uri = TempApkProvider.getApksUri(repo, toDelete); - return ContentProviderOperation.newDelete(uri).build(); - } - /** * This cannot be offloaded to the database (as we did with the query which * updates apps, depending on whether their apks are compatible or not). 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 c10996b7c..da3e80565 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -85,6 +85,8 @@ public interface Schema { String NAME = "fdroid_categoryAppMetadataJoin"; interface Cols { + String ROW_ID = "rowid"; + /** * Foreign key to {@link AppMetadataTable}. * @see AppMetadataTable @@ -100,7 +102,7 @@ public interface Schema { /** * @see AppMetadataTable.Cols#ALL_COLS */ - String[] ALL_COLS = {APP_METADATA_ID, CATEGORY_ID}; + String[] ALL_COLS = {ROW_ID, APP_METADATA_ID, CATEGORY_ID}; } } 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 412688121..5a45c827b 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -7,8 +7,7 @@ import android.database.sqlite.SQLiteDatabase; import android.net.Uri; import org.fdroid.fdroid.data.Schema.ApkTable; - -import java.util.List; +import org.fdroid.fdroid.data.Schema.ApkTable.Cols; /** * This class does all of its operations in a temporary sqlite table. @@ -27,10 +26,9 @@ public class TempApkProvider extends ApkProvider { private static final UriMatcher MATCHER = new UriMatcher(-1); static { - MATCHER.addURI(getAuthority(), PATH_INIT, CODE_INIT); + MATCHER.addURI(getAuthority(), PATH_INIT + "/#", CODE_INIT); MATCHER.addURI(getAuthority(), PATH_APK_FROM_ANY_REPO + "/#/*", CODE_APK_FROM_ANY_REPO); MATCHER.addURI(getAuthority(), PATH_APK_FROM_REPO + "/#/#", CODE_APK_FROM_REPO); - MATCHER.addURI(getAuthority(), PATH_REPO_APK + "/#/*", CODE_REPO_APK); } @Override @@ -51,24 +49,6 @@ public class TempApkProvider extends ApkProvider { return Uri.parse("content://" + getAuthority()); } - public static Uri getApkUri(Apk apk) { - return getContentUri() - .buildUpon() - .appendPath(PATH_APK_FROM_REPO) - .appendPath(Long.toString(apk.appId)) - .appendPath(Integer.toString(apk.versionCode)) - .build(); - } - - public static Uri getApksUri(Repo repo, List apks) { - return getContentUri() - .buildUpon() - .appendPath(PATH_REPO_APK) - .appendPath(Long.toString(repo.id)) - .appendPath(buildApkString(apks)) - .build(); - } - public static class Helper { /** @@ -76,12 +56,15 @@ public class TempApkProvider extends ApkProvider { * table and populates it with all the data from the real apk provider table. * * This is package local because it must be invoked after - * {@link org.fdroid.fdroid.data.TempAppProvider.Helper#init(Context)}. Due to this + * {@link org.fdroid.fdroid.data.TempAppProvider.Helper#init(Context, long)}. Due to this * dependence, that method invokes this one itself, rather than leaving it to the * {@link RepoPersister}. */ - static void init(Context context) { - Uri uri = Uri.withAppendedPath(getContentUri(), PATH_INIT); + static void init(Context context, long repoIdToUpdate) { + Uri uri = getContentUri().buildUpon() + .appendPath(PATH_INIT) + .appendPath(Long.toString(repoIdToUpdate)) + .build(); context.getContentResolver().insert(uri, new ContentValues()); } } @@ -89,7 +72,7 @@ public class TempApkProvider extends ApkProvider { @Override public Uri insert(Uri uri, ContentValues values) { if (MATCHER.match(uri) == CODE_INIT) { - initTable(); + initTable(Long.parseLong(uri.getLastPathSegment())); return null; } @@ -98,39 +81,25 @@ public class TempApkProvider extends ApkProvider { @Override public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { - if (MATCHER.match(uri) != CODE_APK_FROM_REPO) { - throw new UnsupportedOperationException("Cannot update anything other than a single apk."); - } - - return performUpdateUnchecked(uri, values, where, whereArgs); + throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); } @Override public int delete(Uri uri, String where, String[] whereArgs) { - if (MATCHER.match(uri) != CODE_REPO_APK) { - throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); - } - - List pathSegments = uri.getPathSegments(); - QuerySelection query = new QuerySelection(where, whereArgs) - .add(queryRepo(Long.parseLong(pathSegments.get(1)), false)) - .add(queryApks(pathSegments.get(2), false)); - - int rowsAffected = db().delete(getTableName(), query.getSelection(), query.getArgs()); - if (!isApplyingBatch()) { - getContext().getContentResolver().notifyChange(uri, null); - } - return rowsAffected; - + throw new UnsupportedOperationException("Invalid URI for apk content provider: " + uri); } - private void initTable() { + private void initTable(long repoIdBeingUpdated) { final SQLiteDatabase db = db(); final String memoryDbName = TempAppProvider.DB; - db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(Schema.ApkTable.NAME, memoryDbName + "." + getTableName())); - db.execSQL(TempAppProvider.copyData(Schema.ApkTable.Cols.ALL_COLS, Schema.ApkTable.NAME, memoryDbName + "." + getTableName())); - db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + ApkTable.Cols.APP_ID + ");"); - db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + ApkTable.Cols.IS_COMPATIBLE + ");"); + db.execSQL(DBHelper.CREATE_TABLE_APK.replaceFirst(ApkTable.NAME, memoryDbName + "." + getTableName())); + + String where = ApkTable.NAME + "." + Cols.REPO_ID + " != ?"; + String[] whereArgs = new String[]{Long.toString(repoIdBeingUpdated)}; + db.execSQL(TempAppProvider.copyData(Cols.ALL_COLS, ApkTable.NAME, memoryDbName + "." + getTableName(), where), whereArgs); + + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_appId on " + getTableName() + " (" + Cols.APP_ID + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + Cols.IS_COMPATIBLE + ");"); } } 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 79d61817c..1f5ebb3a4 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java @@ -8,7 +8,6 @@ import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteException; import android.net.Uri; import android.text.TextUtils; -import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Schema.ApkTable; import org.fdroid.fdroid.data.Schema.AppMetadataTable; import org.fdroid.fdroid.data.Schema.AppMetadataTable.Cols; @@ -43,8 +42,8 @@ public class TempAppProvider extends AppProvider { 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_INIT + "/#", CODE_INIT); + MATCHER.addURI(getAuthority(), PATH_COMMIT + "/#", CODE_COMMIT); MATCHER.addURI(getAuthority(), PATH_APPS + "/#/*", APPS); MATCHER.addURI(getAuthority(), PATH_SPECIFIC_APP + "/#/*", CODE_SINGLE); } @@ -67,19 +66,6 @@ public class TempAppProvider extends AppProvider { return Uri.parse("content://" + getAuthority()); } - /** - * Same as {@link AppProvider#getSpecificAppUri(String, long)}, except loads data from the temp - * table being used during a repo update rather than the persistent table. - */ - public static Uri getSpecificTempAppUri(String packageName, long repoId) { - return getContentUri() - .buildUpon() - .appendPath(PATH_SPECIFIC_APP) - .appendPath(Long.toString(repoId)) - .appendPath(packageName) - .build(); - } - public static Uri getAppsUri(List apps, long repoId) { return getContentUri().buildUpon() .appendPath(PATH_APPS) @@ -105,10 +91,13 @@ public class TempAppProvider extends AppProvider { * Deletes the old temporary table (if it exists). Then creates a new temporary apk provider * table and populates it with all the data from the real apk provider table. */ - public static void init(Context context) { - Uri uri = Uri.withAppendedPath(getContentUri(), PATH_INIT); + public static void init(Context context, long repoIdToUpdate) { + Uri uri = getContentUri().buildUpon() + .appendPath(PATH_INIT) + .appendPath(Long.toString(repoIdToUpdate)) + .build(); context.getContentResolver().insert(uri, new ContentValues()); - TempApkProvider.Helper.init(context); + TempApkProvider.Helper.init(context, repoIdToUpdate); } public static List findByPackageNames(Context context, @@ -122,8 +111,11 @@ public class TempAppProvider extends AppProvider { * 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. */ - public static void commitAppsAndApks(Context context) { - Uri uri = Uri.withAppendedPath(getContentUri(), PATH_COMMIT); + public static void commitAppsAndApks(Context context, long repoIdToCommit) { + Uri uri = getContentUri().buildUpon() + .appendPath(PATH_COMMIT) + .appendPath(Long.toString(repoIdToCommit)) + .build(); context.getContentResolver().insert(uri, new ContentValues()); } } @@ -137,11 +129,11 @@ public class TempAppProvider extends AppProvider { public Uri insert(Uri uri, ContentValues values) { switch (MATCHER.match(uri)) { case CODE_INIT: - initTable(); + initTable(Long.parseLong(uri.getLastPathSegment())); return null; case CODE_COMMIT: updateAllAppDetails(); - commitTable(); + commitTable(Long.parseLong(uri.getLastPathSegment())); return null; default: return super.insert(uri, values); @@ -150,47 +142,7 @@ public class TempAppProvider extends AppProvider { @Override public int update(Uri uri, ContentValues values, String where, String[] whereArgs) { - if (MATCHER.match(uri) != CODE_SINGLE) { - throw new UnsupportedOperationException("Update not supported for " + uri + "."); - } - - if (values.containsKey(Cols.DESCRIPTION) && values.getAsString(Cols.DESCRIPTION) == null) { - // the database does not let a description be set as null - values.put(Cols.DESCRIPTION, ""); - } - - List pathParts = uri.getPathSegments(); - String packageName = pathParts.get(2); - long repoId = Long.parseLong(pathParts.get(1)); - QuerySelection query = new QuerySelection(where, whereArgs).add(querySingleForUpdate(packageName, repoId)); - - // Package names for apps cannot change... - values.remove(Cols.Package.PACKAGE_NAME); - - if (values.containsKey(Cols.ForWriting.Categories.CATEGORIES)) { - String[] categories = Utils.parseCommaSeparatedString( - values.getAsString(Cols.ForWriting.Categories.CATEGORIES)); - ensureCategories(categories, packageName, repoId); - values.remove(Cols.ForWriting.Categories.CATEGORIES); - } - - int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); - if (!isApplyingBatch()) { - getContext().getContentResolver().notifyChange(getHighestPriorityMetadataUri(packageName), null); - } - return count; - } - - private void ensureCategories(String[] categories, String packageName, long repoId) { - Query query = new AppProvider.Query(); - query.addField(Cols.ROW_ID); - query.addSelection(querySingle(packageName, repoId)); - Cursor cursor = db().rawQuery(query.toString(), query.getArgs()); - cursor.moveToFirst(); - long appMetadataId = cursor.getLong(0); - cursor.close(); - - ensureCategories(categories, appMetadataId); + throw new UnsupportedOperationException("Update not supported for " + uri + "."); } @Override @@ -220,29 +172,45 @@ public class TempAppProvider extends AppProvider { } } - private void initTable() { + private void initTable(long repoIdBeingUpdated) { final SQLiteDatabase db = db(); + + String mainApp = AppMetadataTable.NAME; + String tempApp = DB + "." + getTableName(); + String mainCat = CatJoinTable.NAME; + String tempCat = DB + "." + getCatJoinTableName(); + ensureTempTableDetached(db); db.execSQL("ATTACH DATABASE ':memory:' AS " + DB); - db.execSQL(DBHelper.CREATE_TABLE_APP_METADATA.replaceFirst(AppMetadataTable.NAME, DB + "." + getTableName())); - db.execSQL(DBHelper.CREATE_TABLE_CAT_JOIN.replaceFirst(CatJoinTable.NAME, DB + "." + getCatJoinTableName())); - db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, AppMetadataTable.NAME, DB + "." + getTableName())); - db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, CatJoinTable.NAME, DB + "." + getCatJoinTableName())); - db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + AppMetadataTable.Cols.PACKAGE_ID + ");"); - db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (" + AppMetadataTable.Cols.UPSTREAM_VERSION_CODE + ");"); - db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_compatible ON " + getTableName() + " (" + AppMetadataTable.Cols.IS_COMPATIBLE + ");"); + db.execSQL(DBHelper.CREATE_TABLE_APP_METADATA.replaceFirst(AppMetadataTable.NAME, tempApp)); + db.execSQL(DBHelper.CREATE_TABLE_CAT_JOIN.replaceFirst(CatJoinTable.NAME, tempCat)); + + String appWhere = mainApp + "." + Cols.REPO_ID + " != ?"; + String[] repoArgs = new String[]{Long.toString(repoIdBeingUpdated)}; + db.execSQL(copyData(Cols.ALL_COLS, mainApp, tempApp, appWhere), repoArgs); + + // TODO: String catWhere = mainCat + "." + CatJoinTable.Cols..Cols.REPO_ID + " != ?"; + db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, mainCat, tempCat, null)); + + db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + Cols.PACKAGE_ID + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_upstreamVercode ON " + getTableName() + " (" + Cols.UPSTREAM_VERSION_CODE + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_compatible ON " + getTableName() + " (" + Cols.IS_COMPATIBLE + ");"); } /** * Constructs an INSERT INTO ... SELECT statement as a means from getting data from one table * into another. The list of columns to copy are explicitly specified using colsToCopy. */ - static String copyData(String[] colsToCopy, String fromTable, String toTable) { + static String copyData(String[] colsToCopy, String fromTable, String toTable, String where) { String cols = TextUtils.join(", ", colsToCopy); - return "INSERT INTO " + toTable + " (" + cols + ") SELECT " + cols + " FROM " + fromTable; + String sql = "INSERT INTO " + toTable + " (" + cols + ") SELECT " + cols + " FROM " + fromTable; + if (!TextUtils.isEmpty(where)) { + sql += " WHERE " + where; + } + return sql; } - private void commitTable() { + private void commitTable(long repoIdToCommit) { final SQLiteDatabase db = db(); try { db.beginTransaction(); @@ -251,14 +219,16 @@ public class TempAppProvider extends AppProvider { final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK; final String tempCatJoin = DB + "." + TABLE_TEMP_CAT_JOIN; - db.execSQL("DELETE FROM " + AppMetadataTable.NAME + " WHERE 1"); - db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, tempApp, AppMetadataTable.NAME)); + final String[] repoArgs = new String[]{Long.toString(repoIdToCommit)}; - db.execSQL("DELETE FROM " + ApkTable.NAME + " WHERE 1"); - db.execSQL(copyData(ApkTable.Cols.ALL_COLS, tempApk, ApkTable.NAME)); + db.execSQL("DELETE FROM " + AppMetadataTable.NAME + " WHERE " + Cols.REPO_ID + " = ?", repoArgs); + db.execSQL(copyData(Cols.ALL_COLS, tempApp, AppMetadataTable.NAME, Cols.REPO_ID + " = ?"), repoArgs); - db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE 1"); - db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME)); + db.execSQL("DELETE FROM " + ApkTable.NAME + " WHERE " + ApkTable.Cols.REPO_ID + " = ?", repoArgs); + db.execSQL(copyData(ApkTable.Cols.ALL_COLS, tempApk, ApkTable.NAME, ApkTable.Cols.REPO_ID + " = ?"), repoArgs); + + db.execSQL("DELETE FROM " + CatJoinTable.NAME + " WHERE " + getCatRepoWhere(CatJoinTable.NAME), repoArgs); + db.execSQL(copyData(CatJoinTable.Cols.ALL_COLS, tempCatJoin, CatJoinTable.NAME, getCatRepoWhere(tempCatJoin)), repoArgs); db.setTransactionSuccessful(); @@ -270,4 +240,14 @@ public class TempAppProvider extends AppProvider { db.execSQL("DETACH DATABASE " + DB); // Can't be done in a transaction. } } + + private String getCatRepoWhere(String categoryTable) { + String catRepoSubquery = + "SELECT DISTINCT innerCatJoin." + CatJoinTable.Cols.ROW_ID + " " + + "FROM " + categoryTable + " AS innerCatJoin " + + "JOIN " + getTableName() + " AS app ON (app." + Cols.ROW_ID + " = innerCatJoin." + CatJoinTable.Cols.APP_METADATA_ID + ") " + + "WHERE app." + Cols.REPO_ID + " = ?"; + + return CatJoinTable.Cols.ROW_ID + " IN (" + catRepoSubquery + ")"; + } } 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 8553b0509..11833cf9e 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -238,69 +238,6 @@ public class ApkProviderTest extends FDroidProviderTest { assertEquals(10, apk.repoId); } - @Test - public void testKnownApks() { - - App fdroid = Assert.ensureApp(context, "org.fdroid.fdroid"); - for (int i = 0; i < 7; i++) { - Assert.insertApk(context, fdroid, i); - } - - App exampleOrg = Assert.ensureApp(context, "org.example"); - for (int i = 0; i < 9; i++) { - Assert.insertApk(context, exampleOrg, i); - } - - App exampleCom = Assert.ensureApp(context, "com.example"); - for (int i = 0; i < 3; i++) { - Assert.insertApk(context, exampleCom, i); - } - - App thingo = Assert.ensureApp(context, "com.apk.thingo"); - Assert.insertApk(context, thingo, 1); - - Apk[] known = { - new MockApk(fdroid, 1), - new MockApk(fdroid, 3), - new MockApk(fdroid, 5), - - new MockApk(exampleCom, 1), - new MockApk(exampleCom, 2), - }; - - Apk[] unknown = { - new MockApk(fdroid, 7), - new MockApk(fdroid, 9), - new MockApk(fdroid, 11), - new MockApk(fdroid, 13), - - new MockApk(exampleCom, 3), - new MockApk(exampleCom, 4), - new MockApk(exampleCom, 5), - - new MockApk(-10, 1), - new MockApk(-10, 2), - }; - - List apksToCheck = new ArrayList<>(known.length + unknown.length); - Collections.addAll(apksToCheck, known); - Collections.addAll(apksToCheck, unknown); - - String[] projection = { - Cols.Package.PACKAGE_NAME, - Cols.APP_ID, - Cols.VERSION_CODE, - }; - - List knownApks = ApkProvider.Helper.knownApks(context, apksToCheck, projection); - - assertResultCount(known.length, knownApks); - - for (Apk knownApk : knownApks) { - assertContains(knownApks, knownApk); - } - } - @Test public void testFindByApp() { 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 c1f2463ce..d03b610a5 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/FDroidProviderTest.java @@ -23,6 +23,7 @@ public abstract class FDroidProviderTest { @After public final void tearDownBase() { + CategoryProvider.Helper.clearCategoryIdCache(); FDroidProvider.clearDbHelperSingleton(); } 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 93ec81e7b..05cb646d4 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ProviderUriTests.java @@ -110,7 +110,7 @@ public class ProviderUriTests { // Required so that the `assertValidUri` calls below will indeed have a real temp_fdroid_app // table to query. - TempAppProvider.Helper.init(TestUtils.createContextWithContentResolver(resolver)); + TempAppProvider.Helper.init(TestUtils.createContextWithContentResolver(resolver), 123); List packageNames = new ArrayList<>(2); packageNames.add("org.fdroid.fdroid"); @@ -140,29 +140,7 @@ public class ProviderUriTests { assertValidUri(resolver, ApkProvider.getContentUri(), "content://org.fdroid.fdroid.data.ApkProvider", projection); assertValidUri(resolver, ApkProvider.getAppUri("org.fdroid.fdroid"), "content://org.fdroid.fdroid.data.ApkProvider/app/org.fdroid.fdroid", projection); assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri(new MockApk("org.fdroid.fdroid", 100)), "content://org.fdroid.fdroid.data.ApkProvider/apk-any-repo/100/org.fdroid.fdroid", projection); - assertValidUri(resolver, ApkProvider.getContentUri(apks), projection); assertValidUri(resolver, ApkProvider.getApkFromAnyRepoUri("org.fdroid.fdroid", 100, null), "content://org.fdroid.fdroid.data.ApkProvider/apk-any-repo/100/org.fdroid.fdroid", projection); assertValidUri(resolver, ApkProvider.getRepoUri(1000), "content://org.fdroid.fdroid.data.ApkProvider/repo/1000", projection); } - - @Test(expected = IllegalArgumentException.class) - public void invalidApkUrisWithTooManyApks() { - String[] projection = Schema.ApkTable.Cols.ALL; - - List manyApks = new ArrayList<>(ApkProvider.MAX_APKS_TO_QUERY - 5); - for (int i = 0; i < ApkProvider.MAX_APKS_TO_QUERY - 1; i++) { - manyApks.add(new MockApk("com.example." + i, i)); - } - assertValidUri(resolver, ApkProvider.getContentUri(manyApks), projection); - - manyApks.add(new MockApk("org.fdroid.fdroid.1", 1)); - manyApks.add(new MockApk("org.fdroid.fdroid.2", 2)); - - // Technically, it is a valid URI, because it doesn't - // throw an UnsupportedOperationException. However it - // is still not okay (we run out of bindable parameters - // in the sqlite query. - assertValidUri(resolver, ApkProvider.getContentUri(manyApks), projection); - } - } 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 d887fdb7b..2936110fa 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/ProperMultiRepoUpdaterTest.java @@ -45,8 +45,6 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { @StringDef({"Conflicting", "Normal"}) public @interface RepoIdentifier { } - /* - *This test fails due to issue #568 (https://gitlab.com/fdroid/fdroidclient/issues/568). @Test public void appsRemovedFromRepo() throws RepoUpdater.UpdateException { assertEquals(0, AppProvider.Helper.all(context.getContentResolver()).size()); @@ -67,7 +65,7 @@ public class ProperMultiRepoUpdaterTest extends MultiRepoUpdaterTest { assertEquals(6, ApkProvider.Helper.findByRepo(context, repo, Schema.ApkTable.Cols.ALL).size()); assertEquals(4, ApkProvider.Helper.findByPackageName(context, "org.adaway").size()); assertEquals(2, ApkProvider.Helper.findByPackageName(context, "org.dgtale.icsimport").size()); - }*/ + } @Test public void mainRepo() throws RepoUpdater.UpdateException {