Bug 1081108 - Implement reorder in Bookmarks.jsm. r=Mano

--HG--
extra : rebase_source : d9327202388828855c8579646ace390887eadaf6
This commit is contained in:
Marco Bonardo 2014-12-30 21:33:14 +01:00
parent 2f2112c2a0
commit 7b98d880fd
4 changed files with 283 additions and 4 deletions

View File

@ -654,14 +654,39 @@ let Bookmarks = Object.freeze({
* @rejects if an error happens while reordering.
* @throws if the arguments are invalid.
*/
// TODO must implement these methods yet:
// void setItemIndex(in long long aItemId, in long aNewIndex);
reorder(parentGuid, orderedChildrenGuids) {
throw new Error("Not yet implemented");
let info = { guid: parentGuid };
info = validateBookmarkObject(info, { guid: { required: true } });
if (!Array.isArray(orderedChildrenGuids) || !orderedChildrenGuids.length)
throw new Error("Must provide a sorted array of children GUIDs.");
try {
orderedChildrenGuids.forEach(VALIDATORS.guid);
} catch (ex) {
throw new Error("Invalid GUID found in the sorted children array.");
}
return Task.spawn(function* () {
let parent = yield fetchBookmark(info);
if (!parent || parent.type != this.TYPE_FOLDER)
throw new Error("No folder found for the provided GUID.");
let sortedChildren = yield reorderChildren(parent, orderedChildrenGuids);
let observers = PlacesUtils.bookmarks.getObservers();
// Note that child.index is the old index.
for (let i = 0; i < sortedChildren.length; ++i) {
let child = sortedChildren[i];
notify(observers, "onItemMoved", [ child._id, child._parentId,
child.index, child._parentId,
i, child.type,
child.guid, child.parentGuid,
child.parentGuid ]);
}
}.bind(this));
}
});
////////////////////////////////////////////////////////////////////////////////
// Globals.
@ -949,6 +974,26 @@ function* fetchBookmarksByKeyword(info) {
return rows.length ? rowsToItemsArray(rows) : null;
}
function* fetchBookmarksByParent(info) {
let db = yield DBConnPromised;
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
ORDER BY b.position ASC
`, { parentGuid: info.parentGuid });
return rowsToItemsArray(rows);
}
////////////////////////////////////////////////////////////////////////////////
// Remove implementation.
@ -996,6 +1041,77 @@ function* removeBookmark(item) {
return item;
}
////////////////////////////////////////////////////////////////////////////////
// Reorder implementation.
function* reorderChildren(parent, orderedChildrenGuids) {
let db = yield DBConnPromised;
return db.executeTransaction(function* () {
// Select all of the direct children for the given parent.
let children = yield fetchBookmarksByParent({ parentGuid: parent.guid });
if (!children.length)
return;
// Reorder the children array according to the specified order, provided
// GUIDs come first, others are appended in somehow random order.
children.sort((a, b) => {
let i = orderedChildrenGuids.indexOf(a.guid);
let j = orderedChildrenGuids.indexOf(b.guid);
// This works provided fetchBookmarksByParent returns sorted children.
return (i == -1 && j == -1) ? 0 :
(i != -1 && j != -1 && i < j) || (i != -1 && j == -1) ? -1 : 1;
});
// Update the bookmarks position now. If any unknown guid have been
// inserted meanwhile, its position will be set to -position, and we'll
// handle it later.
// To do the update in a single step, we build a VALUES (guid, position)
// table. We then use count() in the sorting table to avoid skipping values
// when no more existing GUIDs have been provided.
let valuesTable = children.map((child, i) => `("${child.guid}", ${i})`)
.join();
yield db.execute(
`WITH sorting(g, p) AS (
VALUES ${valuesTable}
)
UPDATE moz_bookmarks SET position = (
SELECT CASE count(a.g) WHEN 0 THEN -position
ELSE count(a.g) - 1
END
FROM sorting a
JOIN sorting b ON b.p <= a.p
WHERE a.g = guid
AND parent = :parentId
)`, { parentId: parent._id});
// Update position of items that could have been inserted in the meanwhile.
// Since this can happen rarely and it's only done for schema coherence
// resonds, we won't notify about these changes.
yield db.executeCached(
`CREATE TEMP TRIGGER moz_bookmarks_reorder_trigger
AFTER UPDATE OF position ON moz_bookmarks
WHEN NEW.position = -1
BEGIN
UPDATE moz_bookmarks
SET position = (SELECT MAX(position) FROM moz_bookmarks
WHERE parent = NEW.parent) +
(SELECT count(*) FROM moz_bookmarks
WHERE parent = NEW.parent
AND position BETWEEN OLD.position AND -1)
WHERE guid = NEW.guid;
END
`);
yield db.executeCached(
`UPDATE moz_bookmarks SET position = -1 WHERE position < 0`);
yield db.executeCached(`DROP TRIGGER moz_bookmarks_reorder_trigger`);
return children;
}.bind(this));
}
////////////////////////////////////////////////////////////////////////////////
// Helpers.

View File

@ -414,6 +414,58 @@ add_task(function* eraseEverything_notification() {
]);
});
add_task(function* reorder_notification() {
let bookmarks = [
{ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example1.com/",
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example2.com/",
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example3.com/",
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
];
let sorted = [];
for (let bm of bookmarks){
sorted.push(yield PlacesUtils.bookmarks.insert(bm));
}
// Randomly reorder the array.
sorted.sort(() => 0.5 - Math.random());
let observer = expectNotifications();
yield PlacesUtils.bookmarks.reorder(PlacesUtils.bookmarks.unfiledGuid,
sorted.map(bm => bm.guid));
let expectedNotifications = [];
for (let i = 0; i < sorted.length; ++i) {
let child = sorted[i];
let childId = yield PlacesUtils.promiseItemId(child.guid);
expectedNotifications.push({ name: "onItemMoved",
arguments: [ childId,
PlacesUtils.unfiledBookmarksFolderId,
child.index,
PlacesUtils.unfiledBookmarksFolderId,
i,
child.type,
child.guid,
child.parentGuid,
child.parentGuid
] });
}
observer.check(expectedNotifications);
});
function expectNotifications() {
let notifications = [];
let observer = new Proxy(NavBookmarkObserver, {

View File

@ -0,0 +1,110 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
add_task(function* invalid_input_throws() {
Assert.throws(() => PlacesUtils.bookmarks.reorder(),
/Invalid value for property 'guid'/);
Assert.throws(() => PlacesUtils.bookmarks.reorder(null),
/Invalid value for property 'guid'/);
Assert.throws(() => PlacesUtils.bookmarks.reorder("test"),
/Invalid value for property 'guid'/);
Assert.throws(() => PlacesUtils.bookmarks.reorder(123),
/Invalid value for property 'guid'/);
Assert.throws(() => PlacesUtils.bookmarks.reorder({ guid: "test" }),
/Invalid value for property 'guid'/);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012"),
/Must provide a sorted array of children GUIDs./);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", {}),
/Must provide a sorted array of children GUIDs./);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", null),
/Must provide a sorted array of children GUIDs./);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", []),
/Must provide a sorted array of children GUIDs./);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", [ null ]),
/Invalid GUID found in the sorted children array/);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", [ "" ]),
/Invalid GUID found in the sorted children array/);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", [ {} ]),
/Invalid GUID found in the sorted children array/);
Assert.throws(() => PlacesUtils.bookmarks.reorder("123456789012", [ "012345678901" , null ]),
/Invalid GUID found in the sorted children array/);
});
add_task(function* reorder_nonexistent_guid() {
yield Assert.rejects(PlacesUtils.bookmarks.reorder("123456789012", [ "012345678901" ]),
/No folder found for the provided GUID/,
"Should throw for nonexisting guid");
});
add_task(function* reorder() {
let bookmarks = [
{ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example1.com/",
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example2.com/",
parentGuid: PlacesUtils.bookmarks.unfiledGuid
},
{ type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "http://example3.com/",
parentGuid: PlacesUtils.bookmarks.unfiledGuid
}
];
let sorted = [for (bm of bookmarks) yield PlacesUtils.bookmarks.insert(bm)]
// Check the initial append sorting.
Assert.ok(sorted.every((bm, i) => bm.index == i),
"Initial bookmarks sorting is correct");
// Apply random sorting and run multiple tests.
for (let t = 0; t < 4; t++) {
sorted.sort(() => 0.5 - Math.random());
let sortedGuids = sorted.map(child => child.guid);
dump("Expected order: " + sortedGuids.join() + "\n");
// Add a nonexisting guid to the array, to ensure nothing will break.
sortedGuids.push("123456789012");
yield PlacesUtils.bookmarks.reorder(PlacesUtils.bookmarks.unfiledGuid,
sortedGuids);
for (let i = 0; i < sorted.length; ++i) {
let item = yield PlacesUtils.bookmarks.fetch(sorted[i].guid);
Assert.equal(item.index, i);
}
}
do_print("Test partial sorting");
// Try a partial sorting by passing only 2 entries.
// The unspecified entries should retain the original order.
sorted = [ sorted[1], sorted[0] ].concat(sorted.slice(2));
let sortedGuids = [ sorted[0].guid, sorted[1].guid ];
dump("Expected order: " + [b.guid for (b of sorted)].join() + "\n");
yield PlacesUtils.bookmarks.reorder(PlacesUtils.bookmarks.unfiledGuid,
sortedGuids);
for (let i = 0; i < sorted.length; ++i) {
let item = yield PlacesUtils.bookmarks.fetch(sorted[i].guid);
Assert.equal(item.index, i);
}
// Use triangular numbers to detect skipped position.
let db = yield PlacesUtils.promiseDBConnection();
let rows = yield db.execute(
`SELECT parent
FROM moz_bookmarks
GROUP BY parent
HAVING (SUM(DISTINCT position + 1) - (count(*) * (count(*) + 1) / 2)) <> 0`);
Assert.equal(rows.length, 0, "All the bookmarks should have consistent positions");
});
function run_test() {
run_next_test();
}

View File

@ -34,6 +34,7 @@ skip-if = toolkit == 'android' || toolkit == 'gonk'
[test_bookmarks_insert.js]
[test_bookmarks_notifications.js]
[test_bookmarks_remove.js]
[test_bookmarks_reorder.js]
[test_bookmarks_update.js]
[test_changeBookmarkURI.js]
[test_getBookmarkedURIFor.js]