From 9c9d73d42e041a38ab5b5de86c45b81a0e2613e3 Mon Sep 17 00:00:00 2001 From: Asaf Romano Date: Tue, 25 Nov 2014 09:20:00 +0200 Subject: [PATCH] Bug 1103622 - PlacesTransactions.Annotate for multiple items. r=mak. --- .../components/places/PlacesTransactions.jsm | 52 ++++--- .../tests/unit/test_async_transactions.js | 127 +++++++++++------- 2 files changed, 108 insertions(+), 71 deletions(-) diff --git a/toolkit/components/places/PlacesTransactions.jsm b/toolkit/components/places/PlacesTransactions.jsm index 8e086265ba78..72c621ec51b4 100644 --- a/toolkit/components/places/PlacesTransactions.jsm +++ b/toolkit/components/places/PlacesTransactions.jsm @@ -67,7 +67,8 @@ this.EXPORTED_SYMBOLS = ["PlacesTransactions"]; * a live bookmark is associated. * - tag - a string. * - tags: an array of strings. - * - guid, parentGuid, newParentGuid: a valid places GUID string. + * - guid, parentGuid, newParentGuid: a valid Places GUID string. + * - guids: an array of valid Places GUID strings. * - title: a string * - index, newIndex: the position of an item in its containing folder, * starting from 0. @@ -463,7 +464,6 @@ Enqueuer.prototype = { get promise() this._promise }; - let TransactionsManager = { // See the documentation at the top of this file. |transact| calls are not // serialized with |batch| calls. @@ -869,6 +869,7 @@ DefineTransaction.defineInputProps(["index", "newIndex"], PlacesUtils.bookmarks.DEFAULT_INDEX); DefineTransaction.defineInputProps(["annotation"], DefineTransaction.annotationObjectValidate); +DefineTransaction.defineArrayInputProp("guids", "guid"); DefineTransaction.defineArrayInputProp("urls", "url"); DefineTransaction.defineArrayInputProp("tags", "tag"); DefineTransaction.defineArrayInputProp("annotations", "annotation"); @@ -1269,29 +1270,40 @@ PT.EditUrl.prototype = Object.seal({ * * Required Input Properties: guid, annotationObject */ -PT.Annotate = DefineTransaction(["guid", "annotations"]); +PT.Annotate = DefineTransaction(["guids", "annotations"]); PT.Annotate.prototype = { - execute: function* (aGuid, aNewAnnos) { - let itemId = yield PlacesUtils.promiseItemId(aGuid); - let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId); - let undoAnnos = []; - for (let newAnno of aNewAnnos) { - let currentAnno = currentAnnos.find( a => a.name == newAnno.name ); - if (!!currentAnno) { - undoAnnos.push(currentAnno); - } - else { - // An unset value removes the annotation. - undoAnnos.push({ name: newAnno.name }); + *execute(aGuids, aNewAnnos) { + let undoAnnosForItem = new Map(); // itemId => undoAnnos; + for (let guid of aGuids) { + let itemId = yield PlacesUtils.promiseItemId(guid); + let currentAnnos = PlacesUtils.getAnnotationsForItem(itemId); + + let undoAnnos = []; + for (let newAnno of aNewAnnos) { + let currentAnno = currentAnnos.find(a => a.name == newAnno.name); + if (!!currentAnno) { + undoAnnos.push(currentAnno); + } + else { + // An unset value removes the annotation. + undoAnnos.push({ name: newAnno.name }); + } } + undoAnnosForItem.set(itemId, undoAnnos); + + PlacesUtils.setAnnotationsForItem(itemId, aNewAnnos); } - PlacesUtils.setAnnotationsForItem(itemId, aNewAnnos); - this.undo = () => { - PlacesUtils.setAnnotationsForItem(itemId, undoAnnos); + this.undo = function() { + for (let [itemId, undoAnnos] of undoAnnosForItem) { + PlacesUtils.setAnnotationsForItem(itemId, undoAnnos); + } }; - this.redo = () => { - PlacesUtils.setAnnotationsForItem(itemId, aNewAnnos); + this.redo = function* () { + for (let guid of aGuids) { + let itemId = yield PlacesUtils.promiseItemId(guid); + PlacesUtils.setAnnotationsForItem(itemId, aNewAnnos); + } }; } }; diff --git a/toolkit/components/places/tests/unit/test_async_transactions.js b/toolkit/components/places/tests/unit/test_async_transactions.js index c16bb7a7d281..a1f61a4534bd 100644 --- a/toolkit/components/places/tests/unit/test_async_transactions.js +++ b/toolkit/components/places/tests/unit/test_async_transactions.js @@ -4,10 +4,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -const bmsvc = PlacesUtils.bookmarks; -const tagssvc = PlacesUtils.tagging; -const annosvc = PlacesUtils.annotations; -const PT = PlacesTransactions; +const bmsvc = PlacesUtils.bookmarks; +const tagssvc = PlacesUtils.tagging; +const annosvc = PlacesUtils.annotations; +const PT = PlacesTransactions; +const rootGuid = PlacesUtils.bookmarks.rootGuid; Components.utils.importGlobalProperties(["URL"]); @@ -106,9 +107,6 @@ observer.reset(); // index at which items should begin let bmStartIndex = 0; -// get bookmarks root id -let root = PlacesUtils.bookmarksMenuFolderId; - function run_test() { bmsvc.addObserver(observer, false); do_register_cleanup(function () { @@ -248,9 +246,8 @@ function ensureTagsForURI(aURI, aTags) { do_check_true(aTags.every( t => tagsSet.indexOf(t) != -1 )); } -function* createTestFolderInfo(aTitle = "Test Folder") { - return { parentGuid: yield PlacesUtils.promiseItemGuid(root) - , title: "Test Folder" }; +function createTestFolderInfo(aTitle = "Test Folder") { + return { parentGuid: rootGuid, title: "Test Folder" }; } function isLivemarkTree(aTree) { @@ -338,7 +335,7 @@ add_task(function* test_recycled_transactions() { ensureUndoState(txns, undoPosition); } - let txn_a = PT.NewFolder(yield createTestFolderInfo()); + let txn_a = PT.NewFolder(createTestFolderInfo()); yield txn_a.transact(); ensureUndoState([[txn_a]], 0); yield ensureTransactThrowsFor(txn_a); @@ -351,7 +348,7 @@ add_task(function* test_recycled_transactions() { ensureUndoState(); ensureTransactThrowsFor(txn_a); - let txn_b = PT.NewFolder(yield createTestFolderInfo()); + let txn_b = PT.NewFolder(createTestFolderInfo()); yield PT.batch(function* () { try { yield txn_a.transact(); @@ -375,7 +372,7 @@ add_task(function* test_recycled_transactions() { add_task(function* test_new_folder_with_annotation() { const ANNO = { name: "TestAnno", value: "TestValue" }; - let folder_info = yield createTestFolderInfo(); + let folder_info = createTestFolderInfo(); folder_info.index = bmStartIndex; folder_info.annotations = [ANNO]; ensureUndoState(); @@ -410,7 +407,7 @@ add_task(function* test_new_folder_with_annotation() { }); add_task(function* test_new_bookmark() { - let bm_info = { parentGuid: yield PlacesUtils.promiseItemGuid(root) + let bm_info = { parentGuid: rootGuid , url: NetUtil.newURI("http://test_create_item.com") , index: bmStartIndex , title: "Test creating an item" }; @@ -447,7 +444,7 @@ add_task(function* test_new_bookmark() { }); add_task(function* test_merge_create_folder_and_item() { - let folder_info = yield createTestFolderInfo(); + let folder_info = createTestFolderInfo(); let bm_info = { url: NetUtil.newURI("http://test_create_item_to_folder.com") , title: "Test Bookmark" , index: bmStartIndex }; @@ -485,7 +482,7 @@ add_task(function* test_merge_create_folder_and_item() { }); add_task(function* test_move_items_to_folder() { - let folder_a_info = yield createTestFolderInfo("Folder A"); + let folder_a_info = createTestFolderInfo("Folder A"); let bkm_a_info = { url: new URL("http://test_move_items.com") , title: "Bookmark A" }; let bkm_b_info = { url: NetUtil.newURI("http://test_move_items.com") @@ -541,7 +538,7 @@ add_task(function* test_move_items_to_folder() { ensureUndoState([[bkm_b_txn, bkm_a_txn, folder_a_txn]], 0); // Test moving items between folders. - let folder_b_info = yield createTestFolderInfo("Folder B"); + let folder_b_info = createTestFolderInfo("Folder B"); let folder_b_txn = PT.NewFolder(folder_b_info); folder_b_info.guid = yield folder_b_txn.transact(); ensureUndoState([ [folder_b_txn] @@ -595,7 +592,7 @@ add_task(function* test_move_items_to_folder() { }); add_task(function* test_remove_folder() { - let folder_level_1_info = yield createTestFolderInfo("Folder Level 1"); + let folder_level_1_info = createTestFolderInfo("Folder Level 1"); let folder_level_2_info = { title: "Folder Level 2" }; let [folder_level_1_txn, folder_level_2_txn] = yield PT.batch(function* () { @@ -688,7 +685,7 @@ add_task(function* test_add_and_remove_bookmarks_with_additional_info() { , POST_DATA = "post_data" , ANNO = { name: "TestAnno", value: "TestAnnoValue" }; - let folder_info = yield createTestFolderInfo(); + let folder_info = createTestFolderInfo(); folder_info.guid = yield PT.NewFolder(folder_info).transact(); let ensureTags = ensureTagsForURI.bind(null, testURI); @@ -803,7 +800,7 @@ add_task(function* test_add_and_remove_bookmarks_with_additional_info() { }); add_task(function* test_creating_and_removing_a_separator() { - let folder_info = yield createTestFolderInfo(); + let folder_info = createTestFolderInfo(); let separator_info = {}; let undoEntries = []; @@ -872,7 +869,7 @@ add_task(function* test_creating_and_removing_a_separator() { add_task(function* test_add_and_remove_livemark() { let createLivemarkTxn = PT.NewLivemark( { feedUrl: NetUtil.newURI("http://test.remove.livemark") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) + , parentGuid: rootGuid , title: "Test Remove Livemark" }); let guid = yield createLivemarkTxn.transact(); let originalInfo = yield PlacesUtils.promiseBookmarksTree(guid); @@ -913,7 +910,7 @@ add_task(function* test_add_and_remove_livemark() { }); add_task(function* test_edit_title() { - let bm_info = { parentGuid: yield PlacesUtils.promiseItemGuid(root) + let bm_info = { parentGuid: rootGuid , url: NetUtil.newURI("http://test_create_item.com") , title: "Original Title" }; @@ -951,10 +948,7 @@ add_task(function* test_edit_title() { add_task(function* test_edit_url() { let oldURI = NetUtil.newURI("http://old.test_editing_item_uri.com/"); let newURI = NetUtil.newURI("http://new.test_editing_item_uri.com/"); - let bm_info = { parentGuid: yield PlacesUtils.promiseItemGuid(root) - , url: oldURI - , tags: ["TestTag"]}; - + let bm_info = { parentGuid: rootGuid, url: oldURI, tags: ["TestTag"] }; function ensureURIAndTags(aPreChangeURI, aPostChangeURI, aOLdURITagsPreserved) { ensureItemsChanged({ guid: bm_info.guid , property: "uri" @@ -1016,8 +1010,8 @@ add_task(function* test_edit_url() { }); add_task(function* test_edit_keyword() { - let bm_info = { parentGuid: yield PlacesUtils.promiseItemGuid(root) - , url: NetUtil.newURI("http://test.edit.keyword") }; + let bm_info = { parentGuid: rootGuid + , url: NetUtil.newURI("http://test.edit.keyword") }; const KEYWORD = "test_keyword"; bm_info.guid = yield PT.NewBookmark(bm_info).transact(); function ensureKeywordChange(aCurrentKeyword = "") { @@ -1054,9 +1048,9 @@ add_task(function* test_edit_keyword() { add_task(function* test_tag_uri() { // This also tests passing uri specs. let bm_info_a = { url: "http://bookmarked.uri" - , parentGuid: yield PlacesUtils.promiseItemGuid(root) }; + , parentGuid: rootGuid }; let bm_info_b = { url: NetUtil.newURI("http://bookmarked2.uri") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) }; + , parentGuid: rootGuid }; let unbookmarked_uri = NetUtil.newURI("http://un.bookmarked.uri"); function* promiseIsBookmarked(aURI) { @@ -1128,10 +1122,10 @@ add_task(function* test_tag_uri() { add_task(function* test_untag_uri() { let bm_info_a = { url: NetUtil.newURI("http://bookmarked.uri") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) + , parentGuid: rootGuid , tags: ["A", "B"] }; let bm_info_b = { url: NetUtil.newURI("http://bookmarked2.uri") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) + , parentGuid: rootGuid , tag: "B" }; yield PT.batch(function* () { @@ -1206,7 +1200,7 @@ add_task(function* test_untag_uri() { add_task(function* test_annotate() { let bm_info = { url: NetUtil.newURI("http://test.item.annotation") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) }; + , parentGuid: rootGuid }; let anno_info = { name: "TestAnno", value: "TestValue" }; function ensureAnnoState(aSet) { ensureAnnotationsSet(bm_info.guid, @@ -1248,7 +1242,7 @@ add_task(function* test_annotate() { }); add_task(function* test_annotate_multiple() { - let guid = yield PT.NewFolder(yield createTestFolderInfo()).transact(); + let guid = yield PT.NewFolder(createTestFolderInfo()).transact(); let itemId = yield PlacesUtils.promiseItemId(guid); function AnnoObj(aName, aValue) { @@ -1302,7 +1296,7 @@ add_task(function* test_annotate_multiple() { }); add_task(function* test_sort_folder_by_name() { - let folder_info = yield createTestFolderInfo(); + let folder_info = createTestFolderInfo(); let url = NetUtil.newURI("http://sort.by.name/"); let preSep = [{ title: i, url } for (i of ["3","2","1"])]; @@ -1349,7 +1343,7 @@ add_task(function* test_sort_folder_by_name() { add_task(function* test_livemark_txns() { let livemark_info = { feedUrl: NetUtil.newURI("http://test.feed.uri") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) + , parentGuid: rootGuid , title: "Test Livemark" }; function ensureLivemarkAdded() { ensureItemsAdded({ guid: livemark_info.guid @@ -1396,11 +1390,9 @@ add_task(function* test_livemark_txns() { }); add_task(function* test_copy() { - let rootGuid = yield PlacesUtils.promiseItemGuid(root); - function* duplicate_and_test(aOriginalGuid) { - yield duplicateGuid = - yield PT.Copy({ guid: aOriginalGuid, newParentGuid: rootGuid }).transact(); + let txn = PT.Copy({ guid: aOriginalGuid, newParentGuid: rootGuid }); + yield duplicateGuid = yield txn.transact(); let originalInfo = yield PlacesUtils.promiseBookmarksTree(aOriginalGuid); let duplicateInfo = yield PlacesUtils.promiseBookmarksTree(duplicateGuid); yield ensureEqualBookmarksTrees(originalInfo, duplicateInfo, false); @@ -1436,9 +1428,9 @@ add_task(function* test_copy() { let sepTxn = PT.NewSeparator({ parentGuid: rootGuid, index: 1 }); let livemarkTxn = PT.NewLivemark( { feedUrl: new URL("http://test.feed.uri") - , parentGuid: yield PlacesUtils.promiseItemGuid(root) + , parentGuid: rootGuid , title: "Test Livemark", index: 1 }); - let emptyFolderTxn = PT.NewFolder(yield createTestFolderInfo()); + let emptyFolderTxn = PT.NewFolder(createTestFolderInfo()); for (let txn of [livemarkTxn, sepTxn, emptyFolderTxn]) { let guid = yield txn.transact(); yield duplicate_and_test(guid); @@ -1446,8 +1438,7 @@ add_task(function* test_copy() { // Test duplicating a folder having some contents. let filledFolderGuid = yield PT.batch(function *() { - let folderGuid = - yield PT.NewFolder(yield createTestFolderInfo()).transact(); + let folderGuid = yield PT.NewFolder(createTestFolderInfo()).transact(); let nestedFolderGuid = yield PT.NewFolder({ parentGuid: folderGuid , title: "Nested Folder" }).transact(); @@ -1469,9 +1460,7 @@ add_task(function* test_copy() { }); add_task(function* test_array_input_for_batch() { - let rootGuid = yield PlacesUtils.promiseItemGuid(root); - - let folderTxn = PT.NewFolder(yield createTestFolderInfo()); + let folderTxn = PT.NewFolder(createTestFolderInfo()); let folderGuid = yield folderTxn.transact(); let sep1_txn = PT.NewSeparator({ parentGuid: folderGuid }); @@ -1503,9 +1492,7 @@ add_task(function* test_array_input_for_batch() { }); add_task(function* test_copy_excluding_annotations() { - let rootGuid = yield PlacesUtils.promiseItemGuid(root); - - let folderInfo = yield createTestFolderInfo(); + let folderInfo = createTestFolderInfo(); let anno = n => { return { name: n, value: 1 } }; folderInfo.annotations = [anno("a"), anno("b"), anno("c")]; let folderGuid = yield PT.NewFolder(folderInfo).transact(); @@ -1539,7 +1526,6 @@ add_task(function* test_copy_excluding_annotations() { }); add_task(function* test_invalid_uri_spec_throws() { - let rootGuid = yield PlacesUtils.promiseItemGuid(root); Assert.throws(() => PT.NewBookmark({ parentGuid: rootGuid , url: "invalid uri spec" @@ -1551,3 +1537,42 @@ add_task(function* test_invalid_uri_spec_throws() { PT.Tag({ tag: "TheTag" , urls: ["about:blank", "invalid uri spec"] })); }); + +add_task(function* test_annotate_multiple_items() { + let parentGuid = rootGuid; + let guids = [ + yield PT.NewBookmark({ url: "about:blank", parentGuid }).transact(), + yield PT.NewFolder({ title: "Test Folder", parentGuid }).transact()]; + + let annotation = { name: "TestAnno", value: "TestValue" }; + yield PT.Annotate({ guids, annotation }).transact(); + + function *ensureAnnoSet() { + for (let guid of guids) { + let itemId = yield PlacesUtils.promiseItemId(guid); + Assert.equal(annosvc.getItemAnnotation(itemId, annotation.name), + annotation.value); + } + } + function *ensureAnnoUnset() { + for (let guid of guids) { + let itemId = yield PlacesUtils.promiseItemId(guid); + Assert.ok(!annosvc.itemHasAnnotation(itemId, annotation.name)); + } + } + + yield ensureAnnoSet(); + yield PT.undo(); + yield ensureAnnoUnset(); + yield PT.redo(); + yield ensureAnnoSet(); + yield PT.undo(); + yield ensureAnnoUnset(); + + // Cleanup + yield PT.undo(); + yield PT.undo(); + yield ensureNonExistent(...guids); + PT.clearTransactionsHistory(); + observer.reset(); +});