commit 3a0e0cb0886bdcd105ec03d8bac12fff5079e2f7
parent 58edb3143ed83867a503eb4738442b2de20bcd33
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 8 Feb 2017 21:53:29 -0500
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.
Diffstat:
5 files changed, 85 insertions(+), 15 deletions(-)
diff --git 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
@@ -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
@@ -103,12 +103,23 @@ Zotero.Items = function() {
this._relationsTable = "itemRelations";
+
+ /**
+ * @param {Integer} libraryID
+ * @return {Promise<Boolean>} - 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<Zotero.Item[]|Integer[]>}
*/
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<ids.length; i++) {
- let id = ids[i];
- let item = yield this.getAsync(id);
+ var items = [];
+ for (let id of ids) {
+ let item = this.get(id);
if (!item) {
Zotero.debug('Item ' + id + ' does not exist in Items.trash()!', 1);
- Zotero.Notifier.queue('delete', 'item', id);
+ Zotero.Notifier.queue('trash', 'item', id);
continue;
}
@@ -833,15 +844,32 @@ Zotero.Items = function() {
throw new Error(item._ObjectType + " " + item.libraryKey + " is not editable");
}
- if (!Zotero.Libraries.hasTrash(item.libraryID)) {
- throw new Error(Zotero.Libraries.getName(item.libraryID) + " does not have Trash");
+ if (!Zotero.Libraries.get(item.libraryID).hasTrash) {
+ throw new Error(Zotero.Libraries.getName(item.libraryID) + " does not have a trash");
}
- item.deleted = true;
- yield item.save({
- skipDateModifiedUpdate: true
- });
+ items.push(item);
+ libraryIDs.add(item.libraryID);
}
+
+ items.forEach(item => {
+ 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
@@ -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
@@ -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');
