зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1635859 - ensure all bookmarks have correct syncStatus after reconcilliation. r=lina
Differential Revision: https://phabricator.services.mozilla.com/D87218
This commit is contained in:
Mark Hammond
Родитель
6fb51e8cb7
Коммит
ec83e7c737
|
|
@ -81,7 +81,7 @@ const DB_TITLE_LENGTH_MAX = 4096;
|
|||
|
||||
// The current mirror database schema version. Bump for migrations, then add
|
||||
// migration code to `migrateMirrorSchema`.
|
||||
const MIRROR_SCHEMA_VERSION = 7;
|
||||
const MIRROR_SCHEMA_VERSION = 8;
|
||||
|
||||
const DEFAULT_MAX_FRECENCIES_TO_RECALCULATE = 400;
|
||||
|
||||
|
|
@ -1514,6 +1514,15 @@ async function migrateMirrorSchema(db, currentSchemaVersion) {
|
|||
await db.execute(`CREATE INDEX IF NOT EXISTS mirror.structurePositions ON
|
||||
structure(parentGuid, position)`);
|
||||
}
|
||||
if (currentSchemaVersion < 8) {
|
||||
// Not really a "schema" update, but addresses the defect from bug 1635859.
|
||||
// In short, every bookmark with a corresponding entry in the mirror should
|
||||
// have syncStatus = NORMAL.
|
||||
await db.execute(`UPDATE moz_bookmarks AS b
|
||||
SET syncStatus = ${PlacesUtils.bookmarks.SYNC_STATUS.NORMAL}
|
||||
WHERE EXISTS (SELECT 1 FROM mirror.items
|
||||
WHERE guid = b.guid)`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -26,6 +26,19 @@ fn total_sync_changes() -> i64 {
|
|||
unsafe { NS_NavBookmarksTotalSyncChanges() }
|
||||
}
|
||||
|
||||
// Return all the non-root-roots as a 'sql set' (ie, suitable for use in an
|
||||
// IN statement)
|
||||
fn user_roots_as_sql_set() -> String {
|
||||
format!(
|
||||
"('{0}', '{1}', '{2}', '{3}', '{4}')",
|
||||
dogear::MENU_GUID,
|
||||
dogear::MOBILE_GUID,
|
||||
dogear::TAGS_GUID,
|
||||
dogear::TOOLBAR_GUID,
|
||||
dogear::UNFILED_GUID
|
||||
)
|
||||
}
|
||||
|
||||
pub struct Store<'s> {
|
||||
db: &'s mut Conn,
|
||||
driver: &'s Driver,
|
||||
|
|
@ -79,15 +92,11 @@ impl<'s> Store<'s> {
|
|||
) AND NOT EXISTS(
|
||||
SELECT 1 FROM moz_bookmarks b
|
||||
JOIN moz_bookmarks p ON p.id = b.parent
|
||||
WHERE b.guid IN ('{1}', '{2}', '{3}', '{4}', '{5}') AND
|
||||
p.guid <> '{0}'
|
||||
WHERE b.guid IN {user_roots} AND
|
||||
p.guid <> '{root}'
|
||||
)",
|
||||
dogear::ROOT_GUID,
|
||||
dogear::MENU_GUID,
|
||||
dogear::MOBILE_GUID,
|
||||
dogear::TAGS_GUID,
|
||||
dogear::TOOLBAR_GUID,
|
||||
dogear::UNFILED_GUID,
|
||||
root = dogear::ROOT_GUID,
|
||||
user_roots = user_roots_as_sql_set(),
|
||||
))?;
|
||||
let has_valid_roots = match statement.step()? {
|
||||
Some(row) => row.get_by_index::<i64>(0)? == 1,
|
||||
|
|
@ -891,18 +900,33 @@ fn apply_remote_items(db: &Conn, driver: &Driver, controller: &AbortController)
|
|||
/* The last modified date should always be newer than the date
|
||||
added, so we pick the newer of the two here. */
|
||||
MAX(lastModifiedMicroseconds, remoteDateAddedMicroseconds),
|
||||
{}, 0
|
||||
{syncStatusNormal}, 0
|
||||
FROM itemsToApply
|
||||
WHERE 1
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
title = excluded.title,
|
||||
dateAdded = excluded.dateAdded,
|
||||
lastModified = excluded.lastModified,
|
||||
syncStatus = {syncStatusNormal},
|
||||
syncChangeCounter = 0,
|
||||
/* It's important that we update the URL *after* removing old keywords
|
||||
and *before* inserting new ones, so that the above DELETEs select
|
||||
the correct affected items. */
|
||||
fk = excluded.fk",
|
||||
nsINavBookmarksService::SYNC_STATUS_NORMAL
|
||||
syncStatusNormal = nsINavBookmarksService::SYNC_STATUS_NORMAL
|
||||
))?;
|
||||
// The roots are never in `itemsToApply` but still need to (well, at least
|
||||
// *should*) have a syncStatus of NORMAL after a reconcilliation. The
|
||||
// ROOT_GUID doesn't matter in practice, but we include it to be consistent.
|
||||
db.exec(format!(
|
||||
"UPDATE moz_bookmarks SET
|
||||
syncStatus={syncStatusNormal}
|
||||
WHERE guid IN {user_roots} OR
|
||||
guid = '{root}'
|
||||
",
|
||||
syncStatusNormal = nsINavBookmarksService::SYNC_STATUS_NORMAL,
|
||||
root = dogear::ROOT_GUID,
|
||||
user_roots = user_roots_as_sql_set(),
|
||||
))?;
|
||||
|
||||
// Flag frecencies for recalculation. This is a multi-index OR that uses the
|
||||
|
|
|
|||
|
|
@ -66,7 +66,7 @@ const MobileBookmarksTitle = "mobile";
|
|||
|
||||
function run_test() {
|
||||
let bufLog = Log.repository.getLogger("Sync.Engine.Bookmarks.Mirror");
|
||||
bufLog.level = Log.Level.Error;
|
||||
bufLog.level = Log.Level.All;
|
||||
|
||||
let sqliteLog = Log.repository.getLogger("Sqlite");
|
||||
sqliteLog.level = Log.Level.Error;
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
* http://creativecommons.org/publicdomain/zero/1.0/ */
|
||||
|
||||
// Keep in sync with `SyncedBookmarksMirror.jsm`.
|
||||
const CURRENT_MIRROR_SCHEMA_VERSION = 7;
|
||||
const CURRENT_MIRROR_SCHEMA_VERSION = 8;
|
||||
|
||||
// The oldest schema version that we support. Any databases with schemas older
|
||||
// than this will be dropped and recreated.
|
||||
|
|
@ -158,3 +158,71 @@ add_task(async function test_database_corrupt() {
|
|||
ok(buf.wasCorrupt, "Opening corrupt database should mark it as such");
|
||||
await buf.finalize();
|
||||
});
|
||||
|
||||
add_task(async function test_migrate_v8() {
|
||||
let buf = await openMirror("test_migrate_v8");
|
||||
|
||||
await PlacesUtils.bookmarks.insertTree({
|
||||
guid: PlacesUtils.bookmarks.menuGuid,
|
||||
children: [
|
||||
{
|
||||
guid: "bookmarkAAAA",
|
||||
url: "http://example.com/a",
|
||||
title: "A",
|
||||
},
|
||||
{
|
||||
guid: "bookmarkBBBB",
|
||||
url: "http://example.com/b",
|
||||
title: "B",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await buf.db.execute(
|
||||
`UPDATE moz_bookmarks
|
||||
SET syncChangeCounter = 0,
|
||||
syncStatus = ${PlacesUtils.bookmarks.SYNC_STATUS.NEW}`
|
||||
);
|
||||
|
||||
// setup the mirror.
|
||||
await storeRecords(buf, [
|
||||
{
|
||||
id: "bookmarkAAAA",
|
||||
parentid: "menu",
|
||||
type: "bookmark",
|
||||
title: "B",
|
||||
bmkUri: "http://example.com/b",
|
||||
},
|
||||
{
|
||||
id: "menu",
|
||||
parentid: "places",
|
||||
type: "folder",
|
||||
children: [],
|
||||
},
|
||||
]);
|
||||
|
||||
await buf.db.setSchemaVersion(7, "mirror");
|
||||
await buf.finalize();
|
||||
|
||||
// reopen it.
|
||||
buf = await openMirror("test_migrate_v8");
|
||||
Assert.equal(await buf.db.getSchemaVersion("mirror"), 8, "did upgrade");
|
||||
|
||||
let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
|
||||
"bookmarkAAAA",
|
||||
"bookmarkBBBB",
|
||||
PlacesUtils.bookmarks.menuGuid
|
||||
);
|
||||
let [fieldsA, fieldsB, fieldsMenu] = fields;
|
||||
|
||||
// 'A' was in the mirror - should now be _NORMAL
|
||||
Assert.equal(fieldsA.guid, "bookmarkAAAA");
|
||||
Assert.equal(fieldsA.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL);
|
||||
// 'B' was not in the mirror so should be untouched.
|
||||
Assert.equal(fieldsB.guid, "bookmarkBBBB");
|
||||
Assert.equal(fieldsB.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NEW);
|
||||
// 'menu' was in the mirror - should now be _NORMAL
|
||||
Assert.equal(fieldsMenu.guid, PlacesUtils.bookmarks.menuGuid);
|
||||
Assert.equal(fieldsMenu.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL);
|
||||
await buf.finalize();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,191 @@
|
|||
// Get bookmarks which aren't marked as normally syncing and with no pending
|
||||
// changes.
|
||||
async function getBookmarksNotMarkedAsSynced() {
|
||||
let db = await PlacesUtils.promiseDBConnection();
|
||||
let rows = await db.executeCached(
|
||||
`
|
||||
SELECT guid, syncStatus, syncChangeCounter FROM moz_bookmarks
|
||||
WHERE syncChangeCounter > 1 OR syncStatus != :syncStatus
|
||||
ORDER BY guid
|
||||
`,
|
||||
{ syncStatus: PlacesUtils.bookmarks.SYNC_STATUS.NORMAL }
|
||||
);
|
||||
return rows.map(row => {
|
||||
return {
|
||||
guid: row.getResultByName("guid"),
|
||||
syncStatus: row.getResultByName("syncStatus"),
|
||||
syncChangeCounter: row.getResultByName("syncChangeCounter"),
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
add_task(async function test_reconcile_metadata() {
|
||||
let buf = await openMirror("test_reconcile_metadata");
|
||||
|
||||
let olderDate = new Date(Date.now() - 100000);
|
||||
info("Set up local tree");
|
||||
await PlacesUtils.bookmarks.insertTree({
|
||||
guid: PlacesUtils.bookmarks.menuGuid,
|
||||
children: [
|
||||
{
|
||||
// this folder is going to reconcile exactly
|
||||
guid: "folderAAAAAA",
|
||||
type: PlacesUtils.bookmarks.TYPE_FOLDER,
|
||||
title: "A",
|
||||
children: [
|
||||
{
|
||||
guid: "bookmarkBBBB",
|
||||
url: "http://example.com/b",
|
||||
title: "B",
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
// this folder's existing child isn't on the server (so will be
|
||||
// outgoing) and also will take a new child from the server.
|
||||
guid: "folderCCCCCC",
|
||||
type: PlacesUtils.bookmarks.TYPE_FOLDER,
|
||||
title: "C",
|
||||
children: [
|
||||
{
|
||||
guid: "bookmarkEEEE",
|
||||
url: "http://example.com/e",
|
||||
title: "E",
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
// This bookmark is going to take the remote title.
|
||||
guid: "bookmarkFFFF",
|
||||
url: "http://example.com/f",
|
||||
title: "f",
|
||||
dateAdded: olderDate,
|
||||
lastModified: olderDate,
|
||||
},
|
||||
],
|
||||
});
|
||||
// And a single, local-only bookmark in the toolbar.
|
||||
await PlacesUtils.bookmarks.insertTree({
|
||||
guid: PlacesUtils.bookmarks.toolbarGuid,
|
||||
children: [
|
||||
{
|
||||
guid: "bookmarkTTTT",
|
||||
url: "http://example.com/t",
|
||||
title: "in the toolbar",
|
||||
dateAdded: olderDate,
|
||||
lastModified: olderDate,
|
||||
},
|
||||
],
|
||||
});
|
||||
// Reset to prepare for our reconciled sync.
|
||||
await PlacesSyncUtils.bookmarks.reset();
|
||||
// setup the mirror.
|
||||
await storeRecords(
|
||||
buf,
|
||||
shuffle([
|
||||
{
|
||||
id: "menu",
|
||||
parentid: "places",
|
||||
type: "folder",
|
||||
children: ["folderAAAAAA", "folderCCCCCC", "bookmarkFFFF"],
|
||||
modified: Date.now() / 1000 - 60,
|
||||
},
|
||||
{
|
||||
id: "folderAAAAAA",
|
||||
parentid: "menu",
|
||||
type: "folder",
|
||||
title: "A",
|
||||
children: ["bookmarkBBBB"],
|
||||
modified: Date.now() / 1000 - 60,
|
||||
},
|
||||
{
|
||||
id: "bookmarkBBBB",
|
||||
parentid: "folderAAAAAA",
|
||||
type: "bookmark",
|
||||
title: "B",
|
||||
bmkUri: "http://example.com/b",
|
||||
modified: Date.now() / 1000 - 60,
|
||||
},
|
||||
{
|
||||
id: "folderCCCCCC",
|
||||
parentid: "menu",
|
||||
type: "folder",
|
||||
title: "C",
|
||||
children: ["bookmarkDDDD"],
|
||||
modified: Date.now() / 1000 - 60,
|
||||
},
|
||||
{
|
||||
id: "bookmarkDDDD",
|
||||
parentid: "folderCCCCCC",
|
||||
type: "bookmark",
|
||||
title: "D",
|
||||
bmkUri: "http://example.com/d",
|
||||
modified: Date.now() / 1000 - 60,
|
||||
},
|
||||
{
|
||||
id: "bookmarkFFFF",
|
||||
parentid: "menu",
|
||||
type: "bookmark",
|
||||
title: "F",
|
||||
bmkUri: "http://example.com/f",
|
||||
dateAdded: olderDate,
|
||||
modified: Date.now() / 1000 + 60,
|
||||
},
|
||||
{
|
||||
id: "toolbar",
|
||||
parentid: "places",
|
||||
type: "folder",
|
||||
children: [],
|
||||
index: 1,
|
||||
},
|
||||
{
|
||||
id: "unfiled",
|
||||
parentid: "places",
|
||||
type: "folder",
|
||||
children: [],
|
||||
index: 3,
|
||||
},
|
||||
])
|
||||
);
|
||||
info("Applying");
|
||||
let changesToUpload = await buf.apply();
|
||||
// We need to upload a bookmark and the parent as they didn't exist on the
|
||||
// server. Since we always use the local state for roots (bug 1472241), we'll
|
||||
// reupload them too.
|
||||
let idsToUpload = inspectChangeRecords(changesToUpload);
|
||||
deepEqual(
|
||||
idsToUpload,
|
||||
{
|
||||
updated: [
|
||||
"bookmarkEEEE",
|
||||
"bookmarkTTTT",
|
||||
"folderCCCCCC",
|
||||
"menu",
|
||||
"mobile",
|
||||
"toolbar",
|
||||
"unfiled",
|
||||
],
|
||||
deleted: [],
|
||||
},
|
||||
"Should upload the 2 local-only bookmarks and their parents"
|
||||
);
|
||||
// Check it took the remote thing we were expecting.
|
||||
Assert.equal((await PlacesUtils.bookmarks.fetch("bookmarkFFFF")).title, "F");
|
||||
// Most things should be synced and have no change counter.
|
||||
let badGuids = await getBookmarksNotMarkedAsSynced();
|
||||
Assert.deepEqual(badGuids, [
|
||||
{
|
||||
// The bookmark that was only on the server. Still have SYNC_STATUS_NEW
|
||||
// as it's yet to be uploaded.
|
||||
guid: "bookmarkEEEE",
|
||||
syncStatus: PlacesUtils.bookmarks.SYNC_STATUS.NEW,
|
||||
syncChangeCounter: 1,
|
||||
},
|
||||
{
|
||||
// This bookmark is local only so is yet to be uploaded.
|
||||
guid: "bookmarkTTTT",
|
||||
syncStatus: PlacesUtils.bookmarks.SYNC_STATUS.NEW,
|
||||
syncChangeCounter: 1,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
|
@ -18,6 +18,7 @@ support-files =
|
|||
[test_bookmark_mirror_meta.js]
|
||||
[test_bookmark_mirror_migration.js]
|
||||
[test_bookmark_observer_recorder.js]
|
||||
[test_bookmark_reconcile.js]
|
||||
[test_bookmark_structure_changes.js]
|
||||
[test_bookmark_value_changes.js]
|
||||
[test_sync_utils.js]
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче