Bug 1453804 - Ensure sync password validator will ever run r?markh
MozReview-Commit-ID: 8weadIdJHjl
--- a/services/sync/modules/collection_validator.js
+++ b/services/sync/modules/collection_validator.js
@@ -227,12 +227,33 @@ class CollectionValidator {
}
return {
problemData: problems,
clientRecords,
records: serverRecords,
deletedRecords: [...serverDeleted]
};
}
+
+ async validate(engine) {
+ let start = Cu.now();
+ let clientItems = await this.getClientItems();
+ let serverItems = await this.getServerItems(engine);
+ let serverRecordCount = serverItems.length;
+ let result = await this.compareClientWithServer(clientItems, serverItems);
+ let end = Cu.now();
+ let duration = end - start;
+ engine._log.debug(`Validated ${this.name} in ${duration}ms`);
+ engine._log.debug(`Problem summary`);
+ for (let { name, count } of result.problemData.getSummary()) {
+ engine._log.debug(` ${name}: ${count}`);
+ }
+ return {
+ duration,
+ version: this.version,
+ problems: result.problemData,
+ recordCount: serverRecordCount
+ };
+ }
}
// Default to 0, some engines may override.
CollectionValidator.prototype.version = 0;
--- a/services/sync/modules/doctor.js
+++ b/services/sync/modules/doctor.js
@@ -149,16 +149,19 @@ var Doctor = {
if (maxRecords >= 0 && recordCount > maxRecords) {
log.debug(`Skipping validation for ${engineName} because ` +
`the number of records (${recordCount}) is greater ` +
`than the maximum allowed (${maxRecords}).`);
continue;
}
let validator = engine.getValidator();
if (!validator) {
+ // This is probably only possible in profile downgrade cases.
+ log.warn(`engine.getValidator returned null for ${engineName
+ } but the pref that controls validation is enabled.`);
continue;
}
if (!await validator.canValidate()) {
log.debug(`Skipping validation for ${engineName} because validator.canValidate() is false`);
continue;
}
--- a/services/sync/modules/engines/passwords.js
+++ b/services/sync/modules/engines/passwords.js
@@ -136,17 +136,21 @@ PasswordEngine.prototype = {
async pullAllChanges() {
let changes = {};
let ids = await this._store.getAllIDs();
for (let [id, info] of Object.entries(ids)) {
changes[id] = info.timePasswordChanged / 1000;
}
return changes;
- }
+ },
+
+ getValidator() {
+ return new PasswordValidator();
+ },
};
function PasswordStore(name, engine) {
Store.call(this, name, engine);
this._nsLoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1", Ci.nsILoginInfo, "init");
}
PasswordStore.prototype = {
__proto__: Store.prototype,
@@ -373,20 +377,16 @@ PasswordTracker.prototype = {
case "removeAllLogins":
this._log.trace(data);
this.score += SCORE_INCREMENT_XLARGE;
break;
}
},
- getValidator() {
- return new PasswordValidator();
- },
-
async _trackLogin(login) {
if (Utils.getSyncCredentialsHosts().has(login.hostname)) {
// Skip over Weave password/passphrase changes.
return false;
}
const added = await this.addChangedID(login.guid);
if (!added) {
return false;
--- a/services/sync/tests/unit/test_password_engine.js
+++ b/services/sync/tests/unit/test_password_engine.js
@@ -213,8 +213,37 @@ add_task(async function test_password_du
equal(logins[0].QueryInterface(Ci.nsILoginMetaInfo).guid, guid2);
equal(null, collection.payload(guid1));
} finally {
await cleanup(engine, server);
}
});
+
+add_task(async function test_sync_password_validation() {
+ // This test isn't in test_password_validator to avoid duplicating cleanup.
+ _("Ensure that if a password validation happens, it ends up in the ping");
+
+ let engine = Service.engineManager.get("passwords");
+
+ let server = await serverForFoo(engine);
+ await SyncTestingInfrastructure(server);
+
+ Svc.Prefs.set("engine.passwords.validation.interval", 0);
+ Svc.Prefs.set("engine.passwords.validation.percentageChance", 100);
+ Svc.Prefs.set("engine.passwords.validation.maxRecords", -1);
+ Svc.Prefs.set("engine.passwords.validation.enabled", true);
+
+ try {
+
+ let ping = await wait_for_ping(() => Service.sync());
+
+ let engineInfo = ping.engines.find(e => e.name == "passwords");
+ ok(engineInfo, "Engine should be in ping");
+
+ let validation = engineInfo.validation;
+ ok(validation, "Engine should have validation info");
+
+ } finally {
+ await cleanup(engine, server);
+ }
+});