www

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | Submodules | README | LICENSE

commit fe9fc8bc5a4a34eb45855ff62451422f51022629
parent 182b9a937bb180151d92d74b09f85798c60fa83d
Author: Dan Stillman <dstillman@zotero.org>
Date:   Fri,  3 Mar 2017 16:33:48 -0500

Asyncify various functions to fix cross-library drag-and-drop error

When dragging an item to another library, we have to check if there's a
linked item in the target library, but items might not yet be laoded in
the other library, so item.getLinkedItem() can fail with "Item [n] not
yet loaded].

Fixing required asyncifying the follow functions:

- Zotero.Item::getLinkedItem()
- Zotero.Collection::getLinkedCollection()
- Zotero.URI.getURIItem()
- Zotero.URI.getURICollection()
- Various integration functions

Diffstat:
Mchrome/content/zotero/xpcom/collectionTreeView.js | 4++--
Mchrome/content/zotero/xpcom/data/collection.js | 2++
Mchrome/content/zotero/xpcom/data/dataObject.js | 8++++----
Mchrome/content/zotero/xpcom/data/relations.js | 2+-
Mchrome/content/zotero/xpcom/integration.js | 142++++++++++++++++++++++++++++++++++++++-----------------------------------------
Mchrome/content/zotero/xpcom/report.js | 2+-
Mchrome/content/zotero/xpcom/uri.js | 20++++++++------------
Mtest/tests/collectionTreeViewTest.js | 6+++---
Mtest/tests/dataObjectTest.js | 6+++---
9 files changed, 93 insertions(+), 99 deletions(-)

diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -1759,7 +1759,7 @@ Zotero.CollectionTreeView.prototype.canDropCheckAsync = Zotero.Promise.coroutine // Cross-library drag if (treeRow.ref.libraryID != item.libraryID) { - let linkedItem = item.getLinkedItem(treeRow.ref.libraryID, true); + let linkedItem = yield item.getLinkedItem(treeRow.ref.libraryID, true); if (linkedItem && !linkedItem.deleted) { // For drag to root, skip if linked item exists if (treeRow.isLibrary(true)) { @@ -1865,7 +1865,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r var targetLibraryType = Zotero.Libraries.get(targetLibraryID).libraryType; // Check if there's already a copy of this item in the library - var linkedItem = item.getLinkedItem(targetLibraryID, true); + var linkedItem = yield item.getLinkedItem(targetLibraryID, true); if (linkedItem) { // If linked item is in the trash, undelete it and remove it from collections // (since it shouldn't be restored to previous collections) diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js @@ -810,6 +810,8 @@ Zotero.Collection.prototype.getDescendents = function (nested, type, includeDele /** * Return a collection in the specified library equivalent to this collection + * + * @return {Promise<Zotero.Collection>} */ Zotero.Collection.prototype.getLinkedCollection = function (libraryID, bidrectional) { return this._getLinkedObject(libraryID, bidrectional); diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js @@ -451,9 +451,9 @@ Zotero.DataObject.prototype.setRelations = function (newRelations) { * calling this directly. * * @param {Integer} [libraryID] - * @return {Zotero.DataObject|false} Linked object, or false if not found + * @return {Promise<Zotero.DataObject|false>} Linked object, or false if not found */ -Zotero.DataObject.prototype._getLinkedObject = function (libraryID, bidirectional) { +Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function* (libraryID, bidirectional) { if (!libraryID) { throw new Error("libraryID not provided"); } @@ -471,7 +471,7 @@ Zotero.DataObject.prototype._getLinkedObject = function (libraryID, bidirectiona for (let i = 0; i < uris.length; i++) { let uri = uris[i]; if (uri.startsWith(libraryObjectPrefix)) { - let obj = Zotero.URI['getURI' + this._ObjectType](uri); + let obj = yield Zotero.URI['getURI' + this._ObjectType](uri); if (!obj) { Zotero.debug("Referenced linked " + this._objectType + " '" + uri + "' not found " + "in Zotero." + this._ObjectType + "::getLinked" + this._ObjectType + "()", 2); @@ -501,7 +501,7 @@ Zotero.DataObject.prototype._getLinkedObject = function (libraryID, bidirectiona } return false; -}; +}); /** diff --git a/chrome/content/zotero/xpcom/data/relations.js b/chrome/content/zotero/xpcom/data/relations.js @@ -232,7 +232,7 @@ Zotero.Relations = new function () { // removing if (uri.indexOf(prefix) != -1 && uri.indexOf("/" + type + "s/") != -1 - && !Zotero.URI[getFunc](uri)) { + && !(yield Zotero.URI[getFunc](uri))) { if (!objects[row.id]) { objects[row.id] = yield objectsClass.getAsync(row.id, { noCache: true }); } diff --git a/chrome/content/zotero/xpcom/integration.js b/chrome/content/zotero/xpcom/integration.js @@ -786,7 +786,7 @@ Zotero.Integration.MissingItemException.prototype = { // Now try again Zotero.Integration.currentWindow = oldCurrentWindow; fieldGetter._doc.activate(); - fieldGetter._processFields(fieldIndex); + return fieldGetter._processFields(fieldIndex); }); } } @@ -1420,61 +1420,56 @@ Zotero.Integration.Fields.prototype.get = function get() { /** * Updates Zotero.Integration.Session attached to Zotero.Integration.Fields in line with document */ -Zotero.Integration.Fields.prototype.updateSession = function() { - var me = this, collectFieldsTime; - return this.get().then(function() { - me._session.resetRequest(me._doc); - - me._removeCodeKeys = {}; - me._removeCodeFields = {}; - me._bibliographyFields = []; - me._bibliographyData = ""; - - collectFieldsTime = (new Date()).getTime(); - return me._processFields(); - }).then(function() { - var endTime = (new Date()).getTime(); - if(Zotero.Debug.enabled) { - Zotero.debug("Integration: Updated session data for "+me._fields.length+" fields in "+ - (endTime-collectFieldsTime)/1000+"; "+ - 1000/((endTime-collectFieldsTime)/me._fields.length)+" fields/second"); - } - - // Load uncited items from bibliography - if(me._bibliographyData && !me._session.bibliographyData) { - try { - me._session.loadBibliographyData(me._bibliographyData); - } catch(e) { - var exception = new Zotero.Integration.CorruptBibliographyException(me, e); - exception.setContext(me); - throw exception; - } - } - - // if we are reloading this session, assume no item IDs to be updated except for - // edited items - if(me._session.reload) { - //this._session.restoreProcessorState(); TODO doesn't appear to be working properly - me._session.updateUpdateIndices(); - // Iterate through citations, yielding for UI updates - return Zotero.Promise.each(me._session._updateCitations(), () => {}) - .then(function() { - me._session.updateIndices = {}; - me._session.updateItemIDs = {}; - me._session.citationText = {}; - me._session.bibliographyHasChanged = false; - delete me._session.reload; - }); - } else { - return; +Zotero.Integration.Fields.prototype.updateSession = Zotero.Promise.coroutine(function* () { + var collectFieldsTime; + yield this.get(); + this._session.resetRequest(this._doc); + + this._removeCodeKeys = {}; + this._removeCodeFields = {}; + this._bibliographyFields = []; + this._bibliographyData = ""; + + collectFieldsTime = (new Date()).getTime(); + yield this._processFields(); + + var endTime = (new Date()).getTime(); + if(Zotero.Debug.enabled) { + Zotero.debug("Integration: Updated session data for " + this._fields.length + " fields in " + + (endTime - collectFieldsTime) / 1000 + "; " + + 1000/ ((endTime - collectFieldsTime) / this._fields.length) + " fields/second"); + } + + // Load uncited items from bibliography + if (this._bibliographyData && !this._session.bibliographyData) { + try { + yield this._session.loadBibliographyData(this._bibliographyData); + } catch(e) { + var exception = new Zotero.Integration.CorruptBibliographyException(me, e); + exception.setContext(me); + throw exception; } - }); -} + } + + // if we are reloading this session, assume no item IDs to be updated except for + // edited items + if (this._session.reload) { + //this._session.restoreProcessorState(); TODO doesn't appear to be working properly + this._session.updateUpdateIndices(); + // Iterate through citations, yielding for UI updates + yield Zotero.Promise.each(this._session._updateCitations(), () => {}); + this._session.updateIndices = {}; + this._session.updateItemIDs = {}; + this._session.citationText = {}; + this._session.bibliographyHasChanged = false; + delete this._session.reload; + } +}); /** * Keep processing fields until all have been processed */ -Zotero.Integration.Fields.prototype._processFields = function(i) { +Zotero.Integration.Fields.prototype._processFields = Zotero.Promise.coroutine(function* (i) { if(!i) i = 0; for(var n = this._fields.length; i<n; i++) { @@ -1493,7 +1488,7 @@ Zotero.Integration.Fields.prototype._processFields = function(i) { if(type === INTEGRATION_TYPE_ITEM) { var noteIndex = field.getNoteIndex(); try { - this._session.addCitation(i, noteIndex, content); + yield this._session.addCitation(i, noteIndex, content); } catch(e) { var removeCode = false; @@ -1527,7 +1522,8 @@ Zotero.Integration.Fields.prototype._processFields = function(i) { } } } -} +}); + /** * Updates bibliographies and fields within a document * @param {Boolean} forceCitations Whether to regenerate all citations @@ -1716,7 +1712,7 @@ Zotero.Integration.Fields.prototype._updateDocument = function* (forceCitations, /** * Brings up the addCitationDialog, prepopulated if a citation is provided */ -Zotero.Integration.Fields.prototype.addEditCitation = function(field) { +Zotero.Integration.Fields.prototype.addEditCitation = Zotero.Promise.coroutine(function* (field) { var newField, citation, fieldIndex, session = this._session; // if there's already a citation, make sure we have item IDs in addition to keys @@ -1737,7 +1733,7 @@ Zotero.Integration.Fields.prototype.addEditCitation = function(field) { if(citation) { try { - session.lookupItems(citation); + yield session.lookupItems(citation); } catch(e) { if(e instanceof Zotero.Integration.MissingItemException) { citation.citationItems = []; @@ -1805,7 +1801,7 @@ Zotero.Integration.Fields.prototype.addEditCitation = function(field) { return io.promise; } }); -} +}); /** * Citation editing functions and propertiesaccessible to quickFormat.js and addCitationDialog.js @@ -1848,9 +1844,9 @@ Zotero.Integration.CitationEditInterface = function(citation, field, fieldGetter } me._fieldGetter.progressCallback = progressCallback; return me._updateSession(true); - }).then(function() { + }).then(Zotero.Promise.coroutine(function* () { // Add new citation - me._session.addCitation(me._fieldIndex, me._field.getNoteIndex(), me.citation); + yield me._session.addCitation(me._fieldIndex, me._field.getNoteIndex(), me.citation); me._session.updateIndices[me._fieldIndex] = true; // Check if bibliography changed @@ -1867,7 +1863,7 @@ Zotero.Integration.CitationEditInterface = function(citation, field, fieldGetter // Update document return me._fieldGetter.updateDocument(FORCE_CITATIONS_FALSE, false, false); - }); + })); } Zotero.Integration.CitationEditInterface.prototype = { @@ -2270,7 +2266,7 @@ Zotero.Integration._oldCitationLocatorMap = { /** * Adds a citation to the arrays representing the document */ -Zotero.Integration.Session.prototype.addCitation = function(index, noteIndex, arg) { +Zotero.Integration.Session.prototype.addCitation = Zotero.Promise.coroutine(function* (index, noteIndex, arg) { var index = parseInt(index, 10); if(typeof(arg) == "string") { // text field @@ -2282,7 +2278,7 @@ Zotero.Integration.Session.prototype.addCitation = function(index, noteIndex, ar } // get items - this.lookupItems(citation, index); + yield this.lookupItems(citation, index); citation.properties.added = true; citation.properties.zoteroIndex = index; @@ -2320,13 +2316,13 @@ Zotero.Integration.Session.prototype.addCitation = function(index, noteIndex, ar } Zotero.debug("Integration: Adding citationID "+citation.citationID); this.documentCitationIDs[citation.citationID] = citation.citationID; -} +}); /** * Looks up item IDs to correspond with keys or generates embedded items for given citation object. * Throws a MissingItemException if item was not found. */ -Zotero.Integration.Session.prototype.lookupItems = function(citation, index) { +Zotero.Integration.Session.prototype.lookupItems = Zotero.Promise.coroutine(function* (citation, index) { for(var i=0, n=citation.citationItems.length; i<n; i++) { var citationItem = citation.citationItems[i]; @@ -2334,7 +2330,7 @@ Zotero.Integration.Session.prototype.lookupItems = function(citation, index) { var zoteroItem = false, needUpdate; if(citationItem.uris) { - [zoteroItem, needUpdate] = this.uriMap.getZoteroItemForURIs(citationItem.uris); + [zoteroItem, needUpdate] = yield this.uriMap.getZoteroItemForURIs(citationItem.uris); if(needUpdate && index) this.updateIndices[index] = true; // Unfortunately, people do weird things with their documents. One weird thing people @@ -2421,7 +2417,7 @@ Zotero.Integration.Session.prototype.lookupItems = function(citation, index) { citationItem.id = zoteroItem.cslItemID ? zoteroItem.cslItemID : zoteroItem.id; } } -} +}); /** * Unserializes a JSON citation into a citation object (sans items) @@ -2716,7 +2712,7 @@ Zotero.Integration.Session.prototype.restoreProcessorState = function() { /** * Loads document data from a JSON object */ -Zotero.Integration.Session.prototype.loadBibliographyData = function(json) { +Zotero.Integration.Session.prototype.loadBibliographyData = Zotero.Promise.coroutine(function* (json) { var openBraceIndex = json.indexOf("{"); if(openBraceIndex == -1) return; @@ -2738,7 +2734,7 @@ Zotero.Integration.Session.prototype.loadBibliographyData = function(json) { // new style array of arrays with URIs let zoteroItem, needUpdate; for (let uris of documentData.uncited) { - [zoteroItem, needUpdate] = this.uriMap.getZoteroItemForURIs(uris); + [zoteroItem, needUpdate] = yield this.uriMap.getZoteroItemForURIs(uris); var id = zoteroItem.cslItemID ? zoteroItem.cslItemID : zoteroItem.id; if(zoteroItem && !this.citationsByItemID[id]) { this.uncitedItems[id] = true; @@ -2767,7 +2763,7 @@ Zotero.Integration.Session.prototype.loadBibliographyData = function(json) { // new style array of arrays with URIs var zoteroItem, needUpdate; for (let custom of documentData.custom) { - [zoteroItem, needUpdate] = this.uriMap.getZoteroItemForURIs(custom[0]); + [zoteroItem, needUpdate] = yield this.uriMap.getZoteroItemForURIs(custom[0]); if(!zoteroItem) continue; if(needUpdate) this.bibliographyDataHasChanged = true; @@ -2796,7 +2792,7 @@ Zotero.Integration.Session.prototype.loadBibliographyData = function(json) { if(documentData.omitted) { let zoteroItem, needUpdate; for (let uris of documentData.omitted) { - [zoteroItem, update] = this.uriMap.getZoteroItemForURIs(uris); + [zoteroItem, update] = yield this.uriMap.getZoteroItemForURIs(uris); var id = zoteroItem.cslItemID ? zoteroItem.cslItemID : zoteroItem.id; if(zoteroItem && this.citationsByItemID[id]) { this.omittedItems[id] = true; @@ -2808,7 +2804,7 @@ Zotero.Integration.Session.prototype.loadBibliographyData = function(json) { } this.bibliographyData = json; -} +}); /** * Saves document data from a JSON object @@ -3132,7 +3128,7 @@ Zotero.Integration.URIMap.prototype.getURIsForItemID = function(id) { /** * Gets Zotero item for a given set of URIs */ -Zotero.Integration.URIMap.prototype.getZoteroItemForURIs = function(uris) { +Zotero.Integration.URIMap.prototype.getZoteroItemForURIs = Zotero.Promise.coroutine(function* (uris) { var zoteroItem = false; var needUpdate = false; var embeddedItem = false;; @@ -3147,7 +3143,7 @@ Zotero.Integration.URIMap.prototype.getZoteroItemForURIs = function(uris) { // Next try getting URI directly try { - zoteroItem = Zotero.URI.getURIItem(uri); + zoteroItem = yield Zotero.URI.getURIItem(uri); if(zoteroItem) { // Ignore items in the trash if(zoteroItem.deleted) { @@ -3183,4 +3179,4 @@ Zotero.Integration.URIMap.prototype.getZoteroItemForURIs = function(uris) { } return [zoteroItem, needUpdate]; -} +}); diff --git a/chrome/content/zotero/xpcom/report.js b/chrome/content/zotero/xpcom/report.js @@ -110,7 +110,7 @@ Zotero.Report.HTML = new function () { } for (let i=0; i<rels.length; i++) { let rel = rels[i]; - let relItem = Zotero.URI.getURIItem(rel); + let relItem = yield Zotero.URI.getURIItem(rel); if (relItem) { content += '\t\t\t\t\t<li id="item_' + relItem.key + '">'; content += escapeXML(relItem.getDisplayTitle()); diff --git a/chrome/content/zotero/xpcom/uri.js b/chrome/content/zotero/xpcom/uri.js @@ -235,13 +235,13 @@ Zotero.URI = new function () { * Convert an item URI into an item * * @param {String} itemURI - * @return {Zotero.Item|false} + * @return {Promise<Zotero.Item|false>} */ - this.getURIItem = function (itemURI) { + this.getURIItem = Zotero.Promise.method(function (itemURI) { var obj = this._getURIObject(itemURI, 'item'); if (!obj) return false; - return Zotero.Items.getByLibraryAndKey(obj.libraryID, obj.key); - }; + return Zotero.Items.getByLibraryAndKeyAsync(obj.libraryID, obj.key); + }); /** @@ -252,10 +252,6 @@ Zotero.URI = new function () { return this._getURIObject(itemURI, 'item'); } - this.getURIFeedItem = function (feedItemURI) { - return this._getURIObject(feedItemURI, 'feedItem'); - } - /** * @param {String} itemURI * @return {Integer|FALSE} - itemID of matching item, or FALSE if none @@ -272,13 +268,13 @@ Zotero.URI = new function () { * * @param {String} collectionURI * @param {Zotero.Collection|FALSE} - * @return {Zotero.Collection|false} + * @return {Promise<Zotero.Collection|false>} */ - this.getURICollection = function (collectionURI) { + this.getURICollection = Zotero.Promise.method(function (collectionURI) { var obj = this._getURIObject(collectionURI, 'collection'); if (!obj) return false; - return Zotero.Collections.getByLibraryAndKey(obj.libraryID, obj.key); - }; + return Zotero.Collections.getByLibraryAndKeyAsync(obj.libraryID, obj.key); + }); /** diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js @@ -631,7 +631,7 @@ describe("Zotero.CollectionTreeView", function() { assert.equal(treeRow.ref.libraryID, group.libraryID); assert.equal(treeRow.ref.id, ids[0]); // New item should link back to original - var linked = item.getLinkedItem(group.libraryID); + var linked = yield item.getLinkedItem(group.libraryID); assert.equal(linked.id, treeRow.ref.id); // Check attachment @@ -641,7 +641,7 @@ describe("Zotero.CollectionTreeView", function() { treeRow = itemsView.getRow(1); assert.equal(treeRow.ref.id, ids[1]); // New attachment should link back to original - linked = attachment.getLinkedItem(group.libraryID); + linked = yield attachment.getLinkedItem(group.libraryID); assert.equal(linked.id, treeRow.ref.id); return group.eraseTx(); @@ -673,7 +673,7 @@ describe("Zotero.CollectionTreeView", function() { var item = yield createDataObject('item', false, { skipSelect: true }); yield drop('item', 'L' + group.libraryID, [item.id]); - var droppedItem = item.getLinkedItem(group.libraryID); + var droppedItem = yield item.getLinkedItem(group.libraryID); droppedItem.setCollections([collection.id]); droppedItem.deleted = true; yield droppedItem.saveTx(); diff --git a/test/tests/dataObjectTest.js b/test/tests/dataObjectTest.js @@ -451,7 +451,7 @@ describe("Zotero.DataObject", function() { var item2URI = Zotero.URI.getItemURI(item2); yield item2.addLinkedItem(item1); - var linkedItem = item1.getLinkedItem(item2.libraryID); + var linkedItem = yield item1.getLinkedItem(item2.libraryID); assert.equal(linkedItem.id, item2.id); }) @@ -462,7 +462,7 @@ describe("Zotero.DataObject", function() { var item2 = yield createDataObject('item', { libraryID: group.libraryID }); yield item2.addLinkedItem(item1); - var linkedItem = item2.getLinkedItem(item1.libraryID); + var linkedItem = yield item2.getLinkedItem(item1.libraryID); assert.isFalse(linkedItem); }) @@ -473,7 +473,7 @@ describe("Zotero.DataObject", function() { var item2 = yield createDataObject('item', { libraryID: group.libraryID }); yield item2.addLinkedItem(item1); - var linkedItem = item2.getLinkedItem(item1.libraryID, true); + var linkedItem = yield item2.getLinkedItem(item1.libraryID, true); assert.equal(linkedItem.id, item1.id); }) })