Merge branch 'fix-511--remove-packageName-from-apk' into 'master'

Remove now unused package name from apk table

The package name is only stored in the `fdroid_app` table now, so we need to remove it form the `fdroid_apk` table. Under normal circumstances, I'd normally just leave unused fields in the DDL (the SQL which defines the tables) and never use it from within the Java code. However in this case, the package name formed part of the primary key of this table. Seeing as we are not inserting into that column any more, it isn't okay to leave it there but instead it must be removed so that we can put a more appropriate primary key on the table. In this case, the new primary key is `appId` + `vercode` + `repoId`.

I think this is the final merge request before I submit a MR with repo priorities.

See merge request !359
This commit is contained in:
Daniel Martí 2016-07-25 09:53:42 +00:00
commit 7e451c87c7
9 changed files with 132 additions and 27 deletions

View File

@ -94,7 +94,7 @@ public class Apk extends ValueObject implements Comparable<Apk> {
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<Apk> {
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);

View File

@ -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

View File

@ -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 ";

View File

@ -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,92 @@ class DBHelper extends SQLiteOpenHelper {
recreateInstalledAppTable(db, oldVersion);
addTargetSdkVersionToApk(db, oldVersion);
migrateAppPrimaryKeyToRowId(db, oldVersion);
removeApkPackageNameColumn(db, oldVersion);
}
/**
* Ordinarily, if a column is no longer used, we'd err on the side of just leaving it in the
* database but stop referring to it in Java. However because it forms part of the primary
* key of this table, we need to change the primary key to something which _is_ used. Thus,
* this function will rename the old table, create the new table, and then insert all of the
* data from the old into the new with the new primary key.
*/
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 + ";");
// Now that the old table has been dropped, we can create indexes again.
// Attempting this before dropping the old table will not work, because the
// indexes exist on the _old_ table, and so are unable to be added (with the
// same name) to the _new_ table.
ensureIndexes(db);
db.setTransactionSuccessful();
} finally {
db.endTransaction();
}
}
}
private void migrateAppPrimaryKeyToRowId(SQLiteDatabase db, int oldVersion) {
@ -328,10 +413,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);
@ -607,7 +694,7 @@ class DBHelper extends SQLiteOpenHelper {
Utils.debugLog(TAG, "Ensuring indexes exist for " + ApkTable.NAME);
db.execSQL("CREATE INDEX IF NOT EXISTS apk_vercode on " + ApkTable.NAME + " (" + ApkTable.Cols.VERSION_CODE + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS apk_appId on " + ApkTable.NAME + " (" + ApkTable.Cols.APP_ID + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS apk_id on " + ApkTable.NAME + " (" + AppTable.Cols.PACKAGE_NAME + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS repoId ON " + ApkTable.NAME + " (" + ApkTable.Cols.REPO_ID + ");");
}
/**

View File

@ -189,7 +189,7 @@ public class RepoPersister {
*/
private ArrayList<ContentProviderOperation> insertOrUpdateApks(List<Apk> packages) {
String[] projection = new String[]{
Schema.ApkTable.Cols.PACKAGE_NAME,
Schema.ApkTable.Cols.App.PACKAGE_NAME,
Schema.ApkTable.Cols.VERSION_CODE,
};
List<Apk> existingApks = ApkProvider.Helper.knownApks(context, packages, projection);
@ -275,7 +275,7 @@ public class RepoPersister {
*/
@Nullable
private ContentProviderOperation deleteOrphanedApks(List<App> apps, Map<String, List<Apk>> 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<Apk> existing = ApkProvider.Helper.find(context, repo, apps, projection);
List<Apk> toDelete = new ArrayList<>();

View File

@ -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,

View File

@ -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 + ");");
}

View File

@ -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).

View File

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