Karl/BobStore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

More robust fix for #763, specifying column names to copy explicitly.

Browse Source 
This is far less brittle at runtime, but slightly more work at dev time.
The following things are undesirable but make it much easier to write:
 * Use of `CREATE_TABLE_APP.replaceFirst(...)` to create the temp tables.
 * Having to specify a list fo columns twice in `Schema` (`ALL_COLS` + `COLS`).

The `replaceFirst` means we don't need to maintain two separate create table
statements. It is a little messy because there is no compile time guarantee
that we are creating a valid SQL statement at the end, just our knowledge
that a create table statment tends to have the table name first and it
probably wont cause problems.

The `ALL_COLS` + `COLS` is required so that we don't have to type out a list
of fields when copying data in `TempAppProvider`. Otherwise, whenever a new
column is added, developers would need to know that it also needs to be added
to this third place. Currently it is in the `Schema` and the `CREATE_TABLE_*`
statements where one needs to add a new column. These are both intuitive and
hopefully easily discoverable. Having to add it to the `TempAppProvider` is
less intuitive and likely to result in bugs.
This commit is contained in:
Peter Serwylo 2016-09-24 08:10:01 +10:00
parent 1ba6034e19
commit 1678223cab
4 changed files with 47 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/src/main/java/org/fdroid/fdroid/data/DBHelper.java
View File

@ -45,7 +45,7 @@ class DBHelper extends SQLiteOpenHelper {
+ RepoTable.Cols.TIMESTAMP + " integer not null default 0"
+ ");";
private static final String CREATE_TABLE_APK =
static final String CREATE_TABLE_APK =
"CREATE TABLE " + ApkTable.NAME + " ( "
+ ApkTable.Cols.APP_ID + " integer not null, "
+ ApkTable.Cols.VERSION_NAME + " text not null, "

32
app/src/main/java/org/fdroid/fdroid/data/Schema.java
View File

@ -76,6 +76,25 @@ public interface Schema {
String SIGNATURE = "installedSig";
}
/**
* Each of the physical columns in the sqlite table. Differs from {@link Cols#ALL} in
* that it doesn't include fields which are aliases of other fields (e.g. {@link Cols#_ID}
* or which are from other related tables (e.g. {@link Cols.SuggestedApk#VERSION_NAME}).
*/
String[] ALL_COLS = {
ROW_ID, IS_COMPATIBLE, PACKAGE_NAME, NAME, SUMMARY, ICON, DESCRIPTION,
LICENSE, AUTHOR, EMAIL, WEB_URL, TRACKER_URL, SOURCE_URL,
CHANGELOG_URL, DONATE_URL, BITCOIN_ADDR, LITECOIN_ADDR, FLATTR_ID,
UPSTREAM_VERSION_NAME, UPSTREAM_VERSION_CODE, ADDED, LAST_UPDATED,
CATEGORIES, ANTI_FEATURES, REQUIREMENTS, ICON_URL, ICON_URL_LARGE,
SUGGESTED_VERSION_CODE,
};
/**
* Superset of {@link Cols#ALL_COLS} including fields from other tables and also an alias
* to satisfy the Android requirement for an "_ID" field.
* @see Cols#ALL_COLS
*/
String[] ALL = {
_ID, ROW_ID, IS_COMPATIBLE, PACKAGE_NAME, NAME, SUMMARY, ICON, DESCRIPTION,
LICENSE, AUTHOR, EMAIL, WEB_URL, TRACKER_URL, SOURCE_URL,
@ -134,6 +153,19 @@ public interface Schema {
String PACKAGE_NAME = "appPackageName";
}
/**
* @see AppMetadataTable.Cols#ALL_COLS
*/
String[] ALL_COLS = {
APP_ID, 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, INCOMPATIBLE_REASONS,
};
/**
* @see AppMetadataTable.Cols#ALL
*/
String[] ALL = {
_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,

3
app/src/main/java/org/fdroid/fdroid/data/TempApkProvider.java
View File

@ -125,7 +125,8 @@ public class TempApkProvider extends ApkProvider {
private void initTable() {
final SQLiteDatabase db = db();
final String memoryDbName = TempAppProvider.DB;
db.execSQL("CREATE TABLE " + memoryDbName + "." + getTableName() + " AS SELECT * FROM main." + ApkTable.NAME);
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_vercode on " + getTableName() + " (" + ApkTable.Cols.VERSION_CODE + ");");
db.execSQL("CREATE INDEX IF NOT EXISTS " + memoryDbName + ".apk_compatible ON " + getTableName() + " (" + ApkTable.Cols.IS_COMPATIBLE + ");");
}

15
app/src/main/java/org/fdroid/fdroid/data/TempAppProvider.java
View File

@ -162,12 +162,21 @@ public class TempAppProvider extends AppProvider {
ensureTempTableDetached(db);
db.execSQL("ATTACH DATABASE ':memory:' AS " + DB);
db.execSQL(DBHelper.CREATE_TABLE_APP.replaceFirst(AppMetadataTable.NAME, DB + "." + getTableName()));
db.execSQL("INSERT INTO " + DB + "." + getTableName() + " SELECT * FROM " + AppMetadataTable.NAME);
db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, AppMetadataTable.NAME, DB + "." + getTableName()));
db.execSQL("CREATE INDEX IF NOT EXISTS " + DB + ".app_id ON " + getTableName() + " (" + AppMetadataTable.Cols.PACKAGE_NAME + ");");
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 + ");");
}
/**
* 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) {
String cols = TextUtils.join(", ", colsToCopy);
return "INSERT INTO " + toTable + " (" + cols + ") SELECT " + cols + " FROM " + fromTable;
}
private void commitTable() {
final SQLiteDatabase db = db();
try {
@ -177,10 +186,10 @@ public class TempAppProvider extends AppProvider {
final String tempApk = DB + "." + TempApkProvider.TABLE_TEMP_APK;
db.execSQL("DELETE FROM " + AppMetadataTable.NAME + " WHERE 1");
db.execSQL("INSERT INTO " + AppMetadataTable.NAME + " SELECT * FROM " + tempApp);
db.execSQL(copyData(AppMetadataTable.Cols.ALL_COLS, tempApp, AppMetadataTable.NAME));
db.execSQL("DELETE FROM " + ApkTable.NAME + " WHERE 1");
db.execSQL("INSERT INTO " + ApkTable.NAME + " SELECT * FROM " + tempApk);
db.execSQL(copyData(ApkTable.Cols.ALL_COLS, tempApk, ApkTable.NAME));
db.setTransactionSuccessful();