Bug 1342851 - record the last-modified time of a Sync client record. r=rnewman
☠☠ backed out by 3bc19b609193 ☠ ☠
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 27 Feb 2017 12:44:12 +1100
changeset 394219 b8fb83d1d8513246187f3392e1020c7f56e29951
parent 394218 317ab487c1234b8159d81b29fd32a34e8d69823c
child 394220 ccc9257cda4d4d42c77b2ddbae043f587f6039fb
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1342851
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1342851 - record the last-modified time of a Sync client record. r=rnewman MozReview-Commit-ID: 2jtCsl3sy3X
services/sync/modules/engines/clients.js
services/sync/tests/unit/test_clients_engine.js
--- 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.remoteClient(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.remoteClient(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.remoteClient(activeID).name = "New name";
+    engine._modified.set(activeID, 0);
+    engine._uploadOutgoing();
+
+    _("Local record should have updated timestamp");
+    ok(engine.remoteClient(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);