Bug 1342851 - record the last-modified time of a Sync client record. r=rnewman
MozReview-Commit-ID: 2jtCsl3sy3X
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -295,18 +295,20 @@ ClientEngine.prototype = {
}
// Bug 1264498: Mobile clients don't remove themselves from the clients
// collection when the user disconnects Sync, so we mark as stale clients
// with the same name that haven't synced in over a week.
// (Note we can't simply delete them, or we re-apply them next sync - see
// bug 1287687)
delete this._incomingClients[this.localID];
let names = new Set([this.localName]);
- for (let id in this._incomingClients) {
+ for (let [id, serverLastModified] of Object.entries(this._incomingClients)) {
let record = this._store._remoteClients[id];
+ // stash the server last-modified time on the record.
+ record.serverLastModified = serverLastModified;
if (!names.has(record.name)) {
names.add(record.name);
continue;
}
let remoteAge = AsyncResource.serverTime - this._incomingClients[id];
if (remoteAge > STALE_CLIENT_REMOTE_AGE) {
this._log.info(`Hiding stale client ${id} with age ${remoteAge}`);
record.stale = true;
@@ -320,17 +322,24 @@ ClientEngine.prototype = {
_uploadOutgoing() {
this._currentlySyncingCommands = this._prepareCommandsForUpload();
const clientWithPendingCommands = Object.keys(this._currentlySyncingCommands);
for (let clientId of clientWithPendingCommands) {
if (this._store._remoteClients[clientId] || this.localID == clientId) {
this._modified.set(clientId, 0);
}
}
+ let updatedIDs = this._modified.ids();
SyncEngine.prototype._uploadOutgoing.call(this);
+ // Record the response time as the server time for each item we uploaded.
+ for (let id of updatedIDs) {
+ if (id != this.localID) {
+ this._store._remoteClients[id].serverLastModified = this.lastSync;
+ }
+ }
},
_onRecordsWritten(succeeded, failed) {
// Reconcile the status of the local records with what we just wrote on the
// server
for (let id of succeeded) {
const commandChanges = this._currentlySyncingCommands[id];
if (id == this.localID) {
@@ -742,17 +751,18 @@ ClientStore.prototype = {
record.os = Services.appinfo.OS; // "Darwin"
record.appPackage = Services.appinfo.ID;
record.application = this.engine.brandName // "Nightly"
// We can't compute these yet.
// record.device = ""; // Bug 1100723
// record.formfactor = ""; // Bug 1100722
} else {
- record.cleartext = this._remoteClients[id];
+ record.cleartext = Object.assign({}, this._remoteClients[id]);
+ delete record.cleartext.serverLastModified; // serverLastModified is a local only attribute.
// Add the commands we have to send
if (commandsChanges && commandsChanges.length) {
const recordCommands = record.cleartext.commands || [];
const newCommands = commandsChanges.filter(command => !hasDupeCommand(recordCommands, command));
record.cleartext.commands = recordCommands.concat(newCommands);
}
--- a/services/sync/tests/unit/test_clients_engine.js
+++ b/services/sync/tests/unit/test_clients_engine.js
@@ -361,16 +361,72 @@ add_task(async function test_client_name
ok(tracker.score > initialScore);
ok(tracker.score >= SCORE_INCREMENT_XLARGE);
Svc.Obs.notify("weave:engine:stop-tracking");
cleanup();
});
+add_task(async function test_last_modified() {
+ _("Ensure that remote records have a sane serverLastModified attribute.");
+
+ let now = Date.now() / 1000;
+ let contents = {
+ meta: {global: {engines: {clients: {version: engine.version,
+ syncID: engine.syncID}}}},
+ clients: {},
+ crypto: {}
+ };
+ let server = serverForUsers({"foo": "password"}, contents);
+ let user = server.user("foo");
+
+ await SyncTestingInfrastructure(server);
+ generateNewKeys(Service.collectionKeys);
+
+ let activeID = Utils.makeGUID();
+ server.insertWBO("foo", "clients", new ServerWBO(activeID, encryptPayload({
+ id: activeID,
+ name: "Active client",
+ type: "desktop",
+ commands: [],
+ version: "48",
+ protocols: ["1.5"],
+ }), now - 10));
+
+ try {
+ let collection = user.collection("clients");
+
+ _("Sync to download the record");
+ engine._sync();
+
+ equal(engine._store._remoteClients[activeID].serverLastModified, now - 10,
+ "last modified in the local record is correctly the server last-modified");
+
+ _("Modify the record and re-upload it");
+ // set a new name to make sure we really did upload.
+ engine._store._remoteClients[activeID].name = "New name";
+ engine._modified.set(activeID, 0);
+ engine._uploadOutgoing();
+
+ _("Local record should have updated timestamp");
+ ok(engine._store._remoteClients[activeID].serverLastModified >= now);
+
+ _("Record on the server should have new name but not serverLastModified");
+ let payload = JSON.parse(JSON.parse(collection.payload(activeID)).ciphertext);
+ equal(payload.name, "New name");
+ equal(payload.serverLastModified, undefined);
+
+ } finally {
+ cleanup();
+ server.deleteCollections("foo");
+ await promiseStopServer(server);
+ }
+});
+
add_task(async function test_send_command() {
_("Verifies _sendCommandToClient puts commands in the outbound queue.");
let store = engine._store;
let tracker = engine._tracker;
let remoteId = Utils.makeGUID();
let rec = new ClientsRec("clients", remoteId);