Bug 1794411 - Add option to force a sync in .get() r=nalexander

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

services/settings/RemoteSettingsClient.jsm

@@ -398,6 +398,7 @@ class RemoteSettingsClient extends EventEmitter {
    * @param  {boolean} options.dumpFallback      Fallback to dump data if read of local DB fails (default: `true`).
    * @param  {boolean} options.emptyListFallback Fallback to empty list if no dump data and read of local DB fails (default: `true`).
    * @param  {boolean} options.loadDumpIfNewer   Use dump data if it is newer than local data (default: `true`).
+   * @param  {boolean} options.forceSync         Always synchronize from server before returning results (default: `false`).
    * @param  {boolean} options.syncIfEmpty       Synchronize from server if local data is empty (default: `true`).
    * @param  {boolean} options.verifySignature   Verify the signature of the local data (default: `false`).
    * @return {Promise}
@@ -408,6 +409,7 @@ class RemoteSettingsClient extends EventEmitter {
       order = "", // not sorted by default.
       dumpFallback = true,
       emptyListFallback = true,
+      forceSync = false,
       loadDumpIfNewer = true,
       syncIfEmpty = true,
     } = options;
@@ -416,10 +418,17 @@ class RemoteSettingsClient extends EventEmitter {
     const hasParallelCall = !!this._importingPromise;
     let data;
     try {
-      let lastModified = await this.db.getLastModified();
+      let lastModified = forceSync ? null : await this.db.getLastModified();
       let hasLocalData = lastModified !== null;
 
-      if (syncIfEmpty && !hasLocalData) {
+      if (forceSync) {
+        if (!this._importingPromise) {
+          this._importingPromise = (async () => {
+            await this.sync({ sendEvents: false, trigger: "forced" });
+            return true; // No need to re-verify signature after sync.
+          })();
+        }
+      } else if (syncIfEmpty && !hasLocalData) {
         // .get() was called before we had the chance to synchronize the local database.
         // We'll try to avoid returning an empty list.
         if (!this._importingPromise) {

services/settings/test/unit/test_remote_settings.js

@@ -587,6 +587,22 @@ add_task(
 );
 add_task(clear_state);
 
+add_task(async function test_get_can_force_a_sync() {
+  const step0 = await client.db.getLastModified();
+  await client.get({ forceSync: true });
+  const step1 = await client.db.getLastModified();
+  await client.get();
+  const step2 = await client.db.getLastModified();
+  await client.get({ forceSync: true });
+  const step3 = await client.db.getLastModified();
+
+  equal(step0, null);
+  equal(step1, 3000);
+  equal(step2, 3000);
+  equal(step3, 3001);
+});
+add_task(clear_state);
+
 add_task(async function test_sync_runs_once_only() {
   const backup = Utils.log.warn;
   const messages = [];
@@ -1471,6 +1487,33 @@ wNuvFqc=
         ],
       },
     },
+    "GET:/v1/buckets/main/collections/password-fields/changeset?_expected=1337&_since=%223000%22": {
+      sampleHeaders: [
+        "Access-Control-Allow-Origin: *",
+        "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff",
+        "Content-Type: application/json; charset=UTF-8",
+        "Server: waitress",
+        'Etag: "3001"',
+      ],
+      status: { status: 200, statusText: "OK" },
+      responseBody: {
+        metadata: {
+          signature: {
+            signature: "some-sig",
+            x5u: `http://localhost:${port}/fake-x5u`,
+          },
+        },
+        timestamp: 3001,
+        changes: [
+          {
+            id: "312cc78d-9c1f-4291-a4fa-a1be56f6cc69",
+            last_modified: 3001,
+            website: "https://some-website-2.com",
+            selector: "#webpage[field-pwd]",
+          },
+        ],
+      },
+    },
     "GET:/v1/buckets/main/collections/language-dictionaries/changeset": {
       sampleHeaders: [
         "Access-Control-Allow-Origin: *",

toolkit/components/backgroundtasks/BackgroundTask_message.sys.mjs

@@ -90,14 +90,8 @@ function monkeyPatchRemoteSettingsClient({
   last_modified = new Date().getTime(),
   data = [],
 }) {
-  lazy.RemoteSettingsClient.prototype.sync = async () => {
-    let response = { last_modified };
-    outputInfo({ "RemoteSettingsClient.sync": { response } });
-    return [response];
-  };
-
-  lazy.RemoteSettingsClient.prototype.get = async () => {
-    outputInfo({ "RemoteSettingsClient.get": { response: { data } } });
+  lazy.RemoteSettingsClient.prototype.get = async (options = {}) => {
+    outputInfo({ "RemoteSettingsClient.get": { options, response: { data } } });
     return data;
   };
 }

toolkit/components/backgroundtasks/tests/xpcshell/test_backgroundtask_experiments.js

@@ -372,26 +372,19 @@ add_task(
     let { infoArray, infoMap } = await doMessage({});
 
     Assert.ok(
-      "RemoteSettingsClient.sync" in infoMap,
-      "RemoteSettingsClient.sync was invoked"
+      "RemoteSettingsClient.get" in infoMap,
+      "RemoteSettingsClient.get was invoked"
     );
 
     for (let info of infoArray) {
-      if ("RemoteSettingsClient.sync" in info) {
-        break;
-      }
-
       if ("RemoteSettingsClient.get" in info) {
+        const { options: calledOptions } = info["RemoteSettingsClient.get"];
         Assert.ok(
-          false,
-          "RemoteSettingsClient.sync not invoked before RemoteSettingsClient.get"
+          calledOptions.forceSync,
+          "RemoteSettingsClient.get was first called with `forceSync`"
         );
+        return;
       }
     }
-
-    Assert.ok(
-      true,
-      "RemoteSettingsClient.sync was invoked before any RemoteSettingsClient.get"
-    );
   }
 );

toolkit/components/backgroundtasks/tests/xpcshell/xpcshell.ini

@@ -13,6 +13,7 @@ support-files =
 [test_backgroundtask_deletes_profile.js]
 [test_backgroundtask_exitcodes.js]
 [test_backgroundtask_experiments.js]
+tags = remote-settings
 run-if = buildapp == "browser"
 reason = "ASRouter is Firefox-only."
 [test_backgroundtask_help.js]

toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.jsm

@@ -244,23 +244,9 @@ class _RemoteSettingsExperimentLoader {
     let recipes;
     let loadingError = false;
 
-    if (forceSync) {
-      try {
-        await this.remoteSettingsClient.sync({
-          trigger: "RemoteSettingsExperimentLoader",
-        });
-      } catch (e) {
-        lazy.log.debug(
-          "Error forcing sync of recipes from remote settings.",
-          e
-        );
-        Cu.reportError(e);
-        throw e;
-      }
-    }
-
     try {
       recipes = await this.remoteSettingsClient.get({
+        forceSync,
         // Throw instead of returning an empty list.
         emptyListFallback: false,
       });