Bug 1591790 - Return bookmark paths in fetch. r=Standard8
authorLebar <lebar@internet-mail.org>
Wed, 28 Apr 2021 14:30:28 +0000
changeset 577838 f897c6bb3c38a1d7cc09ac1512d34a64a7cdf75f
parent 577837 6c013650718f4f6f3aed165da700b4e15e2f3d08
child 577839 2801837a06b22ffc82605ea68c0e6382efdc62c5
push id38417
push userimoraru@mozilla.com
push dateThu, 29 Apr 2021 09:26:05 +0000
treeherdermozilla-central@36de91ab59b9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs1591790
milestone90.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 1591790 - Return bookmark paths in fetch. r=Standard8 Differential Revision: https://phabricator.services.mozilla.com/D110786
toolkit/components/places/Bookmarks.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
--- a/toolkit/components/places/Bookmarks.jsm
+++ b/toolkit/components/places/Bookmarks.jsm
@@ -1451,16 +1451,21 @@ var Bookmarks = Object.freeze({
    *        object representing it, as defined above.
    * @param onResult [optional]
    *        Callback invoked for each found bookmark.
    * @param options [optional]
    *        an optional object whose properties describe options for the fetch:
    *         - concurrent: fetches concurrently to any writes, returning results
    *                       faster. On the negative side, it may return stale
    *                       information missing the currently ongoing write.
+   *         - includePath: additionally fetches the path for the bookmarks.
+   *                        This is a potentially expensive operation.  When
+   *                        set to true, the path property is set on results
+   *                        containing an array of {title, guid} objects
+   *                        ordered from root to leaf.
    *
    * @return {Promise} resolved when the fetch is complete.
    * @resolves to an object representing the found item, as described above, or
    *           an array of such objects.  if no item is found, the returned
    *           promise is resolved to null.
    * @rejects if an error happens while fetching.
    * @throws if the arguments are invalid.
    *
@@ -1501,16 +1506,17 @@ var Bookmarks = Object.freeze({
         );
       }
     }
 
     // Create a new options object with just the support properties, because
     // we may augment it and hand it down to other methods.
     options = {
       concurrent: !!options.concurrent,
+      includePath: !!options.includePath,
     };
 
     let behavior = {};
     if (info.hasOwnProperty("parentGuid") || info.hasOwnProperty("index")) {
       behavior = {
         parentGuid: { requiredIf: b => b.hasOwnProperty("index") },
         index: {
           validIf: b =>
@@ -1551,16 +1557,25 @@ var Bookmarks = Object.freeze({
       }
 
       if (!Array.isArray(results)) {
         results = [results];
       }
       // Remove non-enumerable properties.
       results = results.map(r => Object.assign({}, r));
 
+      if (options.includePath) {
+        for (let result of results) {
+          let folderPath = await retrieveFullBookmarkPath(result.parentGuid);
+          if (folderPath) {
+            result.path = folderPath;
+          }
+        }
+      }
+
       // Ideally this should handle an incremental behavior and thus be invoked
       // while we fetch.  Though, the likelihood of 2 or more bookmarks for the
       // same match is very low, so it's not worth the added code complication.
       if (onResult) {
         for (let result of results) {
           try {
             onResult(result);
           } catch (ex) {
@@ -3423,8 +3438,59 @@ function adjustSeparatorsSyncCounter(
  * @param {string} [prefix] a string to prefix to the placeholder.
  * @param {string} [suffix] a string to suffix to the placeholder.
  * @returns {string} placeholders is a string made of question marks and commas,
  *          one per value.
  */
 function sqlBindPlaceholders(values, prefix = "", suffix = "") {
   return new Array(values.length).fill(prefix + "?" + suffix).join(",");
 }
+
+/**
+ * Return the full path, from parent to root folder, of a bookmark.
+ *
+ * @param guid
+ *        The globally unique identifier of the item to determine the full
+ *        bookmark path for.
+ * @param options [optional]
+ *        an optional object whose properties describe options for the query:
+ *         - concurrent:  Queries concurrently to any writes, returning results
+ *                        faster. On the negative side, it may return stale
+ *                        information missing the currently ongoing write.
+ *         - db:          A specific connection to be used.
+ * @return {Promise} resolved when the query is complete.
+ * @resolves to an array of {guid, title} objects that represent the full path
+ *           from parent to root for the passed in bookmark.
+ * @rejects if an error happens while querying.
+ */
+async function retrieveFullBookmarkPath(guid, options = {}) {
+  let query = async function(db) {
+    let rows = await db.executeCached(
+      `WITH RECURSIVE parents(guid, _id, _parent, title) AS
+          (SELECT guid, id AS _id, parent AS _parent,
+                  IFNULL(title, '') AS title
+           FROM moz_bookmarks
+           WHERE guid = :pguid
+           UNION ALL
+           SELECT b.guid, b.id AS _id, b.parent AS _parent,
+                  IFNULL(b.title, '') AS title
+           FROM moz_bookmarks b
+           INNER JOIN parents ON b.id=parents._parent)
+        SELECT * FROM parents WHERE guid != :rootGuid;
+      `,
+      { pguid: guid, rootGuid: PlacesUtils.bookmarks.rootGuid }
+    );
+
+    return rows.reverse().map(r => ({
+      guid: r.getResultByName("guid"),
+      title: r.getResultByName("title"),
+    }));
+  };
+
+  if (options.concurrent) {
+    let db = await PlacesUtils.promiseDBConnection();
+    return query(db);
+  }
+  return PlacesUtils.withConnectionWrapper(
+    "Bookmarks.jsm: retrieveFullBookmarkPath",
+    query
+  );
+}
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
@@ -539,8 +539,39 @@ add_task(async function fetch_by_parent(
   Assert.equal(gAccumulator.results.length, 3);
 
   Assert.equal(bm3.url.href, gAccumulator.results[0].url.href);
   Assert.equal(bm1.url.href, gAccumulator.results[1].url.href);
   Assert.equal(bm2.url.href, gAccumulator.results[2].url.href);
 
   await PlacesUtils.bookmarks.remove(folder1);
 });
+
+add_task(async function fetch_with_bookmark_path() {
+  let folder1 = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    title: "Parent",
+  });
+  checkBookmarkObject(folder1);
+
+  let bm1 = await PlacesUtils.bookmarks.insert({
+    parentGuid: folder1.guid,
+    url: "http://bookmarkpath.example.com/",
+    title: "Child Bookmark",
+  });
+  checkBookmarkObject(bm1);
+
+  let bm2 = await PlacesUtils.bookmarks.fetch(
+    { guid: bm1.guid },
+    gAccumulator.callback,
+    { includePath: true }
+  );
+  checkBookmarkObject(bm2);
+  Assert.equal(gAccumulator.results.length, 1);
+  Assert.equal(bm2.path.length, 2);
+  Assert.equal(bm2.path[0].guid, PlacesUtils.bookmarks.unfiledGuid);
+  Assert.equal(bm2.path[0].title, "unfiled");
+  Assert.equal(bm2.path[1].guid, folder1.guid);
+  Assert.equal(bm2.path[1].title, folder1.title);
+
+  await PlacesUtils.bookmarks.remove(folder1);
+});