mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-11-28 23:31:56 +00:00
Bug 676563 - Add dateAdded field to synced bookmarks r=markh,rnewman
MozReview-Commit-ID: 5dxoTGrypqm --HG-- extra : rebase_source : 94af07afc4c0ae2faccc33247fa8f2df91683cce
This commit is contained in:
Thom Chiovoloni
parent
7e0188b58b
commit
577f26289e
@ -763,6 +763,22 @@ this.SyncEngine = function SyncEngine(name, service) {
|
||||
|
||||
this.loadToFetch();
|
||||
this.loadPreviousFailed();
|
||||
// The set of records needing a weak reupload.
|
||||
// The difference between this and a "normal" reupload is that these records
|
||||
// are only tracked in memory, and if the reupload attempt fails (shutdown,
|
||||
// 412, etc), we abort uploading the "weak" set.
|
||||
//
|
||||
// The rationale here is for the cases where we receive a record from the
|
||||
// server that we know is wrong in some (small) way. For example, the
|
||||
// dateAdded field on bookmarks -- maybe we have a better date, or the server
|
||||
// record is entirely missing the date, etc.
|
||||
//
|
||||
// In these cases, we fix our local copy of the record, and mark it for
|
||||
// weak reupload. A normal ("strong") reupload is problematic here because
|
||||
// in the case of a conflict from the server, there's a window where our
|
||||
// record would be marked as modified more recently than a change that occurs
|
||||
// on another device change, and we lose data from the user.
|
||||
this._needWeakReupload = new Set();
|
||||
}
|
||||
|
||||
// Enumeration to define approaches to handling bad records.
|
||||
@ -1008,6 +1024,7 @@ SyncEngine.prototype = {
|
||||
|
||||
// Keep track of what to delete at the end of sync
|
||||
this._delete = {};
|
||||
this._needWeakReupload.clear();
|
||||
},
|
||||
|
||||
/**
|
||||
@ -1551,15 +1568,15 @@ SyncEngine.prototype = {
|
||||
_uploadOutgoing() {
|
||||
this._log.trace("Uploading local changes to server.");
|
||||
|
||||
// collection we'll upload
|
||||
let up = new Collection(this.engineURL, null, this.service);
|
||||
let modifiedIDs = this._modified.ids();
|
||||
let counts = { failed: 0, sent: 0 };
|
||||
if (modifiedIDs.length) {
|
||||
this._log.trace("Preparing " + modifiedIDs.length +
|
||||
" outgoing records");
|
||||
|
||||
let counts = { sent: modifiedIDs.length, failed: 0 };
|
||||
|
||||
// collection we'll upload
|
||||
let up = new Collection(this.engineURL, null, this.service);
|
||||
counts.sent = modifiedIDs.length;
|
||||
|
||||
let failed = [];
|
||||
let successful = [];
|
||||
@ -1625,6 +1642,7 @@ SyncEngine.prototype = {
|
||||
throw ex;
|
||||
}
|
||||
}
|
||||
this._needWeakReupload.delete(id);
|
||||
if (ok) {
|
||||
let { enqueued, error } = postQueue.enqueue(out);
|
||||
if (!enqueued) {
|
||||
@ -1639,10 +1657,70 @@ SyncEngine.prototype = {
|
||||
this._store._sleep(0);
|
||||
}
|
||||
postQueue.flush(true);
|
||||
}
|
||||
|
||||
if (this._needWeakReupload.size) {
|
||||
try {
|
||||
const { sent, failed } = this._weakReupload(up);
|
||||
counts.sent += sent;
|
||||
counts.failed += failed;
|
||||
} catch (e) {
|
||||
if (Async.isShutdownException(e)) {
|
||||
throw e;
|
||||
}
|
||||
this._log.warn("Weak reupload failed", e);
|
||||
}
|
||||
}
|
||||
if (counts.sent || counts.failed) {
|
||||
Observers.notify("weave:engine:sync:uploaded", counts, this.name);
|
||||
}
|
||||
},
|
||||
|
||||
_weakReupload(collection) {
|
||||
const counts = { sent: 0, failed: 0 };
|
||||
let pendingSent = 0;
|
||||
let postQueue = collection.newPostQueue(this._log, this.lastSync, (resp, batchOngoing = false) => {
|
||||
if (!resp.success) {
|
||||
this._needWeakReupload.clear();
|
||||
this._log.warn("Uploading records (weak) failed: " + resp);
|
||||
resp.failureCode = resp.status == 412 ? ENGINE_BATCH_INTERRUPTED : ENGINE_UPLOAD_FAIL;
|
||||
throw resp;
|
||||
}
|
||||
if (!batchOngoing) {
|
||||
counts.sent += pendingSent;
|
||||
pendingSent = 0;
|
||||
}
|
||||
});
|
||||
|
||||
let pendingWeakReupload = this.buildWeakReuploadMap(this._needWeakReupload);
|
||||
for (let [id, encodedRecord] of pendingWeakReupload) {
|
||||
try {
|
||||
this._log.trace("Outgoing (weak)", encodedRecord);
|
||||
encodedRecord.encrypt(this.service.collectionKeys.keyForCollection(this.name));
|
||||
} catch (ex) {
|
||||
if (Async.isShutdownException(ex)) {
|
||||
throw ex;
|
||||
}
|
||||
this._log.warn(`Failed to encrypt record "${id}" during weak reupload`, ex);
|
||||
++counts.failed;
|
||||
continue;
|
||||
}
|
||||
// Note that general errors (network error, 412, etc.) will throw here.
|
||||
// `enqueued` is only false if the specific item failed to enqueue, but
|
||||
// other items should be/are fine. For example, enqueued will be false if
|
||||
// it is larger than the max post or WBO size.
|
||||
let { enqueued } = postQueue.enqueue(encodedRecord);
|
||||
if (!enqueued) {
|
||||
++counts.failed;
|
||||
} else {
|
||||
++pendingSent;
|
||||
}
|
||||
this._store._sleep(0);
|
||||
}
|
||||
postQueue.flush(true);
|
||||
return counts;
|
||||
},
|
||||
|
||||
_onRecordsWritten(succeeded, failed) {
|
||||
// Implement this method to take specific actions against successfully
|
||||
// uploaded records and failed records.
|
||||
@ -1678,6 +1756,7 @@ SyncEngine.prototype = {
|
||||
},
|
||||
|
||||
_syncCleanup() {
|
||||
this._needWeakReupload.clear();
|
||||
if (!this._modified) {
|
||||
return;
|
||||
}
|
||||
@ -1814,6 +1893,27 @@ SyncEngine.prototype = {
|
||||
this._tracker.addChangedID(id, change);
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Returns a map of (id, unencrypted record) that will be used to perform
|
||||
* the weak reupload. Subclasses may override this to filter out items we
|
||||
* shouldn't upload as part of a weak reupload (items that have changed,
|
||||
* for example).
|
||||
*/
|
||||
buildWeakReuploadMap(idSet) {
|
||||
let result = new Map();
|
||||
for (let id of idSet) {
|
||||
try {
|
||||
result.set(id, this._createRecord(id));
|
||||
} catch (ex) {
|
||||
if (Async.isShutdownException(ex)) {
|
||||
throw ex;
|
||||
}
|
||||
this._log.warn("createRecord failed during weak reupload", ex);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@ -120,11 +120,17 @@ PlacesItem.prototype = {
|
||||
// Converts the record to a Sync bookmark object that can be passed to
|
||||
// `PlacesSyncUtils.bookmarks.{insert, update}`.
|
||||
toSyncBookmark() {
|
||||
return {
|
||||
let result = {
|
||||
kind: this.type,
|
||||
syncId: this.id,
|
||||
parentSyncId: this.parentid,
|
||||
};
|
||||
let dateAdded = PlacesSyncUtils.bookmarks.ratchetTimestampBackwards(
|
||||
this.dateAdded, +this.modified * 1000);
|
||||
if (dateAdded !== undefined) {
|
||||
result.dateAdded = dateAdded;
|
||||
}
|
||||
return result;
|
||||
},
|
||||
|
||||
// Populates the record from a Sync bookmark object returned from
|
||||
@ -132,12 +138,15 @@ PlacesItem.prototype = {
|
||||
fromSyncBookmark(item) {
|
||||
this.parentid = item.parentSyncId;
|
||||
this.parentName = item.parentTitle;
|
||||
if (item.dateAdded) {
|
||||
this.dateAdded = item.dateAdded;
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
Utils.deferGetSet(PlacesItem,
|
||||
"cleartext",
|
||||
["hasDupe", "parentid", "parentName", "type"]);
|
||||
["hasDupe", "parentid", "parentName", "type", "dateAdded"]);
|
||||
|
||||
this.Bookmark = function Bookmark(collection, id, type) {
|
||||
PlacesItem.call(this, collection, id, type || "bookmark");
|
||||
@ -543,6 +552,33 @@ BookmarksEngine.prototype = {
|
||||
return record;
|
||||
},
|
||||
|
||||
buildWeakReuploadMap(idSet) {
|
||||
// We want to avoid uploading records which have changed, since that could
|
||||
// cause an inconsistent state on the server.
|
||||
//
|
||||
// Strictly speaking, it would be correct to just call getChangedIds() after
|
||||
// building the initial weak reupload map, however this is quite slow, since
|
||||
// we might end up doing createRecord() (which runs at least one, and
|
||||
// sometimes multiple database queries) for a potentially large number of
|
||||
// items.
|
||||
//
|
||||
// Since the call to getChangedIds is relatively cheap, we do it once before
|
||||
// building the weakReuploadMap (which is where the calls to createRecord()
|
||||
// occur) as an optimization, and once after for correctness, to handle the
|
||||
// unlikely case that a record was modified while we were building the map.
|
||||
let initialChanges = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.getChangedIds());
|
||||
for (let changed of initialChanges) {
|
||||
idSet.delete(changed);
|
||||
}
|
||||
|
||||
let map = SyncEngine.prototype.buildWeakReuploadMap.call(this, idSet);
|
||||
let changes = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.getChangedIds());
|
||||
for (let id of changes) {
|
||||
map.delete(id);
|
||||
}
|
||||
return map;
|
||||
},
|
||||
|
||||
_findDupe: function _findDupe(item) {
|
||||
this._log.trace("Finding dupe for " + item.id +
|
||||
" (already duped: " + item.hasDupe + ").");
|
||||
@ -666,6 +702,9 @@ BookmarksStore.prototype = {
|
||||
if (item) {
|
||||
this._log.debug(`Created ${item.kind} ${item.syncId} under ${
|
||||
item.parentSyncId}`, item);
|
||||
if (item.dateAdded != record.dateAdded) {
|
||||
this.engine._needWeakReupload.add(item.syncId);
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@ -680,6 +719,9 @@ BookmarksStore.prototype = {
|
||||
if (item) {
|
||||
this._log.debug(`Updated ${item.kind} ${item.syncId} under ${
|
||||
item.parentSyncId}`, item);
|
||||
if (item.dateAdded != record.dateAdded) {
|
||||
this.engine._needWeakReupload.add(item.syncId);
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
|
||||
@ -123,6 +123,7 @@ async function validate(collection, expectedFailures = []) {
|
||||
do_print(JSON.stringify(summary));
|
||||
// print the entire validator output as it has IDs etc.
|
||||
do_print(JSON.stringify(problems, undefined, 2));
|
||||
do_print("Expected: " + JSON.stringify(expectedFailures, undefined, 2));
|
||||
// All server records and the entire bookmark tree.
|
||||
do_print("Server records:\n" + JSON.stringify(collection.payloads(), undefined, 2));
|
||||
let tree = await PlacesUtils.promiseBookmarksTree("", { includeItemIds: true });
|
||||
@ -160,7 +161,7 @@ add_task(async function test_dupe_bookmark() {
|
||||
|
||||
collection.insert(newGUID, encryptPayload(to_apply), Date.now() / 1000 + 500);
|
||||
_("Syncing so new dupe record is processed");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// We should have logically deleted the dupe record.
|
||||
@ -218,7 +219,7 @@ add_task(async function test_dupe_reparented_bookmark() {
|
||||
collection.insert(newGUID, encryptPayload(to_apply), Date.now() / 1000 + 500);
|
||||
|
||||
_("Syncing so new dupe record is processed");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// We should have logically deleted the dupe record.
|
||||
@ -295,7 +296,7 @@ add_task(async function test_dupe_reparented_locally_changed_bookmark() {
|
||||
});
|
||||
|
||||
_("Syncing so new dupe record is processed");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// We should have logically deleted the dupe record.
|
||||
@ -386,7 +387,7 @@ add_task(async function test_dupe_reparented_to_earlier_appearing_parent_bookmar
|
||||
|
||||
|
||||
_("Syncing so new records are processed.");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// Everything should be parented correctly.
|
||||
@ -462,7 +463,7 @@ add_task(async function test_dupe_reparented_to_later_appearing_parent_bookmark(
|
||||
}), Date.now() / 1000 + 500);
|
||||
|
||||
_("Syncing so out-of-order records are processed.");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// The intended parent did end up existing, so it should be parented
|
||||
@ -514,10 +515,11 @@ add_task(async function test_dupe_reparented_to_future_arriving_parent_bookmark(
|
||||
parentName: "Folder 1",
|
||||
parentid: newParentGUID,
|
||||
tags: [],
|
||||
dateAdded: Date.now() - 10000
|
||||
}), Date.now() / 1000 + 500);
|
||||
|
||||
_("Syncing so new dupe record is processed");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// We should have logically deleted the dupe record.
|
||||
@ -557,6 +559,7 @@ add_task(async function test_dupe_reparented_to_future_arriving_parent_bookmark(
|
||||
parentid: folder2_guid,
|
||||
children: [newGUID],
|
||||
tags: [],
|
||||
dateAdded: Date.now() - 10000,
|
||||
}), Date.now() / 1000 + 500);
|
||||
// We also queue an update to "folder 2" that references the new parent.
|
||||
collection.insert(folder2_guid, encryptPayload({
|
||||
@ -567,10 +570,12 @@ add_task(async function test_dupe_reparented_to_future_arriving_parent_bookmark(
|
||||
parentid: "toolbar",
|
||||
children: [newParentGUID],
|
||||
tags: [],
|
||||
dateAdded: Date.now() - 11000,
|
||||
}), Date.now() / 1000 + 500);
|
||||
|
||||
|
||||
_("Syncing so missing parent appears");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
// The intended parent now does exist, so it should have been reparented.
|
||||
@ -626,7 +631,7 @@ add_task(async function test_dupe_empty_folder() {
|
||||
}), Date.now() / 1000 + 500);
|
||||
|
||||
_("Syncing so new dupe records are processed");
|
||||
engine.lastSync = engine.lastSync - 0.01;
|
||||
engine.lastSync = engine.lastSync - 5;
|
||||
engine.sync();
|
||||
|
||||
await validate(collection);
|
||||
|
||||
@ -743,6 +743,165 @@ add_task(async function test_misreconciled_root() {
|
||||
await promiseStopServer(server);
|
||||
});
|
||||
|
||||
add_task(async function test_sync_dateAdded() {
|
||||
await Service.recordManager.clearCache();
|
||||
await PlacesSyncUtils.bookmarks.reset();
|
||||
let engine = new BookmarksEngine(Service);
|
||||
let store = engine._store;
|
||||
let server = serverForFoo(engine);
|
||||
await SyncTestingInfrastructure(server);
|
||||
|
||||
let collection = server.user("foo").collection("bookmarks");
|
||||
|
||||
Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup...
|
||||
|
||||
// Just matters that it's in the past, not how far.
|
||||
let now = Date.now();
|
||||
let oneYearMS = 365 * 24 * 60 * 60 * 1000;
|
||||
|
||||
try {
|
||||
let item1GUID = "abcdefabcdef";
|
||||
let item1 = new Bookmark("bookmarks", item1GUID);
|
||||
item1.bmkUri = "https://example.com";
|
||||
item1.title = "asdf";
|
||||
item1.parentName = "Bookmarks Toolbar";
|
||||
item1.parentid = "toolbar";
|
||||
item1.dateAdded = now - oneYearMS;
|
||||
collection.insert(item1GUID, encryptPayload(item1.cleartext));
|
||||
|
||||
let item2GUID = "aaaaaaaaaaaa";
|
||||
let item2 = new Bookmark("bookmarks", item2GUID);
|
||||
item2.bmkUri = "https://example.com/2";
|
||||
item2.title = "asdf2";
|
||||
item2.parentName = "Bookmarks Toolbar";
|
||||
item2.parentid = "toolbar";
|
||||
item2.dateAdded = now + oneYearMS;
|
||||
const item2LastModified = now / 1000 - 100;
|
||||
collection.insert(item2GUID, encryptPayload(item2.cleartext), item2LastModified);
|
||||
|
||||
let item3GUID = "bbbbbbbbbbbb";
|
||||
let item3 = new Bookmark("bookmarks", item3GUID);
|
||||
item3.bmkUri = "https://example.com/3";
|
||||
item3.title = "asdf3";
|
||||
item3.parentName = "Bookmarks Toolbar";
|
||||
item3.parentid = "toolbar";
|
||||
// no dateAdded
|
||||
collection.insert(item3GUID, encryptPayload(item3.cleartext));
|
||||
|
||||
let item4GUID = "cccccccccccc";
|
||||
let item4 = new Bookmark("bookmarks", item4GUID);
|
||||
item4.bmkUri = "https://example.com/4";
|
||||
item4.title = "asdf4";
|
||||
item4.parentName = "Bookmarks Toolbar";
|
||||
item4.parentid = "toolbar";
|
||||
// no dateAdded, but lastModified in past
|
||||
const item4LastModified = (now - oneYearMS) / 1000;
|
||||
collection.insert(item4GUID, encryptPayload(item4.cleartext), item4LastModified);
|
||||
|
||||
let item5GUID = "dddddddddddd";
|
||||
let item5 = new Bookmark("bookmarks", item5GUID);
|
||||
item5.bmkUri = "https://example.com/5";
|
||||
item5.title = "asdf5";
|
||||
item5.parentName = "Bookmarks Toolbar";
|
||||
item5.parentid = "toolbar";
|
||||
// no dateAdded, lastModified in (near) future.
|
||||
const item5LastModified = (now + 60000) / 1000;
|
||||
collection.insert(item5GUID, encryptPayload(item5.cleartext), item5LastModified);
|
||||
|
||||
let item6GUID = "eeeeeeeeeeee";
|
||||
let item6 = new Bookmark("bookmarks", item6GUID);
|
||||
item6.bmkUri = "https://example.com/6";
|
||||
item6.title = "asdf6";
|
||||
item6.parentName = "Bookmarks Toolbar";
|
||||
item6.parentid = "toolbar";
|
||||
const item6LastModified = (now - oneYearMS) / 1000;
|
||||
collection.insert(item6GUID, encryptPayload(item6.cleartext), item6LastModified);
|
||||
|
||||
let origBuildWeakReuploadMap = engine.buildWeakReuploadMap;
|
||||
engine.buildWeakReuploadMap = set => {
|
||||
let fullMap = origBuildWeakReuploadMap.call(engine, set);
|
||||
fullMap.delete(item6GUID);
|
||||
return fullMap;
|
||||
};
|
||||
|
||||
await sync_engine_and_validate_telem(engine, false);
|
||||
|
||||
let record1 = await store.createRecord(item1GUID);
|
||||
let record2 = await store.createRecord(item2GUID);
|
||||
|
||||
equal(item1.dateAdded, record1.dateAdded, "dateAdded in past should be synced");
|
||||
equal(record2.dateAdded, item2LastModified * 1000, "dateAdded in future should be ignored in favor of last modified");
|
||||
|
||||
let record3 = await store.createRecord(item3GUID);
|
||||
|
||||
ok(record3.dateAdded);
|
||||
// Make sure it's within 24 hours of the right timestamp... This is a little
|
||||
// dodgey but we only really care that it's basically accurate and has the
|
||||
// right day.
|
||||
ok(Math.abs(Date.now() - record3.dateAdded) < 24 * 60 * 60 * 1000);
|
||||
|
||||
let record4 = await store.createRecord(item4GUID);
|
||||
equal(record4.dateAdded, item4LastModified * 1000,
|
||||
"If no dateAdded is provided, lastModified should be used");
|
||||
|
||||
let record5 = await store.createRecord(item5GUID);
|
||||
equal(record5.dateAdded, item5LastModified * 1000,
|
||||
"If no dateAdded is provided, lastModified should be used (even if it's in the future)");
|
||||
|
||||
let item6WBO = JSON.parse(JSON.parse(collection._wbos[item6GUID].payload).ciphertext);
|
||||
ok(!item6WBO.dateAdded,
|
||||
"If we think an item has been modified locally, we don't upload it to the server");
|
||||
|
||||
let record6 = await store.createRecord(item6GUID);
|
||||
equal(record6.dateAdded, item6LastModified * 1000,
|
||||
"We still remember the more accurate dateAdded if we don't upload a record due to local changes");
|
||||
engine.buildWeakReuploadMap = origBuildWeakReuploadMap;
|
||||
|
||||
// Update item2 and try resyncing it.
|
||||
item2.dateAdded = now - 100000;
|
||||
collection.insert(item2GUID, encryptPayload(item2.cleartext), now / 1000 - 50);
|
||||
|
||||
|
||||
// Also, add a local bookmark and make sure it's date added makes it up to the server
|
||||
let bzid = PlacesUtils.bookmarks.insertBookmark(
|
||||
PlacesUtils.bookmarksMenuFolderId, Utils.makeURI("https://bugzilla.mozilla.org/"),
|
||||
PlacesUtils.bookmarks.DEFAULT_INDEX, "Bugzilla");
|
||||
|
||||
let bzguid = await PlacesUtils.promiseItemGuid(bzid);
|
||||
|
||||
|
||||
await sync_engine_and_validate_telem(engine, false);
|
||||
|
||||
let newRecord2 = await store.createRecord(item2GUID);
|
||||
equal(newRecord2.dateAdded, item2.dateAdded, "dateAdded update should work for earlier date");
|
||||
|
||||
let bzWBO = JSON.parse(JSON.parse(collection._wbos[bzguid].payload).ciphertext);
|
||||
ok(bzWBO.dateAdded, "Locally added dateAdded lost");
|
||||
|
||||
let localRecord = await store.createRecord(bzguid);
|
||||
equal(bzWBO.dateAdded, localRecord.dateAdded, "dateAdded should not change during upload");
|
||||
|
||||
item2.dateAdded += 10000;
|
||||
collection.insert(item2GUID, encryptPayload(item2.cleartext), now / 1000 - 10);
|
||||
engine.lastSync = now / 1000 - 20;
|
||||
|
||||
await sync_engine_and_validate_telem(engine, false);
|
||||
|
||||
let newerRecord2 = await store.createRecord(item2GUID);
|
||||
equal(newerRecord2.dateAdded, newRecord2.dateAdded,
|
||||
"dateAdded update should be ignored for later date if we know an earlier one ");
|
||||
|
||||
|
||||
|
||||
} finally {
|
||||
store.wipe();
|
||||
Svc.Prefs.resetBranch("");
|
||||
Service.recordManager.clearCache();
|
||||
await PlacesSyncUtils.bookmarks.reset();
|
||||
await promiseStopServer(server);
|
||||
}
|
||||
});
|
||||
|
||||
function run_test() {
|
||||
initTestLogging("Trace");
|
||||
generateNewKeys(Service.collectionKeys);
|
||||
|
||||
@ -42,6 +42,7 @@ async function resetTracker() {
|
||||
|
||||
async function cleanup() {
|
||||
engine.lastSync = 0;
|
||||
engine._needWeakReupload.clear()
|
||||
store.wipe();
|
||||
await resetTracker();
|
||||
await stopTracking();
|
||||
|
||||
@ -159,9 +159,12 @@ var Bookmarks = Object.freeze({
|
||||
* @throws if the arguments are invalid.
|
||||
*/
|
||||
insert(info) {
|
||||
// Ensure to use the same date for dateAdded and lastModified, even if
|
||||
// dateAdded may be imposed by the caller.
|
||||
let time = (info && info.dateAdded) || new Date();
|
||||
let now = new Date();
|
||||
let addedTime = (info && info.dateAdded) || now;
|
||||
let modTime = addedTime;
|
||||
if (addedTime > now) {
|
||||
modTime = now;
|
||||
}
|
||||
let insertInfo = validateBookmarkObject(info,
|
||||
{ type: { defaultValue: this.TYPE_BOOKMARK }
|
||||
, index: { defaultValue: this.DEFAULT_INDEX }
|
||||
@ -170,12 +173,9 @@ var Bookmarks = Object.freeze({
|
||||
, parentGuid: { required: true }
|
||||
, title: { validIf: b => [ this.TYPE_BOOKMARK
|
||||
, this.TYPE_FOLDER ].includes(b.type) }
|
||||
, dateAdded: { defaultValue: time
|
||||
, validIf: b => !b.lastModified ||
|
||||
b.dateAdded <= b.lastModified }
|
||||
, lastModified: { defaultValue: time,
|
||||
validIf: b => (!b.dateAdded && b.lastModified >= time) ||
|
||||
(b.dateAdded && b.lastModified >= b.dateAdded) }
|
||||
, dateAdded: { defaultValue: addedTime }
|
||||
, lastModified: { defaultValue: modTime,
|
||||
validIf: b => b.lastModified >= now || (b.dateAdded && b.lastModified >= b.dateAdded) }
|
||||
, source: { defaultValue: this.SOURCES.DEFAULT }
|
||||
});
|
||||
|
||||
@ -267,9 +267,6 @@ var Bookmarks = Object.freeze({
|
||||
throw new Error("No bookmarks found for the provided GUID");
|
||||
if (updateInfo.hasOwnProperty("type") && updateInfo.type != item.type)
|
||||
throw new Error("The bookmark type cannot be changed");
|
||||
if (updateInfo.hasOwnProperty("dateAdded") &&
|
||||
updateInfo.dateAdded.getTime() != item.dateAdded.getTime())
|
||||
throw new Error("The bookmark dateAdded cannot be changed");
|
||||
|
||||
// Remove any property that will stay the same.
|
||||
removeSameValueProperties(updateInfo, item);
|
||||
@ -278,13 +275,15 @@ var Bookmarks = Object.freeze({
|
||||
// Remove non-enumerable properties.
|
||||
return Object.assign({}, item);
|
||||
}
|
||||
|
||||
const now = new Date();
|
||||
updateInfo = validateBookmarkObject(updateInfo,
|
||||
{ url: { validIf: () => item.type == this.TYPE_BOOKMARK }
|
||||
, title: { validIf: () => [ this.TYPE_BOOKMARK
|
||||
, this.TYPE_FOLDER ].includes(item.type) }
|
||||
, lastModified: { defaultValue: new Date()
|
||||
, validIf: b => b.lastModified >= item.dateAdded }
|
||||
, lastModified: { defaultValue: now
|
||||
, validIf: b => b.lastModified >= now ||
|
||||
b.lastModified >= (b.dateAdded || item.dateAdded) }
|
||||
, dateAdded: { defaultValue: item.dateAdded }
|
||||
});
|
||||
|
||||
return PlacesUtils.withConnectionWrapper("Bookmarks.jsm: update",
|
||||
@ -842,6 +841,8 @@ function updateBookmark(info, item, newParent) {
|
||||
tuples.set("lastModified", { value: PlacesUtils.toPRTime(info.lastModified) });
|
||||
if (info.hasOwnProperty("title"))
|
||||
tuples.set("title", { value: info.title });
|
||||
if (info.hasOwnProperty("dateAdded"))
|
||||
tuples.set("dateAdded", { value: PlacesUtils.toPRTime(info.dateAdded) });
|
||||
|
||||
yield db.executeTransaction(function* () {
|
||||
let isTagging = item._grandParentId == PlacesUtils.tagsFolderId;
|
||||
@ -899,12 +900,17 @@ function updateBookmark(info, item, newParent) {
|
||||
}
|
||||
|
||||
if (syncChangeDelta) {
|
||||
let isChangingIndex = info.hasOwnProperty("index") &&
|
||||
info.index != item.index;
|
||||
// Sync stores child indices in the parent's record, so we only bump the
|
||||
// item's counter if we're updating at least one more property in
|
||||
// addition to the index and last modified time.
|
||||
let needsSyncChange = isChangingIndex ? tuples.size > 2 : tuples.size > 1;
|
||||
// addition to the index, last modified time, and dateAdded.
|
||||
let sizeThreshold = 1;
|
||||
if (info.hasOwnProperty("index") && info.index != item.index) {
|
||||
++sizeThreshold;
|
||||
}
|
||||
if (tuples.has("dateAdded")) {
|
||||
++sizeThreshold;
|
||||
}
|
||||
let needsSyncChange = tuples.size > sizeThreshold;
|
||||
if (needsSyncChange) {
|
||||
tuples.set("syncChangeDelta", { value: syncChangeDelta
|
||||
, fragment: "syncChangeCounter = syncChangeCounter + :syncChangeDelta" });
|
||||
|
||||
@ -80,6 +80,11 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
|
||||
SYNC_PARENT_ANNO: "sync/parent",
|
||||
SYNC_MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
|
||||
|
||||
// Jan 23, 1993 in milliseconds since 1970. Corresponds roughly to the release
|
||||
// of the original NCSA Mosiac. We can safely assume that any dates before
|
||||
// this time are invalid.
|
||||
EARLIEST_BOOKMARK_TIMESTAMP: Date.UTC(1993, 0, 23),
|
||||
|
||||
KINDS: {
|
||||
BOOKMARK: "bookmark",
|
||||
// Microsummaries were removed from Places in bug 524091. For now, Sync
|
||||
@ -111,6 +116,19 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
|
||||
return ROOT_SYNC_ID_TO_GUID[syncId] || syncId;
|
||||
},
|
||||
|
||||
/**
|
||||
* Resolves to an array of the syncIDs of bookmarks that have a nonzero change
|
||||
* counter
|
||||
*/
|
||||
getChangedIds: Task.async(function* () {
|
||||
let db = yield PlacesUtils.promiseDBConnection();
|
||||
let result = yield db.executeCached(`
|
||||
SELECT guid FROM moz_bookmarks
|
||||
WHERE syncChangeCounter >= 1`);
|
||||
return result.map(row =>
|
||||
BookmarkSyncUtils.guidToSyncId(row.getResultByName("guid")));
|
||||
}),
|
||||
|
||||
/**
|
||||
* Fetches the sync IDs for a folder's children, ordered by their position
|
||||
* within the folder.
|
||||
@ -632,6 +650,8 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
|
||||
* - parentSyncId (all): The sync ID of the item's parent.
|
||||
* - parentTitle (all): The title of the item's parent, used for de-duping.
|
||||
* Omitted for the Places root and parents with empty titles.
|
||||
* - dateAdded (all): Timestamp in milliseconds, when the bookmark was added
|
||||
* or created on a remote device if known.
|
||||
* - title ("bookmark", "folder", "livemark", "query"): The item's title.
|
||||
* Omitted if empty.
|
||||
* - url ("bookmark", "query"): The item's URL.
|
||||
@ -779,6 +799,19 @@ const BookmarkSyncUtils = PlacesSyncUtils.bookmarks = Object.freeze({
|
||||
{ syncChangeDelta, type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
|
||||
url: url.href });
|
||||
},
|
||||
|
||||
/**
|
||||
* Returns `undefined` if no sensible timestamp could be found.
|
||||
* Otherwise, returns the earliest sensible timestamp between `existingMillis`
|
||||
* and `serverMillis`.
|
||||
*/
|
||||
ratchetTimestampBackwards(existingMillis, serverMillis, lowerBound = BookmarkSyncUtils.EARLIEST_BOOKMARK_TIMESTAMP) {
|
||||
const possible = [+existingMillis, +serverMillis].filter(n => !isNaN(n) && n > lowerBound);
|
||||
if (!possible.length) {
|
||||
return undefined;
|
||||
}
|
||||
return Math.min(...possible);
|
||||
}
|
||||
});
|
||||
|
||||
XPCOMUtils.defineLazyGetter(this, "BookmarkSyncLog", () => {
|
||||
@ -1129,6 +1162,16 @@ var updateSyncBookmark = Task.async(function* (updateInfo) {
|
||||
updateInfo.syncId} does not exist`);
|
||||
}
|
||||
|
||||
if (updateInfo.hasOwnProperty("dateAdded")) {
|
||||
let newDateAdded = BookmarkSyncUtils.ratchetTimestampBackwards(
|
||||
oldBookmarkItem.dateAdded, updateInfo.dateAdded);
|
||||
if (!newDateAdded || newDateAdded === oldBookmarkItem.dateAdded) {
|
||||
delete updateInfo.dateAdded;
|
||||
} else {
|
||||
updateInfo.dateAdded = newDateAdded;
|
||||
}
|
||||
}
|
||||
|
||||
let shouldReinsert = false;
|
||||
let oldKind = yield getKindForItem(oldBookmarkItem);
|
||||
if (updateInfo.hasOwnProperty("kind") && updateInfo.kind != oldKind) {
|
||||
@ -1155,6 +1198,9 @@ var updateSyncBookmark = Task.async(function* (updateInfo) {
|
||||
}
|
||||
|
||||
if (shouldReinsert) {
|
||||
if (!updateInfo.hasOwnProperty("dateAdded")) {
|
||||
updateInfo.dateAdded = oldBookmarkItem.dateAdded.getTime();
|
||||
}
|
||||
let newInfo = validateNewBookmark(updateInfo);
|
||||
yield PlacesUtils.bookmarks.remove({
|
||||
guid,
|
||||
@ -1320,6 +1366,7 @@ function validateNewBookmark(info) {
|
||||
, BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
|
||||
, feed: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
|
||||
, site: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
|
||||
, dateAdded: { required: false }
|
||||
});
|
||||
|
||||
return insertInfo;
|
||||
@ -1439,6 +1486,10 @@ var placesBookmarkToSyncBookmark = Task.async(function* (bookmarkItem) {
|
||||
item[prop] = bookmarkItem[prop];
|
||||
break;
|
||||
|
||||
case "dateAdded":
|
||||
item[prop] = new Date(bookmarkItem[prop]).getTime();
|
||||
break;
|
||||
|
||||
// Livemark objects contain additional properties. The feed URL is
|
||||
// required; the site URL is optional.
|
||||
case "feedURI":
|
||||
@ -1477,6 +1528,10 @@ function syncBookmarkToPlacesBookmark(info) {
|
||||
bookmarkInfo.guid = BookmarkSyncUtils.syncIdToGuid(info.syncId);
|
||||
break;
|
||||
|
||||
case "dateAdded":
|
||||
bookmarkInfo.dateAdded = new Date(info.dateAdded);
|
||||
break;
|
||||
|
||||
case "parentSyncId":
|
||||
bookmarkInfo.parentGuid =
|
||||
BookmarkSyncUtils.syncIdToGuid(info.parentSyncId);
|
||||
|
||||
@ -282,6 +282,8 @@ const SYNC_BOOKMARK_VALIDATORS = Object.freeze({
|
||||
keyword: simpleValidateFunc(v => v === null || typeof v == "string"),
|
||||
description: simpleValidateFunc(v => v === null || typeof v == "string"),
|
||||
loadInSidebar: simpleValidateFunc(v => v === true || v === false),
|
||||
dateAdded: simpleValidateFunc(v => typeof v === "number"
|
||||
&& v > PlacesSyncUtils.bookmarks.EARLIEST_BOOKMARK_TIMESTAMP),
|
||||
feed: v => v === null ? v : BOOKMARK_VALIDATORS.url(v),
|
||||
site: v => v === null ? v : BOOKMARK_VALIDATORS.url(v),
|
||||
title: BOOKMARK_VALIDATORS.title,
|
||||
|
||||
@ -42,10 +42,7 @@ add_task(function* invalid_input_throws() {
|
||||
Assert.throws(() => PlacesUtils.bookmarks.insert({ lastModified: Date.now() }),
|
||||
/Invalid value for property 'lastModified'/);
|
||||
let time = new Date();
|
||||
let future = new Date(time + 86400000);
|
||||
Assert.throws(() => PlacesUtils.bookmarks.insert({ dateAdded: future,
|
||||
lastModified: time }),
|
||||
/Invalid value for property 'dateAdded'/);
|
||||
|
||||
let past = new Date(time - 86400000);
|
||||
Assert.throws(() => PlacesUtils.bookmarks.insert({ lastModified: past }),
|
||||
/Invalid value for property 'lastModified'/);
|
||||
|
||||
@ -101,22 +101,6 @@ add_task(function* invalid_properties_for_existing_bookmark() {
|
||||
Assert.ok(/The bookmark type cannot be changed/.test(ex));
|
||||
}
|
||||
|
||||
try {
|
||||
yield PlacesUtils.bookmarks.update({ guid: bm.guid,
|
||||
dateAdded: new Date() });
|
||||
Assert.ok(false, "Should have thrown");
|
||||
} catch (ex) {
|
||||
Assert.ok(/The bookmark dateAdded cannot be changed/.test(ex));
|
||||
}
|
||||
|
||||
try {
|
||||
yield PlacesUtils.bookmarks.update({ guid: bm.guid,
|
||||
dateAdded: new Date() });
|
||||
Assert.ok(false, "Should have thrown");
|
||||
} catch (ex) {
|
||||
Assert.ok(/The bookmark dateAdded cannot be changed/.test(ex));
|
||||
}
|
||||
|
||||
try {
|
||||
yield PlacesUtils.bookmarks.update({ guid: bm.guid,
|
||||
parentGuid: "123456789012",
|
||||
|
||||
@ -1742,6 +1742,7 @@ add_task(function* test_fetch() {
|
||||
description: "Folder description",
|
||||
childSyncIds: [folderBmk.syncId, folderSep.syncId],
|
||||
parentTitle: "Bookmarks Menu",
|
||||
dateAdded: item.dateAdded,
|
||||
title: "",
|
||||
}, "Should include description, children, title, and parent title in folder");
|
||||
}
|
||||
@ -1750,7 +1751,7 @@ add_task(function* test_fetch() {
|
||||
{
|
||||
let item = yield PlacesSyncUtils.bookmarks.fetch(bmk.syncId);
|
||||
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
|
||||
"url", "tags", "description", "loadInSidebar", "parentTitle", "title"].sort(),
|
||||
"url", "tags", "description", "loadInSidebar", "parentTitle", "title", "dateAdded"].sort(),
|
||||
"Should include bookmark-specific properties");
|
||||
equal(item.syncId, bmk.syncId, "Sync ID should match");
|
||||
equal(item.url.href, "https://example.com/", "Should return URL");
|
||||
@ -1766,7 +1767,7 @@ add_task(function* test_fetch() {
|
||||
{
|
||||
let item = yield PlacesSyncUtils.bookmarks.fetch(folderBmk.syncId);
|
||||
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
|
||||
"url", "keyword", "tags", "loadInSidebar", "parentTitle", "title"].sort(),
|
||||
"url", "keyword", "tags", "loadInSidebar", "parentTitle", "title", "dateAdded"].sort(),
|
||||
"Should omit blank bookmark-specific properties");
|
||||
strictEqual(item.loadInSidebar, false, "Should not load bookmark in sidebar");
|
||||
deepEqual(item.tags, [], "Tags should be empty");
|
||||
@ -1785,7 +1786,7 @@ add_task(function* test_fetch() {
|
||||
{
|
||||
let item = yield PlacesSyncUtils.bookmarks.fetch(tagQuery.syncId);
|
||||
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
|
||||
"url", "title", "folder", "parentTitle"].sort(),
|
||||
"url", "title", "folder", "parentTitle", "dateAdded"].sort(),
|
||||
"Should include query-specific properties");
|
||||
equal(item.url.href, `place:type=7&folder=${tagFolderId}`, "Should not rewrite outgoing tag queries");
|
||||
equal(item.folder, "taggy", "Should return tag name for tag queries");
|
||||
@ -1795,7 +1796,7 @@ add_task(function* test_fetch() {
|
||||
{
|
||||
let item = yield PlacesSyncUtils.bookmarks.fetch(smartBmk.syncId);
|
||||
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
|
||||
"url", "title", "query", "parentTitle"].sort(),
|
||||
"url", "title", "query", "parentTitle", "dateAdded"].sort(),
|
||||
"Should include smart bookmark-specific properties");
|
||||
equal(item.query, "BookmarksToolbar", "Should return query name for smart bookmarks");
|
||||
}
|
||||
@ -1821,7 +1822,7 @@ add_task(function* test_fetch_livemark() {
|
||||
do_print("Fetch livemark");
|
||||
let item = yield PlacesSyncUtils.bookmarks.fetch(livemark.guid);
|
||||
deepEqual(Object.keys(item).sort(), ["syncId", "kind", "parentSyncId",
|
||||
"description", "feed", "site", "parentTitle", "title"].sort(),
|
||||
"description", "feed", "site", "parentTitle", "title", "dateAdded"].sort(),
|
||||
"Should include livemark-specific properties");
|
||||
equal(item.description, "Livemark description", "Should return description");
|
||||
equal(item.feed.href, site + "/feed/1", "Should return feed URL");
|
||||
|
||||
Loading…
Reference in New Issue
Block a user