Bug 1309774 - Part 2. Add version number to validation data in sync ping. r?markh
MozReview-Commit-ID: FlDSEsT1ah9
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -17,16 +17,17 @@ this.EXPORTED_SYMBOLS = ["BookmarkValida
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 ||
anno.name == LEFT_PANE_QUERY_ANNO);
}
+const BOOKMARK_VALIDATOR_VERSION = 1;
/**
* 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}):
@@ -758,18 +759,19 @@ class BookmarkValidator {
});
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
};
});
}
};
+BookmarkValidator.prototype.version = BOOKMARK_VALIDATOR_VERSION;
-
--- a/services/sync/modules/collection_validator.js
+++ b/services/sync/modules/collection_validator.js
@@ -194,8 +194,11 @@ class CollectionValidator {
return {
problemData: problems,
clientRecords: clientItems,
records: serverItems,
deletedRecords: [...serverDeleted]
};
}
}
+
+// Default to 0, some engines may override.
+CollectionValidator.prototype.version = 0;
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -168,18 +168,19 @@ class EngineRecord {
}
}
recordValidation(validationResult) {
if (this.validation) {
log.error(`Multiple validations occurred for engine ${this.name}!`);
return;
}
- let { problems, duration, recordCount } = validationResult;
+ let { problems, version, duration, recordCount } = validationResult;
let validation = {
+ version: version || 0,
checked: recordCount || 0,
};
if (duration > 0) {
validation.took = Math.round(duration);
}
let summarized = problems.getSummary(true).filter(({count}) => count > 0);
if (summarized.length) {
validation.problems = summarized;
--- a/services/sync/tests/unit/sync_ping_schema.json
+++ b/services/sync/tests/unit/sync_ping_schema.json
@@ -101,16 +101,17 @@
"anyOf": [
{ "required": ["checked"] },
{ "required": ["failureReason"] }
],
"properties": {
"checked": { "type": "integer", "minimum": 0 },
"failureReason": { "$ref": "#/definitions/error" },
"took": { "type": "integer" },
+ "version": { "type": "integer" },
"problems": {
"type": "array",
"minItems": 1,
"$ref": "#/definitions/validationProblem"
}
}
}
}
--- a/services/sync/tests/unit/test_bookmark_validator.js
+++ b/services/sync/tests/unit/test_bookmark_validator.js
@@ -300,20 +300,23 @@ add_test(function test_cswc_serverUnexpe
});
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 = {
- duration, // We fake this just so that we can verify it's passed through.
+ // We fake duration and version just so that we can verify they're passed through.
+ duration,
+ version: validator.version,
recordCount: server.length,
- problems: new BookmarkValidator().compareServerWithClient(server, client).problemData,
+ 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.
}
add_task(function *test_telemetry_integration() {
let {server, client} = getDummyServerAndClient();
@@ -325,16 +328,17 @@ add_task(function *test_telemetry_integr
ok(ping.engines);
let bme = ping.engines.find(e => e.name === "bookmarks");
ok(bme);
ok(bme.validation);
ok(bme.validation.problems)
equal(bme.validation.checked, server.length);
equal(bme.validation.took, duration);
bme.validation.problems.sort((a, b) => String.localeCompare(a.name, b.name));
+ 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 },
]);
});
--- a/toolkit/components/telemetry/docs/data/sync-ping.rst
+++ b/toolkit/components/telemetry/docs/data/sync-ping.rst
@@ -75,16 +75,18 @@ Structure:
}
],
// Optional, excluded if there were no errors
failureReason: { ... }, // Same as above.
// Optional, excluded if it would be empty or if the engine cannot
// or did not run validation on itself.
validation: {
+ // Optional validator version, default of 0.
+ version: <integer>,
checked: <integer>,
took: <non-monotonic integer duration in milliseconds>,
// Entries with a count of 0 are excluded, the array is excluded if no problems are found.
problems: [
{
name: <string>, // The problem identified.
count: <integer>, // Number of times it occurred.
}