Bug 1426554 - Add a `PlacesUtils.metadata` API. r=mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 08 Mar 2018 14:49:01 -0800
changeset 767784 9697c6d6bbc932e4ac73d41f92a7123608d206e2
parent 767783 f9b8c429f7377e1ee8ce8543d07b178b06c01703
push id102689
push userbmo:kit@mozilla.com
push dateThu, 15 Mar 2018 00:41:35 +0000
reviewersmak
bugs1426554
milestone61.0a1
Bug 1426554 - Add a `PlacesUtils.metadata` API. r=mak MozReview-Commit-ID: 5VLjQxg441K
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/PlacesTestUtils.jsm
toolkit/components/places/tests/unit/test_metadata.js
toolkit/components/places/tests/unit/xpcshell.ini
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -1990,16 +1990,121 @@ XPCOMUtils.defineLazyGetter(this, "gAsyn
       connection: PlacesUtils.history.DBConnection,
   }).then(conn => {
     setupDbForShutdown(conn, "PlacesUtils wrapped connection");
     return conn;
   }).catch(Cu.reportError)
 );
 
 /**
+ * The metadata API allows consumers to store simple key-value metadata in
+ * Places. Keys are strings, values can be any type that SQLite supports:
+ * numbers (integers and doubles), Booleans, strings, and blobs. Values are
+ * cached in memory for faster lookups.
+ *
+ * Since some consumers set metadata as part of an existing operation or active
+ * transaction, the API also exposes a `*withConnection` variant for each
+ * method that takes an open database connection.
+ */
+PlacesUtils.metadata = {
+  cache: new Map(),
+
+  /**
+   * Returns the value associated with a metadata key.
+   *
+   * @param  {String} key
+   *         The metadata key to look up.
+   * @return {*}
+   *         The value associated with the key, or `null` if not set.
+   */
+  get(key) {
+    return PlacesUtils.withConnectionWrapper("PlacesUtils.metadata.get",
+      db => this.getWithConnection(db, key));
+  },
+
+  /**
+   * Sets the value for a metadata key.
+   *
+   * @param {String} key
+   *        The metadata key to update.
+   * @param {*}
+   *        The value to associate with the key.
+   */
+  set(key, value) {
+    return PlacesUtils.withConnectionWrapper("PlacesUtils.metadata.set",
+      db => this.setWithConnection(db, key, value));
+  },
+
+  /**
+   * Removes the values for the given metadata keys.
+   *
+   * @param {String...}
+   *        One or more metadata keys to remove.
+   */
+  delete(...keys) {
+    return PlacesUtils.withConnectionWrapper("PlacesUtils.metadata.delete",
+      db => this.deleteWithConnection(db, ...keys));
+  },
+
+  async getWithConnection(db, key) {
+    key = this.canonicalizeKey(key);
+    if (this.cache.has(key)) {
+      return this.cache.get(key);
+    }
+    let rows = await db.executeCached(`
+      SELECT value FROM moz_meta WHERE key = :key`,
+      { key });
+    let value = null;
+    if (rows.length) {
+      let row = rows[0];
+      let rawValue = row.getResultByName("value");
+      // Convert blobs back to `Uint8Array`s.
+      value = row.getTypeOfIndex(0) == row.VALUE_TYPE_BLOB ?
+              new Uint8Array(rawValue) : rawValue;
+    }
+    this.cache.set(key, value);
+    return value;
+  },
+
+  async setWithConnection(db, key, value) {
+    if (value === null) {
+      await this.deleteWithConnection(db, key);
+      return;
+    }
+    key = this.canonicalizeKey(key);
+    await db.executeCached(`
+      REPLACE INTO moz_meta (key, value)
+      VALUES (:key, :value)`,
+      { key, value });
+    this.cache.set(key, value);
+  },
+
+  async deleteWithConnection(db, ...keys) {
+    keys = keys.map(this.canonicalizeKey);
+    if (!keys.length) {
+      return;
+    }
+    await db.execute(`
+      DELETE FROM moz_meta
+      WHERE key IN (${new Array(keys.length).fill("?").join(",")})`,
+      keys);
+    for (let key of keys) {
+      this.cache.delete(key);
+    }
+  },
+
+  canonicalizeKey(key) {
+    if (typeof key != "string" || !/^[a-zA-Z0-9\/]+$/.test(key)) {
+      throw new TypeError("Invalid metadata key: " + key);
+    }
+    return key.toLowerCase();
+  },
+};
+
+/**
  * Keywords management API.
  * Sooner or later these keywords will merge with search aliases, this is an
  * interim API that should then be replaced by a unified one.
  * Keywords are associated with URLs and can have POST data.
  * The relations between URLs and keywords are the following:
  *  - 1 keyword can only point to 1 URL
  *  - 1 URL can have multiple keywords, iff they differ by POST data (included the empty one).
  */
--- a/toolkit/components/places/tests/PlacesTestUtils.jsm
+++ b/toolkit/components/places/tests/PlacesTestUtils.jsm
@@ -374,9 +374,21 @@ var PlacesTestUtils = Object.freeze({
             break;
         }
       }
       results.push(rowValues.join("\t"));
     }
     results.push("\n");
     dump(results.join("\n"));
   },
+
+  /**
+   * Removes all stored metadata.
+   */
+  clearMetadata() {
+    return PlacesUtils.withConnectionWrapper("PlacesTestUtils: clearMetadata",
+      async db => {
+        await db.execute(`DELETE FROM moz_meta`);
+        PlacesUtils.metadata.cache.clear();
+      }
+    );
+  },
 });
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_metadata.js
@@ -0,0 +1,96 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+add_task(async function test_metadata() {
+  await PlacesUtils.metadata.set("test/integer", 123);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/integer"), 123,
+    "Should store new integer value");
+
+  await PlacesUtils.metadata.set("test/double", 123.45);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/double"), 123.45,
+    "Should store new double value");
+  await PlacesUtils.metadata.set("test/double", 567.89);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/double"), 567.89,
+    "Should update existing double value");
+
+  await PlacesUtils.metadata.set("test/boolean", false);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/boolean"), false,
+    "Should store new Boolean value");
+  await PlacesUtils.metadata.set("test/boolean", true);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/boolean"), true,
+    "Should update existing Boolean value");
+
+  await PlacesUtils.metadata.set("test/string", "hi");
+  Assert.equal(await PlacesUtils.metadata.get("test/string"), "hi",
+    "Should store new string value");
+  await PlacesUtils.metadata.cache.clear();
+  Assert.equal(await PlacesUtils.metadata.get("test/string"), "hi",
+    "Should return string value after clearing cache");
+
+  // Values are untyped; it's OK to store a value of a different type for the
+  // same key.
+  await PlacesUtils.metadata.set("test/string", 111);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/string"), 111,
+    "Should replace string with integer");
+  await PlacesUtils.metadata.set("test/string", null);
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/string"), null,
+    "Should clear value when setting to NULL");
+
+  await PlacesUtils.metadata.delete("test/string", "test/boolean");
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/string"), null,
+    "Should delete string value");
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/boolean"), null,
+    "Should delete Boolean value");
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/integer"), 123,
+    "Should keep undeleted integer value");
+
+  await PlacesTestUtils.clearMetadata();
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/integer"), null,
+    "Should clear integer value");
+  Assert.strictEqual(await PlacesUtils.metadata.get("test/double"), null,
+    "Should clear double value");
+});
+
+add_task(async function test_metadata_canonical_keys() {
+  await PlacesUtils.metadata.set("Test/Integer", 123);
+  Assert.strictEqual(await PlacesUtils.metadata.get("tEsT/integer"), 123,
+    "New keys should be case-insensitive");
+  await PlacesUtils.metadata.set("test/integer", 456);
+  Assert.strictEqual(await PlacesUtils.metadata.get("TEST/INTEGER"), 456,
+    "Existing keys should be case-insensitive");
+
+  await Assert.rejects(PlacesUtils.metadata.set("", 123),
+    /Invalid metadata key/, "Should reject empty keys");
+  await Assert.rejects(PlacesUtils.metadata.get(123),
+    /Invalid metadata key/, "Should reject numeric keys");
+  await Assert.rejects(PlacesUtils.metadata.delete(true),
+    /Invalid metadata key/, "Should reject Boolean keys");
+  await Assert.rejects(PlacesUtils.metadata.set({}),
+    /Invalid metadata key/, "Should reject object keys");
+  await Assert.rejects(PlacesUtils.metadata.get(null),
+    /Invalid metadata key/, "Should reject null keys");
+  await Assert.rejects(PlacesUtils.metadata.delete("!@#$"),
+    /Invalid metadata key/, "Should reject keys with invalid characters");
+});
+
+add_task(async function test_metadata_blobs() {
+  let blob = new Uint8Array([1, 2, 3]);
+  await PlacesUtils.metadata.set("test/blob", blob);
+
+  let sameBlob = await PlacesUtils.metadata.get("test/blob");
+  Assert.equal(ChromeUtils.getClassName(sameBlob), "Uint8Array",
+    "Should cache typed array for blob value");
+  deepEqual(sameBlob, blob,
+    "Should store new blob value");
+
+  info("Remove blob from cache");
+  await PlacesUtils.metadata.cache.clear();
+
+  let newBlob = await PlacesUtils.metadata.get("test/blob");
+  Assert.equal(ChromeUtils.getClassName(newBlob), "Uint8Array",
+    "Should inflate blob into typed array");
+  deepEqual(newBlob, blob,
+    "Should return same blob after clearing cache");
+
+  await PlacesTestUtils.clearMetadata();
+});
--- a/toolkit/components/places/tests/unit/xpcshell.ini
+++ b/toolkit/components/places/tests/unit/xpcshell.ini
@@ -85,16 +85,17 @@ skip-if = (os == 'win' && ccov) # Bug 14
 [test_hosts_triggers.js]
 [test_import_mobile_bookmarks.js]
 [test_isPageInDB.js]
 [test_isURIVisited.js]
 [test_isvisited.js]
 [test_keywords.js]
 [test_lastModified.js]
 [test_markpageas.js]
+[test_metadata.js]
 [test_mozIAsyncLivemarks.js]
 [test_multi_queries.js]
 [test_multi_word_tags.js]
 [test_nsINavHistoryViewer.js]
 # Bug 902248: intermittent timeouts on all platforms
 skip-if = true
 [test_null_interfaces.js]
 [test_onItemChanged_tags.js]