diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 5b112717d..06574410d 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -2087,7 +2087,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r var parentCollectionID = false; } - var unlock = Zotero.Notifier.begin(true); + var commitNotifier = Zotero.Notifier.begin(); try { for (var i=0; i parseInt(id)), - env.notifierData + env.notifierData, + env.options.notifierQueue ); } }); diff --git a/chrome/content/zotero/xpcom/data/feed.js b/chrome/content/zotero/xpcom/data/feed.js index c591c46b0..f5f0dc47f 100644 --- a/chrome/content/zotero/xpcom/data/feed.js +++ b/chrome/content/zotero/xpcom/data/feed.js @@ -271,7 +271,7 @@ Zotero.Feed.prototype._saveData = Zotero.Promise.coroutine(function* (env) { + "VALUES (" + Array(params.length).fill('?').join(', ') + ")"; yield Zotero.DB.queryAsync(sql, params); - Zotero.Notifier.queue('add', 'feed', this.libraryID); + Zotero.Notifier.queue('add', 'feed', this.libraryID, env.options.notifierQueue); } else if (changedCols.length) { let sql = "UPDATE feeds SET " + changedCols.map(v => v + '=?').join(', ') @@ -279,7 +279,7 @@ Zotero.Feed.prototype._saveData = Zotero.Promise.coroutine(function* (env) { params.push(this.libraryID); yield Zotero.DB.queryAsync(sql, params); - Zotero.Notifier.queue('modify', 'feed', this.libraryID); + Zotero.Notifier.queue('modify', 'feed', this.libraryID, env.options.notifierQueue); } else { Zotero.debug("Feed data did not change for feed " + this.libraryID, 5); @@ -305,12 +305,12 @@ Zotero.Feed.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) { }); -Zotero.Feed.prototype._finalizeErase = Zotero.Promise.coroutine(function* (){ +Zotero.Feed.prototype._finalizeErase = Zotero.Promise.coroutine(function* (env) { let notifierData = {}; notifierData[this.libraryID] = { libraryID: this.libraryID }; - Zotero.Notifier.trigger('delete', 'feed', this.id, notifierData); + Zotero.Notifier.queue('delete', 'feed', this.id, notifierData, env.options.notifierQueue); Zotero.Feeds.unregister(this.libraryID); let syncedFeeds = Zotero.SyncedSettings.get(Zotero.Libraries.userLibraryID, 'feeds') || {}; @@ -395,7 +395,7 @@ Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () { } let deferred = Zotero.Promise.defer(); this._updating = deferred.promise; - Zotero.Notifier.trigger('statusChanged', 'feed', this.id); + yield Zotero.Notifier.trigger('statusChanged', 'feed', this.id); this._set('_feedLastCheckError', null); try { @@ -491,7 +491,7 @@ Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () { yield this.updateUnreadCount(); deferred.resolve(); this._updating = false; - Zotero.Notifier.trigger('statusChanged', 'feed', this.id); + yield Zotero.Notifier.trigger('statusChanged', 'feed', this.id); }); Zotero.Feed.prototype.updateFeed = Zotero.Promise.coroutine(function* () { @@ -511,6 +511,6 @@ Zotero.Feed.prototype.updateUnreadCount = Zotero.Promise.coroutine(function* () if (newCount != this._feedUnreadCount) { this._feedUnreadCount = newCount; - Zotero.Notifier.trigger('unreadCountUpdated', 'feed', this.id); + yield Zotero.Notifier.trigger('unreadCountUpdated', 'feed', this.id); } }); diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 9544e3705..038398230 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -1269,7 +1269,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { yield Zotero.DB.queryAsync(sql, env.sqlValues); if (!env.options.skipNotifier) { - Zotero.Notifier.queue('add', 'item', itemID, env.notifierData); + Zotero.Notifier.queue('add', 'item', itemID, env.notifierData, env.options.notifierQueue); } } else { @@ -1278,7 +1278,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { yield Zotero.DB.queryAsync(sql, env.sqlValues); if (!env.options.skipNotifier) { - Zotero.Notifier.queue('modify', 'item', itemID, env.notifierData); + Zotero.Notifier.queue('modify', 'item', itemID, env.notifierData, env.options.notifierQueue); } } @@ -1387,7 +1387,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { let newParentItemNotifierData = {}; //newParentItemNotifierData[newParentItem.id] = {}; - Zotero.Notifier.queue('modify', 'item', parentItemID, newParentItemNotifierData); + Zotero.Notifier.queue( + 'modify', 'item', parentItemID, newParentItemNotifierData, env.options.notifierQueue + ); switch (Zotero.ItemTypes.getName(itemTypeID)) { case 'note': @@ -1411,7 +1413,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { let newParentItemNotifierData = {}; //newParentItemNotifierData[newParentItem.id] = {}; - Zotero.Notifier.queue('modify', 'item', parentItemID, newParentItemNotifierData); + Zotero.Notifier.queue( + 'modify', + 'item', + parentItemID, + newParentItemNotifierData, + env.options.notifierQueue + ); } let oldParentKey = this._previousData.parentKey; @@ -1421,7 +1429,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { if (oldParentItemID) { let oldParentItemNotifierData = {}; //oldParentItemNotifierData[oldParentItemID] = {}; - Zotero.Notifier.queue('modify', 'item', oldParentItemID, oldParentItemNotifierData); + Zotero.Notifier.queue( + 'modify', + 'item', + oldParentItemID, + oldParentItemNotifierData, + env.options.notifierQueue + ); } else { Zotero.debug("Old source item " + oldParentKey @@ -1445,7 +1459,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { Zotero.Notifier.queue( 'remove', 'collection-item', - changedCollections[i] + '-' + this.id + changedCollections[i] + '-' + this.id, + {}, + env.options.notifierQueue ); } let parentOptions = { @@ -1511,9 +1527,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { yield Zotero.DB.queryAsync(sql, itemID); // Refresh trash - Zotero.Notifier.queue('refresh', 'trash', this.libraryID); + Zotero.Notifier.queue('refresh', 'trash', this.libraryID, {}, env.options.notifierQueue); if (this._deleted) { - Zotero.Notifier.queue('trash', 'item', this.id); + Zotero.Notifier.queue('trash', 'item', this.id, {}, env.options.notifierQueue); } if (parentItemID) { @@ -1633,7 +1649,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { tag: tag.tag, type: tagType }; - Zotero.Notifier.queue('add', 'item-tag', this.id + '-' + tagID, notifierData); + Zotero.Notifier.queue( + 'add', 'item-tag', this.id + '-' + tagID, notifierData, env.options.notifierQueue + ); } if (toRemove.length) { @@ -1647,7 +1665,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { libraryID: this.libraryID, tag: tag.tag }; - Zotero.Notifier.queue('remove', 'item-tag', this.id + '-' + tagID, notifierData); + Zotero.Notifier.queue( + 'remove', 'item-tag', this.id + '-' + tagID, notifierData, env.options.notifierQueue + ); } Zotero.Prefs.set('purge.tags', true); } @@ -1680,7 +1700,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { + "(collectionID, itemID, orderIndex) VALUES (?, ?, ?)"; yield Zotero.DB.queryAsync(sql, [collectionID, this.id, orderIndex]); - Zotero.Notifier.queue('add', 'collection-item', collectionID + '-' + this.id); + Zotero.Notifier.queue( + 'add', + 'collection-item', + collectionID + '-' + this.id, + {}, + env.options.notifierQueue + ); } // Add this item to any loaded collections' cached item lists after commit @@ -1700,7 +1726,13 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { for (let i=0; i q.id).join(", ") + "]"); + } + else { + Zotero.debug("Committing notifier event queue" + totals); + } - for (var type in runQueue) { - for (var event in runQueue[type]) { + for (let type in runQueue) { + for (let event in runQueue[type]) { if (runQueue[type][event].ids.length || event == 'refresh') { yield this.trigger( event, @@ -420,41 +400,17 @@ Zotero.Notifier = new function(){ /* * Reset the event queue */ - function reset() { + this.reset = function () { //Zotero.debug("Resetting notifier event queue"); - _locked = false; - _queue = []; + _queue = {}; _inTransaction = false; } - - - // - // These should rarely be used now that we have event queuing - // - - /* - * Disables Notifier notifications - * - * Returns false if the Notifier was already disabled, true otherwise - */ - function disable() { - if (_disabled) { - Zotero.debug('Notifier notifications are already disabled'); - return false; - } - Zotero.debug('Disabling Notifier notifications'); - _disabled = true; - return true; - } - - - function enable() { - Zotero.debug('Enabling Notifier notifications'); - _disabled = false; - } - - - function isEnabled() { - return !_disabled; - } } + + +Zotero.Notifier.Queue = function () { + this.id = Zotero.Utilities.randomString(); + Zotero.debug("Creating notifier queue " + this.id); + this._queue = {}; + this.size = 0; +}; diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js index 9fc9695d3..1a37070f3 100644 --- a/chrome/content/zotero/xpcom/search.js +++ b/chrome/content/zotero/xpcom/search.js @@ -210,10 +210,10 @@ Zotero.Search.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) // Update library searches status yield Zotero.Libraries.get(this.libraryID).updateSearches(); - Zotero.Notifier.queue('add', 'search', this.id, env.notifierData); + Zotero.Notifier.queue('add', 'search', this.id, env.notifierData, env.options.notifierQueue); } else if (!env.options.skipNotifier) { - Zotero.Notifier.queue('modify', 'search', this.id, env.notifierData); + Zotero.Notifier.queue('modify', 'search', this.id, env.notifierData, env.options.notifierQueue); } if (env.isNew && Zotero.Libraries.isGroupLibrary(this.libraryID)) { diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 9370d68e1..60db984dc 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -433,17 +433,15 @@ Zotero.Sync.Data.Local = { } var batchSize = 10; - var batchCounter = 0; + var notifierQueues = []; try { for (let i = 0; i < json.length; i++) { // Batch notifier updates - if (batchCounter == 0) { - Zotero.Notifier.begin(); - } - else if (batchCounter == batchSize || i == json.length - 1) { - Zotero.Notifier.commit(); - Zotero.Notifier.begin(); + if (notifierQueues.length == batchSize) { + yield Zotero.Notifier.commit(notifierQueues); + notifierQueues = []; } + let notifierQueue = new Zotero.Notifier.Queue; let jsonObject = json[i]; let jsonData = jsonObject.data; @@ -454,6 +452,7 @@ Zotero.Sync.Data.Local = { saveOptions.isNewObject = false; saveOptions.skipCache = false; saveOptions.storageDetailsChanged = false; + saveOptions.notifierQueue = notifierQueue; Zotero.debug(`Processing ${objectType} ${libraryID}/${objectKey}`); Zotero.debug(jsonObject); @@ -542,14 +541,14 @@ Zotero.Sync.Data.Local = { obj, jsonObject, { - skipData: true + skipData: true, + notifierQueue } ); results.push(saveResults); if (!saveResults.processed) { throw saveResults.error; } - batchCounter++; return; } @@ -643,8 +642,11 @@ Zotero.Sync.Data.Local = { if (!saveResults.processed) { throw saveResults.error; } - batchCounter++; }.bind(this)); + + if (notifierQueue.size) { + notifierQueues.push(notifierQueue); + } } catch (e) { // Display nicer debug line for known errors @@ -668,20 +670,19 @@ Zotero.Sync.Data.Local = { options.onError(e); } - if (options.stopOnError) { + if (options.stopOnError || e.fatal) { throw e; } } } } - catch (e) { - Zotero.Notifier.reset(); - throw e; - } finally { - Zotero.Notifier.commit(); + if (notifierQueues.length) { + yield Zotero.Notifier.commit(notifierQueues); + } } + // // Conflict resolution // @@ -704,17 +705,15 @@ Zotero.Sync.Data.Local = { Zotero.debug("Processing resolved conflicts"); let batchSize = 50; - let batchCounter = 0; + let notifierQueues = []; try { for (let i = 0; i < mergeData.length; i++) { // Batch notifier updates - if (batchCounter == 0) { - Zotero.Notifier.begin(); - } - else if (batchCounter == batchSize || i == json.length - 1) { - Zotero.Notifier.commit(); - Zotero.Notifier.begin(); + if (notifierQueues.length == batchSize) { + yield Zotero.Notifier.commit(notifierQueues); + notifierQueues = []; } + let notifierQueue = new Zotero.Notifier.Queue; let json = mergeData[i]; @@ -722,6 +721,7 @@ Zotero.Sync.Data.Local = { Object.assign(saveOptions, options); // Tell _saveObjectFromJSON to save as unsynced saveOptions.saveAsChanged = true; + saveOptions.notifierQueue = notifierQueue; // Errors have to be thrown in order to roll back the transaction, so catch // those here and continue @@ -735,7 +735,9 @@ Zotero.Sync.Data.Local = { // Delete local object if (json.deleted) { try { - yield obj.erase(); + yield obj.erase({ + notifierQueue + }); } catch (e) { results.push({ @@ -784,6 +786,10 @@ Zotero.Sync.Data.Local = { } }.bind(this)); + + if (notifierQueue.size) { + notifierQueues.push(notifierQueue); + } } catch (e) { Zotero.logError(e); @@ -798,11 +804,10 @@ Zotero.Sync.Data.Local = { } } } - catch (e) { - Zotero.Notifier.reset(); - } finally { - Zotero.Notifier.commit(); + if (notifierQueues.length) { + yield Zotero.Notifier.commit(notifierQueues); + } } } } @@ -1001,6 +1006,7 @@ Zotero.Sync.Data.Local = { skipDateModifiedUpdate: true, skipSelect: true, skipCache: options.skipCache || false, + notifierQueue: options.notifierQueue, // Errors are logged elsewhere, so skip in DataObject.save() errorHandler: function (e) { return; diff --git a/test/tests/notifierTest.js b/test/tests/notifierTest.js new file mode 100644 index 000000000..1c49f1f4f --- /dev/null +++ b/test/tests/notifierTest.js @@ -0,0 +1,60 @@ +"use strict"; + +describe("Zotero.Notifier", function () { + describe("#trigger()", function () { + it("should trigger add events before modify events", function* () { + var deferred = Zotero.Promise.defer(); + var events = []; + var observer = { + notify: (action, type, ids) => { + events.push(action); + if (events.length == 2) { + deferred.resolve(); + } + } + }; + var id = Zotero.Notifier.registerObserver(observer, null, 'test_trigger'); + + yield Zotero.DB.executeTransaction(function* () { + var item = new Zotero.Item('book'); + item.setField('title', 'A'); + yield item.save(); + item.setField('title', 'B'); + yield item.save(); + + Zotero.Notifier.queue('unknown', 'item', item.id); + }); + + assert.isTrue(deferred.promise.isResolved()); + assert.lengthOf(events, 3); + assert.equal(events[0], 'add'); + assert.equal(events[1], 'modify'); + assert.equal(events[2], 'unknown'); + + Zotero.Notifier.unregisterObserver(id); + }); + + it("should add events to passed queue", function* () { + var collection = yield createDataObject('collection'); + + var deferred = Zotero.Promise.defer(); + var observer = { + notify: () => deferred.resolve() + }; + var id = Zotero.Notifier.registerObserver(observer, null, 'test_trigger'); + + var queue = new Zotero.Notifier.Queue; + var item = createUnsavedDataObject('item'); + item.setCollections([collection.id]); + yield item.saveTx({ + notifierQueue: queue + }); + assert.isTrue(deferred.promise.isPending()); + assert.equal(queue.size, 2); + yield Zotero.Notifier.commit(queue); + assert.isTrue(deferred.promise.isResolved()); + + Zotero.Notifier.unregisterObserver(id); + }); + }); +});