Bug 1559514 - Emit sync event when up-to-date if JSON dump is loaded r=glasserc

Differential Revision: https://phabricator.services.mozilla.com/D35240

--HG--
extra : moz-landing-system : lando

services/settings/RemoteSettingsClient.jsm

@@ -334,32 +334,6 @@ class RemoteSettingsClient extends EventEmitter {
         }
       }
 
-      // If the data is up to date, there's no need to sync. We still need
-      // to record the fact that a check happened.
-      if (expectedTimestamp <= collectionLastModified) {
-        console.debug(`${this.identifier} local data is up-to-date`);
-        // If the data is up-to-date but don't have metadata (records loaded from dump),
-        // we fetch them and validate the signature immediately.
-        if (this.verifySignature && ObjectUtils.isEmpty(await kintoCollection.metadata())) {
-          console.debug(`${this.identifier} verify signature of local data`);
-          let allData = importedFromDump;
-          if (importedFromDump.length == 0) {
-            // If dump was imported at some other point (eg. `.get()`), list local DB.
-            ({ data: allData } = await kintoCollection.list({ order: "" }));
-          }
-          const localRecords = allData.map(r => kintoCollection.cleanLocalFields(r));
-          const metadata = await kintoCollection.pullMetadata(this.httpClient());
-          if (this.verifySignature) {
-            await this._validateCollectionSignature([],
-                                                    collectionLastModified,
-                                                    metadata,
-                                                    { localRecords });
-          }
-        }
-        reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE;
-        return;
-      }
-
       // If signature verification is enabled, then add a synchronization hook
       // for incoming changes that validates the signature.
       if (this.verifySignature) {
@@ -382,16 +356,48 @@ class RemoteSettingsClient extends EventEmitter {
 
       let syncResult;
       try {
-        // Fetch changes from server, and make sure we overwrite local data.
-        const strategy = Kinto.syncStrategy.SERVER_WINS;
-        syncResult = await kintoCollection.sync({ remote: gServerURL, strategy, expectedTimestamp });
-        if (!syncResult.ok) {
-          // With SERVER_WINS, there cannot be any conflicts, but don't silent it anyway.
-          throw new Error("Synced failed");
+        // Is local timestamp up to date with the server?
+        if (expectedTimestamp <= collectionLastModified) {
+          console.debug(`${this.identifier} local data is up-to-date`);
+          reportStatus = UptakeTelemetry.STATUS.UP_TO_DATE;
+
+          // If the data is up-to-date but don't have metadata (records loaded from dump),
+          // we fetch them and validate the signature immediately.
+          if (this.verifySignature && ObjectUtils.isEmpty(await kintoCollection.metadata())) {
+            console.debug(`${this.identifier} pull collection metadata`);
+            const metadata = await kintoCollection.pullMetadata(this.httpClient());
+            // We don't bother validating the signature if the dump was just loaded. We do
+            // if the dump was loaded at some other point (eg. from .get()).
+            if (this.verifySignature && importedFromDump.length == 0) {
+              console.debug(`${this.identifier} verify signature of local data`);
+              const { data: allData } = await kintoCollection.list({ order: "" });
+              const localRecords = allData.map(r => kintoCollection.cleanLocalFields(r));
+              await this._validateCollectionSignature([],
+                                                      collectionLastModified,
+                                                      metadata,
+                                                      { localRecords });
+            }
+          }
+
+          // Since the data is up-to-date, if we didn't load any dump then we're done here.
+          if (importedFromDump.length == 0) {
+            return;
+          }
+          // Otherwise we want to continue with sending the sync event to notify about the created records.
+          syncResult = { current: importedFromDump, created: importedFromDump, updated: [], deleted: [] };
+        } else {
+          // Local data is outdated.
+          // Fetch changes from server, and make sure we overwrite local data.
+          const strategy = Kinto.syncStrategy.SERVER_WINS;
+          syncResult = await kintoCollection.sync({ remote: gServerURL, strategy, expectedTimestamp });
+          if (!syncResult.ok) {
+            // With SERVER_WINS, there cannot be any conflicts, but don't silent it anyway.
+            throw new Error("Synced failed");
+          }
+          // The records imported from the dump should be considered as "created" for the
+          // listeners.
+          syncResult.created = importedFromDump.concat(syncResult.created);
         }
-        // The records imported from the dump should be considered as "created" for the
-        // listeners.
-        syncResult.created = importedFromDump.concat(syncResult.created);
       } catch (e) {
         if (e instanceof RemoteSettingsClient.InvalidSignatureError) {
           // Signature verification failed during synchronization.

services/settings/test/unit/test_remote_settings.js

@@ -102,6 +102,28 @@ add_task(async function test_records_from_dump_are_listed_as_created_in_event()
 });
 add_task(clear_state);
 
+add_task(async function test_sync_event_is_sent_even_if_up_to_date() {
+  if (IS_ANDROID) {
+    // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
+    return;
+  }
+  const startHistogram = getUptakeTelemetrySnapshot(clientWithDump.identifier);
+  let received;
+  clientWithDump.on("sync", ({ data }) => received = data);
+  // Use a timestamp inferior to latest record in dump.
+  const timestamp = 1000000000000; // Sun Sep 09 2001
+
+  await clientWithDump.maybeSync(timestamp);
+
+  ok(received.current.length > 0, "Dump records are listed as created");
+  equal(received.current.length, received.created.length);
+
+  const endHistogram = getUptakeTelemetrySnapshot(clientWithDump.identifier);
+  const expectedIncrements = { [UptakeTelemetry.STATUS.UP_TO_DATE]: 1 };
+  checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements);
+});
+add_task(clear_state);
+
 add_task(async function test_records_can_have_local_fields() {
   const c = RemoteSettings("with-local-fields", { localFields: [ "accepted" ]});
   c.verifySignature = false;