Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-09 19:35:51 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1273234 - Reduce the number of false positives reported by sync's bookmark_validator. r=markh

Browse Source 
MozReview-Commit-ID: 6VulTTDcfs3

--HG--
extra : transplant_source : %B0v%5DV%3Ex_%FC%85%C2%D3%ED%84%F8d.%E2%BE%7CV
This commit is contained in:
Thom Chiovoloni 2016-05-19 16:14:24 -04:00
parent 265ec53ae8
commit 6ed76eba46
2 changed files with 68 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
services/sync/modules/bookmark_validator.js
View File

@ -22,6 +22,7 @@ class BookmarkValidator {
function traverse(treeNode) {
let guid = BookmarkSpecialIds.specialGUIDForId(treeNode.id) || treeNode.guid;
let itemType = 'item';
treeNode.ignored = PlacesUtils.annotations.itemHasAnnotation(treeNode.id, BookmarkAnnos.EXCLUDEBACKUP_ANNO);
treeNode.id = guid;
switch (treeNode.type) {
case PlacesUtils.TYPE_X_MOZ_PLACE:
@ -37,13 +38,30 @@ class BookmarkValidator {
}
break;
case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER:
itemType = 'folder';
let isLivemark = false;
if (treeNode.annos) {
for (let anno of treeNode.annos) {
if (anno.name === PlacesUtils.LMANNO_FEEDURI) {
isLivemark = true;
treeNode.feedUri = anno.value;
} else if (anno.name === PlacesUtils.LMANNO_SITEURI) {
isLivemark = true;
treeNode.siteUri = anno.value;
}
}
}
itemType = isLivemark ? "livemark" : "folder";
break;
case PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR:
itemType = 'separator';
break;
}
if (treeNode.tags) {
treeNode.tags = treeNode.tags.split(",");
} else {
treeNode.tags = [];
}
treeNode.type = itemType;
treeNode.pos = treeNode.index;
treeNode.bmkUri = treeNode.uri;
@ -153,7 +171,7 @@ class BookmarkValidator {
}
idToRecord.set(record.id, record);
}
if (record.children) {
if (record.children && record.type !== "livemark") {
if (record.type !== 'folder') {
problemData.childrenOnNonFolder.push(record.id);
}
@ -345,17 +363,24 @@ class BookmarkValidator {
/**
* Compare the list of server records with the client tree.
*
* Returns the problemData described in the inspectServerRecords comment,
* Returns the same data as described in the inspectServerRecords comment,
* with the following additional fields.
* - clientMissing: Array of ids on the server missing from the client
* - serverMissing: Array of ids on the client missing from the server
* - differences: Array of {id: string, differences: string array} recording
* the properties that are differente between the client and server
* - 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
*/
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;
@ -363,12 +388,15 @@ class BookmarkValidator {
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});
@ -390,11 +418,19 @@ class BookmarkValidator {
continue;
}
if (!server && client) {
problemData.serverMissing.push(id);
if (serverDeletedLookup.has(id)) {
problemData.serverDeleted.push(id);
} else if (!client.ignored && client.id != "places") {
problemData.serverMissing.push(id);
}
continue;
}
if (server && client && client.ignored) {
problemData.serverUnexpected.push(id);
}
let differences = [];
if (client.title !== server.title) {
// We want to treat undefined, null and an empty string as identical
if ((client.title || "") !== (server.title || "")) {
differences.push('title');
}
@ -418,7 +454,15 @@ class BookmarkValidator {
}
}
if (client.type !== server.type) {
let sameType = client.type === server.type;
if (!sameType) {
if (server.type === "query" && client.type === "bookmark" && client.bmkUri.startsWith("place:")) {
sameType = true;
}
}
if (!sameType) {
differences.push('type');
} else {
switch (server.type) {
@ -433,6 +477,14 @@ class BookmarkValidator {
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
@ -454,7 +506,7 @@ class BookmarkValidator {
problemData.differences.push({id, differences});
}
}
return problemData;
return inspectionInfo;
}
};

10
services/sync/tests/unit/test_bookmark_validator.js
View File

@ -185,7 +185,7 @@ function getDummyServerAndClient() {
add_test(function test_cswc_valid() {
let {server, client} = getDummyServerAndClient();
let c = new BookmarkValidator().compareServerWithClient(server, client);
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
equal(c.differences.length, 0);
@ -198,7 +198,7 @@ add_test(function test_cswc_serverMissing() {
server.pop();
server[0].children.pop();
let c = new BookmarkValidator().compareServerWithClient(server, client);
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
deepEqual(c.serverMissing, ['cccccccccccc']);
equal(c.clientMissing.length, 0);
deepEqual(c.differences, [{id: 'aaaaaaaaaaaa', differences: ['childGUIDs']}]);
@ -209,7 +209,7 @@ add_test(function test_cswc_clientMissing() {
let {server, client} = getDummyServerAndClient();
client.children[0].children.pop();
let c = new BookmarkValidator().compareServerWithClient(server, client);
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
deepEqual(c.clientMissing, ['cccccccccccc']);
equal(c.serverMissing.length, 0);
deepEqual(c.differences, [{id: 'aaaaaaaaaaaa', differences: ['childGUIDs']}]);
@ -220,7 +220,7 @@ add_test(function test_cswc_differences() {
{
let {server, client} = getDummyServerAndClient();
client.children[0].children[0].title = 'asdf';
let c = new BookmarkValidator().compareServerWithClient(server, client);
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
deepEqual(c.differences, [{id: 'bbbbbbbbbbbb', differences: ['title']}]);
@ -229,7 +229,7 @@ add_test(function test_cswc_differences() {
{
let {server, client} = getDummyServerAndClient();
server[2].type = 'bookmark';
let c = new BookmarkValidator().compareServerWithClient(server, client);
let c = new BookmarkValidator().compareServerWithClient(server, client).problemData;
equal(c.clientMissing.length, 0);
equal(c.serverMissing.length, 0);
deepEqual(c.differences, [{id: 'cccccccccccc', differences: ['type']}]);