--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -3,20 +3,21 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
const Cu = Components.utils;
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
+Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://gre/modules/Timer.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-
this.EXPORTED_SYMBOLS = ["BookmarkValidator", "BookmarkProblemData"];
const LEFT_PANE_ROOT_ANNO = "PlacesOrganizer/OrganizerFolder";
const LEFT_PANE_QUERY_ANNO = "PlacesOrganizer/OrganizerQuery";
// Indicates if a local bookmark tree node should be excluded from syncing.
function isNodeIgnored(treeNode) {
return treeNode.annos && treeNode.annos.some(anno => anno.name == LEFT_PANE_ROOT_ANNO ||
@@ -209,23 +210,27 @@ class BookmarkValidator {
let concreteItem = recordMap.get(concreteId);
if (!concreteItem) {
continue;
}
entry.concrete = concreteItem;
}
}
- createClientRecordsFromTree(clientTree) {
+ async createClientRecordsFromTree(clientTree) {
// Iterate over the treeNode, converting it to something more similar to what
// the server stores.
let records = [];
let recordsByGuid = new Map();
let syncedRoots = SYNCED_ROOTS;
- function traverse(treeNode, synced) {
+ let yieldCounter = 0;
+ async function traverse(treeNode, synced) {
+ if (++yieldCounter % 50 === 0) {
+ await new Promise(resolve => setTimeout(resolve, 50));
+ }
if (!synced) {
synced = syncedRoots.includes(treeNode.guid);
} else if (isNodeIgnored(treeNode)) {
synced = false;
}
let guid = PlacesSyncUtils.bookmarks.guidToSyncId(treeNode.guid);
let itemType = "item";
treeNode.ignored = !synced;
@@ -275,24 +280,24 @@ class BookmarkValidator {
// We want to use the "real" guid here.
recordsByGuid.set(treeNode.guid, treeNode);
if (treeNode.type === "folder") {
treeNode.childGUIDs = [];
if (!treeNode.children) {
treeNode.children = [];
}
for (let child of treeNode.children) {
- traverse(child, synced);
+ await traverse(child, synced);
child.parent = treeNode;
child.parentid = guid;
treeNode.childGUIDs.push(child.guid);
}
}
}
- traverse(clientTree, false);
+ await traverse(clientTree, false);
clientTree.id = "places";
this._followQueries(recordsByGuid);
return records;
}
/**
* Process the server-side list. Mainly this builds the records into a tree,
* but it also records information about problems, and produces arrays of the
@@ -313,27 +318,28 @@ class BookmarkValidator {
* - root: Root of the server-side bookmark tree. Has the same properties as
* above.
* - deletedRecords: As above, but only contains items that the server sent
* where it also sent indication that the item should be deleted.
* - problemData: a BookmarkProblemData object, with the caveat that
* the fields describing client/server relationship will not have been filled
* out yet.
*/
- inspectServerRecords(serverRecords) {
+ async inspectServerRecords(serverRecords) {
let deletedItemIds = new Set();
let idToRecord = new Map();
let deletedRecords = [];
let folders = [];
let problemData = new BookmarkProblemData();
let resultRecords = [];
+ let yieldCounter = 0;
for (let record of serverRecords) {
if (!record.id) {
++problemData.missingIDs;
continue;
}
if (record.deleted) {
deletedItemIds.add(record.id);
} else if (idToRecord.has(record.id)) {
@@ -362,16 +368,19 @@ class BookmarkValidator {
// The children array stores special guids as their local guid values,
// e.g. 'menu________' instead of 'menu', but all other parts of the
// serverside bookmark info stores it as the special value ('menu').
record.childGUIDs = record.children;
record.children = record.children.map(childID => {
return PlacesSyncUtils.bookmarks.guidToSyncId(childID);
});
}
+ if (++yieldCounter % 50 === 0) {
+ await new Promise(resolve => setTimeout(resolve, 50));
+ }
}
for (let deletedId of deletedItemIds) {
let record = idToRecord.get(deletedId);
if (record && !record.isDeleted) {
deletedRecords.push(record);
record.isDeleted = true;
}
@@ -602,20 +611,20 @@ class BookmarkValidator {
*
* Returns the same data as described in the inspectServerRecords comment,
* with the following additional fields.
* - clientRecords: an array of client records in a similar format to
* the .records (ie, server records) entry.
* - problemData is the same as for inspectServerRecords, except all properties
* will be filled out.
*/
- compareServerWithClient(serverRecords, clientTree) {
+ async compareServerWithClient(serverRecords, clientTree) {
- let clientRecords = this.createClientRecordsFromTree(clientTree);
- let inspectionInfo = this.inspectServerRecords(serverRecords);
+ let clientRecords = await this.createClientRecordsFromTree(clientTree);
+ let inspectionInfo = await this.inspectServerRecords(serverRecords);
inspectionInfo.clientRecords = clientRecords;
// Mainly do this to remove deleted items and normalize child guids.
serverRecords = inspectionInfo.records;
let problemData = inspectionInfo.problemData;
this._validateClient(problemData, clientRecords);
@@ -747,33 +756,30 @@ class BookmarkValidator {
};
let resp = collection.getBatched();
if (!resp.success) {
throw resp;
}
return items;
}
- validate(engine) {
- let self = this;
- return Task.spawn(function*() {
- let start = Date.now();
- let clientTree = yield PlacesUtils.promiseBookmarksTree("", {
- includeItemIds: true
- });
- let serverState = self._getServerState(engine);
- let serverRecordCount = serverState.length;
- let result = self.compareServerWithClient(serverState, clientTree);
- let end = Date.now();
- let duration = end - start;
- return {
- duration,
- version: self.version,
- problems: result.problemData,
- recordCount: serverRecordCount
- };
+ async validate(engine) {
+ let start = Date.now();
+ let clientTree = await PlacesUtils.promiseBookmarksTree("", {
+ includeItemIds: true
});
+ let serverState = this._getServerState(engine);
+ let serverRecordCount = serverState.length;
+ let result = await this.compareServerWithClient(serverState, clientTree);
+ let end = Date.now();
+ let duration = end - start;
+ return {
+ duration,
+ version: this.version,
+ problems: result.problemData,
+ recordCount: serverRecordCount
+ };
}
}
BookmarkValidator.prototype.version = BOOKMARK_VALIDATOR_VERSION;
--- a/services/sync/tests/unit/test_bookmark_validator.js
+++ b/services/sync/tests/unit/test_bookmark_validator.js
@@ -1,125 +1,121 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
Components.utils.import("resource://services-sync/bookmark_validator.js");
Components.utils.import("resource://services-sync/util.js");
-function inspectServerRecords(data) {
- return new BookmarkValidator().inspectServerRecords(data);
+async function inspectServerRecords(data) {
+ let validator = new BookmarkValidator();
+ return validator.inspectServerRecords(data);
}
-add_test(function test_isr_rootOnServer() {
- let c = inspectServerRecords([{
+async function compareServerWithClient(server, client) {
+ let validator = new BookmarkValidator();
+ return validator.compareServerWithClient(server, client);
+}
+
+add_task(async function test_isr_rootOnServer() {
+ let c = await inspectServerRecords([{
id: "places",
type: "folder",
children: [],
}]);
ok(c.problemData.rootOnServer);
- run_next_test();
});
-add_test(function test_isr_empty() {
- let c = inspectServerRecords([]);
+add_task(async function test_isr_empty() {
+ let c = await inspectServerRecords([]);
ok(!c.problemData.rootOnServer);
notEqual(c.root, null);
- run_next_test();
});
-add_test(function test_isr_cycles() {
- let c = inspectServerRecords([
+add_task(async function test_isr_cycles() {
+ let c = (await inspectServerRecords([
{id: "C", type: "folder", children: ["A", "B"], parentid: "places"},
{id: "A", type: "folder", children: ["B"], parentid: "B"},
{id: "B", type: "folder", children: ["A"], parentid: "A"},
- ]).problemData;
+ ])).problemData;
equal(c.cycles.length, 1);
ok(c.cycles[0].indexOf("A") >= 0);
ok(c.cycles[0].indexOf("B") >= 0);
- run_next_test();
});
-add_test(function test_isr_orphansMultiParents() {
- let c = inspectServerRecords([
+add_task(async function test_isr_orphansMultiParents() {
+ let c = (await inspectServerRecords([
{ id: "A", type: "bookmark", parentid: "D" },
{ id: "B", type: "folder", parentid: "places", children: ["A"]},
{ id: "C", type: "folder", parentid: "places", children: ["A"]},
- ]).problemData;
+ ])).problemData;
deepEqual(c.orphans, [{ id: "A", parent: "D" }]);
equal(c.multipleParents.length, 1)
ok(c.multipleParents[0].parents.indexOf("B") >= 0);
ok(c.multipleParents[0].parents.indexOf("C") >= 0);
- run_next_test();
});
-add_test(function test_isr_orphansMultiParents2() {
- let c = inspectServerRecords([
+add_task(async function test_isr_orphansMultiParents2() {
+ let c = (await inspectServerRecords([
{ id: "A", type: "bookmark", parentid: "D" },
{ id: "B", type: "folder", parentid: "places", children: ["A"]},
- ]).problemData;
+ ])).problemData;
equal(c.orphans.length, 1);
equal(c.orphans[0].id, "A");
equal(c.multipleParents.length, 0);
- run_next_test();
});
-add_test(function test_isr_deletedParents() {
- let c = inspectServerRecords([
+add_task(async function test_isr_deletedParents() {
+ let c = (await inspectServerRecords([
{ id: "A", type: "bookmark", parentid: "B" },
{ id: "B", type: "folder", parentid: "places", children: ["A"]},
{ id: "B", type: "item", deleted: true},
- ]).problemData;
- deepEqual(c.deletedParents, ["A"])
- run_next_test();
+ ])).problemData;
+ deepEqual(c.deletedParents, ["A"]);
});
-add_test(function test_isr_badChildren() {
- let c = inspectServerRecords([
+add_task(async function test_isr_badChildren() {
+ let c = (await inspectServerRecords([
{ id: "A", type: "bookmark", parentid: "places", children: ["B", "C"] },
{ id: "C", type: "bookmark", parentid: "A" }
- ]).problemData;
+ ])).problemData;
deepEqual(c.childrenOnNonFolder, ["A"])
deepEqual(c.missingChildren, [{parent: "A", child: "B"}]);
deepEqual(c.parentNotFolder, ["C"]);
- run_next_test();
});
-add_test(function test_isr_parentChildMismatches() {
- let c = inspectServerRecords([
+add_task(async function test_isr_parentChildMismatches() {
+ let c = (await inspectServerRecords([
{ id: "A", type: "folder", parentid: "places", children: [] },
{ id: "B", type: "bookmark", parentid: "A" }
- ]).problemData;
+ ])).problemData;
deepEqual(c.parentChildMismatches, [{parent: "A", child: "B"}]);
- run_next_test();
});
-add_test(function test_isr_duplicatesAndMissingIDs() {
- let c = inspectServerRecords([
+add_task(async function test_isr_duplicatesAndMissingIDs() {
+ let c = (await inspectServerRecords([
{id: "A", type: "folder", parentid: "places", children: []},
{id: "A", type: "folder", parentid: "places", children: []},
{type: "folder", parentid: "places", children: []}
- ]).problemData;
+ ])).problemData;
equal(c.missingIDs, 1);
deepEqual(c.duplicates, ["A"]);
- run_next_test();
});
-add_test(function test_isr_duplicateChildren() {
- let c = inspectServerRecords([
+add_task(async function test_isr_duplicateChildren() {
+ let c = (await inspectServerRecords([
{id: "A", type: "folder", parentid: "places", children: ["B", "B"]},
{id: "B", type: "bookmark", parentid: "A"},
- ]).problemData;
+ ])).problemData;
deepEqual(c.duplicateChildren, ["A"]);
- run_next_test();
});
-// Each compareServerWithClient test mutates these, so we can't just keep them
+// Each compareServerWithClient test mutates these, so we can"t just keep them
// global
function getDummyServerAndClient() {
let server = [
{
id: "menu",
parentid: "places",
type: "folder",
parentName: "",
@@ -179,72 +175,68 @@ function getDummyServerAndClient() {
]
}
]
};
return {server, client};
}
-add_test(function test_cswc_valid() {
+add_task(async function test_cswc_valid() {
let {server, client} = getDummyServerAndClient();
- let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
+ let c = (await compareServerWithClient(server, client)).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
equal(c.differences.length, 0);
- run_next_test();
});
-add_test(function test_cswc_serverMissing() {
+add_task(async function test_cswc_serverMissing() {
let {server, client} = getDummyServerAndClient();
// remove c
server.pop();
server[0].children.pop();
- let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
+ let c = (await compareServerWithClient(server, client)).problemData;
deepEqual(c.serverMissing, ["cccccccccccc"]);
equal(c.clientMissing.length, 0);
deepEqual(c.structuralDifferences, [{id: "menu", differences: ["childGUIDs"]}]);
- run_next_test();
});
-add_test(function test_cswc_clientMissing() {
+add_task(async function test_cswc_clientMissing() {
let {server, client} = getDummyServerAndClient();
client.children[0].children.pop();
- let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
+ let c = (await compareServerWithClient(server, client)).problemData;
deepEqual(c.clientMissing, ["cccccccccccc"]);
equal(c.serverMissing.length, 0);
deepEqual(c.structuralDifferences, [{id: "menu", differences: ["childGUIDs"]}]);
- run_next_test();
});
-add_test(function test_cswc_differences() {
+add_task(async function test_cswc_differences() {
{
let {server, client} = getDummyServerAndClient();
client.children[0].children[0].title = "asdf";
- let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
+ let c = (await compareServerWithClient(server, client)).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
deepEqual(c.differences, [{id: "bbbbbbbbbbbb", differences: ["title"]}]);
}
{
let {server, client} = getDummyServerAndClient();
server[2].type = "bookmark";
- let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
+ let c = (await compareServerWithClient(server, client)).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
deepEqual(c.differences, [{id: "cccccccccccc", differences: ["type"]}]);
}
- run_next_test();
});
-add_test(function test_cswc_serverUnexpected() {
+add_task(async function test_cswc_serverUnexpected() {
let {server, client} = getDummyServerAndClient();
client.children.push({
"guid": "dddddddddddd",
"title": "",
"id": 2000,
"annos": [{
"name": "places/excludeFromBackup",
"flags": 0,
@@ -286,41 +278,41 @@ add_test(function test_cswc_serverUnexpe
id: "eeeeeeeeeeee",
parentid: "dddddddddddd",
parentName: "",
title: "History",
type: "query",
bmkUri: "place:type=3&sort=4"
});
- let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
+ let c = (await compareServerWithClient(server, client)).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
equal(c.serverUnexpected.length, 2);
deepEqual(c.serverUnexpected, ["dddddddddddd", "eeeeeeeeeeee"]);
- run_next_test();
});
-function validationPing(server, client, duration) {
- return wait_for_ping(function() {
- // fake this entirely
- Svc.Obs.notify("weave:service:sync:start");
- Svc.Obs.notify("weave:engine:sync:start", null, "bookmarks");
- Svc.Obs.notify("weave:engine:sync:finish", null, "bookmarks");
- let validator = new BookmarkValidator();
- let data = {
- // We fake duration and version just so that we can verify they're passed through.
- duration,
- version: validator.version,
- recordCount: server.length,
- problems: validator.compareServerWithClient(server, client).problemData,
- };
- Svc.Obs.notify("weave:engine:validate:finish", data, "bookmarks");
- Svc.Obs.notify("weave:service:sync:finish");
- }, true); // Allow "failing" pings, since having validation info indicates failure.
+async function validationPing(server, client, duration) {
+ let pingPromise = wait_for_ping(() => {}, true); // Allow "failing" pings, since having validation info indicates failure.
+ // fake this entirely
+ Svc.Obs.notify("weave:service:sync:start");
+ Svc.Obs.notify("weave:engine:sync:start", null, "bookmarks");
+ Svc.Obs.notify("weave:engine:sync:finish", null, "bookmarks");
+ let validator = new BookmarkValidator();
+ let {problemData} = await validator.compareServerWithClient(server, client);
+ let data = {
+ // We fake duration and version just so that we can verify they"re passed through.
+ duration,
+ version: validator.version,
+ recordCount: server.length,
+ problems: problemData,
+ };
+ Svc.Obs.notify("weave:engine:validate:finish", data, "bookmarks");
+ Svc.Obs.notify("weave:service:sync:finish");
+ return pingPromise;
}
add_task(async function test_telemetry_integration() {
let {server, client} = getDummyServerAndClient();
// remove "c"
server.pop();
server[0].children.pop();
const duration = 50;
@@ -336,12 +328,8 @@ add_task(async function test_telemetry_i
equal(bme.validation.version, new BookmarkValidator().version);
deepEqual(bme.validation.problems, [
{ name: "badClientRoots", count: 3 },
{ name: "sdiff:childGUIDs", count: 1 },
{ name: "serverMissing", count: 1 },
{ name: "structuralDifferences", count: 1 },
]);
});
-
-function run_test() {
- run_next_test();
-}