Bug 1353217 - importing bookmarks from html doesn't need to reset the bookmarks engine. r=markh
MozReview-Commit-ID: 4F7KF5ZkNuX
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -967,23 +967,28 @@ BookmarksTracker.prototype = {
}
break;
case "bookmarks-restore-begin":
this._log.debug("Ignoring changes from importing bookmarks.");
break;
case "bookmarks-restore-success":
this._log.debug("Tracking all items on successful import.");
- this._log.debug("Restore succeeded: wiping server and other clients.");
- Async.promiseSpinningly((async () => {
- await this.engine.service.resetClient([this.name]);
- await this.engine.service.wipeServer([this.name]);
- await this.engine.service.clientsEngine.sendCommand("wipeEngine", [this.name],
- null, { reason: "bookmark-restore" });
- })());
+ if (data == "json") {
+ this._log.debug("Restore succeeded: wiping server and other clients.");
+ Async.promiseSpinningly((async () => {
+ await this.engine.service.resetClient([this.name]);
+ await this.engine.service.wipeServer([this.name]);
+ await this.engine.service.clientsEngine.sendCommand("wipeEngine", [this.name],
+ null, { reason: "bookmark-restore" });
+ })());
+ } else {
+ // "html", "html-initial", or "json-append"
+ this._log.debug("Import succeeded.");
+ }
break;
case "bookmarks-restore-failed":
this._log.debug("Tracking all items on failed import.");
break;
}
},
QueryInterface: XPCOMUtils.generateQI([
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -1,12 +1,13 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
+Cu.import("resource://gre/modules/BookmarkHTMLUtils.jsm");
Cu.import("resource://gre/modules/BookmarkJSONUtils.jsm");
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/engines.js");
Cu.import("resource://services-sync/engines/bookmarks.js");
Cu.import("resource://services-sync/service.js");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://testing-common/services/sync/utils.js");
@@ -190,17 +191,32 @@ add_task(async function test_processInco
Svc.Prefs.resetBranch("");
Service.recordManager.clearCache();
await PlacesSyncUtils.bookmarks.reset();
await promiseStopServer(server);
}
});
add_task(async function test_restorePromptsReupload() {
- _("Ensure that restoring from a backup will reupload all records.");
+ await test_restoreOrImport(true);
+});
+
+add_task(async function test_importPromptsReupload() {
+ await test_restoreOrImport(false);
+});
+
+// Test a JSON restore or HTML import. Use JSON if `aReplace` is `true`, or
+// HTML otherwise.
+async function test_restoreOrImport(aReplace) {
+ let verb = aReplace ? "restore" : "import";
+ let verbing = aReplace ? "restoring" : "importing";
+ let bookmarkUtils = aReplace ? BookmarkJSONUtils : BookmarkHTMLUtils;
+
+ _(`Ensure that ${verbing} from a backup will reupload all records.`);
+
let engine = new BookmarksEngine(Service);
await engine.initialize();
let store = engine._store;
let server = serverForFoo(engine);
await SyncTestingInfrastructure(server);
let collection = server.user("foo").collection("bookmarks");
@@ -215,35 +231,34 @@ add_task(async function test_restoreProm
let fxuri = Utils.makeURI("http://getfirefox.com/");
let tburi = Utils.makeURI("http://getthunderbird.com/");
_("Create a single record.");
let bmk1_id = PlacesUtils.bookmarks.insertBookmark(
folder1_id, fxuri, PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
let bmk1_guid = await store.GUIDForId(bmk1_id);
- _("Get Firefox!: " + bmk1_id + ", " + bmk1_guid);
-
+ _(`Get Firefox!: ${bmk1_id}, ${bmk1_guid}`);
let dirSvc = Cc["@mozilla.org/file/directory_service;1"]
.getService(Ci.nsIProperties);
let backupFile = dirSvc.get("TmpD", Ci.nsILocalFile);
_("Make a backup.");
backupFile.append("t_b_e_" + Date.now() + ".json");
- _("Backing up to file " + backupFile.path);
- await BookmarkJSONUtils.exportToFile(backupFile.path);
+ _(`Backing up to file ${backupFile.path}`);
+ await bookmarkUtils.exportToFile(backupFile.path);
_("Create a different record and sync.");
let bmk2_id = PlacesUtils.bookmarks.insertBookmark(
folder1_id, tburi, PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Thunderbird!");
let bmk2_guid = await store.GUIDForId(bmk2_id);
- _("Get Thunderbird!: " + bmk2_id + ", " + bmk2_guid);
+ _(`Get Thunderbird!: ${bmk2_id}, ${bmk2_guid}`);
PlacesUtils.bookmarks.removeItem(bmk1_id);
let error;
try {
await sync_engine_and_validate_telem(engine, false);
} catch (ex) {
error = ex;
@@ -254,83 +269,138 @@ add_task(async function test_restoreProm
_("Verify that there's only one bookmark on the server, and it's Thunderbird.");
// Of course, there's also the Bookmarks Toolbar and Bookmarks Menu...
let wbos = collection.keys(function(id) {
return ["menu", "toolbar", "mobile", "unfiled", folder1_guid].indexOf(id) == -1;
});
do_check_eq(wbos.length, 1);
do_check_eq(wbos[0], bmk2_guid);
- _("Now restore from a backup.");
- await BookmarkJSONUtils.importFromFile(backupFile, true);
+ _(`Now ${verb} from a backup.`);
+ await bookmarkUtils.importFromFile(backupFile, aReplace);
+
+ let bookmarksCollection = server.user("foo").collection("bookmarks");
+ if (aReplace) {
+ _("Verify that we wiped the server.");
+ do_check_true(!bookmarksCollection);
+ } else {
+ _("Verify that we didn't wipe the server.");
+ do_check_true(!!bookmarksCollection);
+ }
_("Ensure we have the bookmarks we expect locally.");
let guids = await fetchAllSyncIds();
_("GUIDs: " + JSON.stringify([...guids]));
- let found = false;
+ let bookmarkGuids = new Map();
let count = 0;
- let newFX;
for (let guid of guids) {
count++;
let id = await store.idForGUID(guid, true);
// Only one bookmark, so _all_ should be Firefox!
if (PlacesUtils.bookmarks.getItemType(id) == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
let uri = PlacesUtils.bookmarks.getBookmarkURI(id);
- _("Found URI " + uri.spec + " for GUID " + guid);
- do_check_eq(uri.spec, fxuri.spec);
- newFX = guid; // Save the new GUID after restore.
- found = true; // Only runs if the above check passes.
+ _(`Found URI ${uri.spec} for GUID ${guid}`);
+ bookmarkGuids.set(uri.spec, guid);
}
}
- _("We found it: " + found);
- do_check_true(found);
+ do_check_true(bookmarkGuids.has(fxuri.spec));
+ if (!aReplace) {
+ do_check_true(bookmarkGuids.has(tburi.spec));
+ }
_("Have the correct number of IDs locally, too.");
- do_check_eq(count, ["menu", "toolbar", "mobile", "unfiled", folder1_id, bmk1_id].length);
+ let expectedResults = ["menu", "toolbar", "mobile", "unfiled", folder1_id,
+ bmk1_id];
+ if (!aReplace) {
+ expectedResults.push("toolbar", folder1_id, bmk2_id);
+ }
+ do_check_eq(count, expectedResults.length);
_("Sync again. This'll wipe bookmarks from the server.");
try {
await sync_engine_and_validate_telem(engine, false);
} catch (ex) {
error = ex;
_("Got error: " + Log.exceptionStr(ex));
}
do_check_true(!error);
- _("Verify that there's only one bookmark on the server, and it's Firefox.");
+ _("Verify that there's the right bookmarks on the server.");
// Of course, there's also the Bookmarks Toolbar and Bookmarks Menu...
let payloads = server.user("foo").collection("bookmarks").payloads();
let bookmarkWBOs = payloads.filter(function(wbo) {
return wbo.type == "bookmark";
});
+
let folderWBOs = payloads.filter(function(wbo) {
return ((wbo.type == "folder") &&
(wbo.id != "menu") &&
(wbo.id != "toolbar") &&
(wbo.id != "unfiled") &&
- (wbo.id != "mobile"));
+ (wbo.id != "mobile") &&
+ (wbo.parentid != "menu"));
});
- do_check_eq(bookmarkWBOs.length, 1);
- do_check_eq(bookmarkWBOs[0].id, newFX);
- do_check_eq(bookmarkWBOs[0].bmkUri, fxuri.spec);
- do_check_eq(bookmarkWBOs[0].title, "Get Firefox!");
+ let expectedFX = {
+ id: bookmarkGuids.get(fxuri.spec),
+ bmkUri: fxuri.spec,
+ title: "Get Firefox!"
+ };
+ let expectedTB = {
+ id: bookmarkGuids.get(tburi.spec),
+ bmkUri: tburi.spec,
+ title: "Get Thunderbird!"
+ };
+
+ let expectedBookmarks;
+ if (aReplace) {
+ expectedBookmarks = [expectedFX];
+ } else {
+ expectedBookmarks = [expectedTB, expectedFX];
+ }
+
+ doCheckWBOs(bookmarkWBOs, expectedBookmarks);
_("Our old friend Folder 1 is still in play.");
- do_check_eq(folderWBOs.length, 1);
- do_check_eq(folderWBOs[0].title, "Folder 1");
+ let expectedFolder1 = { title: "Folder 1" };
+
+ let expectedFolders;
+ if (aReplace) {
+ expectedFolders = [expectedFolder1];
+ } else {
+ expectedFolders = [expectedFolder1, expectedFolder1];
+ }
+
+ doCheckWBOs(folderWBOs, expectedFolders);
} finally {
await store.wipe();
Svc.Prefs.resetBranch("");
Service.recordManager.clearCache();
await PlacesSyncUtils.bookmarks.reset();
await promiseStopServer(server);
}
-});
+}
+
+function doCheckWBOs(WBOs, expected) {
+ do_check_eq(WBOs.length, expected.length);
+ for (let i = 0; i < expected.length; i++) {
+ let lhs = WBOs[i];
+ let rhs = expected[i];
+ if ("id" in rhs) {
+ do_check_eq(lhs.id, rhs.id);
+ }
+ if ("bmkUri" in rhs) {
+ do_check_eq(lhs.bmkUri, rhs.bmkUri);
+ }
+ if ("title" in rhs) {
+ do_check_eq(lhs.title, rhs.title);
+ }
+ }
+}
function FakeRecord(constructor, r) {
constructor.call(this, "bookmarks", r.id);
for (let x in r) {
this[x] = r[x];
}
// Borrow the constructor's conversion functions.
this.toSyncBookmark = constructor.prototype.toSyncBookmark;
--- a/toolkit/components/places/BookmarkJSONUtils.jsm
+++ b/toolkit/components/places/BookmarkJSONUtils.jsm
@@ -52,25 +52,25 @@ this.BookmarkJSONUtils = Object.freeze({
* Boolean if true, replace existing bookmarks, else merge.
*
* @return {Promise}
* @resolves When the new bookmarks have been created.
* @rejects JavaScript exception.
*/
importFromURL: function BJU_importFromURL(aSpec, aReplace) {
return (async function() {
- notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN);
+ notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN, aReplace);
try {
let importer = new BookmarkImporter(aReplace);
await importer.importFromURL(aSpec);
- notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS);
+ notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aReplace);
} catch (ex) {
Cu.reportError("Failed to restore bookmarks from " + aSpec + ": " + ex);
- notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED);
+ notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aReplace);
}
})();
},
/**
* Restores bookmarks and tags from a JSON file.
* @note any item annotated with "places/excludeFromBackup" won't be removed
* before executing the restore.
@@ -89,31 +89,31 @@ this.BookmarkJSONUtils = Object.freeze({
if (aFilePath instanceof Ci.nsIFile) {
Deprecated.warning("Passing an nsIFile to BookmarksJSONUtils.importFromFile " +
"is deprecated. Please use an OS.File path string instead.",
"https://developer.mozilla.org/docs/JavaScript_OS.File");
aFilePath = aFilePath.path;
}
return (async function() {
- notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN);
+ notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_BEGIN, aReplace);
try {
if (!(await OS.File.exists(aFilePath)))
throw new Error("Cannot restore from nonexisting json file");
let importer = new BookmarkImporter(aReplace);
if (aFilePath.endsWith("jsonlz4")) {
await importer.importFromCompressedFile(aFilePath);
} else {
await importer.importFromURL(OS.Path.toFileURI(aFilePath));
}
- notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS);
+ notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_SUCCESS, aReplace);
} catch (ex) {
Cu.reportError("Failed to restore bookmarks from " + aFilePath + ": " + ex);
- notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED);
+ notifyObservers(PlacesUtils.TOPIC_BOOKMARKS_RESTORE_FAILED, aReplace);
throw ex;
}
})();
},
/**
* Serializes bookmarks using JSON, and writes to the supplied file path.
*
@@ -318,18 +318,18 @@ BookmarkImporter.prototype = {
let url = await fixupQuery(searchBookmark.url, folderIdToGuidMap);
if (url != searchBookmark.url) {
await PlacesUtils.bookmarks.update({ guid, url, source: this._source });
}
}
},
};
-function notifyObservers(topic) {
- Services.obs.notifyObservers(null, topic, "json");
+function notifyObservers(topic, replace) {
+ Services.obs.notifyObservers(null, topic, replace ? "json" : "json-append");
}
/**
* Replaces imported folder ids with their local counterparts in a place: URI.
*
* @param {nsIURI} aQueryURI
* A place: URI with folder ids.
* @param {Object} aFolderIdMap