--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -5,18 +5,115 @@
"use strict";
const Cu = Components.utils;
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/bookmark_utils.js");
-this.EXPORTED_SYMBOLS = ["BookmarkValidator"];
+this.EXPORTED_SYMBOLS = ["BookmarkValidator", "BookmarkProblemData"];
+
+/**
+ * Result of bookmark validation. Contains the following fields which describe
+ * server-side problems unless otherwise specified.
+ *
+ * - missingIDs (number): # of objects with missing ids
+ * - duplicates (array of ids): ids seen more than once
+ * - parentChildMismatches (array of {parent: parentid, child: childid}):
+ * instances where the child's parentid and the parent's children array
+ * do not match
+ * - cycles (array of array of ids). List of cycles found in the "tree".
+ * - orphans (array of {id: string, parent: string}): List of nodes with
+ * either no parentid, or where the parent could not be found.
+ * - missingChildren (array of {parent: id, child: id}):
+ * List of parent/children where the child id couldn't be found
+ * - multipleParents (array of {child: id, parents: array of ids}):
+ * List of children that were part of multiple parent arrays
+ * - deletedParents (array of ids) : List of records that aren't deleted but
+ * had deleted parents
+ * - childrenOnNonFolder (array of ids): list of non-folders that still have
+ * children arrays
+ * - duplicateChildren (array of ids): list of records who have the same
+ * child listed multiple times in their children array
+ * - parentNotFolder (array of ids): list of records that have parents that
+ * aren't folders
+ * - wrongParentName (array of ids): list of records whose parentName does
+ * not match the parent's actual title
+ * - rootOnServer (boolean): true if the root came from the server
+ *
+ * - clientMissing: Array of ids on the server missing from the client
+ * - serverMissing: Array of ids on the client missing from the server
+ * - serverDeleted: Array of ids on the client that the server had marked as deleted.
+ * - serverUnexpected: Array of ids that appear on the server but shouldn't
+ * because the client attempts to never upload them.
+ * - differences: Array of {id: string, differences: string array} recording
+ * the non-structural properties that are differente between the client and server
+ * - structuralDifferences: As above, but contains the items where the differences were
+ * structural, that is, they contained childGUIDs or parentid
+ */
+class BookmarkProblemData {
+ constructor() {
+ this.rootOnServer = false;
+ this.missingIDs = 0;
+ this.duplicates = [];
+ this.parentChildMismatches = [];
+ this.cycles = [];
+ this.orphans = [];
+ this.missingChildren = [];
+ this.multipleParents = [];
+ this.deletedParents = [];
+ this.childrenOnNonFolder = [];
+ this.duplicateChildren = [];
+ this.parentNotFolder = [];
+ this.wrongParentName = [];
+
+ this.clientMissing = [];
+ this.serverMissing = [];
+ this.serverDeleted = [];
+ this.serverUnexpected = [];
+ this.differences = [];
+ this.structuralDifferences = [];
+ }
+
+ /**
+ * Produce a list summarizing problems found. Each entry contains {name, count},
+ * where name is the field name for the problem, and count is the number of times
+ * the problem was encountered.
+ *
+ * Validation has failed if all counts are not 0.
+ */
+ getSummary() {
+ return [
+ { name: "clientMissing", count: this.clientMissing.length },
+ { name: "serverMissing", count: this.serverMissing.length },
+ { name: "serverDeleted", count: this.serverDeleted.length },
+ { name: "serverUnexpected", count: this.serverUnexpected.length },
+
+ { name: "structuralDifferences", count: this.structuralDifferences.length },
+ { name: "differences", count: this.differences.length },
+
+ { name: "missingIDs", count: this.missingIDs },
+ { name: "rootOnServer", count: this.rootOnServer ? 1 : 0 },
+
+ { name: "duplicates", count: this.duplicates.length },
+ { name: "parentChildMismatches", count: this.parentChildMismatches.length },
+ { name: "cycles", count: this.cycles.length },
+ { name: "orphans", count: this.orphans.length },
+ { name: "missingChildren", count: this.missingChildren.length },
+ { name: "multipleParents", count: this.multipleParents.length },
+ { name: "deletedParents", count: this.deletedParents.length },
+ { name: "childrenOnNonFolder", count: this.childrenOnNonFolder.length },
+ { name: "duplicateChildren", count: this.duplicateChildren.length },
+ { name: "parentNotFolder", count: this.parentNotFolder.length },
+ { name: "wrongParentName", count: this.wrongParentName.length },
+ ];
+ }
+}
class BookmarkValidator {
createClientRecordsFromTree(clientTree) {
// Iterate over the treeNode, converting it to something more similar to what
// the server stores.
let records = [];
function traverse(treeNode) {
@@ -101,64 +198,29 @@ class BookmarkValidator {
* contain any records that could not be found.
* - parent (record): The parent to this record.
* - Unchanged properties send down from the server: id, title, type,
* parentName, parentid, bmkURI, keyword, tags, pos, queryId, loadInSidebar
* - 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: Object containing info about problems recorded.
- * - missingIDs (number): # of objects with missing ids
- * - duplicates (array of ids): ids seen more than once
- * - parentChildMismatches (array of {parent: parentid, child: childid}):
- * instances where the child's parentid and the parent's children array
- * do not match
- * - cycles (array of array of ids). List of cycles found in the "tree".
- * - orphans (array of {id: string, parent: string}): List of nodes with
- * either no parentid, or where the parent could not be found.
- * - missingChildren (array of {parent: id, child: id}):
- * List of parent/children where the child id couldn't be found
- * - multipleParents (array of {child: id, parents: array of ids}):
- * List of children that were part of multiple parent arrays
- * - deletedParents (array of ids) : List of records that aren't deleted but
- * had deleted parents
- * - childrenOnNonFolder (array of ids): list of non-folders that still have
- * children arrays
- * - duplicateChildren (array of ids): list of records who have the same
- * child listed multiple times in their children array
- * - parentNotFolder (array of ids): list of records that have parents that
- * aren't folders
- * - wrongParentName (array of ids): list of records whose parentName does
- * not match the parent's actual title
- * - rootOnServer (boolean): true if the root came from the server
+ * - problemData: a BookmarkProblemData object, with the caveat that
+ * the fields describing client/server relationship will not have been filled
+ * out yet.
*/
inspectServerRecords(serverRecords) {
let deletedItemIds = new Set();
let idToRecord = new Map();
let deletedRecords = [];
let folders = [];
let problems = [];
- let problemData = {
- missingIDs: 0,
- duplicates: [],
- parentChildMismatches: [],
- cycles: [],
- orphans: [],
- missingChildren: [],
- multipleParents: [],
- deletedParents: [],
- childrenOnNonFolder: [],
- duplicateChildren: [],
- parentNotFolder: [],
- wrongParentName: [],
- rootOnServer: false
- };
+ let problemData = new BookmarkProblemData();
let resultRecords = [];
for (let record of serverRecords) {
if (!record.id) {
++problemData.missingIDs;
continue;
}
@@ -362,42 +424,29 @@ class BookmarkValidator {
/**
* 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, but with the
- * following additional entries.
- * - clientMissing: Array of ids on the server missing from the client
- * - serverMissing: Array of ids on the client missing from the server
- * - serverUnexpected: Array of ids that appear on the server but shouldn't
- * because the client attempts to never upload them.
- * - differences: Array of {id: string, differences: string array} recording
- * the properties that are differente between the client and server
+ * - problemData is the same as for inspectServerRecords, except all properties
+ * will be filled out.
*/
compareServerWithClient(serverRecords, clientTree) {
let clientRecords = this.createClientRecordsFromTree(clientTree);
let inspectionInfo = this.inspectServerRecords(serverRecords);
inspectionInfo.clientRecords = clientRecords;
// Mainly do this to remove deleted items and normalize child guids.
serverRecords = inspectionInfo.records;
let problemData = inspectionInfo.problemData;
- problemData.clientMissing = [];
- problemData.serverMissing = [];
- problemData.serverDeleted = [];
- problemData.serverUnexpected = [];
- problemData.differences = [];
- problemData.good = [];
-
let matches = [];
let allRecords = new Map();
let serverDeletedLookup = new Set(inspectionInfo.deletedRecords.map(r => r.id));
for (let sr of serverRecords) {
allRecords.set(sr.id, {client: null, server: sr});
}
@@ -424,24 +473,25 @@ class BookmarkValidator {
problemData.serverMissing.push(id);
}
continue;
}
if (server && client && client.ignored) {
problemData.serverUnexpected.push(id);
}
let differences = [];
+ let structuralDifferences = [];
// 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) {
- differences.push('parentid');
+ structuralDifferences.push('parentid');
}
// Need to special case 'unfiled' due to it's recent name change
// ("Other Bookmarks" vs "Unsorted Bookmarks"), otherwise this has a lot
// of false positives.
if (client.parentName !== server.parentName && server.parentid !== 'unfiled') {
differences.push('parentName');
}
}
@@ -490,25 +540,28 @@ class BookmarkValidator {
// 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 (cl.length !== sl.length || !cl.every((id, i) => sl[i] === id)) {
- differences.push('childGUIDs');
+ structuralDifferences.push('childGUIDs');
}
}
break;
}
}
if (differences.length) {
problemData.differences.push({id, differences});
}
+ if (structuralDifferences.length) {
+ problemData.structuralDifferences.push({ id, differences: structuralDifferences });
+ }
}
return inspectionInfo;
}
};
--- a/services/sync/tests/unit/test_bookmark_validator.js
+++ b/services/sync/tests/unit/test_bookmark_validator.js
@@ -196,28 +196,28 @@ add_test(function test_cswc_serverMissin
let {server, client} = getDummyServerAndClient();
// remove c
server.pop();
server[0].children.pop();
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
deepEqual(c.serverMissing, ['cccccccccccc']);
equal(c.clientMissing.length, 0);
- deepEqual(c.differences, [{id: 'aaaaaaaaaaaa', differences: ['childGUIDs']}]);
+ deepEqual(c.structuralDifferences, [{id: 'aaaaaaaaaaaa', differences: ['childGUIDs']}]);
run_next_test();
});
add_test(function test_cswc_clientMissing() {
let {server, client} = getDummyServerAndClient();
client.children[0].children.pop();
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
deepEqual(c.clientMissing, ['cccccccccccc']);
equal(c.serverMissing.length, 0);
- deepEqual(c.differences, [{id: 'aaaaaaaaaaaa', differences: ['childGUIDs']}]);
+ deepEqual(c.structuralDifferences, [{id: 'aaaaaaaaaaaa', differences: ['childGUIDs']}]);
run_next_test();
});
add_test(function test_cswc_differences() {
{
let {server, client} = getDummyServerAndClient();
client.children[0].children[0].title = 'asdf';
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;