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.

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) {
+    public static Uri getAppUri(String packageName) {
-        return getContentUri().buildUpon().appendPath(PATH_APP_ID).appendPath(Long.toString(appId)).build();
+        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) {
+    protected QuerySelection querySingle(String packageName) {
-        final String selection = getTableName() + "." + Cols.APP_ID + " = ?";
+        final String selection = getTableName() + "." + Cols.PACKAGE_NAME + " = ?";
-        final String[] args = {Long.toString(appId)};
+        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;

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;
             }
         }

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) {
+    private void resetTransient(SQLiteDatabase db) {
-        context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit()
+        Utils.debugLog(TAG, "Removing app + apk tables so they can be recreated. Next time F-Droid updates it should trigger an index update.");
-                .putBoolean("triedEmptyUpdate", false).apply();
+        context.getSharedPreferences("FDroid", Context.MODE_PRIVATE)
-        db.execSQL("drop table " + AppTable.NAME);
+                .edit()
-        db.execSQL("drop table " + ApkTable.NAME);
+                .putBoolean("triedEmptyUpdate", false)
-        clearRepoEtags(db);
+                .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);
+        if (tableExists(db, AppPrefsTable.NAME)) {
-        db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.APP_ID + ");");
+            Utils.debugLog(TAG, "Ensuring indexes exist for " + AppPrefsTable.NAME);
-        db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_appId_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" +
+            db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_packageName on " + AppPrefsTable.NAME + " (" + AppPrefsTable.Cols.PACKAGE_NAME + ");");
-                AppPrefsTable.Cols.APP_ID + ", " +
+            db.execSQL("CREATE INDEX IF NOT EXISTS appPrefs_packageName_ignoreAll_ignoreThis on " + AppPrefsTable.NAME + " (" +
-                AppPrefsTable.Cols.IGNORE_ALL_UPDATES + ", " +
+                    AppPrefsTable.Cols.PACKAGE_NAME + ", " +
-                AppPrefsTable.Cols.IGNORE_THIS_UPDATE + ");");
+                    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,
+    private static boolean columnExists(SQLiteDatabase db, String table, String column) {
-            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;
+    }
+
 }

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};
         }
     }