--- a/services/sync/modules/bookmark_repair.js
+++ b/services/sync/modules/bookmark_repair.js
@@ -1,20 +1,23 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
const Cu = Components.utils;
-this.EXPORTED_SYMBOLS = ["BookmarkRepairRequestor"];
+this.EXPORTED_SYMBOLS = ["BookmarkRepairRequestor", "BookmarkRepairResponder"];
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/Log.jsm");
+Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
+Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/collection_repair.js");
Cu.import("resource://services-sync/constants.js");
const log = Log.repository.getLogger("Sync.Engine.Bookmarks.Repair");
const PREF_BRANCH = "services.sync.repairs.bookmarks.";
// How long should we wait after sending a repair request before we give up?
@@ -425,8 +428,153 @@ class BookmarkRepairRequestor extends Co
this.prefs.set(PREF.REPAIR_ID, val);
}
// Used for test mocks.
_now() {
return Date.now();
}
}
+
+/* An object that responds to repair requests initiated by some other device.
+*/
+class BookmarkRepairResponder extends CollectionRepairResponder {
+ async repair(request, rawCommand) {
+ if (request.request != "upload") {
+ this._abortRepair(request, rawCommand,
+ `Don't understand request type ${request.request}`);
+ return;
+ }
+ if (this._currentState) {
+ this._abortRepair(request, rawCommand,
+ `A repair is already in progress`);
+ return;
+ }
+ let engine = this.service.engineManager.get("bookmarks");
+ // Some items have been requested, but we need to be careful about how we
+ // handle them. Potential issues include:
+ // * The item exists locally, but isn't in the tree of items we sync (eg, it
+ // might be a left-pane item or similar.)
+ // * The item exists locally as a folder - and the children of the folder
+ // also don't exist on the server - just uploading the folder isn't going
+ // to help. (Note that we assume the parents *do* exist, otherwise the
+ // device requesting the item be uploaded wouldn't be aware it exists)
+ // XXX - is this OK? Maybe we should verify the parents are the same as
+ // we have recorded?
+ // * ???
+ let maybeToDelete = new Set(); // items we *may* delete.
+ let toUpload = new Set(); // items we will upload.
+ let existsRemotely = new Set(); // items we determine already exist on the server
+ let results = await PlacesSyncUtils.bookmarks.fetchSyncIdsForRepair(request.ids);
+ for (let { syncId: id, syncable } of results) {
+ if (syncable) {
+ toUpload.add(id);
+ } else {
+ log.debug(`repair request to upload item ${id} but it isn't under a syncable root`);
+ maybeToDelete.add(id);
+ }
+ }
+ if (log.level <= Log.Level.Debug) {
+ let missingItems = request.ids.filter(id =>
+ !toUpload.has(id) && !maybeToDelete.has(id)
+ );
+ if (missingItems.length) {
+ log.debug("repair request to upload items that don't exist locally",
+ missingItems);
+ }
+ }
+ // So we've now got items we know should potentially be uploaded or deleted.
+ // Query the server to find out what it actually has.
+ let itemSource = engine.itemSource();
+ itemSource.ids = request.ids;
+ log.trace(`checking the server for items`, request.ids);
+ for (let remoteID of JSON.parse(itemSource.get())) {
+ log.trace(`the server has "${remoteID}"`);
+ existsRemotely.add(remoteID);
+ // This item exists on the server, so remove it from toUpload.
+ toUpload.delete(remoteID);
+ }
+ // We only need to flag as deleted items that actually are on the server.
+ let toDelete = new Set();
+ for (let id of maybeToDelete) {
+ if (!existsRemotely.has(id)) {
+ toDelete.add(id);
+ }
+ }
+ // whew - now add these items to the tracker "weakly" (ie, they will not
+ // persist in the case of a restart, but that's OK - we'll then end up here
+ // again.)
+ log.debug(`repair request will upload ${toUpload.size} items and delete ${toDelete.size} items`);
+ for (let id of toUpload) {
+ engine._modified.setWeak(id, { tombstone: false });
+ }
+ for (let id of toDelete) {
+ engine._modified.setWeak(id, { tombstone: true });
+ }
+
+ // Add an observer for the engine sync being complete.
+ this._currentState = {
+ request,
+ rawCommand,
+ toUpload,
+ toDelete,
+ }
+ if (toUpload.size || toDelete.size) {
+ // We have arranged for stuff to be uploaded, so wait until that's done.
+ Svc.Obs.add("weave:engine:sync:uploaded", this.onUploaded, this);
+ // and record in telemetry that we got this far - just incase we never
+ // end up doing the upload for some obscure reason.
+ let eventExtra = {
+ flowID: request.flowID,
+ numIDs: (toUpload.size + toDelete.size).toString(),
+ };
+ this.service.recordTelemetryEvent("repairResponse", "uploading", undefined, eventExtra);
+ } else {
+ // We were unable to help with the repair, so report that we are done.
+ this._finishRepair();
+ }
+ }
+
+ onUploaded(subject, data) {
+ if (data != "bookmarks") {
+ return;
+ }
+ Svc.Obs.remove("weave:engine:sync:uploaded", this.onUploaded, this);
+ log.debug(`bookmarks engine has uploaded stuff - creating a repair response`);
+ this._finishRepair();
+ }
+
+ _finishRepair() {
+ let clientsEngine = this.service.clientsEngine;
+ let flowID = this._currentState.request.flowID;
+ let response = {
+ request: this._currentState.request.request,
+ collection: "bookmarks",
+ clientID: clientsEngine.localID,
+ flowID,
+ ids: [],
+ }
+ for (let id of this._currentState.toUpload) {
+ response.ids.push(id);
+ }
+ for (let id of this._currentState.toDelete) {
+ response.ids.push(id);
+ }
+ let clientID = this._currentState.request.requestor;
+ clientsEngine.sendCommand("repairResponse", [response], clientID, { flowID });
+ // and nuke the request from our client.
+ clientsEngine.removeLocalCommand(this._currentState.rawCommand);
+ let eventExtra = {
+ flowID,
+ numIDs: response.ids.length.toString(),
+ }
+ this.service.recordTelemetryEvent("repairResponse", "finished", undefined, eventExtra);
+ this._currentState = null;
+ }
+
+ _abortRepair(request, rawCommand, why) {
+ // XXX - probably want to try and use the same "error" handling as the
+ // rest of our telemetry code?
+ log.warn(`aborting repair request: ${why}`);
+ this.service.clientsEngine.removeLocalCommand(rawCommand);
+ // probably want to record telemetry and write a response???
+ }
+}
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_bookmark_repair_responder.js
@@ -0,0 +1,448 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
+Cu.import("resource://testing-common/PlacesTestUtils.jsm");
+Cu.import("resource:///modules/PlacesUIUtils.jsm");
+Cu.import("resource://gre/modules/Log.jsm");
+
+Cu.import("resource://services-sync/engines/bookmarks.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/bookmark_repair.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
+
+initTestLogging("Trace");
+Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace;
+
+// stub telemetry so we can easily check the right things are recorded.
+var recordedEvents = [];
+Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
+ recordedEvents.push({ object, method, value, extra });
+}
+
+function checkRecordedEvents(expected) {
+ deepEqual(recordedEvents, expected);
+ // and clear the list so future checks are easier to write.
+ recordedEvents = [];
+}
+
+function getServerBookmarks(server) {
+ return server.user("foo").collection("bookmarks");
+}
+
+async function setup() {
+ let clientsEngine = Service.clientsEngine;
+ let bookmarksEngine = Service.engineManager.get("bookmarks");
+
+ let server = serverForUsers({"foo": "password"}, {
+ meta: {
+ global: {
+ syncID: Service.syncID,
+ storageVersion: STORAGE_VERSION,
+ engines: {
+ clients: {
+ version: clientsEngine.version,
+ syncID: clientsEngine.syncID,
+ },
+ bookmarks: {
+ version: bookmarksEngine.version,
+ syncID: bookmarksEngine.syncID,
+ },
+ },
+ },
+ },
+ crypto: {
+ keys: encryptPayload({
+ id: "keys",
+ // Generate a fake default key bundle to avoid resetting the client
+ // before the first sync.
+ default: [
+ Svc.Crypto.generateRandomKey(),
+ Svc.Crypto.generateRandomKey(),
+ ],
+ }),
+ },
+ });
+
+ await SyncTestingInfrastructure(server);
+
+ // Disable validation so that we don't try to automatically repair the server
+ // when we sync.
+ Svc.Prefs.set("engine.bookmarks.validation.enabled", false);
+
+ return server;
+}
+
+async function cleanup(server) {
+ await promiseStopServer(server);
+ await PlacesSyncUtils.bookmarks.wipe();
+ Svc.Prefs.reset("engine.bookmarks.validation.enabled");
+}
+
+add_task(async function test_responder_no_items() {
+ let server = await setup();
+
+ let request = {
+ request: "upload",
+ ids: [Utils.makeGUID()],
+ flowID: Utils.makeGUID(),
+ }
+ let responder = new BookmarkRepairResponder();
+ await responder.repair(request, null);
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "finished",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "0"},
+ },
+ ]);
+
+ await cleanup(server);
+});
+
+// One item requested and we have it locally, but it's not yet on the server.
+add_task(async function test_responder_upload() {
+ let server = await setup();
+
+ // Pretend we've already synced this bookmark, so that we can ensure it's
+ // uploaded in response to our repair request.
+ let bm = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ title: "Get Firefox",
+ url: "http://getfirefox.com/",
+ source: PlacesUtils.bookmarks.SOURCES.SYNC });
+
+ Service.sync();
+ deepEqual(getServerBookmarks(server).keys().sort(), [
+ "menu",
+ "mobile",
+ "toolbar",
+ "unfiled",
+ ], "Should only upload roots on first sync");
+
+ let request = {
+ request: "upload",
+ ids: [bm.guid],
+ flowID: Utils.makeGUID(),
+ }
+ let responder = new BookmarkRepairResponder();
+ await responder.repair(request, null);
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "uploading",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "1"},
+ },
+ ]);
+
+ Service.sync();
+ deepEqual(getServerBookmarks(server).keys().sort(), [
+ "menu",
+ "mobile",
+ "toolbar",
+ "unfiled",
+ bm.guid,
+ ].sort(), "Should upload requested bookmark on second sync");
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "finished",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "1"},
+ },
+ ]);
+
+ await cleanup(server);
+});
+
+// One item requested and we have it locally, but it's already on the server.
+// We should not upload it.
+add_task(async function test_responder_item_exists_locally() {
+ let server = await setup();
+
+ let bm = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ title: "Get Firefox",
+ url: "http://getfirefox.com/" });
+ // first sync to get the item on the server.
+ _("Syncing to get item on the server");
+ Service.sync();
+
+ // issue a repair request for it.
+ let request = {
+ request: "upload",
+ ids: [bm.guid],
+ flowID: Utils.makeGUID(),
+ }
+ let responder = new BookmarkRepairResponder();
+ await responder.repair(request, null);
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "finished",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "0"},
+ },
+ ]);
+
+ await cleanup(server);
+});
+
+add_task(async function test_responder_tombstone() {
+ let server = await setup();
+
+ // TODO: Request an item for which we have a tombstone locally. Decide if
+ // we want to store tombstones permanently for this. In the integration
+ // test, we can also try requesting a deleted child or ancestor.
+
+ await cleanup(server);
+});
+
+add_task(async function test_responder_missing_items() {
+ let server = await setup();
+
+ let fxBmk = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ title: "Get Firefox",
+ url: "http://getfirefox.com/",
+ });
+ let tbBmk = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ title: "Get Thunderbird",
+ url: "http://getthunderbird.com/",
+ // Pretend we've already synced Thunderbird.
+ source: PlacesUtils.bookmarks.SOURCES.SYNC,
+ });
+
+ Service.sync();
+ deepEqual(getServerBookmarks(server).keys().sort(), [
+ "menu",
+ "mobile",
+ "toolbar",
+ "unfiled",
+ fxBmk.guid,
+ ].sort(), "Should upload roots and Firefox on first sync");
+
+ _("Request Firefox, Thunderbird, and nonexistent GUID");
+ let request = {
+ request: "upload",
+ ids: [fxBmk.guid, tbBmk.guid, Utils.makeGUID()],
+ flowID: Utils.makeGUID(),
+ }
+ let responder = new BookmarkRepairResponder();
+ await responder.repair(request, null);
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "uploading",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "1"},
+ },
+ ]);
+
+ _("Sync after requesting IDs");
+ Service.sync();
+ deepEqual(getServerBookmarks(server).keys().sort(), [
+ "menu",
+ "mobile",
+ "toolbar",
+ "unfiled",
+ fxBmk.guid,
+ tbBmk.guid,
+ ].sort(), "Second sync should upload Thunderbird; skip nonexistent");
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "finished",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "1"},
+ },
+ ]);
+
+ await cleanup(server);
+});
+
+add_task(async function test_non_syncable() {
+ let server = await setup();
+
+ // Creates the left pane queries as a side effect.
+ let leftPaneId = PlacesUIUtils.leftPaneFolderId;
+ _(`Left pane root ID: ${leftPaneId}`);
+ await PlacesTestUtils.promiseAsyncUpdates();
+
+ // A child folder of the left pane root, containing queries for the menu,
+ // toolbar, and unfiled queries.
+ let allBookmarksId = PlacesUIUtils.leftPaneQueries.AllBookmarks;
+ let allBookmarksGuid = await PlacesUtils.promiseItemGuid(allBookmarksId);
+
+ // Explicitly request the unfiled query; we should also upload tombstones
+ // for the menu and toolbar queries.
+ let unfiledQueryId = PlacesUIUtils.leftPaneQueries.UnfiledBookmarks;
+ let unfiledQueryGuid = await PlacesUtils.promiseItemGuid(unfiledQueryId);
+
+ let request = {
+ request: "upload",
+ ids: [allBookmarksGuid, unfiledQueryGuid],
+ flowID: Utils.makeGUID(),
+ }
+ let responder = new BookmarkRepairResponder();
+ await responder.repair(request, null);
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "uploading",
+ value: undefined,
+ // Tombstones for the folder and its 3 children.
+ extra: {flowID: request.flowID, numIDs: "4"},
+ },
+ ]);
+
+ _("Sync to upload tombstones for items");
+ Service.sync();
+
+ let toolbarQueryId = PlacesUIUtils.leftPaneQueries.BookmarksToolbar;
+ let menuQueryId = PlacesUIUtils.leftPaneQueries.BookmarksMenu;
+ let queryGuids = [
+ allBookmarksGuid,
+ await PlacesUtils.promiseItemGuid(toolbarQueryId),
+ await PlacesUtils.promiseItemGuid(menuQueryId),
+ unfiledQueryGuid,
+ ];
+
+ let collection = getServerBookmarks(server);
+ deepEqual(collection.keys().sort(), [
+ // We always upload roots on the first sync.
+ "menu",
+ "mobile",
+ "toolbar",
+ "unfiled",
+ ...queryGuids,
+ ].sort(), "Should upload roots and queries on first sync");
+
+ for (let guid of queryGuids) {
+ let wbo = collection.wbo(guid);
+ let payload = JSON.parse(JSON.parse(wbo.payload).ciphertext);
+ ok(payload.deleted, `Should upload tombstone for left pane query ${guid}`);
+ }
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "finished",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "4"},
+ },
+ ]);
+
+ await cleanup(server);
+});
+
+add_task(async function test_folder_descendants() {
+ let server = await setup();
+
+ let parentFolder = await PlacesUtils.bookmarks.insert({
+ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ parentGuid: PlacesUtils.bookmarks.menuGuid,
+ title: "Parent folder",
+ });
+ let childFolder = await PlacesUtils.bookmarks.insert({
+ type: PlacesUtils.bookmarks.TYPE_FOLDER,
+ parentGuid: parentFolder.guid,
+ title: "Child folder",
+ });
+ let childSiblingBmk = await PlacesUtils.bookmarks.insert({
+ parentGuid: parentFolder.guid,
+ title: "Get Thunderbird",
+ url: "http://getthunderbird.com",
+ });
+
+ _("Initial sync to upload roots and parent folder");
+ Service.sync();
+
+ let initialSyncIds = [
+ "menu",
+ "mobile",
+ "toolbar",
+ "unfiled",
+ parentFolder.guid,
+ childFolder.guid,
+ childSiblingBmk.guid,
+ ].sort();
+ deepEqual(getServerBookmarks(server).keys().sort(), initialSyncIds,
+ "Should upload roots and partial folder contents on first sync");
+
+ _("Insert missing bookmarks locally to request later");
+ let childBmk = await PlacesSyncUtils.bookmarks.insert({
+ kind: "bookmark",
+ syncId: Utils.makeGUID(),
+ parentSyncId: parentFolder.guid,
+ title: "Get Firefox",
+ url: "http://getfirefox.com",
+ });
+ let grandChildBmk = await PlacesSyncUtils.bookmarks.insert({
+ kind: "bookmark",
+ syncId: Utils.makeGUID(),
+ parentSyncId: childFolder.guid,
+ title: "Bugzilla",
+ url: "https://bugzilla.mozilla.org",
+ });
+ let grandChildSiblingBmk = await PlacesSyncUtils.bookmarks.insert({
+ kind: "bookmark",
+ syncId: Utils.makeGUID(),
+ parentSyncId: childFolder.guid,
+ title: "Mozilla",
+ url: "https://mozilla.org",
+ });
+
+ _("Sync again; server contents shouldn't change");
+ Service.sync();
+ deepEqual(getServerBookmarks(server).keys().sort(), initialSyncIds,
+ "Second sync should not upload missing bookmarks");
+
+ // This assumes the parent record on the server is correct, and the server
+ // is just missing the children. This isn't a correct assumption if the
+ // parent's `children` array is wrong, or if the parent and children disagree.
+ _("Request missing bookmarks");
+ let request = {
+ request: "upload",
+ ids: [
+ // Already on server.
+ parentFolder.guid,
+ childSiblingBmk.guid,
+ // Explicitly upload these. We should also upload `grandChildBmk`,
+ // since it's a descendant of `parentFolder` and we requested its
+ // ancestor.
+ childBmk.syncId,
+ grandChildSiblingBmk.syncId],
+ flowID: Utils.makeGUID(),
+ }
+ let responder = new BookmarkRepairResponder();
+ await responder.repair(request, null);
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "uploading",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "4"},
+ },
+ ]);
+
+ _("Sync after requesting repair; should upload missing records");
+ Service.sync();
+ deepEqual(getServerBookmarks(server).keys().sort(), [
+ ...initialSyncIds,
+ childBmk.syncId,
+ grandChildBmk.syncId,
+ grandChildSiblingBmk.syncId,
+ ].sort(), "Third sync should upload requested items");
+
+ checkRecordedEvents([
+ { object: "repairResponse",
+ method: "finished",
+ value: undefined,
+ extra: {flowID: request.flowID, numIDs: "4"},
+ },
+ ]);
+
+ await cleanup(server);
+});
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -15,17 +15,17 @@
"assertions.js": ["Assert", "Expect"],
"async.js": ["Async"],
"AsyncSpellCheckTestHelper.jsm": ["onSpellCheck"],
"AutoMigrate.jsm": ["AutoMigrate"],
"Battery.jsm": ["GetBattery", "Battery"],
"blocklist-clients.js": ["AddonBlocklistClient", "GfxBlocklistClient", "OneCRLBlocklistClient", "PluginBlocklistClient", "FILENAME_ADDONS_JSON", "FILENAME_GFX_JSON", "FILENAME_PLUGINS_JSON"],
"blocklist-updater.js": ["checkVersions", "addTestBlocklistClient"],
"bogus_element_type.jsm": [],
- "bookmark_repair.js": ["BookmarkRepairRequestor"],
+ "bookmark_repair.js": ["BookmarkRepairRequestor", "BookmarkRepairResponder"],
"bookmark_validator.js": ["BookmarkValidator", "BookmarkProblemData"],
"bookmarks.js": ["BookmarksEngine", "PlacesItem", "Bookmark", "BookmarkFolder", "BookmarkQuery", "Livemark", "BookmarkSeparator"],
"bookmarks.jsm": ["PlacesItem", "Bookmark", "Separator", "Livemark", "BookmarkFolder", "DumpBookmarks"],
"BootstrapMonitor.jsm": ["monitor"],
"browser-loader.js": ["BrowserLoader"],
"browserid_identity.js": ["BrowserIDManager", "AuthenticationError"],
"CertUtils.jsm": ["BadCertHandler", "checkCert", "readCertPrefs", "validateCert"],
"clients.js": ["ClientEngine", "ClientsRec"],
