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 1343093 - Add integration test for bookmark repair. r=markh

Browse Source 
This test replaces the clients engine to simulate two different
clients, and uses `SOURCE_SYNC` to bypass change tracking.

MozReview-Commit-ID: IF3b3WZtain

--HG--
extra : rebase_source : deff5eb74dcbc29d7e7556b4ce6e15590ee4d4a0
This commit is contained in:
Kit Cambridge 2017-03-21 11:54:06 -07:00
parent 7070fa0cb7
commit c4565c993a
5 changed files with 235 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
services/sync/modules/bookmark_repair.js
View File

@ -89,12 +89,6 @@ class BookmarkRepairRequestor extends CollectionRepairRequestor {
this.prefs = new Preferences(PREF_BRANCH);
}
/* Exposed incase another module needs to understand our state
*/
get STATE() {
return STATE;
}
/* Check if any other clients connected to our account are current performing
a repair. A thin wrapper which exists mainly for mocking during tests.
*/
@ -186,13 +180,6 @@ class BookmarkRepairRequestor extends CollectionRepairRequestor {
return false;
}
if (this.anyClientsRepairing()) {
log.info("Can't start repair, since other clients are already repairing bookmarks");
let extra = { flowID, reason: "other clients repairing" };
this.service.recordTelemetryEvent("repair", "aborted", undefined, extra)
return false;
}
let ids = this.getProblemIDs(validationInfo);
if (ids.size > MAX_REQUESTED_IDS) {
log.info("Not starting a repair as there are over " + MAX_REQUESTED_IDS + " problems");
@ -206,6 +193,13 @@ class BookmarkRepairRequestor extends CollectionRepairRequestor {
return false;
}
if (this.anyClientsRepairing()) {
log.info("Can't start repair, since other clients are already repairing bookmarks");
let extra = { flowID, reason: "other clients repairing" };
this.service.recordTelemetryEvent("repair", "aborted", undefined, extra)
return false;
}
log.info(`Starting a repair, looking for ${ids.size} missing item(s)`);
// setup our prefs to indicate we are on our way.
this._flowID = flowID;
@ -720,3 +714,8 @@ class BookmarkRepairResponder extends CollectionRepairResponder {
// that should be rare, so let's wait and see what telemetry tells us.
}
}
/* Exposed in case another module needs to understand our state.
*/
BookmarkRepairRequestor.STATE = STATE;
BookmarkRepairRequestor.PREF = PREF;

2
services/sync/modules/engines/clients.js
View File

@ -406,7 +406,7 @@ ClientEngine.prototype = {
continue;
}
// fixup the client record, so our copy of _remoteClients matches what we uploaded.
clientRecord.commands = this._store.createRecord(id);
this._store._remoteClients[id] = this._store.createRecord(id);
// we could do better and pass the reference to the record we just uploaded,
// but this will do for now
}

251
services/sync/tests/unit/test_bookmark_repair.js
View File

@ -5,36 +5,63 @@
// many mocks)
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://gre/modules/osfile.jsm");
Cu.import("resource://services-sync/bookmark_repair.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/doctor.js");
Cu.import("resource://services-sync/service.js");
Cu.import("resource://services-sync/engines/clients.js");
Cu.import("resource://services-sync/engines/bookmarks.js");
Cu.import("resource://testing-common/services/sync/utils.js");
const LAST_BOOKMARK_SYNC_PREFS = [
"bookmarks.lastSync",
"bookmarks.lastSyncLocal",
];
const BOOKMARK_REPAIR_STATE_PREFS = [
"client.GUID",
"doctor.lastRepairAdvance",
...LAST_BOOKMARK_SYNC_PREFS,
...Object.values(BookmarkRepairRequestor.PREF).map(name =>
`repairs.bookmarks.${name}`
),
];
initTestLogging("Trace");
Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace
Log.repository.getLogger("Sync.Engine.Clients").level = Log.Level.Trace
Log.repository.getLogger("Sqlite").level = Log.Level.Info; // less noisy
const bms = PlacesUtils.bookmarks;
//Service.engineManager.register(BookmarksEngine);
let clientsEngine = Service.clientsEngine;
let bookmarksEngine = Service.engineManager.get("bookmarks");
generateNewKeys(Service.collectionKeys);
function createFolder(parentId, title) {
let id = bms.createFolder(parentId, title, 0);
let guid = bookmarksEngine._store.GUIDForId(id);
return { id, guid };
var recordedEvents = [];
Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
recordedEvents.push({ object, method, value, extra });
};
function checkRecordedEvents(expected, message) {
deepEqual(recordedEvents, expected, message);
// and clear the list so future checks are easier to write.
recordedEvents = [];
}
function createBookmark(parentId, url, title, index = bms.DEFAULT_INDEX) {
let uri = Utils.makeURI(url);
let id = bms.insertBookmark(parentId, uri, index, title)
let guid = bookmarksEngine._store.GUIDForId(id);
return { id, guid };
// Backs up and resets all preferences to their default values. Returns a
// function that restores the preferences when called.
function backupPrefs(names) {
let state = new Map();
for (let name of names) {
state.set(name, Svc.Prefs.get(name));
Svc.Prefs.reset(name);
}
return () => {
for (let [name, value] of state) {
Svc.Prefs.set(name, value);
}
};
}
async function promiseValidationDone(expected) {
@ -57,7 +84,7 @@ async function cleanup(server) {
await promiseStopServer(server);
}
add_task(async function test_something() {
add_task(async function test_bookmark_repair_integration() {
enableValidationPrefs();
_("Ensure that a validation error triggers a repair request.");
@ -103,50 +130,194 @@ add_task(async function test_something() {
protocols: ["1.5"],
}), Date.now() / 1000));
// Create a couple of bookmarks.
let folderInfo = createFolder(bms.toolbarFolder, "Folder 1");
let bookmarkInfo = createBookmark(folderInfo.id, "http://getfirefox.com/", "Get Firefox!");
let validationPromise = promiseValidationDone([]);
_("Syncing.");
Service.sync();
// should have 2 clients
equal(clientsEngine.stats.numClients, 2)
await validationPromise;
// Now we will reach into the server and hard-delete the bookmark
user.collection("bookmarks").wbo(bookmarkInfo.guid).delete();
// And delete the bookmark, but cheat by telling places that Sync did
// it, so we don't end up with an orphan.
// and use SQL to hard-delete the bookmark from the store.
await bms.remove(bookmarkInfo.guid, {
source: bms.SOURCE_SYNC,
_("Create bookmark and folder");
let folderInfo = await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.toolbarGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
title: "Folder 1",
});
// sanity check we aren't going to sync this removal.
do_check_empty(bookmarksEngine.pullNewChanges());
let bookmarkInfo = await PlacesUtils.bookmarks.insert({
parentGuid: folderInfo.guid,
url: "http://getfirefox.com/",
title: "Get Firefox!",
});
_(`Upload ${folderInfo.guid} and ${bookmarkInfo.guid} to server`);
let validationPromise = promiseValidationDone([]);
Service.sync();
equal(clientsEngine.stats.numClients, 2, "Clients collection should have 2 records");
await validationPromise;
checkRecordedEvents([], "Should not start repair after first sync");
_("Back up last sync timestamps for remote client");
let restoreRemoteLastBookmarkSync = backupPrefs(LAST_BOOKMARK_SYNC_PREFS);
_(`Delete ${bookmarkInfo.guid} locally and on server`);
// Now we will reach into the server and hard-delete the bookmark
user.collection("bookmarks").remove(bookmarkInfo.guid);
// And delete the bookmark, but cheat by telling places that Sync did
// it, so we don't end up with a tombstone.
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
deepEqual(bookmarksEngine.pullNewChanges(), {},
`Should not upload tombstone for ${bookmarkInfo.guid}`);
// sync again - we should have a few problems...
_("Syncing again.");
_("Sync again to trigger repair");
validationPromise = promiseValidationDone([
{"name":"missingChildren","count":1},
{"name":"structuralDifferences","count":1},
]);
Service.sync();
await validationPromise;
let flowID = Svc.Prefs.get("repairs.bookmarks.flowID");
checkRecordedEvents([{
object: "repair",
method: "started",
value: undefined,
extra: {
flowID,
numIDs: "1",
},
}, {
object: "sendcommand",
method: "repairRequest",
value: undefined,
extra: {
flowID,
deviceID: Service.identity.hashedDeviceID(remoteID),
},
}, {
object: "repair",
method: "request",
value: "upload",
extra: {
deviceID: Service.identity.hashedDeviceID(remoteID),
flowID,
numIDs: "1",
},
}], "Should record telemetry events for repair request");
// We should have started a repair with our second client.
equal(clientsEngine.getClientCommands(remoteID).length, 1);
_("Syncing so the outgoing client command is sent.");
equal(clientsEngine.getClientCommands(remoteID).length, 1,
"Should queue repair request for remote client after repair");
_("Sync to send outgoing repair request");
Service.sync();
equal(clientsEngine.getClientCommands(remoteID).length, 0);
equal(clientsEngine.getClientCommands(remoteID).length, 0,
"Should send repair request to remote client after next sync");
checkRecordedEvents([],
"Should not record repair telemetry after sending repair request");
_("Back up repair state to restore later");
let restoreInitialRepairState = backupPrefs(BOOKMARK_REPAIR_STATE_PREFS);
// so now let's take over the role of that other client!
_("Create new clients engine pretending to be remote client");
let remoteClientsEngine = Service.clientsEngine = new ClientEngine(Service);
remoteClientsEngine.localID = remoteID;
_("what could possibly go wrong?");
Service.sync();
// TODO - make the rest of this work!
_("Restore missing bookmark");
// Pretend Sync wrote the bookmark, so that we upload it as part of the
// repair instead of the sync.
bookmarkInfo.source = PlacesUtils.bookmarks.SOURCE_SYNC;
await PlacesUtils.bookmarks.insert(bookmarkInfo);
restoreRemoteLastBookmarkSync();
_("Sync as remote client");
Service.sync();
checkRecordedEvents([{
object: "processcommand",
method: "repairRequest",
value: undefined,
extra: {
flowID,
},
}, {
object: "repairResponse",
method: "uploading",
value: undefined,
extra: {
flowID,
numIDs: "1",
},
}, {
object: "sendcommand",
method: "repairResponse",
value: undefined,
extra: {
flowID,
deviceID: Service.identity.hashedDeviceID(initialID),
},
}, {
object: "repairResponse",
method: "finished",
value: undefined,
extra: {
flowID,
numIDs: "1",
}
}], "Should record telemetry events for repair response");
// We should queue the repair response for the initial client.
equal(remoteClientsEngine.getClientCommands(initialID).length, 1,
"Should queue repair response for initial client after repair");
ok(user.collection("bookmarks").wbo(bookmarkInfo.guid),
"Should upload missing bookmark");
_("Sync to upload bookmark and send outgoing repair response");
Service.sync();
equal(remoteClientsEngine.getClientCommands(initialID).length, 0,
"Should send repair response to initial client after next sync");
checkRecordedEvents([],
"Should not record repair telemetry after sending repair response");
ok(!Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"),
"Remote client should not be repairing");
_("Pretend to be initial client again");
Service.clientsEngine = clientsEngine;
_("Restore incomplete Places database and prefs");
await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
source: PlacesUtils.bookmarks.SOURCE_SYNC,
});
restoreInitialRepairState();
ok(Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"),
"Initial client should still be repairing");
_("Sync as initial client");
let revalidationPromise = promiseValidationDone([]);
Service.sync();
let restoredBookmarkInfo = await PlacesUtils.bookmarks.fetch(bookmarkInfo.guid);
ok(restoredBookmarkInfo, "Missing bookmark should be downloaded to initial client");
checkRecordedEvents([{
object: "processcommand",
method: "repairResponse",
value: undefined,
extra: {
flowID,
},
}, {
object: "repair",
method: "response",
value: "upload",
extra: {
flowID,
deviceID: Service.identity.hashedDeviceID(remoteID),
numIDs: "1",
},
}, {
object: "repair",
method: "finished",
value: undefined,
extra: {
flowID,
numIDs: "0",
},
}]);
await revalidationPromise;
ok(!Services.prefs.prefHasUserValue("services.sync.repairs.bookmarks.state"),
"Should clear repair pref after successfully completing repair");
} finally {
await cleanup(server);
}

20
services/sync/tests/unit/test_bookmark_repair_requestor.js
View File

@ -133,16 +133,16 @@ add_task(async function test_requestor_one_client_no_response() {
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// asking it to continue stays in that state until we timeout or the command
// is removed.
requestor.continueRepairs();
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// now pretend that client synced.
mockService.clientsEngine._sentCommands = {};
requestor.continueRepairs();
checkState(requestor.STATE.SENT_SECOND_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_SECOND_REQUEST);
// the command should be outgoing again.
checkOutgoingCommand(mockService, "client-a");
@ -194,7 +194,7 @@ add_task(async function test_requestor_one_client_no_sync() {
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// pretend we are now in the future.
let theFuture = Date.now() + 300000000;
@ -245,7 +245,7 @@ add_task(async function test_requestor_latest_client_used() {
requestor.startRepairs(validationInfo, Utils.makeGUID());
// the repair command should be outgoing to the most-recent client.
checkOutgoingCommand(mockService, "client-late");
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
// and this test is done - reset the repair.
requestor.prefs.resetBranch();
});
@ -271,7 +271,7 @@ add_task(async function test_requestor_client_vanishes() {
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
mockService.clientsEngine._sentCommands = {};
// Now let's pretend the client vanished.
@ -279,7 +279,7 @@ add_task(async function test_requestor_client_vanishes() {
requestor.continueRepairs();
// We should have moved on to client-b.
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
checkOutgoingCommand(mockService, "client-b");
// Now let's pretend client B wrote all missing IDs.
@ -349,7 +349,7 @@ add_task(async function test_requestor_success_responses() {
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
mockService.clientsEngine._sentCommands = {};
// Now let's pretend the client wrote a response.
@ -362,7 +362,7 @@ add_task(async function test_requestor_success_responses() {
}
requestor.continueRepairs(response);
// We should have moved on to client 2.
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
checkOutgoingCommand(mockService, "client-b");
// Now let's pretend client B write the missing ID.
@ -469,7 +469,7 @@ add_task(async function test_requestor_already_repairing_continue() {
// the command should now be outgoing.
checkOutgoingCommand(mockService, "client-a");
checkState(requestor.STATE.SENT_REQUEST);
checkState(BookmarkRepairRequestor.STATE.SENT_REQUEST);
mockService.clientsEngine._sentCommands = {};
// Now let's pretend the client wrote a response (it doesn't matter what's in here)

2
services/sync/tests/unit/test_bookmark_repair_responder.js
View File

@ -21,7 +21,7 @@ Log.repository.getLogger("Sqlite").level = Log.Level.Error;
var recordedEvents = [];
Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
recordedEvents.push({ object, method, value, extra });
}
};
function checkRecordedEvents(expected) {
deepEqual(recordedEvents, expected);