From 485d5e82ed63d709335bd315acb6243144acce17 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Wed, 6 Jul 2016 06:11:59 +1000 Subject: [PATCH] Remove `Schema.ApkTable.Cols.PACKAGE_NAME` completely. Wherever the "package name" of an apk is required, it can be requested by asking for `Schema.ApkTable.Cols.App.PACKAGE_NAME`. Note the `App` which indicates that it is in fact pulling this data from the `fdroid_app` table rather than the `fdroid_apk` table. --- .../main/java/org/fdroid/fdroid/data/Apk.java | 3 +- .../org/fdroid/fdroid/data/ApkProvider.java | 34 ++++++-- .../org/fdroid/fdroid/data/AppProvider.java | 8 +- .../java/org/fdroid/fdroid/data/DBHelper.java | 81 ++++++++++++++++++- .../org/fdroid/fdroid/data/RepoPersister.java | 4 +- .../java/org/fdroid/fdroid/data/Schema.java | 4 +- .../fdroid/fdroid/data/TempApkProvider.java | 1 - .../test/java/org/fdroid/fdroid/Assert.java | 1 - .../fdroid/fdroid/data/ApkProviderTest.java | 7 +- 9 files changed, 117 insertions(+), 26 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 745050bd4..7561d2793 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java @@ -94,7 +94,7 @@ public class Apk extends ValueObject implements Comparable { case Cols.FEATURES: features = Utils.parseCommaSeparatedString(cursor.getString(i)); break; - case Cols.PACKAGE_NAME: + case Cols.App.PACKAGE_NAME: packageName = cursor.getString(i); break; case Cols.IS_COMPATIBLE: @@ -201,7 +201,6 @@ 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); values.put(Cols.REPO_ID, repo); 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 ed3787c45..82f7ee8f1 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java @@ -194,6 +194,7 @@ public class ApkProvider extends FDroidProvider { 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 String PROVIDER_NAME = "ApkProvider"; protected static final String PATH_APK = "apk"; @@ -202,6 +203,7 @@ public class ApkProvider extends FDroidProvider { 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); @@ -219,6 +221,7 @@ public class ApkProvider extends FDroidProvider { MATCHER.addURI(getAuthority(), PATH_APP + "/*", CODE_APP); 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); } @@ -230,6 +233,13 @@ public class ApkProvider extends FDroidProvider { return Uri.parse("content://" + getAuthority()); } + private Uri getApkUri(long apkRowId) { + return getContentUri().buildUpon() + .appendPath(PATH_APK_ROW_ID) + .appendPath(Long.toString(apkRowId)) + .build(); + } + public static Uri getAppUri(String packageName) { return getContentUri() .buildUpon() @@ -384,7 +394,7 @@ public class ApkProvider extends FDroidProvider { 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 selection = alias + Cols.VERSION_CODE + " = ? and " + alias + Cols.APP_ID + " = (" + getAppIdFromPackageNameQuery() + ")"; final String[] args = { // First (0th) path segment is the word "apk", // and we are not interested in it. @@ -394,6 +404,17 @@ public class ApkProvider extends FDroidProvider { return new QuerySelection(selection, args); } + private QuerySelection querySingle(long apkRowId) { + return querySingle(apkRowId, true); + } + + private QuerySelection querySingle(long apkRowId, boolean includeAlias) { + String alias = includeAlias ? "apk." : ""; + final String selection = alias + Cols.ROW_ID + " = ?"; + final String[] args = {Long.toString(apkRowId)}; + return new QuerySelection(selection, args); + } + protected QuerySelection queryRepo(long repoId) { return queryRepo(repoId, true); } @@ -462,6 +483,10 @@ public class ApkProvider extends FDroidProvider { query = query.add(querySingle(uri)); break; + case CODE_APK_ROW_ID: + query = query.add(querySingle(Long.parseLong(uri.getLastPathSegment()))); + break; + case CODE_APP: query = query.add(queryApp(uri.getLastPathSegment())); break; @@ -516,14 +541,11 @@ public class ApkProvider extends FDroidProvider { public Uri insert(Uri uri, ContentValues values) { removeFieldsFromOtherTables(values); validateFields(Cols.ALL, values); - db().insertOrThrow(getTableName(), null, values); + long newId = db().insertOrThrow(getTableName(), null, values); if (!isApplyingBatch()) { getContext().getContentResolver().notifyChange(uri, null); } - return getContentUri( - values.getAsString(Cols.PACKAGE_NAME), - values.getAsInteger(Cols.VERSION_CODE)); - + return getApkUri(newId); } @Override 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 a034fc01e..9653ba1c5 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/AppProvider.java @@ -653,7 +653,7 @@ public class AppProvider extends FDroidProvider { private AppQuerySelection queryNoApks() { final String apk = getApkTableName(); final String app = getTableName(); - String selection = "(SELECT COUNT(*) FROM " + apk + " WHERE " + apk + "." + ApkTable.Cols.PACKAGE_NAME + " = " + app + "." + Cols.PACKAGE_NAME + ") = 0"; + String selection = "(SELECT COUNT(*) FROM " + apk + " WHERE " + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + ") = 0"; return new AppQuerySelection(selection); } @@ -843,7 +843,7 @@ public class AppProvider extends FDroidProvider { "UPDATE " + app + " SET " + Cols.IS_COMPATIBLE + " = ( " + " SELECT TOTAL( " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + ") > 0 " + " FROM " + apk + - " WHERE " + apk + "." + ApkTable.Cols.PACKAGE_NAME + " = " + app + "." + Cols.PACKAGE_NAME + " );"; + " WHERE " + apk + "." + ApkTable.Cols.APP_ID + " = " + app + "." + Cols.ROW_ID + " );"; db().execSQL(updateSql); } @@ -867,7 +867,7 @@ public class AppProvider extends FDroidProvider { " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " + " FROM " + apk + " WHERE " + - app + "." + Cols.PACKAGE_NAME + " = " + apk + "." + ApkTable.Cols.PACKAGE_NAME + " AND " + + app + "." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + " AND " + restrictToStable + " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + Cols.IS_COMPATIBLE + " = 1 ) ) " + " WHERE " + Cols.UPSTREAM_VERSION_CODE + " > 0 "; @@ -894,7 +894,7 @@ public class AppProvider extends FDroidProvider { " SELECT MAX( " + apk + "." + ApkTable.Cols.VERSION_CODE + " ) " + " FROM " + apk + " WHERE " + - app + "." + Cols.PACKAGE_NAME + " = " + apk + "." + ApkTable.Cols.PACKAGE_NAME + " AND " + + app + "." + Cols.ROW_ID + " = " + apk + "." + ApkTable.Cols.APP_ID + " AND " + " ( " + app + "." + Cols.IS_COMPATIBLE + " = 0 OR " + apk + "." + ApkTable.Cols.IS_COMPATIBLE + " = 1 ) ) " + " WHERE " + Cols.UPSTREAM_VERSION_CODE + " = 0 OR " + Cols.UPSTREAM_VERSION_CODE + " IS NULL OR " + Cols.SUGGESTED_VERSION_CODE + " IS NULL "; 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 c64a51b70..c2643dc16 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -46,7 +46,6 @@ 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 + " integer not null, " + ApkTable.Cols.VERSION_NAME + " text not null, " + ApkTable.Cols.REPO_ID + " integer not null, " @@ -66,7 +65,7 @@ class DBHelper extends SQLiteOpenHelper { + ApkTable.Cols.ADDED_DATE + " string, " + ApkTable.Cols.IS_COMPATIBLE + " int not null, " + ApkTable.Cols.INCOMPATIBLE_REASONS + " text, " - + "primary key(" + ApkTable.Cols.PACKAGE_NAME + ", " + ApkTable.Cols.VERSION_CODE + ")" + + "PRIMARY KEY (" + ApkTable.Cols.APP_ID + ", " + ApkTable.Cols.VERSION_CODE + ", " + ApkTable.Cols.REPO_ID + ")" + ");"; private static final String CREATE_TABLE_APP = "CREATE TABLE " + AppTable.NAME @@ -115,7 +114,7 @@ class DBHelper extends SQLiteOpenHelper { + " );"; private static final String DROP_TABLE_INSTALLED_APP = "DROP TABLE " + InstalledAppTable.NAME + ";"; - private static final int DB_VERSION = 58; + private static final int DB_VERSION = 59; private final Context context; @@ -317,6 +316,78 @@ class DBHelper extends SQLiteOpenHelper { recreateInstalledAppTable(db, oldVersion); addTargetSdkVersionToApk(db, oldVersion); migrateAppPrimaryKeyToRowId(db, oldVersion); + removeApkPackageNameColumn(db, oldVersion); + } + + private void removeApkPackageNameColumn(SQLiteDatabase db, int oldVersion) { + if (oldVersion < 59) { + + Utils.debugLog(TAG, "Changing primary key of " + ApkTable.NAME + " from package + vercode to app + vercode + repo"); + db.beginTransaction(); + + try { + // http://stackoverflow.com/questions/805363/how-do-i-rename-a-column-in-a-sqlite-database-table#805508 + String tempTableName = ApkTable.NAME + "__temp__"; + db.execSQL("ALTER TABLE " + ApkTable.NAME + " RENAME TO " + tempTableName + ";"); + + String createTableDdl = "CREATE TABLE " + ApkTable.NAME + " ( " + + ApkTable.Cols.APP_ID + " integer not null, " + + ApkTable.Cols.VERSION_NAME + " text not null, " + + ApkTable.Cols.REPO_ID + " integer not null, " + + ApkTable.Cols.HASH + " text not null, " + + ApkTable.Cols.VERSION_CODE + " int not null," + + ApkTable.Cols.NAME + " text not null, " + + ApkTable.Cols.SIZE + " int not null, " + + ApkTable.Cols.SIGNATURE + " string, " + + ApkTable.Cols.SOURCE_NAME + " string, " + + ApkTable.Cols.MIN_SDK_VERSION + " integer, " + + ApkTable.Cols.TARGET_SDK_VERSION + " integer, " + + ApkTable.Cols.MAX_SDK_VERSION + " integer, " + + ApkTable.Cols.PERMISSIONS + " string, " + + ApkTable.Cols.FEATURES + " string, " + + ApkTable.Cols.NATIVE_CODE + " string, " + + ApkTable.Cols.HASH_TYPE + " string, " + + ApkTable.Cols.ADDED_DATE + " string, " + + ApkTable.Cols.IS_COMPATIBLE + " int not null, " + + ApkTable.Cols.INCOMPATIBLE_REASONS + " text, " + + "PRIMARY KEY (" + ApkTable.Cols.APP_ID + ", " + ApkTable.Cols.VERSION_CODE + ", " + ApkTable.Cols.REPO_ID + ")" + + ");"; + + db.execSQL(createTableDdl); + + String nonPackageNameFields = TextUtils.join(", ", new String[] { + ApkTable.Cols.APP_ID, + ApkTable.Cols.VERSION_NAME, + ApkTable.Cols.REPO_ID, + ApkTable.Cols.HASH, + ApkTable.Cols.VERSION_CODE, + ApkTable.Cols.NAME, + ApkTable.Cols.SIZE, + ApkTable.Cols.SIGNATURE, + ApkTable.Cols.SOURCE_NAME, + ApkTable.Cols.MIN_SDK_VERSION, + ApkTable.Cols.TARGET_SDK_VERSION, + ApkTable.Cols.MAX_SDK_VERSION, + ApkTable.Cols.PERMISSIONS, + ApkTable.Cols.FEATURES, + ApkTable.Cols.NATIVE_CODE, + ApkTable.Cols.HASH_TYPE, + ApkTable.Cols.ADDED_DATE, + ApkTable.Cols.IS_COMPATIBLE, + ApkTable.Cols.INCOMPATIBLE_REASONS, + }); + + String insertSql = "INSERT INTO " + ApkTable.NAME + + "(" + nonPackageNameFields + " ) " + + "SELECT " + nonPackageNameFields + " FROM " + tempTableName + ";"; + + db.execSQL(insertSql); + db.execSQL("DROP TABLE " + tempTableName + ";"); + db.setTransactionSuccessful(); + } finally { + db.endTransaction(); + } + } } private void migrateAppPrimaryKeyToRowId(SQLiteDatabase db, int oldVersion) { @@ -328,10 +399,12 @@ class DBHelper extends SQLiteOpenHelper { Utils.debugLog(TAG, alter); db.execSQL(alter); + // Hard coded the string literal ".id" as ApkTable.Cols.PACKAGE_NAME was removed in + // the subsequent migration (DB_VERSION 59) final String update = "UPDATE " + ApkTable.NAME + " SET " + ApkTable.Cols.APP_ID + " = ( " + "SELECT app." + AppTable.Cols.ROW_ID + " " + "FROM " + AppTable.NAME + " AS app " + - "WHERE " + ApkTable.NAME + "." + ApkTable.Cols.PACKAGE_NAME + " = app." + AppTable.Cols.PACKAGE_NAME + ")"; + "WHERE " + ApkTable.NAME + ".id = app." + AppTable.Cols.PACKAGE_NAME + ")"; Log.i(TAG, "Updating foreign key from " + ApkTable.NAME + " to " + AppTable.NAME + " to use numeric foreign key."); Utils.debugLog(TAG, update); db.execSQL(update); 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 98fdd849a..43e1dd177 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java @@ -189,7 +189,7 @@ public class RepoPersister { */ private ArrayList insertOrUpdateApks(List packages) { String[] projection = new String[]{ - Schema.ApkTable.Cols.PACKAGE_NAME, + Schema.ApkTable.Cols.App.PACKAGE_NAME, Schema.ApkTable.Cols.VERSION_CODE, }; List existingApks = ApkProvider.Helper.knownApks(context, packages, projection); @@ -275,7 +275,7 @@ public class RepoPersister { */ @Nullable private ContentProviderOperation deleteOrphanedApks(List apps, Map> packages) { - String[] projection = new String[]{Schema.ApkTable.Cols.PACKAGE_NAME, Schema.ApkTable.Cols.VERSION_CODE}; + String[] projection = new String[]{Schema.ApkTable.Cols.App.PACKAGE_NAME, Schema.ApkTable.Cols.VERSION_CODE}; List existing = ApkProvider.Helper.find(context, repo, apps, projection); List toDelete = new ArrayList<>(); 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 d9c12d7f0..7b39a045a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Schema.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Schema.java @@ -91,7 +91,7 @@ public interface Schema { * Foreign key to the {@link AppTable}. */ String APP_ID = "appId"; - String PACKAGE_NAME = "id"; + String ROW_ID = "rowid"; String VERSION_NAME = "version"; String REPO_ID = "repo"; String HASH = "hash"; @@ -121,7 +121,7 @@ public interface Schema { } String[] ALL = { - _ID, APP_ID, PACKAGE_NAME, VERSION_NAME, REPO_ID, HASH, VERSION_CODE, NAME, + _ID, APP_ID, App.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, 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 3f0335b69..8d35fdf1a 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java @@ -137,7 +137,6 @@ public class TempApkProvider extends ApkProvider { final String memoryDbName = TempAppProvider.DB; db.execSQL("CREATE TABLE " + memoryDbName + "." + getTableName() + " AS SELECT * FROM main." + ApkTable.NAME); db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_vercode on " + getTableName() + " (" + ApkTable.Cols.VERSION_CODE + ");"); - db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_id on " + getTableName() + " (" + ApkTable.Cols.PACKAGE_NAME + ");"); db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + ApkTable.Cols.IS_COMPATIBLE + ");"); } diff --git a/app/src/test/java/org/fdroid/fdroid/Assert.java b/app/src/test/java/org/fdroid/fdroid/Assert.java index 97d3dff7c..40b59e71e 100644 --- a/app/src/test/java/org/fdroid/fdroid/Assert.java +++ b/app/src/test/java/org/fdroid/fdroid/Assert.java @@ -229,7 +229,6 @@ public class Assert { ContentValues values = new ContentValues(); 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). 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 20d9b609c..fcbdf38e5 100644 --- a/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java +++ b/app/src/test/java/org/fdroid/fdroid/data/ApkProviderTest.java @@ -191,8 +191,7 @@ public class ApkProviderTest extends FDroidProviderTest { Apk apk = new MockApk("org.fdroid.fdroid", 13); // Insert a new record... - Uri newUri = Assert.insertApk(context, apk.packageName, apk.versionCode); - assertEquals(ApkProvider.getContentUri(apk).toString(), newUri.toString()); + Assert.insertApk(context, apk.packageName, apk.versionCode); cursor = queryAllApks(); assertNotNull(cursor); assertEquals(1, cursor.getCount()); @@ -345,7 +344,7 @@ public class ApkProviderTest extends FDroidProviderTest { Collections.addAll(apksToCheck, unknown); String[] projection = { - Cols.PACKAGE_NAME, + Cols.App.PACKAGE_NAME, Cols.VERSION_CODE, }; @@ -485,7 +484,7 @@ public class ApkProviderTest extends FDroidProviderTest { assertEquals("a hash type", apk.hashType); String[] projection = { - Cols.PACKAGE_NAME, + Cols.App.PACKAGE_NAME, Cols.HASH, };