Bug 1342851 - record the last-modified time of a Sync client record. r=rnewman

MozReview-Commit-ID: 2jtCsl3sy3X

--HG--
extra : rebase_source : 13d1324f3be41f8f020c0978b8b1077ce0fdea6a

services/sync/modules/engines/clients.js

@@ -300,8 +300,10 @@ ClientEngine.prototype = {
       // 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;
@@ -325,7 +327,14 @@ ClientEngine.prototype = {
         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) {
@@ -747,7 +756,8 @@ ClientStore.prototype = {
       // 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) {

services/sync/tests/unit/test_clients_engine.js

@@ -366,6 +366,62 @@ add_task(async function test_client_name_change() {
   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.");