From 3a0e0cb0886bdcd105ec03d8bac12fff5079e2f7 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 8 Feb 2017 21:53:29 -0500 Subject: [PATCH] Drastically speed up moving items to the trash E.g., moving 3,600 items to the trash now takes 4 seconds instead of 62 Instead of saving each item, update internal state and database directly (which is more brittle but worth it). Also avoid unnecessary sorting after removing an item from the items tree. --- .../zotero/xpcom/collectionTreeView.js | 3 +- chrome/content/zotero/xpcom/data/item.js | 22 ++++++++ chrome/content/zotero/xpcom/data/items.js | 52 ++++++++++++++----- chrome/content/zotero/xpcom/itemTreeView.js | 1 - test/tests/itemsTest.js | 22 ++++++++ 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index c3de245fa..db1e90033 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -338,8 +338,7 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function* // If trash is refreshed, we probably need to update the icon from full to empty if (type == 'trash') { // libraryID is passed as parameter to 'refresh' - let deleted = yield Zotero.Items.getDeleted(ids[0], true); - this._trashNotEmpty[ids[0]] = !!deleted.length; + this._trashNotEmpty[ids[0]] = yield Zotero.Items.hasDeleted(ids[0]); let row = this.getRowIndexByID("T" + ids[0]); this._treebox.invalidateRow(row); } diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index cc9e1d802..8ab56825e 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3603,6 +3603,28 @@ Zotero.Item.prototype.inCollection = function (collectionID) { }; +/** + * Update item deleted (i.e., trash) state without marking as changed or modifying DB + * + * This is used by Zotero.Items.trash(). + * + * Database state must be set separately! + * + * @param {Boolean} deleted + */ +Zotero.DataObject.prototype.setDeleted = Zotero.Promise.coroutine(function* (deleted) { + if (!this.id) { + throw new Error("Cannot update deleted state of unsaved item"); + } + + this._deleted = !!deleted; + + if (this._changed.deleted) { + delete this._changed.deleted; + } +}); + + Zotero.Item.prototype.getImageSrc = function() { var itemType = Zotero.ItemTypes.getName(this.itemTypeID); if (itemType == 'attachment') { diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index 0391fa760..3e57e79ae 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -103,12 +103,23 @@ Zotero.Items = function() { this._relationsTable = "itemRelations"; + + /** + * @param {Integer} libraryID + * @return {Promise} - True if library has items in trash, false otherwise + */ + this.hasDeleted = Zotero.Promise.coroutine(function* (libraryID) { + var sql = "SELECT COUNT(*) > 0 FROM items JOIN deletedItems USING (itemID) WHERE libraryID=?"; + return !!(yield Zotero.DB.valueQueryAsync(sql, [libraryID])); + }); + + /** * Return items marked as deleted * * @param {Integer} libraryID - Library to search * @param {Boolean} [asIDs] - Return itemIDs instead of Zotero.Item objects - * @return {Zotero.Item[]|Integer[]} + * @return {Promise} */ this.getDeleted = Zotero.Promise.coroutine(function* (libraryID, asIDs, days) { var sql = "SELECT itemID FROM items JOIN deletedItems USING (itemID) " @@ -818,14 +829,14 @@ Zotero.Items = function() { this.trash = Zotero.Promise.coroutine(function* (ids) { Zotero.DB.requireTransaction(); + var libraryIDs = new Set(); ids = Zotero.flattenArguments(ids); - - for (let i=0; i { + item.setDeleted(true); + }); + yield Zotero.Utilities.Internal.forEachChunkAsync(ids, 250, Zotero.Promise.coroutine(function* (chunk) { + yield Zotero.DB.queryAsync( + "UPDATE items SET synced=1, clientDateModified=CURRENT_TIMESTAMP " + + `WHERE itemID IN (${chunk.map(id => parseInt(id)).join(", ")})` + ); + yield Zotero.DB.queryAsync( + "INSERT OR IGNORE INTO deletedItems (itemID) VALUES " + + chunk.map(id => "(" + id + ")").join(", ") + ); + }.bind(this))); + + Zotero.Notifier.queue('trash', 'item', ids); + Array.from(libraryIDs).forEach(libraryID => { + Zotero.Notifier.queue('refresh', 'trash', libraryID); + }); }); diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index 3126da6df..316ea5888 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -641,7 +641,6 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio } madeChanges = true; - sort = true; } } else if (type == 'item' && action == 'modify') diff --git a/test/tests/itemsTest.js b/test/tests/itemsTest.js index a7a44d1b9..d558fdfa9 100644 --- a/test/tests/itemsTest.js +++ b/test/tests/itemsTest.js @@ -87,6 +87,28 @@ describe("Zotero.Items", function () { }) }) + + describe("#trash()", function () { + it("should send items to the trash", function* () { + var items = []; + items.push( + (yield createDataObject('item')), + (yield createDataObject('item')), + (yield createDataObject('item')) + ); + var ids = items.map(item => item.id); + yield Zotero.Items.trashTx(ids); + items.forEach(item => { + assert.isTrue(item.deleted); + assert.isFalse(item.hasChanged()); + }); + assert.equal((yield Zotero.DB.valueQueryAsync( + `SELECT COUNT(*) FROM deletedItems WHERE itemID IN (${ids})` + )), 3); + }); + }); + + describe("#emptyTrash()", function () { it("should delete items in the trash", function* () { var item1 = createUnsavedDataObject('item');