зеркало из https://github.com/mozilla/pjs.git
Bug 656513: part 2: revamp lazy GUID map handling (_lazyMap). r=philiKON
This commit is contained in:
Richard Newman
Родитель
405156adc7
Коммит
12f1796ffb
|
|
@ -262,11 +262,128 @@ BookmarksEngine.prototype = {
|
|||
}, null);
|
||||
|
||||
// Expose the exception if something inside the batch failed
|
||||
if (batchEx!= null) {
|
||||
if (batchEx != null) {
|
||||
throw batchEx;
|
||||
}
|
||||
},
|
||||
|
||||
_guidMapFailed: false,
|
||||
_buildGUIDMap: function _buildGUIDMap() {
|
||||
let guidMap = {};
|
||||
for (let guid in this._store.getAllIDs()) {
|
||||
// Figure out with which key to store the mapping.
|
||||
let key;
|
||||
let id = this._store.idForGUID(guid);
|
||||
switch (PlacesUtils.bookmarks.getItemType(id)) {
|
||||
case PlacesUtils.bookmarks.TYPE_BOOKMARK:
|
||||
|
||||
// Smart bookmarks map to their annotation value.
|
||||
let queryId;
|
||||
try {
|
||||
queryId = PlacesUtils.annotations.getItemAnnotation(
|
||||
id, SMART_BOOKMARKS_ANNO);
|
||||
} catch(ex) {}
|
||||
|
||||
if (queryId)
|
||||
key = "q" + queryId;
|
||||
else
|
||||
key = "b" + PlacesUtils.bookmarks.getBookmarkURI(id).spec + ":" +
|
||||
PlacesUtils.bookmarks.getItemTitle(id);
|
||||
break;
|
||||
case PlacesUtils.bookmarks.TYPE_FOLDER:
|
||||
key = "f" + PlacesUtils.bookmarks.getItemTitle(id);
|
||||
break;
|
||||
case PlacesUtils.bookmarks.TYPE_SEPARATOR:
|
||||
key = "s" + PlacesUtils.bookmarks.getItemIndex(id);
|
||||
break;
|
||||
default:
|
||||
continue;
|
||||
}
|
||||
|
||||
// The mapping is on a per parent-folder-name basis.
|
||||
let parent = PlacesUtils.bookmarks.getFolderIdForItem(id);
|
||||
if (parent <= 0)
|
||||
continue;
|
||||
|
||||
let parentName = PlacesUtils.bookmarks.getItemTitle(parent);
|
||||
if (guidMap[parentName] == null)
|
||||
guidMap[parentName] = {};
|
||||
|
||||
// If the entry already exists, remember that there are explicit dupes.
|
||||
let entry = new String(guid);
|
||||
entry.hasDupe = guidMap[parentName][key] != null;
|
||||
|
||||
// Remember this item's GUID for its parent-name/key pair.
|
||||
guidMap[parentName][key] = entry;
|
||||
this._log.trace("Mapped: " + [parentName, key, entry, entry.hasDupe]);
|
||||
}
|
||||
|
||||
return guidMap;
|
||||
},
|
||||
|
||||
// Helper function to get a dupe GUID for an item.
|
||||
_mapDupe: function _mapDupe(item) {
|
||||
// Figure out if we have something to key with.
|
||||
let key;
|
||||
let altKey;
|
||||
switch (item.type) {
|
||||
case "query":
|
||||
// Prior to Bug 610501, records didn't carry their Smart Bookmark
|
||||
// anno, so we won't be able to dupe them correctly. This altKey
|
||||
// hack should get them to dupe correctly.
|
||||
if (item.queryId) {
|
||||
key = "q" + item.queryId;
|
||||
altKey = "b" + item.bmkUri + ":" + item.title;
|
||||
break;
|
||||
}
|
||||
// No queryID? Fall through to the regular bookmark case.
|
||||
case "bookmark":
|
||||
case "microsummary":
|
||||
key = "b" + item.bmkUri + ":" + item.title;
|
||||
break;
|
||||
case "folder":
|
||||
case "livemark":
|
||||
key = "f" + item.title;
|
||||
break;
|
||||
case "separator":
|
||||
key = "s" + item.pos;
|
||||
break;
|
||||
default:
|
||||
return;
|
||||
}
|
||||
|
||||
// Figure out if we have a map to use!
|
||||
// This will throw in some circumstances. That's fine.
|
||||
let guidMap = this._guidMap;
|
||||
|
||||
// Give the GUID if we have the matching pair.
|
||||
this._log.trace("Finding mapping: " + item.parentName + ", " + key);
|
||||
let parent = guidMap[item.parentName];
|
||||
|
||||
if (!parent) {
|
||||
this._log.trace("No parent => no dupe.");
|
||||
return undefined;
|
||||
}
|
||||
|
||||
let dupe = parent[key];
|
||||
|
||||
if (dupe) {
|
||||
this._log.trace("Mapped dupe: " + dupe);
|
||||
return dupe;
|
||||
}
|
||||
|
||||
if (altKey) {
|
||||
dupe = parent[altKey];
|
||||
if (dupe) {
|
||||
this._log.trace("Mapped dupe using altKey " + altKey + ": " + dupe);
|
||||
return dupe;
|
||||
}
|
||||
}
|
||||
|
||||
this._log.trace("No dupe found for key " + key + "/" + altKey + ".");
|
||||
return undefined;
|
||||
},
|
||||
|
||||
_syncStartup: function _syncStart() {
|
||||
SyncEngine.prototype._syncStartup.call(this);
|
||||
|
||||
|
|
@ -274,117 +391,19 @@ BookmarksEngine.prototype = {
|
|||
if (this.lastSync == 0)
|
||||
archiveBookmarks();
|
||||
|
||||
// Lazily create a mapping of folder titles and separator positions to GUID
|
||||
this.__defineGetter__("_lazyMap", function() {
|
||||
delete this._lazyMap;
|
||||
|
||||
let lazyMap = {};
|
||||
for (let guid in this._store.getAllIDs()) {
|
||||
// Figure out what key to store the mapping
|
||||
let key;
|
||||
let id = this._store.idForGUID(guid);
|
||||
switch (PlacesUtils.bookmarks.getItemType(id)) {
|
||||
case PlacesUtils.bookmarks.TYPE_BOOKMARK:
|
||||
|
||||
// Smart bookmarks map to their annotation value.
|
||||
let queryId;
|
||||
try {
|
||||
queryId = PlacesUtils.annotations.getItemAnnotation(
|
||||
id, SMART_BOOKMARKS_ANNO);
|
||||
} catch(ex) {}
|
||||
|
||||
if (queryId)
|
||||
key = "q" + queryId;
|
||||
else
|
||||
key = "b" + PlacesUtils.bookmarks.getBookmarkURI(id).spec + ":" +
|
||||
PlacesUtils.bookmarks.getItemTitle(id);
|
||||
break;
|
||||
case PlacesUtils.bookmarks.TYPE_FOLDER:
|
||||
key = "f" + PlacesUtils.bookmarks.getItemTitle(id);
|
||||
break;
|
||||
case PlacesUtils.bookmarks.TYPE_SEPARATOR:
|
||||
key = "s" + PlacesUtils.bookmarks.getItemIndex(id);
|
||||
break;
|
||||
default:
|
||||
continue;
|
||||
}
|
||||
|
||||
// The mapping is on a per parent-folder-name basis
|
||||
let parent = PlacesUtils.bookmarks.getFolderIdForItem(id);
|
||||
if (parent <= 0)
|
||||
continue;
|
||||
|
||||
let parentName = PlacesUtils.bookmarks.getItemTitle(parent);
|
||||
if (lazyMap[parentName] == null)
|
||||
lazyMap[parentName] = {};
|
||||
|
||||
// If the entry already exists, remember that there are explicit dupes
|
||||
let entry = new String(guid);
|
||||
entry.hasDupe = lazyMap[parentName][key] != null;
|
||||
|
||||
// Remember this item's guid for its parent-name/key pair
|
||||
lazyMap[parentName][key] = entry;
|
||||
this._log.trace("Mapped: " + [parentName, key, entry, entry.hasDupe]);
|
||||
this.__defineGetter__("_guidMap", function() {
|
||||
// Create a mapping of folder titles and separator positions to GUID.
|
||||
// We do this lazily so that we don't do any work unless we reconcile
|
||||
// incoming items.
|
||||
try {
|
||||
return this._guidMap = this._buildGUIDMap();
|
||||
} catch (ex) {
|
||||
this._log.warn("Got exception \"" + Utils.exceptionStr(ex) +
|
||||
"\" building GUID map." +
|
||||
" Skipping all other incoming items.");
|
||||
throw {code: Engine.prototype.eEngineAbortApplyIncoming,
|
||||
cause: ex};
|
||||
}
|
||||
|
||||
// Expose a helper function to get a dupe guid for an item
|
||||
return this._lazyMap = function(item) {
|
||||
// Figure out if we have something to key with
|
||||
let key;
|
||||
let altKey;
|
||||
switch (item.type) {
|
||||
case "query":
|
||||
// Prior to Bug 610501, records didn't carry their Smart Bookmark
|
||||
// anno, so we won't be able to dupe them correctly. This altKey
|
||||
// hack should get them to dupe correctly.
|
||||
if (item.queryId) {
|
||||
key = "q" + item.queryId;
|
||||
altKey = "b" + item.bmkUri + ":" + item.title;
|
||||
break;
|
||||
}
|
||||
// No queryID? Fall through to the regular bookmark case.
|
||||
case "bookmark":
|
||||
case "microsummary":
|
||||
key = "b" + item.bmkUri + ":" + item.title;
|
||||
break;
|
||||
case "folder":
|
||||
case "livemark":
|
||||
key = "f" + item.title;
|
||||
break;
|
||||
case "separator":
|
||||
key = "s" + item.pos;
|
||||
break;
|
||||
default:
|
||||
return;
|
||||
}
|
||||
|
||||
// Give the guid if we have the matching pair
|
||||
this._log.trace("Finding mapping: " + item.parentName + ", " + key);
|
||||
let parent = lazyMap[item.parentName];
|
||||
|
||||
if (!parent) {
|
||||
this._log.trace("No parent => no dupe.");
|
||||
return undefined;
|
||||
}
|
||||
|
||||
let dupe = parent[key];
|
||||
|
||||
if (dupe) {
|
||||
this._log.trace("Mapped dupe: " + dupe);
|
||||
return dupe;
|
||||
}
|
||||
|
||||
if (altKey) {
|
||||
dupe = parent[altKey];
|
||||
if (dupe) {
|
||||
this._log.trace("Mapped dupe using altKey " + altKey + ": " + dupe);
|
||||
return dupe;
|
||||
}
|
||||
}
|
||||
|
||||
this._log.trace("No dupe found for key " + key + "/" + altKey + ".");
|
||||
return undefined;
|
||||
};
|
||||
});
|
||||
|
||||
this._store._childrenToOrder = {};
|
||||
|
|
@ -404,14 +423,18 @@ BookmarksEngine.prototype = {
|
|||
|
||||
_syncFinish: function _syncFinish() {
|
||||
SyncEngine.prototype._syncFinish.call(this);
|
||||
delete this._lazyMap;
|
||||
this._tracker._ensureMobileQuery();
|
||||
},
|
||||
|
||||
_syncCleanup: function _syncCleanup() {
|
||||
SyncEngine.prototype._syncCleanup.call(this);
|
||||
delete this._guidMap;
|
||||
},
|
||||
|
||||
_createRecord: function _createRecord(id) {
|
||||
// Create the record like normal but mark it as having dupes if necessary
|
||||
let record = SyncEngine.prototype._createRecord.call(this, id);
|
||||
let entry = this._lazyMap(record);
|
||||
let entry = this._mapDupe(record);
|
||||
if (entry != null && entry.hasDupe)
|
||||
record.hasDupe = true;
|
||||
return record;
|
||||
|
|
@ -421,7 +444,7 @@ BookmarksEngine.prototype = {
|
|||
// Don't bother finding a dupe if the incoming item has duplicates
|
||||
if (item.hasDupe)
|
||||
return;
|
||||
return this._lazyMap(item);
|
||||
return this._mapDupe(item);
|
||||
},
|
||||
|
||||
_handleDupe: function _handleDupe(item, dupeId) {
|
||||
|
|
@ -1020,7 +1043,7 @@ BookmarksStore.prototype = {
|
|||
record = new BookmarkSeparator(collection, id);
|
||||
if (parent > 0)
|
||||
record.parentName = PlacesUtils.bookmarks.getItemTitle(parent);
|
||||
// Create a positioning identifier for the separator, used by _lazyMap
|
||||
// Create a positioning identifier for the separator, used by _mapDupe
|
||||
record.pos = PlacesUtils.bookmarks.getItemIndex(placeId);
|
||||
break;
|
||||
|
||||
|
|
|
|||
|
|
@ -32,17 +32,16 @@ add_test(function bad_record_allIDs() {
|
|||
|
||||
_("Fetching all IDs.");
|
||||
let all = store.getAllIDs();
|
||||
|
||||
|
||||
_("All IDs: " + JSON.stringify(all));
|
||||
do_check_true("menu" in all);
|
||||
do_check_true("toolbar" in all);
|
||||
|
||||
|
||||
_("Clean up.");
|
||||
PlacesUtils.bookmarks.removeItem(badRecordID);
|
||||
run_next_test();
|
||||
});
|
||||
|
||||
|
||||
|
||||
add_test(function test_ID_caching() {
|
||||
let syncTesting = new SyncTestingInfrastructure();
|
||||
|
||||
|
|
@ -170,7 +169,7 @@ add_test(function test_restorePromptsReupload() {
|
|||
Service.clusterURL = "http://localhost:8080/";
|
||||
|
||||
let collection = new ServerCollection({}, true);
|
||||
|
||||
|
||||
let engine = new BookmarksEngine();
|
||||
let store = engine._store;
|
||||
let global = new ServerWBO('global',
|
||||
|
|
@ -262,7 +261,7 @@ add_test(function test_restorePromptsReupload() {
|
|||
|
||||
_("Have the correct number of IDs locally, too.");
|
||||
do_check_eq(count, ["menu", "toolbar", folder1_id, bmk1_id].length);
|
||||
|
||||
|
||||
_("Sync again. This'll wipe bookmarks from the server.");
|
||||
try {
|
||||
engine.sync();
|
||||
|
|
@ -385,6 +384,64 @@ add_test(function test_mismatched_types() {
|
|||
}
|
||||
});
|
||||
|
||||
add_test(function test_bookmark_guidMap_fail() {
|
||||
_("Ensure that failures building the GUID map cause early death.");
|
||||
|
||||
let syncTesting = new SyncTestingInfrastructure();
|
||||
Svc.Prefs.set("clusterURL", "http://localhost:8080/");
|
||||
Svc.Prefs.set("username", "foo");
|
||||
let collection = new ServerCollection();
|
||||
let engine = new BookmarksEngine();
|
||||
let store = engine._store;
|
||||
let global = new ServerWBO('global',
|
||||
{engines: {bookmarks: {version: engine.version,
|
||||
syncID: engine.syncID}}});
|
||||
let server = httpd_setup({
|
||||
"/1.1/foo/storage/meta/global": global.handler(),
|
||||
"/1.1/foo/storage/bookmarks": collection.handler()
|
||||
});
|
||||
|
||||
// Add one item to the server.
|
||||
let itemID = PlacesUtils.bookmarks.createFolder(
|
||||
PlacesUtils.bookmarks.toolbarFolder, "Folder 1", 0);
|
||||
let itemGUID = store.GUIDForId(itemID);
|
||||
let itemPayload = store.createRecord(itemGUID).cleartext;
|
||||
let encPayload = encryptPayload(itemPayload);
|
||||
collection.wbos[itemGUID] = new ServerWBO(itemGUID, encPayload);
|
||||
|
||||
engine.lastSync = 1; // So we don't back up.
|
||||
|
||||
// Make building the GUID map fail.
|
||||
store.getAllIDs = function () { throw "Nooo"; };
|
||||
|
||||
// Ensure that we throw when accessing _guidMap.
|
||||
engine._syncStartup();
|
||||
_("No error.");
|
||||
do_check_false(engine._guidMapFailed);
|
||||
|
||||
_("We get an error if building _guidMap fails in use.");
|
||||
let err;
|
||||
try {
|
||||
_(engine._guidMap);
|
||||
} catch (ex) {
|
||||
err = ex;
|
||||
}
|
||||
do_check_eq(err.code, Engine.prototype.eEngineAbortApplyIncoming);
|
||||
do_check_eq(err.cause, "Nooo");
|
||||
|
||||
_("We get an error and abort during processIncoming.");
|
||||
err = undefined;
|
||||
try {
|
||||
engine._processIncoming();
|
||||
} catch (ex) {
|
||||
err = ex;
|
||||
}
|
||||
do_check_eq(err, "Nooo");
|
||||
|
||||
server.stop(run_next_test);
|
||||
});
|
||||
|
||||
|
||||
function run_test() {
|
||||
initTestLogging("Trace");
|
||||
Log4Moz.repository.getLogger("Sync.Engine.Bookmarks").level = Log4Moz.Level.Trace;
|
||||
|
|
|
|||
|
|
@ -205,11 +205,11 @@ add_test(function test_smart_bookmarks_duped() {
|
|||
try {
|
||||
engine._syncStartup();
|
||||
|
||||
_("Verify that lazyMap uses the anno, discovering a dupe regardless of URI.");
|
||||
do_check_eq(mostVisitedGUID, engine._lazyMap(record));
|
||||
_("Verify that mapDupe uses the anno, discovering a dupe regardless of URI.");
|
||||
do_check_eq(mostVisitedGUID, engine._mapDupe(record));
|
||||
|
||||
record.bmkUri = "http://foo/";
|
||||
do_check_eq(mostVisitedGUID, engine._lazyMap(record));
|
||||
do_check_eq(mostVisitedGUID, engine._mapDupe(record));
|
||||
do_check_neq(PlacesUtils.bookmarks.getBookmarkURI(mostVisitedID).spec,
|
||||
record.bmkUri);
|
||||
|
||||
|
|
@ -224,7 +224,7 @@ add_test(function test_smart_bookmarks_duped() {
|
|||
_("Handle records without a queryId entry.");
|
||||
record.bmkUri = uri;
|
||||
delete record.queryId;
|
||||
do_check_eq(mostVisitedGUID, engine._lazyMap(record));
|
||||
do_check_eq(mostVisitedGUID, engine._mapDupe(record));
|
||||
|
||||
engine._syncFinish();
|
||||
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче