Bug 1620185 - Remove usage of .openCollection() r=glasserc

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

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

services/common/docs/RemoteSettings.rst

@@ -357,17 +357,17 @@ You can forge a ``payload`` that contains the events attributes as described abo
 Manipulate local data
 ---------------------
 
-A handle on the local collection can be obtained with ``openCollection()``.
+A handle on the underlying database can be obtained through the ``.db`` attribute.
 
 .. code-block:: js
 
-    const collection = await RemoteSettings("a-key").openCollection();
+    const db = await RemoteSettings("a-key").db;
 
 And records can be created manually (as if they were synchronized from the server):
 
 .. code-block:: js
 
-    const record = await collection.create({
+    const record = await db.create({
       id: "a-custom-string-or-uuid",
       domain: "website.com",
       usernameSelector: "#login-account",
@@ -378,13 +378,13 @@ If no timestamp is set, any call to ``.get()`` will trigger the load of initial
 
 .. code-block:: js
 
-    await collection.db.saveLastModified(42);
+    await db.saveLastModified(42);
 
 In order to bypass the potential target filtering of ``RemoteSettings("key").get()``, the low-level listing of records can be obtained with ``collection.list()``:
 
 .. code-block:: js
 
-    const { data: subset } = await collection.list({
+    const { data: subset } = await db.list({
       filters: {
         "property": "value"
       }
@@ -394,9 +394,7 @@ The local data can be flushed with ``clear()``:
 
 .. code-block:: js
 
-    await collection.clear()
-
-For further documentation in collection API, checkout the `kinto.js library <https://kintojs.readthedocs.io/>`_, which is in charge of the IndexedDB interactions behind-the-scenes.
+    await db.clear()
 
 
 Misc

services/settings/RemoteSettingsClient.jsm

@@ -12,6 +12,7 @@ const { XPCOMUtils } = ChromeUtils.import(
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
+  CommonUtils: "resource://services-common/utils.js",
   ClientEnvironmentBase:
     "resource://gre/modules/components-utils/ClientEnvironment.jsm",
   Downloader: "resource://services-settings/Attachments.jsm",
@@ -188,6 +189,87 @@ class AttachmentDownloader extends Downloader {
   }
 }
 
+/**
+ * Database is a tiny wrapper with the objective
+ * of providing major kinto-offline-client collection API.
+ * (with the objective of getting rid of kinto-offline-client)
+ */
+class Database {
+  constructor(identifier) {
+    this._idb = new Kinto.adapters.IDB(identifier, {
+      dbName: DB_NAME,
+      migrateOldData: false,
+    });
+  }
+
+  async list(options) {
+    return this._idb.list(options);
+  }
+
+  async importBulk(toInsert) {
+    return this._idb.importBulk(toInsert);
+  }
+
+  async deleteAll(toDelete) {
+    return this._idb.execute(transaction => {
+      toDelete.forEach(r => {
+        transaction.delete(r.id);
+      });
+    });
+  }
+
+  async getLastModified() {
+    return this._idb.getLastModified();
+  }
+
+  async saveLastModified(remoteTimestamp) {
+    return this._idb.saveLastModified(remoteTimestamp);
+  }
+
+  async getMetadata() {
+    return this._idb.getMetadata();
+  }
+
+  async saveMetadata(metadata) {
+    return this._idb.saveMetadata(metadata);
+  }
+
+  async clear() {
+    await this._idb.clear();
+    await this._idb.saveLastModified(null);
+    await this._idb.saveMetadata(null);
+  }
+
+  async close() {
+    return this._idb.close();
+  }
+
+  /*
+   * Methods used by unit tests.
+   */
+
+  async create(record) {
+    if (!("id" in record)) {
+      record = { ...record, id: CommonUtils.generateUUID() };
+    }
+    return this._idb.execute(transaction => {
+      transaction.create(record);
+    });
+  }
+
+  async update(record) {
+    return this._idb.execute(transaction => {
+      transaction.update(record);
+    });
+  }
+
+  async delete(recordId) {
+    return this._idb.execute(transaction => {
+      transaction.delete(recordId);
+    });
+  }
+}
+
 class RemoteSettingsClient extends EventEmitter {
   static get NetworkOfflineError() {
     return NetworkOfflineError;
@@ -235,12 +317,11 @@ class RemoteSettingsClient extends EventEmitter {
       this.bucketNamePref
     );
 
-    XPCOMUtils.defineLazyGetter(this, "db", () => {
-      return new Kinto.adapters.IDB(this.identifier, {
-        dbName: DB_NAME,
-        migrateOldData: false,
-      });
-    });
+    XPCOMUtils.defineLazyGetter(
+      this,
+      "db",
+      () => new Database(this.identifier)
+    );
 
     XPCOMUtils.defineLazyGetter(
       this,
@@ -287,22 +368,13 @@ class RemoteSettingsClient extends EventEmitter {
     return timestamp;
   }
 
-  /**
-   * Clear local data and metadata.
-   */
-  async clear() {
-    await this.db.clear();
-    await this.db.saveLastModified(null);
-    await this.db.saveMetadata(null);
-    await this.db.close();
-  }
-
   /**
    * Open the underlying Kinto collection, using the appropriate adapter and options.
    */
   async openCollection() {
     // This is inefficient, and not DRY, but one step towards removal of kinto.js.
-    // So far, only unit tests use `.openCollection()`.
+    // So far, only «foreign» unit tests use `.openCollection()`.
+    console.warn("RemoteSettingsClient.openCollection() is deprecated");
     const kinto = new Kinto({
       bucket: this.bucketName,
       adapter: Kinto.adapters.IDB,
@@ -822,9 +894,7 @@ class RemoteSettingsClient extends EventEmitter {
     }
 
     const toDelete = remoteRecords.filter(r => r.deleted);
-    const toInsert = remoteRecords
-      .filter(r => !r.deleted)
-      .map(r => ({ ...r, _status: "synced" }));
+    const toInsert = remoteRecords.filter(r => !r.deleted);
 
     console.debug(
       `${this.identifier} ${toDelete.length} to delete, ${toInsert.length} to insert`
@@ -835,14 +905,10 @@ class RemoteSettingsClient extends EventEmitter {
       // In the retry situation, we fetched all server data,
       // and we clear all local data before applying updates.
       console.debug(`${this.identifier} clear local data`);
-      await this.clear();
+      await this.db.clear();
     } else {
       // Otherwise delete local records for each tombstone.
-      await this.db.execute(transaction => {
-        toDelete.forEach(r => {
-          transaction.delete(r.id);
-        });
-      });
+      await this.db.deleteAll(toDelete);
     }
     // Overwrite all other data.
     await this.db.importBulk(toInsert);

services/settings/Utils.jsm

@@ -95,8 +95,7 @@ var Utils = {
    * @return {bool} Whether it exists or not.
    */
   async hasLocalData(client) {
-    const kintoCol = await client.openCollection();
-    const timestamp = await kintoCol.db.getLastModified();
+    const timestamp = await client.db.getLastModified();
     return timestamp !== null;
   },
 

services/settings/remote-settings.js

@@ -422,7 +422,8 @@ function remoteSettingsFunction() {
         // Delete all potential attachments.
         await client.attachments.deleteAll();
         // Delete local data.
-        await client.clear();
+        await client.db.clear();
+        await client.db.close();
         // Remove status pref.
         Services.prefs.clearUserPref(client.lastCheckTimePref);
       })

services/settings/test/unit/test_attachments_downloader.js

@@ -16,6 +16,7 @@ const { TelemetryTestUtils } = ChromeUtils.import(
 const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
 
 const RECORD = {
+  id: "1f3a0802-648d-11ea-bd79-876a8b69c377",
   attachment: {
     hash: "f41ed47d0f43325c9f089d03415c972ce1d3f1ecab6e4d6260665baf3db3ccee",
     size: 1597,
@@ -202,8 +203,7 @@ add_task(clear_state);
 
 add_task(async function test_delete_all() {
   const client = RemoteSettings("some-collection");
-  const kintoCol = await client.openCollection();
-  await kintoCol.create(RECORD);
+  await client.db.create(RECORD);
   const fileURL = await downloader.download(RECORD);
   const localFilePath = pathFromURL(fileURL);
   Assert.ok(await OS.File.exists(localFilePath));

services/settings/test/unit/test_remote_settings.js

@@ -35,13 +35,11 @@ async function clear_state() {
   clientWithDump.verifySignature = false;
 
   // Clear local DB.
-  const collection = await client.openCollection();
-  await collection.clear();
+  await client.db.clear();
   // Reset event listeners.
   client._listeners.set("sync", []);
 
-  const collectionWithDump = await clientWithDump.openCollection();
-  await collectionWithDump.clear();
+  await clientWithDump.db.clear();
 
   Services.prefs.clearUserPref("services.settings.default_bucket");
 
@@ -211,8 +209,7 @@ add_task(async function test_records_can_have_local_fields() {
 
   await c.maybeSync(2000);
 
-  const col = await c.openCollection();
-  await col.update({
+  await c.db.update({
     id: "c74279ce-fb0a-42a6-ae11-386b567a6119",
     accepted: true,
   });
@@ -223,8 +220,7 @@ add_task(clear_state);
 add_task(
   async function test_records_changes_are_overwritten_by_server_changes() {
     // Create some local conflicting data, and make sure it syncs without error.
-    const collection = await client.openCollection();
-    await collection.create(
+    await client.db.create(
       {
         website: "",
         id: "9d500963-d80e-3a91-6e74-66f3811b99cc",
@@ -326,8 +322,7 @@ add_task(async function test_get_can_verify_signature() {
   ok(calledSignature.endsWith("abcdef"));
 
   // It throws when signature does not verify.
-  const col = await client.openCollection();
-  await col.delete("9d500963-d80e-3a91-6e74-66f3811b99cc", { virtual: false });
+  await client.db.delete("9d500963-d80e-3a91-6e74-66f3811b99cc");
   error = null;
   try {
     await client.get({ verifySignature: true });
@@ -373,7 +368,7 @@ add_task(async function test_get_does_not_verify_signature_if_load_dump() {
 
   // If metadata is missing locally, it is fetched by default (`syncIfEmpty: true`)
   await clientWithDump.get({ verifySignature: true });
-  const metadata = await (await clientWithDump.openCollection()).metadata();
+  const metadata = await clientWithDump.db.getMetadata();
   ok(Object.keys(metadata).length > 0, "metadata was fetched");
   ok(called, "signature was verified for the data that was in dump");
 });
@@ -419,7 +414,7 @@ add_task(
     // Signature verification is disabled (see `clear_state()`), so we don't bother with
     // fetching metadata.
     await clientWithDump.maybeSync(42);
-    let metadata = await (await clientWithDump.openCollection()).metadata();
+    let metadata = await clientWithDump.db.getMetadata();
     ok(!metadata, "metadata was not fetched");
 
     // Synchronize again the collection (up-to-date, since collection last modified still > 42)
@@ -427,7 +422,7 @@ add_task(
     await clientWithDump.maybeSync(42);
 
     // With signature verification, metadata was fetched.
-    metadata = await (await clientWithDump.openCollection()).metadata();
+    metadata = await clientWithDump.db.getMetadata();
     ok(Object.keys(metadata).length > 0, "metadata was fetched");
     ok(called, "signature was verified for the data that was in dump");
 
@@ -645,8 +640,7 @@ add_task(async function test_telemetry_reports_if_application_fails() {
 add_task(clear_state);
 
 add_task(async function test_telemetry_reports_if_sync_fails() {
-  const collection = await client.openCollection();
-  await collection.db.saveLastModified(9999);
+  await client.db.saveLastModified(9999);
 
   const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
 
@@ -661,8 +655,7 @@ add_task(async function test_telemetry_reports_if_sync_fails() {
 add_task(clear_state);
 
 add_task(async function test_telemetry_reports_if_parsing_fails() {
-  const collection = await client.openCollection();
-  await collection.db.saveLastModified(10000);
+  await client.db.saveLastModified(10000);
 
   const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
 
@@ -677,8 +670,7 @@ add_task(async function test_telemetry_reports_if_parsing_fails() {
 add_task(clear_state);
 
 add_task(async function test_telemetry_reports_if_fetching_signature_fails() {
-  const collection = await client.openCollection();
-  await collection.db.saveLastModified(11000);
+  await client.db.saveLastModified(11000);
 
   const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
 

services/settings/test/unit/test_remote_settings_jexl_filters.js

@@ -6,12 +6,11 @@ const { RemoteSettings } = ChromeUtils.import(
 let client;
 
 async function createRecords(records) {
-  const collection = await client.openCollection();
-  await collection.clear();
+  await client.db.clear();
   for (const record of records) {
-    await collection.create(record);
+    await client.db.create(record);
   }
-  await collection.db.saveLastModified(42); // Prevent from loading JSON dump.
+  await client.db.saveLastModified(42); // Prevent from loading JSON dump.
 }
 
 function run_test() {
@@ -39,11 +38,10 @@ add_task(async function test_returns_all_without_target() {
 
 add_task(async function test_filters_can_be_disabled() {
   const c = RemoteSettings("no-jexl", { filterFunc: null });
-  const collection = await c.openCollection();
-  await collection.create({
+  await c.db.create({
     filter_expression: "1 == 2",
   });
-  await collection.db.saveLastModified(42); // Prevent from loading JSON dump.
+  await c.db.saveLastModified(42); // Prevent from loading JSON dump.
 
   const list = await c.get();
   equal(list.length, 1);

services/settings/test/unit/test_remote_settings_signatures.js

@@ -625,14 +625,13 @@ add_task(async function test_check_synchronization_with_signatures() {
   // properly contains created, updated, and deleted records.
   // the local DB contains same id as RECORD2 and a fake record.
   // the final server collection contains RECORD2 and RECORD3
-  const kintoCol = await client.openCollection();
-  await kintoCol.clear();
-  await kintoCol.create(
+  await client.db.clear();
+  await client.db.create(
     { ...RECORD2, last_modified: 1234567890, serialNumber: "abc" },
     { synced: true, useRecordId: true }
   );
   const localId = "0602b1b2-12ab-4d3a-b6fb-593244e7b035";
-  await kintoCol.create({ id: localId }, { synced: true, useRecordId: true });
+  await client.db.create({ id: localId }, { synced: true, useRecordId: true });
 
   let syncData = null;
   client.on("sync", ({ data }) => {