WIP
Bug 1320806 - Add type column to ActivityStream blocklist table
MozReview-Commit-ID: G9lPXr0xdZN
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -635,16 +635,20 @@ public class BrowserContract {
public static final class ActivityStreamBlocklist implements CommonColumns {
private ActivityStreamBlocklist() {}
public static final String TABLE_NAME = "activity_stream_blocklist";
public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, TABLE_NAME);
public static final String URL = "url";
public static final String CREATED = "created";
+ public static final String TYPE = "type";
+
+ public static final int TYPE_TOPSITES = 0;
+ public static final int TYPE_HIGHLIGHTS = 1;
}
@RobocopTarget
public static final class UrlAnnotations implements CommonColumns, DateSyncColumns {
private UrlAnnotations() {}
public static final String TABLE_NAME = "urlannotations";
public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, TABLE_NAME);
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
@@ -928,17 +928,19 @@ public final class BrowserDatabaseHelper
}
private void createActivityStreamBlocklistTable(final SQLiteDatabase db) {
debug("Creating " + ActivityStreamBlocklist.TABLE_NAME + " table");
db.execSQL("CREATE TABLE " + ActivityStreamBlocklist.TABLE_NAME + "(" +
ActivityStreamBlocklist._ID + " INTEGER PRIMARY KEY AUTOINCREMENT, " +
ActivityStreamBlocklist.URL + " TEXT UNIQUE NOT NULL, " +
- ActivityStreamBlocklist.CREATED + " INTEGER NOT NULL)");
+ ActivityStreamBlocklist.CREATED + " INTEGER NOT NULL, " +
+ ActivityStreamBlocklist.TYPE + " INTEGER NOT NULL " +
+ ")");
}
private void createReadingListTable(final SQLiteDatabase db, final String tableName) {
debug("Creating " + TABLE_READING_LIST + " table");
db.execSQL("CREATE TABLE " + tableName + "(" +
ReadingListItems._ID + " INTEGER PRIMARY KEY AUTOINCREMENT, " +
ReadingListItems.GUID + " TEXT UNIQUE, " + // Server-assigned.
@@ -1967,22 +1969,42 @@ public final class BrowserDatabaseHelper
}
private void upgradeDatabaseFrom33to34(final SQLiteDatabase db) {
updateHistoryTableAddVisitAggregates(db);
calculateHistoryTableVisitAggregates(db);
createV34CombinedView(db);
}
- private void upgradeDatabaseFrom34to35(final SQLiteDatabase db) {
- createActivityStreamBlocklistTable(db);
+ private void upgradeDatabaseFrom35to36(final SQLiteDatabase db) {
+ createPageMetadataTable(db);
}
- private void upgradeDatabaseFrom35to36(final SQLiteDatabase db) {
- createPageMetadataTable(db);
+ private void addTypeColumnToActivityStreamBlocklist(final SQLiteDatabase db) {
+ // We're merely adding a column here, however doing it manually is the best choice:
+ // Under the hood, sqlite performs this operation by creating a temp table with the new schema,
+ // followed by copying data over - doing this ourselves does not result in a loss of performance.
+ // We specifically want to add a new column that is NOT NULL, but that does not have a default.
+ // This is impossible with the sqlite ADD column syntax: we can either set NOT NULL, in which case
+ // we must supply a DEFAULT (which we don't want); or we don't set a DEFAULT, and we can't use NOT NULL.
+ // Instead we explicityly create our own new table with desired schema, and do copy from a temp table
+ // where we can stuff in the desired value for our new column:
+ final String BLOCKLIST_TMP = ActivityStreamBlocklist.TABLE_NAME + "_tmp";
+
+ db.execSQL("ALTER TABLE " + ActivityStreamBlocklist.TABLE_NAME +
+ " RENAME TO " + BLOCKLIST_TMP);
+
+ createActivityStreamBlocklistTable(db);
+
+ db.execSQL("INSERT INTO " + ActivityStreamBlocklist.TABLE_NAME +
+ " SELECT *, " +
+ ActivityStreamBlocklist.TYPE_HIGHLIGHTS + " AS " + ActivityStreamBlocklist.TYPE +
+ " FROM " + ActivityStreamBlocklist.TABLE_NAME);
+
+ db.execSQL("DROP TABLE " + BLOCKLIST_TMP);
}
private void createV33CombinedView(final SQLiteDatabase db) {
db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED);
db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED_WITH_FAVICONS);
createCombinedViewOn33(db);
}
@@ -2103,23 +2125,39 @@ public final class BrowserDatabaseHelper
case 33:
upgradeDatabaseFrom32to33(db);
break;
case 34:
upgradeDatabaseFrom33to34(db);
break;
- case 35:
- upgradeDatabaseFrom34to35(db);
- break;
+ // case 35:
+ // created the ActivityStreamBlocklist table, is handled separately below in a
+ // table-by-table upgrade process
case 36:
upgradeDatabaseFrom35to36(db);
break;
+
+ // case 37:
+ // adds type column to ActivityStreamBlocklist, is handled separately below
+ }
+
+ // Activity Stream blocklist migrations: it's easier to reason about migrations, and
+ // to write cleaner migration code, if we take into account individual iterations
+ // of a given table. This also means we only need to run one migration for any given
+ // table to get it from the input state to desired state (e.g. if the table doesn't
+ // exist, create it once with an up-to-date schema, if we have v1 of a table run one
+ // migration to bring it to the current schema, if we have v2 run a different migration
+ // to update it to the current schema, etc.).
+ if (oldVersion < 35) {
+ createActivityStreamBlocklistTable(db);
+ } else if (oldVersion >= 35 && oldVersion < 37) {
+ addTypeColumnToActivityStreamBlocklist(db);
}
}
for (Table table : BrowserProvider.sTables) {
table.onUpgrade(db, oldVersion, newVersion);
}
// Delete the obsolete favicon database after all other upgrades complete.