Bug 1047819 - Add an async version of PlacesUtils.getAnnotationsForItem. r?mak draft
authorMark Banner <standard8@mozilla.com>
Thu, 29 Mar 2018 09:57:43 +0100
changeset 774806 d5b7ef502b44948e021bfbbeb5eb4caaad7b510c
parent 774805 a6506c7edd7041b2541fab68771e551cdabb8d52
child 774807 114a5127fbf7cd3afff3e23d791b7d8a17ebe0a9
push id104510
push userbmo:standard8@mozilla.com
push dateThu, 29 Mar 2018 16:11:54 +0000
reviewersmak
bugs1047819
milestone61.0a1
Bug 1047819 - Add an async version of PlacesUtils.getAnnotationsForItem. r?mak MozReview-Commit-ID: yGPd0mhuPt
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/unit/test_PlacesUtils_annotations.js
toolkit/components/places/tests/unit/test_async_transactions.js
toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -1351,17 +1351,17 @@ PT.EditUrl.prototype = Object.seal({
  * Required Input Properties: guid, annotationObject
  */
 PT.Annotate = DefineTransaction(["guids", "annotations"]);
 PT.Annotate.prototype = {
   async execute({ guids, annotations }) {
     let undoAnnosForItemId = new Map();
     for (let guid of guids) {
       let itemId = await PlacesUtils.promiseItemId(guid);
-      let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId);
+      let currentAnnos = await PlacesUtils.promiseAnnotationsForItem(itemId);
 
       let undoAnnos = [];
       for (let newAnno of annotations) {
         let currentAnno = currentAnnos.find(a => a.name == newAnno.name);
         if (currentAnno) {
           undoAnnos.push(currentAnno);
         } else {
           // An unset value removes the annotation.
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -91,16 +91,44 @@ async function notifyKeywordChange(url, 
                                          bookmark.parentId,
                                          bookmark.guid, bookmark.parentGuid,
                                          "", source
                                        ]);
   }
 }
 
 /**
+ * Synchonously fetches all annotations for an item, including all properties of
+ * each annotation which would be required to recreate it.
+ * @note The async version (PlacesUtils.promiseAnnotationsForItem) should be
+ *       used, unless there's absolutely no way to make the caller async.
+ * @param aItemId
+ *        The identifier of the itme for which annotations are to be
+ *        retrieved.
+ * @return Array of objects, each containing the following properties:
+ *         name, flags, expires, mimeType, type, value
+ */
+function getAnnotationsForItem(aItemId) {
+  var annos = [];
+  var annoNames = PlacesUtils.annotations.getItemAnnotationNames(aItemId);
+  for (let name of annoNames) {
+    let value = {}, flags = {}, exp = {}, storageType = {};
+    PlacesUtils.annotations.getItemAnnotationInfo(aItemId, name, value,
+                                                  flags, exp, storageType);
+    annos.push({
+      name,
+      flags: flags.value,
+      expires: exp.value,
+      value: value.value
+    });
+  }
+  return annos;
+}
+
+/**
  * Serializes the given node in JSON format.
  *
  * @param aNode
  *        An nsINavHistoryResultNode
  * @param aIsLivemark
  *        Whether the node represents a livemark.
  */
 function serializeNode(aNode, aIsLivemark) {
@@ -131,17 +159,17 @@ function serializeNode(aNode, aIsLivemar
     let grandParent = aNode.parent && aNode.parent.parent;
     if (grandParent) {
       grandParentId = grandParent.itemId;
     }
 
     data.dateAdded = aNode.dateAdded;
     data.lastModified = aNode.lastModified;
 
-    let annos = PlacesUtils.getAnnotationsForItem(data.id);
+    let annos = getAnnotationsForItem(data.id);
     if (annos.length > 0)
       data.annos = annos;
   }
 
   if (PlacesUtils.nodeIsURI(aNode)) {
     // Check for url validity.
     NetUtil.newURI(aNode.uri);
 
@@ -1172,35 +1200,43 @@ var PlacesUtils = {
     var result = this.history.executeQuery(query, options);
     result.root.containerOpen = true;
     return result;
   },
 
   /**
    * Fetch all annotations for an item, including all properties of each
    * annotation which would be required to recreate it.
-   * @param aItemId
+   * @param itemId
    *        The identifier of the itme for which annotations are to be
    *        retrieved.
    * @return Array of objects, each containing the following properties:
    *         name, flags, expires, mimeType, type, value
    */
-  getAnnotationsForItem: function PU_getAnnotationsForItem(aItemId) {
-    var annosvc = this.annotations;
-    var annos = [];
-    var annoNames = annosvc.getItemAnnotationNames(aItemId);
-    for (var i = 0; i < annoNames.length; i++) {
-      let value = {}, flags = {}, exp = {}, storageType = {};
-      annosvc.getItemAnnotationInfo(aItemId, annoNames[i], value, flags, exp, storageType);
-      annos.push({name: annoNames[i],
-                  flags: flags.value,
-                  expires: exp.value,
-                  value: value.value});
+  async promiseAnnotationsForItem(itemId) {
+    let db =  await PlacesUtils.promiseDBConnection();
+    let rows = await db.executeCached(
+      `SELECT n.name, a.content, a.expiration, a.flags
+       FROM moz_items_annos a
+       JOIN moz_anno_attributes n ON a.anno_attribute_id = n.id
+       WHERE a.item_id = :itemId
+      `, { itemId });
+
+    let result = [];
+    for (let row of rows) {
+      let anno = {
+        name: row.getResultByName("name"),
+        value: row.getResultByName("content"),
+        expires: row.getResultByName("expiration"),
+        flags: row.getResultByName("flags"),
+      };
+      result.push(anno);
     }
-    return annos;
+
+    return result;
   },
 
   /**
    * Annotate an item with a batch of annotations.
    * @param aItemId
    *        The identifier of the item for which annotations are to be set
    * @param aAnnotations
    *        Array of objects, each containing the following properties:
@@ -1704,17 +1740,17 @@ var PlacesUtils = {
       let type = aRow.getResultByName("type");
       item.typeCode = type;
       if (type == Ci.nsINavBookmarksService.TYPE_BOOKMARK)
         copyProps("charset", "tags", "iconuri");
 
       // Add annotations.
       if (aRow.getResultByName("has_annos")) {
         try {
-          item.annos = PlacesUtils.getAnnotationsForItem(itemId);
+          item.annos = await PlacesUtils.promiseAnnotationsForItem(itemId);
         } catch (ex) {
           Cu.reportError("Unexpected error while reading annotations " + ex);
         }
       }
 
       switch (type) {
         case Ci.nsINavBookmarksService.TYPE_BOOKMARK:
           item.type = PlacesUtils.TYPE_X_MOZ_PLACE;
--- a/toolkit/components/places/tests/unit/test_PlacesUtils_annotations.js
+++ b/toolkit/components/places/tests/unit/test_PlacesUtils_annotations.js
@@ -40,12 +40,12 @@ add_task(async function test_getAnnotati
       url: "http://example.com/2",
       annos: TEST_ANNOTATIONS
     }],
   });
 
   let ids = await PlacesUtils.promiseManyItemIds(bms.map(bm => bm.guid));
 
   for (let i = 0; i < bms.length; i++) {
-    let annotations = PlacesUtils.getAnnotationsForItem(ids.get(bms[i].guid));
+    let annotations = await PlacesUtils.promiseAnnotationsForItem(ids.get(bms[i].guid));
     checkAnnotations(annotations, TEST_ANNOTATIONS.slice(0, i));
   }
 });
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -1471,51 +1471,51 @@ add_task(async function test_annotate_mu
     this.flags = 0;
     this.expires = Ci.nsIAnnotationService.EXPIRE_NEVER;
   }
 
   function annos(a = null, b = null) {
     return [new AnnoObj("A", a), new AnnoObj("B", b)];
   }
 
-  function verifyAnnoValues(a = null, b = null) {
-    let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId);
+  async function verifyAnnoValues(a = null, b = null) {
+    let currentAnnos = await PlacesUtils.promiseAnnotationsForItem(itemId);
     let expectedAnnos = [];
     if (a !== null)
       expectedAnnos.push(new AnnoObj("A", a));
     if (b !== null)
       expectedAnnos.push(new AnnoObj("B", b));
 
     Assert.deepEqual(currentAnnos, expectedAnnos);
   }
 
   await PT.Annotate({ guid, annotations: annos(1, 2) }).transact();
-  verifyAnnoValues(1, 2);
+  await verifyAnnoValues(1, 2);
   await PT.undo();
-  verifyAnnoValues();
+  await verifyAnnoValues();
   await PT.redo();
-  verifyAnnoValues(1, 2);
+  await verifyAnnoValues(1, 2);
 
   await PT.Annotate({ guid,
                       annotation: { name: "A" } }).transact();
-  verifyAnnoValues(null, 2);
+  await verifyAnnoValues(null, 2);
 
   await PT.Annotate({ guid,
                       annotation: { name: "B", value: 0 } }).transact();
-  verifyAnnoValues(null, 0);
+  await verifyAnnoValues(null, 0);
   await PT.undo();
-  verifyAnnoValues(null, 2);
+  await verifyAnnoValues(null, 2);
   await PT.redo();
-  verifyAnnoValues(null, 0);
+  await verifyAnnoValues(null, 0);
   await PT.undo();
-  verifyAnnoValues(null, 2);
+  await verifyAnnoValues(null, 2);
   await PT.undo();
-  verifyAnnoValues(1, 2);
+  await verifyAnnoValues(1, 2);
   await PT.undo();
-  verifyAnnoValues();
+  await verifyAnnoValues();
 
   // Cleanup
   await PT.undo();
   observer.reset();
 });
 
 add_task(async function test_sort_folder_by_name() {
   let folder_info = createTestFolderInfo();
--- a/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
+++ b/toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
@@ -40,17 +40,17 @@ async function compareToNode(aItem, aNod
 
   if (aIsRootItem && aNode.itemId != PlacesUtils.placesRootId) {
     Assert.ok("parentGuid" in aItem);
     await check_has_child(aItem.parentGuid, aItem.guid);
   } else {
     check_unset("parentGuid");
   }
 
-  let expectedAnnos = PlacesUtils.getAnnotationsForItem(aItem.id);
+  let expectedAnnos = await PlacesUtils.promiseAnnotationsForItem(aItem.id);
   if (expectedAnnos.length > 0)
     Assert.deepEqual(aItem.annos, expectedAnnos);
   else
     check_unset("annos");
 
   const BOOKMARK_ONLY_PROPS = ["uri", "iconuri", "tags", "charset", "keyword"];
   const FOLDER_ONLY_PROPS = ["children", "root"];