--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -84,64 +84,106 @@ PlacesItem.prototype = {
case "item":
return PlacesItem;
}
throw "Unknown places item object type: " + type;
},
__proto__: CryptoWrapper.prototype,
_logName: "Sync.Record.PlacesItem",
+
+ // Converts the record to a Sync bookmark object that can be passed to
+ // `PlacesSyncUtils.bookmarks.{insert, update}`.
+ toSyncBookmark() {
+ return {
+ kind: this.type,
+ syncId: this.id,
+ parentSyncId: this.parentid,
+ };
+ },
};
Utils.deferGetSet(PlacesItem,
"cleartext",
["hasDupe", "parentid", "parentName", "type"]);
this.Bookmark = function Bookmark(collection, id, type) {
PlacesItem.call(this, collection, id, type || "bookmark");
}
Bookmark.prototype = {
__proto__: PlacesItem.prototype,
_logName: "Sync.Record.Bookmark",
+
+ toSyncBookmark() {
+ let info = PlacesItem.prototype.toSyncBookmark.call(this);
+ info.title = this.title;
+ info.url = this.bmkUri;
+ info.description = this.description;
+ info.loadInSidebar = this.loadInSidebar;
+ info.tags = this.tags;
+ info.keyword = this.keyword;
+ return info;
+ },
};
Utils.deferGetSet(Bookmark,
"cleartext",
["title", "bmkUri", "description",
"loadInSidebar", "tags", "keyword"]);
this.BookmarkQuery = function BookmarkQuery(collection, id) {
Bookmark.call(this, collection, id, "query");
}
BookmarkQuery.prototype = {
__proto__: Bookmark.prototype,
_logName: "Sync.Record.BookmarkQuery",
+
+ toSyncBookmark() {
+ let info = Bookmark.prototype.toSyncBookmark.call(this);
+ info.folder = this.folderName;
+ info.query = this.queryId;
+ return info;
+ },
};
Utils.deferGetSet(BookmarkQuery,
"cleartext",
["folderName", "queryId"]);
this.BookmarkFolder = function BookmarkFolder(collection, id, type) {
PlacesItem.call(this, collection, id, type || "folder");
}
BookmarkFolder.prototype = {
__proto__: PlacesItem.prototype,
_logName: "Sync.Record.Folder",
+
+ toSyncBookmark() {
+ let info = PlacesItem.prototype.toSyncBookmark.call(this);
+ info.description = this.description;
+ info.title = this.title;
+ return info;
+ },
};
Utils.deferGetSet(BookmarkFolder, "cleartext", ["description", "title",
"children"]);
this.Livemark = function Livemark(collection, id) {
BookmarkFolder.call(this, collection, id, "livemark");
}
Livemark.prototype = {
__proto__: BookmarkFolder.prototype,
_logName: "Sync.Record.Livemark",
+
+ toSyncBookmark() {
+ let info = BookmarkFolder.prototype.toSyncBookmark.call(this);
+ info.feed = this.feedUri;
+ info.site = this.siteUri;
+ return info;
+ },
};
Utils.deferGetSet(Livemark, "cleartext", ["siteUri", "feedUri"]);
this.BookmarkSeparator = function BookmarkSeparator(collection, id) {
PlacesItem.call(this, collection, id, "separator");
}
BookmarkSeparator.prototype = {
@@ -621,122 +663,46 @@ BookmarksStore.prototype = {
Store.prototype.applyIncoming.call(this, record);
if (record.type == "folder" && record.children) {
this._childrenToOrder[record.id] = record.children;
}
},
create: function BStore_create(record) {
- switch (record.type) {
- case "bookmark":
- case "query":
- case "microsummary": {
- let info = {
- kind: record.type,
- url: record.bmkUri,
- parentSyncId: record.parentid,
- title: record.title,
- syncId: record.id,
- tags: record.tags,
- keyword: record.keyword,
- loadInSidebar: record.loadInSidebar,
- query: record.queryId,
- folder: record.folderName,
- description: record.description,
- };
-
- let bmk = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
- this._log.debug("created bookmark " + bmk.syncId + " under " + bmk.parentSyncId
- + " as " + bmk.title + " " + bmk.url.href);
-
- } break;
- case "folder": {
- let info = {
- kind: PlacesSyncUtils.bookmarks.KINDS.FOLDER,
- parentSyncId: record.parentid,
- syncId: record.id,
- title: record.title,
- description: record.description,
- };
-
- let folder = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
- this._log.debug("created folder " + folder.syncId + " under " + folder.parentSyncId
- + " as " + folder.title);
-
- // record.children will be dealt with in _orderChildren.
- } break;
- case "livemark":
- if (!record.feedUri) {
- this._log.debug("No feed URI: skipping livemark record " + record.id);
- return;
- }
- let info = {
- kind: PlacesSyncUtils.bookmarks.KINDS.LIVEMARK,
- title: record.title,
- parentSyncId: record.parentid,
- feed: record.feedUri,
- site: record.siteUri,
- syncId: record.id,
- };
- let livemark = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
- if (livemark) {
- this._log.debug("Created livemark " + livemark.syncId + " under " +
- livemark.parentSyncId + " as " + livemark.title +
- ", " + livemark.site.href + ", " +
- livemark.feed.href);
- }
- break;
- case "separator": {
- let separator = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert({
- kind: PlacesSyncUtils.bookmarks.KINDS.SEPARATOR,
- parentSyncId: record.parentid,
- syncId: record.id,
- }));
- this._log.debug("created separator " + separator.syncId + " under " + separator.parentSyncId);
- } break;
- case "item":
- this._log.debug(" -> got a generic places item.. do nothing?");
- return;
- default:
- this._log.error("_create: Unknown item type: " + record.type);
- return;
+ let info = record.toSyncBookmark();
+ // This can throw if we're inserting an invalid or incomplete bookmark.
+ // That's fine; the exception will be caught by `applyIncomingBatch`
+ // without aborting further processing.
+ let item = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
+ if (item) {
+ this._log.debug(`Created ${item.kind} ${item.syncId} under ${
+ item.parentSyncId}`, item);
}
},
remove: function BStore_remove(record) {
try {
let info = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.remove(record.id));
if (info) {
this._log.debug(`Removed item ${record.id} with type ${record.type}`);
}
} catch (ex) {
// Likely already removed.
this._log.debug(`Error removing ${record.id}`, ex);
}
},
update: function BStore_update(record) {
- let info = {
- parentSyncId: record.parentid,
- syncId: record.id,
- kind: record.type,
- title: record.title,
- url: record.bmkUri,
- tags: record.tags,
- keyword: record.keyword,
- description: record.description,
- loadInSidebar: record.loadInSidebar,
- query: record.queryId,
- site: record.siteUri,
- feed: record.feedUri,
- };
-
- let bmk = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.update(info));
- this._log.debug("updated bookmark " + bmk.syncId + " under " + bmk.parentSyncId);
+ let info = record.toSyncBookmark();
+ let item = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.update(info));
+ if (item) {
+ this._log.debug(`Updated ${item.kind} ${item.syncId} under ${
+ item.parentSyncId}`, item);
+ }
},
_orderChildren: function _orderChildren() {
let promises = Object.keys(this._childrenToOrder).map(syncID => {
let children = this._childrenToOrder[syncID];
return PlacesSyncUtils.bookmarks.order(syncID, children).catch(ex => {
this._log.debug(`Could not order children for ${syncID}`, ex);
});
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -354,16 +354,18 @@ add_task(function* test_restorePromptsRe
}
});
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;
}
// Bug 632287.
add_task(function* test_mismatched_types() {
_("Ensure that handling a record that changes type causes deletion " +
"then re-adding.");
let oldRecord = {
@@ -509,27 +511,24 @@ add_task(function* test_bookmark_tag_but
yield PlacesSyncUtils.bookmarks.insert({
kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
syncId: Utils.makeGUID(),
url: "about:fake",
parentSyncId: "toolbar",
tags: null,
});
- let record = {
+ let record = new FakeRecord(BookmarkFolder, {
parentid: "toolbar",
id: Utils.makeGUID(),
description: "",
tags: ["foo"],
title: "Taggy tag",
type: "folder"
- };
-
- // Because update() walks the cleartext.
- record.cleartext = record;
+ });
store.create(record);
record.tags = ["bar"];
store.update(record);
});
add_task(function* test_misreconciled_root() {
_("Ensure that we don't reconcile an arbitrary record with a root.");
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -453,16 +453,21 @@ var insertSyncBookmark = Task.async(func
// Reparent all orphans that expect this folder as the parent.
yield reparentOrphans(newItem);
return newItem;
});
// Inserts a synced livemark.
var insertSyncLivemark = Task.async(function* (insertInfo) {
+ if (!insertInfo.feed) {
+ BookmarkSyncLog.debug(`insertSyncLivemark: ${
+ insertInfo.syncId} missing feed URL`);
+ return null;
+ }
let livemarkInfo = syncBookmarkToPlacesBookmark(insertInfo);
let parentId = yield PlacesUtils.promiseItemId(livemarkInfo.parentGuid);
let parentIsLivemark = PlacesUtils.annotations.itemHasAnnotation(parentId,
PlacesUtils.LMANNO_FEEDURI);
if (parentIsLivemark) {
// A livemark can't be a descendant of another livemark.
BookmarkSyncLog.debug(`insertSyncLivemark: Invalid parent ${
insertInfo.parentSyncId}; skipping livemark record ${
@@ -801,18 +806,17 @@ function validateNewBookmark(info) {
, description: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
, BookmarkSyncUtils.KINDS.MICROSUMMARY
, BookmarkSyncUtils.KINDS.QUERY
, BookmarkSyncUtils.KINDS.FOLDER
, BookmarkSyncUtils.KINDS.LIVEMARK ].includes(b.kind) }
, loadInSidebar: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
, BookmarkSyncUtils.KINDS.MICROSUMMARY
, BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
- , feed: { requiredIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK
- , validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
+ , feed: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
, site: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
});
return insertInfo;
}
function findAnnoItems(anno, val) {
let annos = PlacesUtils.annotations;