--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -2,16 +2,17 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
ChromeUtils.import("resource://services-common/utils.js");
+ChromeUtils.import("resource://services-sync/util.js");
ChromeUtils.defineModuleGetter(this, "Async",
"resource://services-common/async.js");
ChromeUtils.defineModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
ChromeUtils.defineModuleGetter(this, "PlacesSyncUtils",
@@ -235,24 +236,391 @@ XPCOMUtils.defineLazyGetter(this, "ROOT_
// report them just in case.
[PlacesUtils.bookmarks.tagsGuid]: "TAGS",
[PlacesUtils.bookmarks.unfiledGuid]: "UNFILED_BOOKMARKS",
[PlacesUtils.bookmarks.toolbarGuid]: "TOOLBAR",
[PlacesUtils.bookmarks.mobileGuid]: "MOBILE_BOOKMARKS",
}));
+async function detectCycles(records) {
+ // currentPath and pathLookup contain the same data. pathLookup is faster to
+ // query, but currentPath gives is the order of traversal that we need in
+ // order to report the members of the cycles.
+ let pathLookup = new Set();
+ let currentPath = [];
+ let cycles = [];
+ let seenEver = new Set();
+ const maybeYield = Async.jankYielder();
+ const traverse = async node => {
+ await maybeYield();
+ if (pathLookup.has(node)) {
+ let cycleStart = currentPath.lastIndexOf(node);
+ let cyclePath = currentPath.slice(cycleStart).map(n => n.id);
+ cycles.push(cyclePath);
+ return;
+ } else if (seenEver.has(node)) {
+ // If we're checking the server, this is a problem, but it should already be reported.
+ // On the client, this could happen due to including `node.concrete` in the child list.
+ return;
+ }
+ seenEver.add(node);
+ let children = node.children || [];
+ if (node.concreteItems) {
+ children.push(...node.concreteItems);
+ }
+ if (children.length) {
+ pathLookup.add(node);
+ currentPath.push(node);
+ for (let child of children) {
+ await traverse(child);
+ }
+ currentPath.pop();
+ pathLookup.delete(node);
+ }
+ };
+ for (let record of records) {
+ if (!seenEver.has(record)) {
+ await traverse(record);
+ }
+ }
+
+ return cycles;
+}
+
+class ServerRecordInspection {
+ constructor() {
+ this.serverRecords = null;
+ this.liveRecords = [];
+
+ this.folders = [];
+
+ this.root = null;
+
+ this.idToRecord = new Map();
+
+ this.deletedIds = new Set();
+ this.deletedRecords = [];
+
+ this.problemData = new BookmarkProblemData();
+
+ // These are handled outside of problemData
+ this._orphans = new Map();
+ this._multipleParents = new Map();
+
+ this.maybeYield = Async.jankYielder();
+ }
+
+ static async create(records) {
+ return new ServerRecordInspection().performInspection(records);
+ }
+
+ async performInspection(records) {
+ await this._setRecords(records);
+ await this._linkParentIDs();
+ await this._linkChildren();
+ await this._findOrphans();
+ await this._finish();
+ return this;
+ }
+
+ // We don't set orphans in this.problemData. Instead, we walk the tree at the
+ // end to find unreachable items.
+ _noteOrphan(id, parentId = undefined) {
+ // This probably shouldn't be called with a parentId twice, but if it
+ // happens we take the most recent one.
+ if (parentId || !this._orphans.has(id)) {
+ this._orphans.set(id, parentId);
+ }
+ }
+
+ noteParent(child, parent) {
+ let parents = this._multipleParents.get(child);
+ if (!parents) {
+ this._multipleParents.set(child, [parent]);
+ } else {
+ parents.push(parent);
+ }
+ }
+
+ noteMismatch(child, parent) {
+ let exists = this.problemData.parentChildMismatches.some(match =>
+ match.child == child && match.parent == parent);
+ if (!exists) {
+ this.problemData.parentChildMismatches.push({child, parent});
+ }
+ }
+
+ // - Populates `this.deletedIds`, `this.folders`, and `this.idToRecord`
+ // - calls `_initRoot` (thus initializing `this.root`).
+ async _setRecords(records) {
+ if (this.serverRecords) {
+ // In general this class is expected to be created, have
+ // `performInspection` called, and then only read from from that point on.
+ throw new Error("Bug: ServerRecordInspection can't `setRecords` twice");
+ }
+ this.serverRecords = records;
+ let rootChildren = [];
+
+ for (let record of this.serverRecords) {
+ await this.maybeYield();
+ if (!record.id) {
+ ++this.problemData.missingIDs;
+ continue;
+ }
+
+ if (record.deleted) {
+ this.deletedIds.add(record.id);
+ }
+ if (this.idToRecord.has(record.id)) {
+ this.problemData.duplicates.push(record.id);
+ continue;
+ }
+
+ this.idToRecord.set(record.id, record);
+
+ if (!record.deleted) {
+ this.liveRecords.push(record);
+
+ if (record.parentid == "places") {
+ rootChildren.push(record);
+ }
+ }
+
+ if (!record.children) {
+ continue;
+ }
+
+ if (record.type != "folder") {
+ // Due to implementation details in engines/bookmarks.js, (Livemark
+ // subclassing BookmarkFolder) Livemarks will have a children array,
+ // but it should still be empty.
+ if (!record.children.length) {
+ continue;
+ }
+ // Otherwise we mark it as an error and still try to resolve the children
+ this.problemData.childrenOnNonFolder.push(record.id);
+ }
+
+ this.folders.push(record);
+
+ if (new Set(record.children).size !== record.children.length) {
+ this.problemData.duplicateChildren.push(record.id);
+ }
+
+ // After we're through with them, folder records store 3 (ugh) arrays that
+ // represent their folder information. The final fields looks like:
+ //
+ // - childGUIDs: The original `children` array, which is an array of
+ // record IDs.
+ //
+ // - unfilteredChildren: Contains more or less `childGUIDs.map(id =>
+ // idToRecord.get(id))`, without the nulls for missing children. It will
+ // still have deleted, duplicate, mismatching, etc. children.
+ //
+ // - children: This is the 'cleaned' version of the child records that are
+ // safe to iterate over, etc.. If there are no reported problems, it should
+ // be identical to unfilteredChildren.
+ //
+ // The last two are left alone until later `this._linkChildren`, however.
+ record.childGUIDs = record.children;
+ for (let id of record.childGUIDs) {
+ await this.maybeYield();
+ this.noteParent(id, record.id);
+ }
+ record.children = [];
+ }
+
+ // Finish up some parts we can easily do now that we have idToRecord.
+ this.deletedRecords = Array.from(this.deletedIds,
+ id => this.idToRecord.get(id));
+
+ this._initRoot(rootChildren);
+ }
+
+ _initRoot(rootChildren) {
+ let serverRoot = this.idToRecord.get("places");
+ if (serverRoot) {
+ this.root = serverRoot;
+ this.problemData.rootOnServer = true;
+ return;
+ }
+
+ // Fabricate a root. We want to be able to remember that it's fake, but
+ // would like to avoid it needing too much special casing, so we come up
+ // with children for it too (we just get these while we're iterating over
+ // the records to avoid needing two passes over a potentially large number
+ // of records).
+
+ this.root = {
+ id: "places",
+ fake: true,
+ children: rootChildren,
+ childGUIDs: rootChildren.map(record => record.id),
+ type: "folder",
+ title: "",
+ };
+ this.liveRecords.push(this.root);
+ this.idToRecord.set("places", this.root);
+ }
+
+ // Adds `parent` to all records it can that have `parentid`
+ async _linkParentIDs() {
+ for (let [id, record] of this.idToRecord) {
+ await this.maybeYield();
+ if (record == this.root || record.deleted) {
+ continue;
+ }
+
+ // Check and update our orphan map.
+ let parentID = record.parentid;
+ let parent = this.idToRecord.get(parentID);
+ if (!parentID || !parent) {
+ this._noteOrphan(id, parentID);
+ continue;
+ }
+
+ record.parent = parent;
+
+ if (parent.deleted) {
+ this.problemData.deletedParents.push(id);
+ return;
+ } else if (parent.type != "folder") {
+ this.problemData.parentNotFolder.push(record.id);
+ return;
+ }
+
+ if (parent.id !== "place" || this.problemData.rootOnServer) {
+ if (!parent.childGUIDs.includes(record.id)) {
+ this.noteMismatch(record.id, parent.id);
+ }
+ }
+
+ if (parent.deleted && !record.deleted) {
+ this.problemData.deletedParents.push(record.id);
+ }
+
+ // Note: We used to check if the parentName on the server matches the
+ // actual local parent name, but given this is used only for de-duping a
+ // record the first time it is seen and expensive to keep up-to-date, we
+ // decided to just stop recording it. See bug 1276969 for more.
+ }
+ }
+
+ // Build the children and unfilteredChildren arrays, (which are of record
+ // objects, not ids)
+ async _linkChildren() {
+ // Check that we aren't missing any children.
+ for (let folder of this.folders) {
+ await this.maybeYield();
+
+ folder.children = [];
+ folder.unfilteredChildren = [];
+
+ let idsThisFolder = new Set();
+
+ for (let i = 0; i < folder.childGUIDs.length; ++i) {
+ await this.maybeYield();
+ let childID = folder.childGUIDs[i];
+
+ let child = this.idToRecord.get(childID);
+
+ if (!child) {
+ this.problemData.missingChildren.push({ parent: folder.id, child: childID });
+ continue;
+ }
+
+ if (child.deleted) {
+ this.problemData.deletedChildren.push({ parent: folder.id, child: childID });
+ continue;
+ }
+
+ if (child.parentid != folder.id) {
+ this.noteMismatch(childID, folder.id);
+ continue;
+ }
+
+ if (idsThisFolder.has(childID)) {
+ // Already recorded earlier, we just don't want to mess up `children`
+ continue;
+ }
+ folder.children.push(child);
+ }
+ }
+ }
+
+ // Finds the orphans in the tree using something similar to a `mark and sweep`
+ // strategy. That is, we iterate over the children from the root, remembering
+ // which items we've seen. Then, we iterate all items, and know the ones we
+ // haven't seen are orphans.
+ async _findOrphans() {
+ let seen = new Set([this.root.id]);
+ for (let [node] of Utils.walkTree(this.root)) {
+ await this.maybeYield();
+ if (seen.has(node.id)) {
+ // We're in an infloop due to a cycle.
+ // Return early to avoid reporting false positives for orphans.
+ return;
+ }
+ seen.add(node.id);
+ }
+
+ for (let i = 0; i < this.liveRecords.length; ++i) {
+ await this.maybeYield();
+ let record = this.liveRecords[i];
+ if (!seen.has(record.id)) {
+ // We intentionally don't record the parentid here, since we only record
+ // that if the record refers to a parent that doesn't exist, which we
+ // have already handled (when linking parentid's).
+ this._noteOrphan(record.id);
+ }
+ }
+
+ for (const [id, parent] of this._orphans) {
+ await this.maybeYield();
+ this.problemData.orphans.push({id, parent});
+ }
+ }
+
+ async _finish() {
+ this.problemData.cycles = await detectCycles(this.liveRecords);
+
+ for (const [child, recordedParents] of this._multipleParents) {
+ let parents = new Set(recordedParents);
+ if (parents.size > 1) {
+ this.problemData.multipleParents.push({ child, parents: [...parents] });
+ }
+ }
+ // Dedupe simple arrays in the problem data, so that we don't have to worry
+ // about it in the code
+ const idArrayProps = [
+ "duplicates",
+ "deletedParents",
+ "childrenOnNonFolder",
+ "duplicateChildren",
+ "parentNotFolder",
+ ];
+ for (let prop of idArrayProps) {
+ this.problemData[prop] = [...new Set(this.problemData[prop])];
+ }
+ }
+}
+
class BookmarkValidator {
+ constructor() {
+ this.maybeYield = Async.jankYielder();
+ }
async canValidate() {
return !await PlacesSyncUtils.bookmarks.havePendingChanges();
}
- _followQueries(recordsByQueryId) {
+ async _followQueries(recordsByQueryId) {
for (let entry of recordsByQueryId.values()) {
+ await this.maybeYield();
if (entry.type !== "query" && (!entry.bmkUri || !entry.bmkUri.startsWith(QUERY_PROTOCOL))) {
continue;
}
let params = new URLSearchParams(entry.bmkUri.slice(QUERY_PROTOCOL.length));
// Queries with `excludeQueries` won't form cycles because they'll
// exclude all queries, including themselves, from the result set.
let excludeQueries = params.get("excludeQueries");
if (excludeQueries === "1" || excludeQueries === "true") {
@@ -276,19 +644,18 @@ class BookmarkValidator {
let records = [];
// A map of local IDs and well-known query folder names to records. Unlike
// GUIDs, local IDs aren't synced, since they're not stable across devices.
// New Places APIs use GUIDs to refer to bookmarks, but the legacy APIs
// still use local IDs. We use this mapping to parse `place:` queries that
// refer to folders via their local IDs.
let recordsByQueryId = new Map();
let syncedRoots = SYNCED_ROOTS;
- let maybeYield = Async.jankYielder();
- async function traverse(treeNode, synced) {
- await maybeYield();
+ const traverse = async (treeNode, synced) => {
+ await this.maybeYield();
if (!synced) {
synced = syncedRoots.includes(treeNode.guid);
} else if (isNodeIgnored(treeNode)) {
synced = false;
}
let localId = treeNode.id;
let guid = PlacesSyncUtils.bookmarks.guidToRecordId(treeNode.guid);
let itemType = "item";
@@ -354,20 +721,20 @@ class BookmarkValidator {
}
for (let child of treeNode.children) {
await traverse(child, synced);
child.parent = treeNode;
child.parentid = guid;
treeNode.childGUIDs.push(child.guid);
}
}
- }
+ };
await traverse(clientTree, false);
clientTree.id = "places";
- this._followQueries(recordsByQueryId);
+ await this._followQueries(recordsByQueryId);
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
* deleted and non-deleted nodes.
*
@@ -386,437 +753,189 @@ 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.
*/
- // XXX This should be split up and the complexity reduced.
- // eslint-disable-next-line complexity
async inspectServerRecords(serverRecords) {
- let deletedItemIds = new Set();
- let idToRecord = new Map();
- let deletedRecords = [];
-
- let folders = [];
-
- let problemData = new BookmarkProblemData();
-
- let resultRecords = [];
-
- let maybeYield = Async.jankYielder();
- for (let record of serverRecords) {
- await maybeYield();
- if (!record.id) {
- ++problemData.missingIDs;
- continue;
- }
- if (record.deleted) {
- deletedItemIds.add(record.id);
- } else if (idToRecord.has(record.id)) {
- problemData.duplicates.push(record.id);
- continue;
- }
- idToRecord.set(record.id, record);
-
- if (record.children) {
- if (record.type !== "folder") {
- // Due to implementation details in engines/bookmarks.js, (Livemark
- // subclassing BookmarkFolder) Livemarks will have a children array,
- // but it should still be empty.
- if (!record.children.length) {
- continue;
- }
- // Otherwise we mark it as an error and still try to resolve the children
- problemData.childrenOnNonFolder.push(record.id);
- }
- folders.push(record);
-
- if (new Set(record.children).size !== record.children.length) {
- problemData.duplicateChildren.push(record.id);
- }
-
- // 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.guidToRecordId(childID);
- });
- }
- }
-
- for (let deletedId of deletedItemIds) {
- let record = idToRecord.get(deletedId);
- if (record && !record.isDeleted) {
- deletedRecords.push(record);
- record.isDeleted = true;
- }
- }
-
- let root = idToRecord.get("places");
-
- if (!root) {
- // Fabricate a root. We want to remember that it's fake so that we can
- // avoid complaining about stuff like it missing it's childGUIDs later.
- root = { id: "places", children: [], type: "folder", title: "", fake: true };
- resultRecords.push(root);
- idToRecord.set("places", root);
- } else {
- problemData.rootOnServer = true;
- }
-
- // Build the tree, find orphans, and record most problems having to do with
- // the tree structure.
- for (let [id, record] of idToRecord) {
- if (record === root) {
- continue;
- }
-
- if (record.isDeleted) {
- continue;
- }
-
- let parentID = record.parentid;
- if (!parentID) {
- problemData.orphans.push({id: record.id, parent: parentID});
- continue;
- }
-
- let parent = idToRecord.get(parentID);
- if (!parent) {
- problemData.orphans.push({id: record.id, parent: parentID});
- continue;
- }
-
- if (parent.type !== "folder") {
- problemData.parentNotFolder.push(record.id);
- if (!parent.children) {
- parent.children = [];
- }
- if (!parent.childGUIDs) {
- parent.childGUIDs = [];
- }
- }
-
- if (!record.isDeleted) {
- resultRecords.push(record);
- }
-
- record.parent = parent;
- if (parent !== root || problemData.rootOnServer) {
- let childIndex = parent.children.indexOf(id);
- if (childIndex < 0) {
- problemData.parentChildMismatches.push({parent: parent.id, child: record.id});
- } else {
- parent.children[childIndex] = record;
- }
- } else {
- parent.children.push(record);
- }
-
- if (parent.isDeleted && !record.isDeleted) {
- problemData.deletedParents.push(record.id);
- }
-
- // We used to check if the parentName on the server matches the actual
- // local parent name, but given this is used only for de-duping a record
- // the first time it is seen and expensive to keep up-to-date, we decided
- // to just stop recording it. See bug 1276969 for more.
- }
-
- // Check that we aren't missing any children.
- for (let folder of folders) {
- folder.unfilteredChildren = folder.children;
- folder.children = [];
- for (let ci = 0; ci < folder.unfilteredChildren.length; ++ci) {
- let child = folder.unfilteredChildren[ci];
- let childObject;
- if (typeof child == "string") {
- // This can happen the parent refers to a child that has a different
- // parentid, or if it refers to a missing or deleted child. It shouldn't
- // be possible with totally valid bookmarks.
- childObject = idToRecord.get(child);
- if (!childObject) {
- problemData.missingChildren.push({parent: folder.id, child});
- } else {
- folder.unfilteredChildren[ci] = childObject;
- if (childObject.isDeleted) {
- problemData.deletedChildren.push({ parent: folder.id, child });
- }
- }
- } else {
- childObject = child;
- }
-
- if (!childObject) {
- continue;
- }
-
- if (childObject.parentid === folder.id) {
- folder.children.push(childObject);
- continue;
- }
-
- // The child is very probably in multiple `children` arrays --
- // see if we already have a problem record about it.
- let currentProblemRecord = problemData.multipleParents.find(pr =>
- pr.child === child);
-
- if (currentProblemRecord) {
- currentProblemRecord.parents.push(folder.id);
- continue;
- }
-
- let otherParent = idToRecord.get(childObject.parentid);
- // it's really an ... orphan ... sort of.
- if (!otherParent) {
- // if we never end up adding to this parent's list, we filter it out after this loop.
- problemData.multipleParents.push({
- child,
- parents: [folder.id]
- });
- if (!problemData.orphans.some(r => r.id === child)) {
- problemData.orphans.push({
- id: child,
- parent: childObject.parentid
- });
- }
- continue;
- }
-
- if (otherParent.isDeleted) {
- if (!problemData.deletedParents.includes(child)) {
- problemData.deletedParents.push(child);
- }
- continue;
- }
-
- if (otherParent.childGUIDs && !otherParent.childGUIDs.includes(child)) {
- if (!problemData.parentChildMismatches.some(r => r.child === child)) {
- // Might not be possible to get here.
- problemData.parentChildMismatches.push({ child, parent: folder.id });
- }
- }
-
- problemData.multipleParents.push({
- child,
- parents: [childObject.parentid, folder.id]
- });
- }
- }
- problemData.multipleParents = problemData.multipleParents.filter(record =>
- record.parents.length >= 2);
-
- problemData.cycles = this._detectCycles(resultRecords);
-
+ const data = await ServerRecordInspection.create(serverRecords);
return {
- deletedRecords,
- records: resultRecords,
- problemData,
- root,
+ deletedRecords: data.deletedRecords,
+ records: data.liveRecords,
+ problemData: data.problemData,
+ root: data.root,
};
}
- // helper for inspectServerRecords
- _detectCycles(records) {
- // currentPath and pathLookup contain the same data. pathLookup is faster to
- // query, but currentPath gives is the order of traversal that we need in
- // order to report the members of the cycles.
- let pathLookup = new Set();
- let currentPath = [];
- let cycles = [];
- let seenEver = new Set();
- const traverse = node => {
- if (pathLookup.has(node)) {
- let cycleStart = currentPath.lastIndexOf(node);
- let cyclePath = currentPath.slice(cycleStart).map(n => n.id);
- cycles.push(cyclePath);
- return;
- } else if (seenEver.has(node)) {
- // If we're checking the server, this is a problem, but it should already be reported.
- // On the client, this could happen due to including `node.concrete` in the child list.
- return;
- }
- seenEver.add(node);
- let children = node.children || [];
- if (node.concreteItems) {
- children.push(...node.concreteItems);
- }
- if (children.length) {
- pathLookup.add(node);
- currentPath.push(node);
- for (let child of children) {
- traverse(child);
- }
- currentPath.pop();
- pathLookup.delete(node);
- }
- };
- for (let record of records) {
- if (!seenEver.has(record)) {
- traverse(record);
- }
- }
-
- return cycles;
- }
-
// Perform client-side sanity checking that doesn't involve server data
- _validateClient(problemData, clientRecords) {
- problemData.clientCycles = this._detectCycles(clientRecords);
+ async _validateClient(problemData, clientRecords) {
+ problemData.clientCycles = await detectCycles(clientRecords);
for (let rootGUID of SYNCED_ROOTS) {
let record = clientRecords.find(record =>
record.guid === rootGUID);
if (!record || record.parentid !== "places") {
problemData.badClientRoots.push(rootGUID);
}
}
}
+ async _computeUnifiedRecordMap(serverRecords, clientRecords) {
+ let allRecords = new Map();
+ for (let sr of serverRecords) {
+ await this.maybeYield();
+ if (sr.fake) {
+ continue;
+ }
+ allRecords.set(sr.id, {client: null, server: sr});
+ }
+
+ for (let cr of clientRecords) {
+ await this.maybeYield();
+ let unified = allRecords.get(cr.id);
+ if (!unified) {
+ allRecords.set(cr.id, {client: cr, server: null});
+ } else {
+ unified.client = cr;
+ }
+ }
+ return allRecords;
+ }
+
+ _recordMissing(problems, id, clientRecord, serverRecord, serverTombstones) {
+ if (!clientRecord && serverRecord) {
+ problems.clientMissing.push(id);
+ }
+ if (!serverRecord && clientRecord) {
+ if (serverTombstones.has(id)) {
+ problems.serverDeleted.push(id);
+ } else if (!clientRecord.ignored && clientRecord.id != "places") {
+ problems.serverMissing.push(id);
+ }
+ }
+ }
+
+ _compareRecords(client, server) {
+ let structuralDifferences = [];
+ let differences = [];
+
+ // Don't bother comparing titles of roots. It's okay if locally it's
+ // "Mobile Bookmarks", but the server thinks it's "mobile".
+ // TODO: We probably should be handing other localized bookmarks (e.g.
+ // default bookmarks) here as well, see bug 1316041.
+ if (!SYNCED_ROOTS.includes(client.guid)) {
+ // We want to treat undefined, null and an empty string as identical
+ if ((client.title || "") !== (server.title || "")) {
+ differences.push("title");
+ }
+ }
+
+ if (client.parentid || server.parentid) {
+ if (client.parentid !== server.parentid) {
+ structuralDifferences.push("parentid");
+ }
+ }
+
+ if (client.tags || server.tags) {
+ let cl = client.tags ? [...client.tags].sort() : [];
+ let sl = server.tags ? [...server.tags].sort() : [];
+ if (!CommonUtils.arrayEqual(cl, sl)) {
+ differences.push("tags");
+ }
+ }
+
+ let sameType = client.type === server.type;
+ if (!sameType) {
+ if (server.type === "query" && client.type === "bookmark" &
+ client.bmkUri.startsWith(QUERY_PROTOCOL)) {
+ sameType = true;
+ }
+ }
+
+ if (!sameType) {
+ differences.push("type");
+ } else {
+ switch (server.type) {
+ case "bookmark":
+ case "query":
+ if (!areURLsEqual(server.bmkUri, client.bmkUri)) {
+ differences.push("bmkUri");
+ }
+ break;
+ case "separator":
+ if (server.pos != client.pos) {
+ differences.push("pos");
+ }
+ break;
+ case "livemark":
+ if (server.feedUri != client.feedUri) {
+ differences.push("feedUri");
+ }
+ if (server.siteUri != client.siteUri) {
+ differences.push("siteUri");
+ }
+ break;
+ case "folder":
+ if (server.id === "places" && server.fake) {
+ // It's the fabricated places root. It won't have the GUIDs, but
+ // it doesn't matter.
+ break;
+ }
+ if (client.childGUIDs || server.childGUIDs) {
+ let cl = client.childGUIDs || [];
+ let sl = server.childGUIDs || [];
+ if (!CommonUtils.arrayEqual(cl, sl)) {
+ structuralDifferences.push("childGUIDs");
+ }
+ }
+ break;
+ }
+ }
+ return { differences, structuralDifferences };
+ }
+
/**
* Compare the list of server records with the client tree.
*
* 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.
*/
- // XXX This should be split up and the complexity reduced.
- // eslint-disable-next-line complexity
async compareServerWithClient(serverRecords, clientTree) {
-
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);
-
- let allRecords = new Map();
- let serverDeletedLookup = new Set(inspectionInfo.deletedRecords.map(r => r.id));
+ await this._validateClient(problemData, clientRecords);
- for (let sr of serverRecords) {
- if (sr.fake) {
- continue;
- }
- allRecords.set(sr.id, {client: null, server: sr});
- }
+ let allRecords = await this._computeUnifiedRecordMap(serverRecords, clientRecords);
- for (let cr of clientRecords) {
- let unified = allRecords.get(cr.id);
- if (!unified) {
- allRecords.set(cr.id, {client: cr, server: null});
- } else {
- unified.client = cr;
- }
- }
-
-
+ let serverDeleted = new Set(inspectionInfo.deletedRecords.map(r => r.id));
for (let [id, {client, server}] of allRecords) {
- if (!client && server) {
- problemData.clientMissing.push(id);
- continue;
- }
- if (!server && client) {
- if (serverDeletedLookup.has(id)) {
- problemData.serverDeleted.push(id);
- } else if (!client.ignored && client.id != "places") {
- problemData.serverMissing.push(id);
- }
+ await this.maybeYield();
+ if (!client || !server) {
+ this._recordMissing(problemData, id, client, server, serverDeleted);
continue;
}
if (server && client && client.ignored) {
problemData.serverUnexpected.push(id);
}
- let differences = [];
- let structuralDifferences = [];
-
- // Don't bother comparing titles of roots. It's okay if locally it's
- // "Mobile Bookmarks", but the server thinks it's "mobile".
- // TODO: We probably should be handing other localized bookmarks (e.g.
- // default bookmarks) here as well, see bug 1316041.
- if (!SYNCED_ROOTS.includes(client.guid)) {
- // We want to treat undefined, null and an empty string as identical
- if ((client.title || "") !== (server.title || "")) {
- differences.push("title");
- }
- }
-
- if (client.parentid || server.parentid) {
- if (client.parentid !== server.parentid) {
- structuralDifferences.push("parentid");
- }
- }
-
- if (client.tags || server.tags) {
- let cl = client.tags ? [...client.tags].sort() : [];
- let sl = server.tags ? [...server.tags].sort() : [];
- if (!CommonUtils.arrayEqual(cl, sl)) {
- differences.push("tags");
- }
- }
-
- let sameType = client.type === server.type;
- if (!sameType) {
- if (server.type === "query" && client.type === "bookmark" && client.bmkUri.startsWith(QUERY_PROTOCOL)) {
- sameType = true;
- }
- }
-
-
- if (!sameType) {
- differences.push("type");
- } else {
- switch (server.type) {
- case "bookmark":
- case "query":
- if (!areURLsEqual(server.bmkUri, client.bmkUri)) {
- differences.push("bmkUri");
- }
- break;
- case "separator":
- if (server.pos != client.pos) {
- differences.push("pos");
- }
- break;
- case "livemark":
- if (server.feedUri != client.feedUri) {
- differences.push("feedUri");
- }
- if (server.siteUri != client.siteUri) {
- differences.push("siteUri");
- }
- break;
- case "folder":
- if (server.id === "places" && !problemData.rootOnServer) {
- // It's the fabricated places root. It won't have the GUIDs, but
- // it doesn't matter.
- break;
- }
- if (client.childGUIDs || server.childGUIDs) {
- let cl = client.childGUIDs || [];
- let sl = server.childGUIDs || [];
- if (!CommonUtils.arrayEqual(cl, sl)) {
- structuralDifferences.push("childGUIDs");
- }
- }
- break;
- }
- }
+ let { differences, structuralDifferences } = this._compareRecords(client, server);
if (differences.length) {
- problemData.differences.push({id, differences});
+ problemData.differences.push({ id, differences });
}
if (structuralDifferences.length) {
problemData.structuralDifferences.push({ id, differences: structuralDifferences });
}
}
return inspectionInfo;
}
@@ -825,20 +944,19 @@ class BookmarkValidator {
// ensure the repairer only applys with if-unmodified-since that date.
let collection = engine.itemSource();
let collectionKey = engine.service.collectionKeys.keyForCollection(engine.name);
collection.full = true;
let result = await collection.getBatched();
if (!result.response.success) {
throw result.response;
}
- let maybeYield = Async.jankYielder();
let cleartexts = [];
for (let record of result.records) {
- await maybeYield();
+ await this.maybeYield();
await record.decrypt(collectionKey);
cleartexts.push(record.cleartext);
}
return cleartexts;
}
async validate(engine) {
let start = Date.now();