Bug 1125374 - Let Bookmarks.fetch() support DEFAULT_INDEX r=mak
authorTim Taubert <ttaubert@mozilla.com>
Fri, 23 Jan 2015 23:13:47 +0100
changeset 225684 ce36da2702b9240315da330f7a5b7ec14d8e2773
parent 225683 0f87f9d002c11ff8b42654664d9a35911ff93017
child 225685 3dd8422c861983f0fb21e15d6ac0b85a1aefbff2
push id10996
push userttaubert@mozilla.com
push dateMon, 26 Jan 2015 19:52:22 +0000
treeherderfx-team@ce36da2702b9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1125374
milestone38.0a1
Bug 1125374 - Let Bookmarks.fetch() support DEFAULT_INDEX r=mak
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
@@ -528,17 +528,17 @@ let Bookmarks = Object.freeze({
       throw new Error(`Unexpected number of conditions provided: ${conditionsCount}`);
 
     // Even if we ignore any other unneeded property, we still validate any
     // known property to reduce likelihood of hidden bugs.
     let fetchInfo = validateBookmarkObject(info,
       { parentGuid: { requiredIf: b => b.hasOwnProperty("index") }
       , index: { requiredIf: b => b.hasOwnProperty("parentGuid")
                , validIf: b => typeof(b.index) == "number" &&
-                               b.index >= 0 }
+                               b.index >= 0 || b.index == this.DEFAULT_INDEX }
       , keyword: { validIf: b => typeof(b.keyword) == "string" &&
                                  b.keyword.length > 0 }
       });
 
     return Task.spawn(function* () {
       let results;
       if (fetchInfo.hasOwnProperty("guid"))
         results = yield fetchBookmark(fetchInfo);
@@ -910,29 +910,33 @@ function* fetchBookmark(info) {
      WHERE b.guid = :guid
     `, { guid: info.guid });
 
   return rows.length ? rowsToItemsArray(rows)[0] : null;
 }
 
 function* fetchBookmarkByPosition(info) {
   let db = yield DBConnPromised;
+  let index = info.index == Bookmarks.DEFAULT_INDEX ? null : info.index;
 
   let rows = yield db.executeCached(
     `SELECT b.guid, IFNULL(p.guid, "") AS parentGuid, b.position AS 'index',
             b.dateAdded, b.lastModified, b.type, b.title, h.url AS url,
             keyword, b.id AS _id, b.parent AS _parentId,
             (SELECT count(*) FROM moz_bookmarks WHERE parent = b.id) AS _childCount,
             p.parent AS _grandParentId
      FROM moz_bookmarks b
      LEFT JOIN moz_bookmarks p ON p.id = b.parent
      LEFT JOIN moz_keywords k ON k.id = b.keyword_id
      LEFT JOIN moz_places h ON h.id = b.fk
-     WHERE p.guid = :parentGuid AND b.position = :index
-    `, { parentGuid: info.parentGuid, index: info.index });
+     WHERE p.guid = :parentGuid
+     AND b.position = IFNULL(:index, (SELECT count(*) - 1
+                                      FROM moz_bookmarks
+                                      WHERE parent = p.id))
+    `, { parentGuid: info.parentGuid, index });
 
   return rows.length ? rowsToItemsArray(rows)[0] : null;
 }
 
 function* fetchBookmarksByURL(info) {
   let db = yield DBConnPromised;
 
   let rows = yield db.executeCached(
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
@@ -61,19 +61,16 @@ add_task(function* invalid_input_throws(
                                                     index: "0" }),
                 /Invalid value for property 'index'/);
   Assert.throws(() => PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012",
                                                     index: null }),
                 /Invalid value for property 'index'/);
   Assert.throws(() => PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012",
                                                     index: -10 }),
                 /Invalid value for property 'index'/);
-  Assert.throws(() => PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012",
-                                                    index: -1 }),
-                /Invalid value for property 'index'/);
 
   Assert.throws(() => PlacesUtils.bookmarks.fetch({ url: "http://te st/" }),
                 /Invalid value for property 'url'/);
   Assert.throws(() => PlacesUtils.bookmarks.fetch({ url: null }),
                 /Invalid value for property 'url'/);
   Assert.throws(() => PlacesUtils.bookmarks.fetch({ url: -10 }),
                 /Invalid value for property 'url'/);
 
@@ -236,16 +233,43 @@ add_task(function* fetch_byposition() {
   Assert.equal(bm2.index, 0);
   Assert.deepEqual(bm2.dateAdded, bm2.lastModified);
   Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_BOOKMARK);
   Assert.equal(bm2.url.href, "http://example.com/");
   Assert.equal(bm2.title, "a bookmark");
   Assert.ok(!("keyword" in bm2));
 });
 
+add_task(function* fetch_byposition_default_index() {
+  let bm1 = yield PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+                                                 type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+                                                 url: "http://example.com/last",
+                                                 title: "last child" });
+  checkBookmarkObject(bm1);
+
+  let bm2 = yield PlacesUtils.bookmarks.fetch({ parentGuid: bm1.parentGuid,
+                                                index: PlacesUtils.bookmarks.DEFAULT_INDEX },
+                                              gAccumulator.callback);
+  checkBookmarkObject(bm2);
+  Assert.equal(gAccumulator.results.length, 1);
+  checkBookmarkObject(gAccumulator.results[0]);
+  Assert.deepEqual(gAccumulator.results[0], bm1);
+
+  Assert.deepEqual(bm1, bm2);
+  Assert.equal(bm2.parentGuid, PlacesUtils.bookmarks.unfiledGuid);
+  Assert.equal(bm2.index, 1);
+  Assert.deepEqual(bm2.dateAdded, bm2.lastModified);
+  Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_BOOKMARK);
+  Assert.equal(bm2.url.href, "http://example.com/last");
+  Assert.equal(bm2.title, "last child");
+  Assert.ok(!("keyword" in bm2));
+
+  yield PlacesUtils.bookmarks.remove(bm1.guid);
+});
+
 add_task(function* fetch_byurl_nonexisting() {
   let bm = yield PlacesUtils.bookmarks.fetch({ url: "http://nonexisting.com/" },
                                              gAccumulator.callback);
   Assert.equal(bm, null);
   Assert.equal(gAccumulator.results.length, 0);
 });
 
 add_task(function* fetch_byurl() {