Bug 1640136 - Load main dump when using preview r=Standard8
authorMathieu Leplatre <mathieu@mozilla.com>
Thu, 04 Jun 2020 09:00:31 +0000
changeset 533859 91da551351b59af72d6fc571cfa55bf3dfff3316
parent 533858 06661f12888f6f02682d60b387769d81c35ccf78
child 533860 86c2932e66d3dce463d7047bc63bf7af76221edc
push id37479
push userapavel@mozilla.com
push dateThu, 04 Jun 2020 15:32:20 +0000
treeherdermozilla-central@0d21bdf3fc01 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1640136
milestone79.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 1640136 - Load main dump when using preview r=Standard8 Differential Revision: https://phabricator.services.mozilla.com/D77786
services/settings/RemoteSettingsClient.jsm
services/settings/RemoteSettingsWorker.js
services/settings/test/unit/test_remote_settings.js
services/settings/test/unit/test_remote_settings_worker.js
--- a/services/settings/RemoteSettingsClient.jsm
+++ b/services/settings/RemoteSettingsClient.jsm
@@ -336,25 +336,19 @@ class RemoteSettingsClient extends Event
       syncIfEmpty = true,
     } = options;
     let { verifySignature = false } = options;
 
     if (syncIfEmpty && !(await Utils.hasLocalData(this))) {
       try {
         // .get() was called before we had the chance to synchronize the local database.
         // We'll try to avoid returning an empty list.
-        if (
-          gLoadDump &&
-          (await Utils.hasLocalDump(this.bucketName, this.collectionName))
-        ) {
-          // Since there is a JSON dump, load it as default data.
-          console.debug(`${this.identifier} Local DB is empty, load JSON dump`);
-          await this._importJSONDump();
-        } else {
-          // There is no JSON dump, force a synchronization from the server.
+        const importedFromDump = gLoadDump ? await this._importJSONDump() : -1;
+        if (importedFromDump < 0) {
+          // There is no JSON dump to load, force a synchronization from the server.
           console.debug(
             `${this.identifier} Local DB is empty, pull data from server`
           );
           await this.sync({ loadDump: false });
         }
         // Either from trusted dump, or already done during sync.
         verifySignature = false;
       } catch (e) {
@@ -715,19 +709,24 @@ class RemoteSettingsClient extends Event
 
     return reportStatus;
   }
 
   /**
    * Import the JSON files from services/settings/dump into the local DB.
    */
   async _importJSONDump() {
+    console.info(`${this.identifier} restore dump`);
+
+    // When using the preview bucket, we still want to load the main dump.
+    const bucketName = this.bucketName.replace("-preview", "");
+
     const start = Cu.now() * 1000;
     const result = await RemoteSettingsWorker.importJSONDump(
-      this.bucketName,
+      bucketName,
       this.collectionName
     );
     if (gTimingEnabled) {
       const end = Cu.now() * 1000;
       PerformanceCounters.storeExecutionTime(
         `remotesettings/${this.identifier}`,
         "importJSONDump",
         end - start,
@@ -916,23 +915,20 @@ class RemoteSettingsClient extends Event
         } else if (localTrustworthy) {
           // Signature of data before importing changes was good.
           // If we retry, we will pull and import changes again on top of them.
           // If we retried already, we will restore the local data before
           // throwing.
           await this.db.importBulk(localRecords);
           await this.db.saveLastModified(localTimestamp);
           await this.db.saveMetadata(localMetadata);
-        } else if (
+        } else {
           // The local data was tampered.
           // We retried and signature failed again.
-          // So restore the dump if available.
-          await Utils.hasLocalDump(this.bucketName, this.collectionName)
-        ) {
-          console.info(`${this.identifier} restore dump`);
+          // So restore the dump if available (no-op if no dump)
           await this._importJSONDump();
         }
 
         throw e;
       }
     } else {
       console.warn(`${this.identifier} has signature disabled`);
     }
--- a/services/settings/RemoteSettingsWorker.js
+++ b/services/settings/RemoteSettingsWorker.js
@@ -59,19 +59,24 @@ const Agent = {
   },
 
   /**
    * If present, import the JSON file into the Remote Settings IndexedDB
    * for the specified bucket and collection.
    * (eg. blocklists/certificates, main/onboarding)
    * @param {String} bucket
    * @param {String} collection
+   * @returns {int} Number of records loaded from dump or -1 if no dump found.
    */
   async importJSONDump(bucket, collection) {
     const { data: records } = await loadJSONDump(bucket, collection);
+    if (records === null) {
+      // Return -1 if file is missing.
+      return -1;
+    }
     if (gShutdown) {
       throw new Error("Can't import when we've started shutting down.");
     }
     await importDumpIDB(bucket, collection, records);
     return records.length;
   },
 
   /**
@@ -136,18 +141,18 @@ self.onmessage = event => {
  * @param {String}  collection
  */
 async function loadJSONDump(bucket, collection) {
   const fileURI = `resource://app/defaults/settings/${bucket}/${collection}.json`;
   let response;
   try {
     response = await fetch(fileURI);
   } catch (e) {
-    // Return empty dataset if file is missing.
-    return { data: [] };
+    // Return null if file is missing.
+    return { data: null };
   }
   if (gShutdown) {
     throw new Error("Can't import when we've started shutting down.");
   }
   // Will throw if JSON is invalid.
   return response.json();
 }
 
--- a/services/settings/test/unit/test_remote_settings.js
+++ b/services/settings/test/unit/test_remote_settings.js
@@ -321,16 +321,17 @@ add_task(async function test_get_can_ver
 
   let calledSignature;
   client._verifier = {
     async asyncVerifyContentSignature(serialized, signature) {
       calledSignature = signature;
       return true;
     },
   };
+  client.verifySignature = true;
 
   // No metadata in local DB, but gets pulled and then verifies.
   ok(ObjectUtils.isEmpty(await client.db.getMetadata()), "Metadata is empty");
 
   await client.get({ verifySignature: true });
 
   ok(
     !ObjectUtils.isEmpty(await client.db.getMetadata()),
@@ -800,16 +801,34 @@ add_task(async function test_bucketname_
     "main-preview"
   );
 
   equal(client.bucketName, "main-preview");
 });
 add_task(clear_state);
 
 add_task(
+  async function test_get_loads_default_records_from_a_local_dump_if_preview_collection() {
+    if (IS_ANDROID) {
+      // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
+      return;
+    }
+    Services.prefs.setCharPref(
+      "services.settings.default_bucket",
+      "main-preview"
+    );
+    // When collection has a dump in services/settings/dumps/{bucket}/{collection}.json
+    const data = await clientWithDump.get();
+    notEqual(data.length, 0);
+    // No synchronization happened (responses are not mocked).
+  }
+);
+add_task(clear_state);
+
+add_task(
   async function test_inspect_changes_the_list_when_bucket_pref_is_changed() {
     if (IS_ANDROID) {
       // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
       return;
     }
     // Register a client only listed in -preview...
     RemoteSettings("crash-rate");
 
--- a/services/settings/test/unit/test_remote_settings_worker.js
+++ b/services/settings/test/unit/test_remote_settings_worker.js
@@ -73,16 +73,20 @@ add_task(async function test_throws_erro
     await RemoteSettingsWorker.canonicalStringify(null, 42);
   } catch (e) {
     error = e;
   }
   Assert.equal(error.message.endsWith("records is null"), true);
 });
 
 add_task(async function test_throws_error_if_worker_fails_async() {
+  if (IS_ANDROID) {
+    // Skip test: we don't ship dump, so importJSONDump() is no-op.
+    return;
+  }
   // Delete the Remote Settings database, and try to import a dump.
   // This is not supported, and the error thrown asynchronously in the worker
   // should be reported to the caller.
   await new Promise((resolve, reject) => {
     const request = indexedDB.deleteDatabase("remote-settings");
     request.onsuccess = event => resolve();
     request.onblocked = event => reject(new Error("Cannot delete DB"));
     request.onerror = event => reject(event.target.error);