From 125acd6276f80aa07d29969486384188340a8367 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 25 Jul 2016 22:40:20 +1000 Subject: [PATCH] Migrate app preferences to different table. In the process, realised that using appId as a foreign key is worse than packageName, because appId can get removed and added again, but it will be different when the same app is inserted a second time. In order to maintain the association of which apps have preferences stored against them, they need to be stored against something with a bit more semantic meaning. Thus, join onto package name instead. --- .../fdroid/fdroid/data/AppPrefsProvider.java | 26 +++---- .../org/fdroid/fdroid/data/AppProvider.java | 2 +- .../java/org/fdroid/fdroid/data/DBHelper.java | 69 ++++++++++++++----- .../java/org/fdroid/fdroid/data/Schema.java | 8 ++- 4 files changed, 72 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java index e4e0cf10c..7ed4cf2f2 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppPrefsProvider.java @@ -25,10 +25,10 @@ public class AppPrefsProvider extends FDroidProvider { values.put(Cols.IGNORE_THIS_UPDATE, prefs.ignoreThisUpdate); if (getPrefsOrNull(context, app) == null) { - values.put(Cols.APP_ID, app.getId()); + values.put(Cols.PACKAGE_NAME, app.packageName); context.getContentResolver().insert(getContentUri(), values); } else { - context.getContentResolver().update(getAppUri(app.getId()), values, null, null); + context.getContentResolver().update(getAppUri(app.packageName), values, null, null); } } @@ -40,7 +40,7 @@ public class AppPrefsProvider extends FDroidProvider { @Nullable public static AppPrefs getPrefsOrNull(Context context, App app) { - Cursor cursor = context.getContentResolver().query(getAppUri(app.getId()), Cols.ALL, null, null, null); + Cursor cursor = context.getContentResolver().query(getAppUri(app.packageName), Cols.ALL, null, null, null); if (cursor == null) { return null; } @@ -77,18 +77,18 @@ public class AppPrefsProvider extends FDroidProvider { private static final UriMatcher MATCHER = new UriMatcher(-1); - private static final String PATH_APP_ID = "appId"; + private static final String PATH_PACKAGE_NAME = "packageName"; static { - MATCHER.addURI(getAuthority(), PATH_APP_ID + "/#", CODE_SINGLE); + MATCHER.addURI(getAuthority(), PATH_PACKAGE_NAME + "/*", CODE_SINGLE); } private static Uri getContentUri() { return Uri.parse("content://" + getAuthority()); } - public static Uri getAppUri(long appId) { - return getContentUri().buildUpon().appendPath(PATH_APP_ID).appendPath(Long.toString(appId)).build(); + public static Uri getAppUri(String packageName) { + return getContentUri().buildUpon().appendPath(PATH_PACKAGE_NAME).appendPath(packageName).build(); } @Override @@ -110,9 +110,9 @@ public class AppPrefsProvider extends FDroidProvider { return MATCHER; } - protected QuerySelection querySingle(long appId) { - final String selection = getTableName() + "." + Cols.APP_ID + " = ?"; - final String[] args = {Long.toString(appId)}; + protected QuerySelection querySingle(String packageName) { + final String selection = getTableName() + "." + Cols.PACKAGE_NAME + " = ?"; + final String[] args = {packageName}; return new QuerySelection(selection, args); } @@ -122,7 +122,7 @@ public class AppPrefsProvider extends FDroidProvider { switch (MATCHER.match(uri)) { case CODE_SINGLE: - selection = selection.add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + selection = selection.add(querySingle(uri.getLastPathSegment())); break; default: @@ -152,7 +152,7 @@ public class AppPrefsProvider extends FDroidProvider { public Uri insert(Uri uri, ContentValues values) { db().insertOrThrow(getTableName(), null, values); getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); - return getAppUri(values.getAsLong(Cols.APP_ID)); + return getAppUri(values.getAsString(Cols.PACKAGE_NAME)); } @Override @@ -160,7 +160,7 @@ public class AppPrefsProvider extends FDroidProvider { switch (MATCHER.match(uri)) { case CODE_SINGLE: QuerySelection query = new QuerySelection(where, whereArgs) - .add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + .add(querySingle(uri.getLastPathSegment())); int count = db().update(getTableName(), values, query.getSelection(), query.getArgs()); getContext().getContentResolver().notifyChange(AppProvider.getCanUpdateUri(), null); return count; 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 4432094e8..8ff64b039 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -300,7 +300,7 @@ public class AppProvider extends FDroidProvider { leftJoin( AppPrefsTable.NAME, "prefs", - "prefs." + AppPrefsTable.Cols.APP_ID + " = " + getTableName() + "." + Cols.ROW_ID); + "prefs." + AppPrefsTable.Cols.PACKAGE_NAME + " = " + getTableName() + "." + Cols.PACKAGE_NAME); requiresLeftJoinToPrefs = true; } } 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 9c0e01b9d..e97ffd63a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -102,7 +102,7 @@ class DBHelper extends SQLiteOpenHelper { private static final String CREATE_TABLE_APP_PREFS = "CREATE TABLE " + AppPrefsTable.NAME + " ( " - + AppPrefsTable.Cols.APP_ID + " INT REFERENCES " + AppTable.NAME + "(" + AppTable.Cols.ROW_ID + ") ON DELETE CASCADE, " + + AppPrefsTable.Cols.PACKAGE_NAME + " TEXT, " + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + " INT BOOLEAN NOT NULL, " + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + " INT NOT NULL " + " );"; @@ -120,7 +120,7 @@ class DBHelper extends SQLiteOpenHelper { + " );"; private static final String DROP_TABLE_INSTALLED_APP = "DROP TABLE " + InstalledAppTable.NAME + ";"; - private static final int DB_VERSION = 59; + private static final int DB_VERSION = 60; private final Context context; @@ -326,6 +326,31 @@ class DBHelper extends SQLiteOpenHelper { addTargetSdkVersionToApk(db, oldVersion); migrateAppPrimaryKeyToRowId(db, oldVersion); removeApkPackageNameColumn(db, oldVersion); + addAppPrefsTable(db, oldVersion); + } + + private void addAppPrefsTable(SQLiteDatabase db, int oldVersion) { + if (oldVersion < 60) { + + Utils.debugLog(TAG, "Creating app preferences table"); + db.execSQL(CREATE_TABLE_APP_PREFS); + + Utils.debugLog(TAG, "Migrating app preferences to separate table"); + db.execSQL( + "INSERT INTO " + AppPrefsTable.NAME + " (" + + AppPrefsTable.Cols.PACKAGE_NAME + ", " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ", " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + + ") SELECT " + + AppTable.Cols.PACKAGE_NAME + ", " + + "ignoreThisUpdate, " + + "ignoreAllUpdates " + + "FROM " + AppTable.NAME + " " + + "WHERE ignoreThisUpdate > 0 OR ignoreAllUpdates > 0" + ); + + resetTransient(db); + } } /** @@ -666,19 +691,23 @@ class DBHelper extends SQLiteOpenHelper { * their repos (either manually or on a scheduled task), they will update regardless of whether * they have changed since last update or not. */ - private void clearRepoEtags(SQLiteDatabase db) { + private static void clearRepoEtags(SQLiteDatabase db) { Utils.debugLog(TAG, "Clearing repo etags, so next update will not be skipped with \"Repos up to date\"."); db.execSQL("update " + RepoTable.NAME + " set " + RepoTable.Cols.LAST_ETAG + " = NULL"); } - private void resetTransient(SQLiteDatabase db, int oldVersion) { - context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit() - .putBoolean("triedEmptyUpdate", false).apply(); - db.execSQL("drop table " + AppTable.NAME); - db.execSQL("drop table " + ApkTable.NAME); - clearRepoEtags(db); + private void resetTransient(SQLiteDatabase db) { + Utils.debugLog(TAG, "Removing app + apk tables so they can be recreated. Next time F-Droid updates it should trigger an index update."); + context.getSharedPreferences("FDroid", Context.MODE_PRIVATE) + .edit() + .putBoolean("triedEmptyUpdate", false) + .apply(); + + db.execSQL("DROP TABLE " + AppTable.NAME); + db.execSQL("DROP TABLE " + ApkTable.NAME); db.execSQL(CREATE_TABLE_APP); db.execSQL(CREATE_TABLE_APK); + clearRepoEtags(db); ensureIndexes(db); } @@ -712,12 +741,14 @@ class DBHelper extends SQLiteOpenHelper { db.execSQL("CREATE INDEX IF NOT EXISTS apk_appId on " + ApkTable.NAME + " (" + ApkTable.Cols.APP_ID + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS repoId ON " + ApkTable.NAME + " (" + ApkTable.Cols.REPO_ID + ");"); - Utils.debugLog(TAG, "Ensuring indexes exist for " + AppPrefsTable.NAME); - db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.APP_ID + ");"); - db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" + - AppPrefsTable.Cols.APP_ID + ", " + - AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", " + - AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");"); + if (tableExists(db, AppPrefsTable.NAME)) { + Utils.debugLog(TAG, "Ensuring indexes exist for " + AppPrefsTable.NAME); + db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_packageName on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.PACKAGE_NAME + ");"); + db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_packageName_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" + + AppPrefsTable.Cols.PACKAGE_NAME + ", " + + AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", " + + AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");"); + } Utils.debugLog(TAG, "Ensuring indexes exist for " + InstalledAppTable.NAME); db.execSQL("CREATE INDEX IF NOT EXISTS installedApp_appId_vercode on " + InstalledAppTable.NAME + " (" + @@ -753,10 +784,14 @@ class DBHelper extends SQLiteOpenHelper { + ApkTable.Cols.TARGET_SDK_VERSION + " integer"); } - private static boolean columnExists(SQLiteDatabase db, - String table, String column) { + private static boolean columnExists(SQLiteDatabase db, String table, String column) { return db.rawQuery("select * from " + table + " limit 0,1", null) .getColumnIndex(column) != -1; } + private static boolean tableExists(SQLiteDatabase db, String table) { + return db.rawQuery("SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?", + new String[] {table}).getCount() > 0; + } + } 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 fd5a1ec48..dfef154c0 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -14,11 +14,15 @@ public interface Schema { String NAME = "fdroid_appPrefs"; interface Cols extends BaseColumns { - String APP_ID = "appId"; + // Join onto app table via packageName, not appId. The corresponding app row could + // be deleted and then re-added in the future with the same metadata but a different + // rowid. This should not cause us to forget the preferences specified by a user. + String PACKAGE_NAME = "packageName"; + String IGNORE_ALL_UPDATES = "ignoreAllUpdates"; String IGNORE_THIS_UPDATE = "ignoreThisUpdate"; - String[] ALL = {APP_ID, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE}; + String[] ALL = {PACKAGE_NAME, IGNORE_ALL_UPDATES, IGNORE_THIS_UPDATE}; } }