Bug 1451537 - Bump bookmarks mirror schema version and add a migration test. r=mak
MozReview-Commit-ID: FKMGxauwSyi
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -80,17 +80,17 @@ XPCOMUtils.defineLazyGetter(this, "UserC
const DB_URL_LENGTH_MAX = 65536;
const DB_TITLE_LENGTH_MAX = 4096;
const DB_DESCRIPTION_LENGTH_MAX = 256;
const SQLITE_MAX_VARIABLE_NUMBER = 999;
// The current mirror database schema version. Bump for migrations, then add
// migration code to `migrateMirrorSchema`.
-const MIRROR_SCHEMA_VERSION = 1;
+const MIRROR_SCHEMA_VERSION = 2;
// Use a shared jankYielder in these functions
XPCOMUtils.defineLazyGetter(this, "maybeYield", () => Async.jankYielder());
function yieldingIterator(collection) {
return Async.yieldingIterator(collection, maybeYield);
}
/**
@@ -158,42 +158,41 @@ class SyncedBookmarksMirror {
* @return {SyncedBookmarksMirror}
* A mirror ready for use.
*/
static async open(options) {
let db = await Sqlite.cloneStorageConnection({
connection: PlacesUtils.history.DBConnection,
readOnly: false,
});
+ let whyFailed = "initialize";
try {
+ await db.execute(`PRAGMA foreign_keys = ON`);
try {
- await db.execute(`ATTACH :mirrorPath AS mirror`,
- { mirrorPath: options.path });
+ await attachAndInitMirrorDatabase(db, options.path);
} catch (ex) {
- if (ex.errors && isDatabaseCorrupt(ex.errors[0])) {
+ if (isDatabaseCorrupt(ex)) {
MirrorLog.warn("Error attaching mirror to Places; removing and " +
"recreating mirror", ex);
- options.recordTelemetryEvent("mirror", "open", "error",
+ options.recordTelemetryEvent("mirror", "open", "retry",
{ why: "corrupt" });
+
+ whyFailed = "remove";
await OS.File.remove(options.path);
- await db.execute(`ATTACH :mirrorPath AS mirror`,
- { mirrorPath: options.path });
+
+ whyFailed = "replace";
+ await attachAndInitMirrorDatabase(db, options.path);
} else {
MirrorLog.warn("Unrecoverable error attaching mirror to Places", ex);
throw ex;
}
}
- await db.execute(`PRAGMA foreign_keys = ON`);
- await db.executeTransaction(async function() {
- await migrateMirrorSchema(db);
- await initializeTempMirrorEntities(db);
- });
} catch (ex) {
options.recordTelemetryEvent("mirror", "open", "error",
- { why: "initialize" });
+ { why: whyFailed });
await db.close();
throw ex;
}
return new SyncedBookmarksMirror(db, options);
}
/**
* Returns the newer of the bookmarks collection last modified time, or the
@@ -1876,42 +1875,97 @@ SyncedBookmarksMirror.META_KEY = {
LAST_MODIFIED: "collection/lastModified",
SYNC_ID: "collection/syncId",
};
/**
* An error thrown when the merge can't proceed because the local or remote
* tree is inconsistent.
*/
-SyncedBookmarksMirror.ConsistencyError =
- class ConsistencyError extends Error {};
+class ConsistencyError extends Error {
+ constructor(message) {
+ super(message);
+ this.name = "ConsistencyError";
+ }
+}
+SyncedBookmarksMirror.ConsistencyError = ConsistencyError;
+
+/**
+ * An error thrown when the mirror database is corrupt, or can't be migrated to
+ * the latest schema version, and must be replaced.
+ */
+class DatabaseCorruptError extends Error {
+ constructor(message) {
+ super(message);
+ this.name = "DatabaseCorruptError";
+ }
+}
// Indicates if the mirror should be replaced because the database file is
// corrupt.
function isDatabaseCorrupt(error) {
- return error instanceof Ci.mozIStorageError &&
- (error.result == Ci.mozIStorageError.CORRUPT ||
- error.result == Ci.mozIStorageError.NOTADB);
+ if (error instanceof DatabaseCorruptError) {
+ return true;
+ }
+ if (error.errors) {
+ return error.errors.some(error =>
+ error instanceof Ci.mozIStorageError &&
+ (error.result == Ci.mozIStorageError.CORRUPT ||
+ error.result == Ci.mozIStorageError.NOTADB));
+ }
+ return false;
+}
+
+/**
+ * Attaches a cloned Places database connection to the mirror database,
+ * migrates the mirror schema to the latest version, and creates temporary
+ * tables, views, and triggers.
+ *
+ * @param {Sqlite.OpenedConnection} db
+ * The Places database connection.
+ * @param {String} path
+ * The full path to the mirror database file.
+ */
+async function attachAndInitMirrorDatabase(db, path) {
+ await db.execute(`ATTACH :path AS mirror`, { path });
+ try {
+ await db.executeTransaction(async function() {
+ let currentSchemaVersion = await db.getSchemaVersion("mirror");
+ if (currentSchemaVersion > 0) {
+ if (currentSchemaVersion < MIRROR_SCHEMA_VERSION) {
+ await migrateMirrorSchema(db, currentSchemaVersion);
+ }
+ } else {
+ await initializeMirrorDatabase(db);
+ }
+ // Downgrading from a newer profile to an older profile rolls back the
+ // schema version, but leaves all new columns in place. We'll run the
+ // migration logic again on the next upgrade.
+ await db.setSchemaVersion(MIRROR_SCHEMA_VERSION, "mirror");
+ await initializeTempMirrorEntities(db);
+ });
+ } catch (ex) {
+ await db.execute(`DETACH mirror`);
+ throw ex;
+ }
}
/**
* Migrates the mirror database schema to the latest version.
*
* @param {Sqlite.OpenedConnection} db
* The mirror database connection.
+ * @param {Number} currentSchemaVersion
+ * The current mirror database schema version.
*/
-async function migrateMirrorSchema(db) {
- let currentSchemaVersion = await db.getSchemaVersion("mirror");
- if (currentSchemaVersion < 1) {
- await initializeMirrorDatabase(db);
+async function migrateMirrorSchema(db, currentSchemaVersion) {
+ if (currentSchemaVersion < 2) {
+ throw new DatabaseCorruptError(`Can't migrate from schema version ${
+ currentSchemaVersion}; too old`);
}
- // Downgrading from a newer profile to an older profile rolls back the
- // schema version, but leaves all new columns in place. We'll run the
- // migration logic again on the next upgrade.
- await db.setSchemaVersion(MIRROR_SCHEMA_VERSION, "mirror");
}
/**
* Initializes a new mirror database, creating persistent tables, indexes, and
* roots.
*
* @param {Sqlite.OpenedConnection} db
* The mirror database connection.
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/sync/mirror_corrupt.sqlite
@@ -0,0 +1,1 @@
+Not a database!
new file mode 100644
index 0000000000000000000000000000000000000000..f0b88536167a2afeae7facad33e828588607570e
GIT binary patch
literal 294912
zc%1Fk%WfQ5835pJU*d~o0U9E2ASo6W_DtsDVTI60ICf=h#<3Z@tz=eBJ6%pn>F&x@
zR|f(}<$%Nj@dzx~vEdbv*sz5MKtdoMfGs<ys`jNl9S24NHH+_)s=Dewm-^2+b*c|P
z-i@;;I7!lOD+|6+JFV60wQmPOtyY^Vj)mH#x#E}^xv!rcuQOG9=gWUx%KuTDfAPy&
zKFI(0<6HT6e)RSa^A~?A0ssI20000000000000000000000000000000001=(+Bf6
zmT%vzAH}_P^n*d#*>3fpME&BTzp!}_ZZyJRXKyQf7+e$u`+Mb0fAvXmyY}*Hb92i#
zZ`OaZlC_RIQPz6gFOT`NwvCOuyJ1j9Ru_XpoJHN8cF|=c{4hKS_VydW-qG%Ea1h=L
z55m38@UW`VUya*qRfFpkNo<9?VeyE~jl<22t?*7&y?9d42p={sdM=`CFW<T`%<-F5
zj%D)YF?S`$YDmKzFZ28E!Os1SgHM8w!cT_%KOPjNR~|5o7p-3vE~jX0u*0G>dcK|X
zvsYx>o1I&}eY^hCx2kOWSvq)>4bsbt8&~qa2(Mm~;P@b(wbH1UeK=loY!%AhUcV04
zEyS~=A7^pW8ykH&#NfeBV|)Lo5ghD4*xCBo+VtG=@^bwb>(zj(Cs(JLD+3*7^f^}O
zXm97^qwq7+{_dMW<4F|sqx5-{2Hm6`pTto+$l`8vCy0ANH|})ee)K5mwfpY`Z@ybK
z8rHslUU}^8-3sq*9PKuO_lD2tMNzwdKT01*!QK7+-Ed>?+NyDXD=KzP(doad`Lx)e
z*FX8~^((`Blt!&;_S>!OHRCEvH`?w0U`Sb<b)sroFW&c&Z9MYcRNLs)FU{qfau|*-
z$<ye?_mi|e(tELs?<Jjfl<u{<qfc!|{YPp1tQy&6w3D>jJH5lW9UZsQ^LZQ=^}DS!
zyPG6Wi{Gc8>FlIfrK5x0kx2R0U5ymex!>!&2#(LX=m)K06+RtAX<U2=yMto?K50FV
zf}+*stZr_Hn;)$fyCDo7Y=?X2D;KP;-+6y6Xq2Jh^o?wZE5hBw@O*|3hszP}ZIx5H
zwl;HP`Ge(pt@>K`KkXD5H(P@&sqUNA7HO_mKeG?()kp8;8`ZAr7N6$wn7Xp3%E;>Z
zA`ExU*oJ!E>I|YQ+xP2JH<rJ3^SUI;KAY?1dAj&}nJ@oimdexVB>&|A0000000000
z0000000000000000000000000003a}7R%G=gp&XO0000000000000000000000000
z00000000000DuWwDvsIwd$s&8`S0>y<)_6Z000000000000000000000000000000
z000000002|*WH@^Qf*~^VZJ_p5$Yz#aVKh4rf<!Rl@EF+MS1(Ie0scn)EhLL=k2G)
z%CjWt9JkW5@_Jc5KU$t9Np{hGu?WoOzp3Sa&;OkNA^&Z02><{9000000000000000
z00000000000000000000|JS}ay;2|aPU23~ZkA@sK$aw(<5pS)roJ$}vXCZ8);v>I
z=qATSi{brj6^MF+^9s|8MZdFor<VUM|9$@J{O83b0000000000000000000000000
z00000000000002|PknV}Wu-pooy47}-7KX^k~PmveWfU!&yu8b+)BqvtEQuY(d&O!
zrQPJX=y<$zy6kwN8}$aSYP(b>b~@pS0RR91000000000000000000000000000000
z0001h30o>prxQ*B000000000000000000000000000000000000000cY_U9@PB;kw
l00000000000000000000000000000000000005Y<e*q^{P^bU^
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/sync/test_bookmark_mirror_migration.js
@@ -0,0 +1,77 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Migrations between 1 and 2 discard the entire database.
+add_task(async function test_migrate_from_1_to_2() {
+ let dbFile = do_get_file("mirror_v1.sqlite");
+ let telemetryEvents = [];
+ let buf = await SyncedBookmarksMirror.open({
+ path: dbFile.path,
+ recordTelemetryEvent(object, method, value, extra) {
+ telemetryEvents.push({ object, method, value, extra });
+ },
+ });
+ deepEqual(telemetryEvents, [{
+ object: "mirror",
+ method: "open",
+ value: "retry",
+ extra: { why: "corrupt" },
+ }]);
+ await buf.finalize();
+});
+
+add_task(async function test_database_corrupt() {
+ let corruptFile = do_get_file("mirror_corrupt.sqlite");
+ let telemetryEvents = [];
+ let buf = await SyncedBookmarksMirror.open({
+ path: corruptFile.path,
+ recordTelemetryEvent(object, method, value, extra) {
+ telemetryEvents.push({ object, method, value, extra });
+ },
+ });
+ deepEqual(telemetryEvents, [{
+ object: "mirror",
+ method: "open",
+ value: "retry",
+ extra: { why: "corrupt" },
+ }]);
+ await buf.finalize();
+});
+
+add_task(async function test_database_readonly() {
+ let dbFile = do_get_file("mirror_v1.sqlite");
+ try {
+ await OS.File.setPermissions(dbFile.path, {
+ unixMode: 0o400,
+ winAttributes: { readOnly: true },
+ });
+ } catch (ex) {
+ if (ex instanceof OS.File.Error &&
+ ex.unixErrno == OS.Constants.libc.EPERM) {
+ info("Skipping test; can't change database mode to read-only");
+ return;
+ }
+ throw ex;
+ }
+ try {
+ let telemetryEvents = [];
+ await Assert.rejects(SyncedBookmarksMirror.open({
+ path: dbFile.path,
+ recordTelemetryEvent(object, method, value, extra) {
+ telemetryEvents.push({ object, method, value, extra });
+ },
+ }), ex => ex.errors && ex.errors[0].result == Ci.mozIStorageError.READONLY,
+ "Should not try to recreate read-only database");
+ deepEqual(telemetryEvents, [{
+ object: "mirror",
+ method: "open",
+ value: "error",
+ extra: { why: "initialize" },
+ }], "Should record event for read-only error");
+ } finally {
+ await OS.File.setPermissions(dbFile.path, {
+ unixMode: 0o644,
+ winAttributes: { readOnly: false },
+ });
+ }
+});
--- a/toolkit/components/places/tests/sync/xpcshell.ini
+++ b/toolkit/components/places/tests/sync/xpcshell.ini
@@ -1,18 +1,21 @@
[DEFAULT]
head = head_sync.js
support-files =
livemark.xml
sync_utils_bookmarks.html
sync_utils_bookmarks.json
+ mirror_corrupt.sqlite
+ mirror_v1.sqlite
[test_bookmark_corruption.js]
[test_bookmark_deduping.js]
[test_bookmark_deletion.js]
[test_bookmark_explicit_weakupload.js]
[test_bookmark_haschanges.js]
[test_bookmark_kinds.js]
[test_bookmark_mirror_meta.js]
+[test_bookmark_mirror_migration.js]
[test_bookmark_structure_changes.js]
[test_bookmark_validation.js]
[test_bookmark_value_changes.js]
[test_sync_utils.js]
