From d871e2540bb6c8efc739f1e670f030a621ed724b Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 7 Mar 2016 01:11:26 -0500 Subject: [PATCH 01/31] Revert "Make citing work via right-click" This reverts commit f95832d4a950b4c2dc6de9757256f5022ae05860. No longer necessary with deasyncification. --- chrome/content/zotero/fileInterface.js | 17 ++--- chrome/content/zotero/xpcom/cite.js | 99 ++++++++++---------------- chrome/content/zotero/xpcom/style.js | 6 +- 3 files changed, 51 insertions(+), 71 deletions(-) diff --git a/chrome/content/zotero/fileInterface.js b/chrome/content/zotero/fileInterface.js index 82fe32c6c9..0b3f58a286 100644 --- a/chrome/content/zotero/fileInterface.js +++ b/chrome/content/zotero/fileInterface.js @@ -126,6 +126,7 @@ var Zotero_File_Interface = new function() { this.exportItems = exportItems; this.bibliographyFromCollection = bibliographyFromCollection; this.bibliographyFromItems = bibliographyFromItems; + this.copyItemsToClipboard = copyItemsToClipboard; this.copyCitationToClipboard = copyCitationToClipboard; /** @@ -407,7 +408,7 @@ var Zotero_File_Interface = new function() { * * Does not check that items are actual references (and not notes or attachments) */ - this.copyItemsToClipboard = Zotero.Promise.coroutine(function* (items, style, locale, asHTML, asCitations) { + function copyItemsToClipboard(items, style, locale, asHTML, asCitations) { // copy to clipboard var transferable = Components.classes["@mozilla.org/widget/transferable;1"]. createInstance(Components.interfaces.nsITransferable); @@ -417,7 +418,7 @@ var Zotero_File_Interface = new function() { var cslEngine = style.getCiteProc(locale); // add HTML - var bibliography = yield Zotero.Cite.makeFormattedBibliographyOrCitationList(cslEngine, items, "html", asCitations); + var bibliography = Zotero.Cite.makeFormattedBibliographyOrCitationList(cslEngine, items, "html", asCitations); var str = Components.classes["@mozilla.org/supports-string;1"]. createInstance(Components.interfaces.nsISupportsString); str.data = bibliography; @@ -427,7 +428,7 @@ var Zotero_File_Interface = new function() { // add text (or HTML source) if(!asHTML) { cslEngine = style.getCiteProc(locale); - var bibliography = yield Zotero.Cite.makeFormattedBibliographyOrCitationList(cslEngine, items, "text", asCitations); + var bibliography = Zotero.Cite.makeFormattedBibliographyOrCitationList(cslEngine, items, "text", asCitations); } var str = Components.classes["@mozilla.org/supports-string;1"]. createInstance(Components.interfaces.nsISupportsString); @@ -436,7 +437,7 @@ var Zotero_File_Interface = new function() { transferable.setTransferData("text/unicode", str, bibliography.length*2); clipboardService.setData(transferable, null, Components.interfaces.nsIClipboard.kGlobalClipboard); - }); + } /* @@ -483,7 +484,7 @@ var Zotero_File_Interface = new function() { /* * Shows bibliography options and creates a bibliography */ - let _doBibliographyOptions = Zotero.Promise.coroutine(function* (name, items) { + function _doBibliographyOptions(name, items) { // make sure at least one item is not a standalone note or attachment var haveRegularItem = false; for each(var item in items) { @@ -515,12 +516,12 @@ var Zotero_File_Interface = new function() { // generate bibliography try { if(io.method == 'copy-to-clipboard') { - yield Zotero_File_Interface.copyItemsToClipboard(items, io.style, locale, false, io.mode === "citations"); + Zotero_File_Interface.copyItemsToClipboard(items, io.style, locale, false, io.mode === "citations"); } else { var style = Zotero.Styles.get(io.style); var cslEngine = style.getCiteProc(locale); - var bibliography = yield Zotero.Cite.makeFormattedBibliographyOrCitationList(cslEngine, + var bibliography = Zotero.Cite.makeFormattedBibliographyOrCitationList(cslEngine, items, format, io.mode === "citations"); } } catch(e) { @@ -598,7 +599,7 @@ var Zotero_File_Interface = new function() { fStream.close(); } } - }); + } function _saveBibliography(name, format) { diff --git a/chrome/content/zotero/xpcom/cite.js b/chrome/content/zotero/xpcom/cite.js index 28adaadde4..c542a99ef5 100644 --- a/chrome/content/zotero/xpcom/cite.js +++ b/chrome/content/zotero/xpcom/cite.js @@ -71,9 +71,9 @@ Zotero.Cite = { * @param {String} format The format of the output (html, text, or rtf) * @return {String} Bibliography or item list in specified format */ - "makeFormattedBibliographyOrCitationList":Zotero.Promise.coroutine(function* (cslEngine, items, format, asCitationList) { + "makeFormattedBibliographyOrCitationList":function(cslEngine, items, format, asCitationList) { cslEngine.setOutputFormat(format); - yield cslEngine.updateItems(items.map(item => item.id)); + cslEngine.updateItems(items.map(item => item.id)); if(!asCitationList) { var bibliography = Zotero.Cite.makeFormattedBibliography(cslEngine, format); @@ -84,7 +84,7 @@ Zotero.Cite = { var citations=[]; for (var i=0, ilen=items.length; i Date: Mon, 7 Mar 2016 16:05:51 -0500 Subject: [PATCH 02/31] Deasyncification :back: :cry: While trying to get translation and citing working with asynchronously generated data, we realized that drag-and-drop support was going to be...problematic. Firefox only supports synchronous methods for providing drag data (unlike, it seems, the DataTransferItem interface supported by Chrome), which means that we'd need to preload all relevant data on item selection (bounded by export.quickCopy.dragLimit) and keep the translate/cite methods synchronous (or maintain two separate versions). What we're trying instead is doing what I said in #518 we weren't going to do: loading most object data on startup and leaving many more functions synchronous. Essentially, this takes the various load*() methods described in #518, moves them to startup, and makes them operate on entire libraries rather than individual objects. The obvious downside here (other than undoing much of the work of the last many months) is that it increases startup time, potentially quite a lot for larger libraries. On my laptop, with a 3,000-item library, this adds about 3 seconds to startup time. I haven't yet tested with larger libraries. But I'm hoping that we can optimize this further to reduce that delay. Among other things, this is loading data for all libraries, when it should be able to load data only for the library being viewed. But this is also fundamentally just doing some SELECT queries and storing the results, so it really shouldn't need to be that slow (though performance may be bounded a bit here by XPCOM overhead). If we can make this fast enough, it means that third-party plugins should be able to remain much closer to their current designs. (Some things, including saving, will still need to be made asynchronous.) --- chrome/content/zotero/bindings/noteeditor.xml | 61 +- chrome/content/zotero/bindings/relatedbox.xml | 182 +++-- chrome/content/zotero/bindings/tagsbox.xml | 33 +- chrome/content/zotero/duplicatesMerge.js | 6 +- chrome/content/zotero/itemPane.js | 3 - chrome/content/zotero/locateMenu.js | 1 - chrome/content/zotero/recognizePDF.js | 1 - chrome/content/zotero/xpcom/api.js | 1 - chrome/content/zotero/xpcom/attachments.js | 3 +- chrome/content/zotero/xpcom/cite.js | 1 - .../content/zotero/xpcom/collectionTreeRow.js | 1 + .../zotero/xpcom/collectionTreeView.js | 22 +- .../content/zotero/xpcom/data/collection.js | 169 +---- .../content/zotero/xpcom/data/collections.js | 100 ++- chrome/content/zotero/xpcom/data/creators.js | 40 +- .../content/zotero/xpcom/data/dataObject.js | 121 +--- .../content/zotero/xpcom/data/dataObjects.js | 273 +++++++- chrome/content/zotero/xpcom/data/item.js | 417 ++++-------- chrome/content/zotero/xpcom/data/items.js | 636 +++++++++++++----- chrome/content/zotero/xpcom/db.js | 8 +- chrome/content/zotero/xpcom/itemTreeView.js | 163 ++--- chrome/content/zotero/xpcom/search.js | 190 +++--- chrome/content/zotero/xpcom/storage.js | 4 +- .../zotero/xpcom/storage/storageEngine.js | 4 +- .../zotero/xpcom/storage/storageLocal.js | 155 +---- chrome/content/zotero/xpcom/storage/webdav.js | 69 +- chrome/content/zotero/xpcom/storage/zfs.js | 47 +- .../content/zotero/xpcom/sync/syncEngine.js | 2 +- chrome/content/zotero/xpcom/sync/syncLocal.js | 6 +- chrome/content/zotero/xpcom/timeline.js | 1 - .../xpcom/translation/translate_item.js | 8 +- chrome/content/zotero/xpcom/utilities.js | 5 +- .../zotero/xpcom/utilities_internal.js | 72 +- chrome/content/zotero/xpcom/zotero.js | 12 +- chrome/content/zotero/zoteroPane.js | 20 +- components/zotero-protocol-handler.js | 18 +- test/content/support.js | 7 +- test/resource/chai | 2 +- test/resource/mocha | 2 +- test/tests/collectionTest.js | 6 - test/tests/collectionTreeViewTest.js | 8 +- test/tests/creatorsTest.js | 21 + test/tests/dataObjectTest.js | 4 +- test/tests/dataObjectUtilitiesTest.js | 4 +- test/tests/fileInterfaceTest.js | 2 +- test/tests/itemTest.js | 60 +- test/tests/preferences_syncTest.js | 2 +- test/tests/relatedboxTest.js | 6 +- test/tests/searchTest.js | 12 +- test/tests/storageLocalTest.js | 72 +- test/tests/syncEngineTest.js | 16 +- test/tests/syncLocalTest.js | 67 +- test/tests/translateTest.js | 2 +- test/tests/utilitiesTest.js | 26 +- test/tests/webdavTest.js | 38 +- test/tests/zfsTest.js | 41 +- test/tests/zoteroPaneTest.js | 2 +- 57 files changed, 1648 insertions(+), 1607 deletions(-) create mode 100644 test/tests/creatorsTest.js diff --git a/chrome/content/zotero/bindings/noteeditor.xml b/chrome/content/zotero/bindings/noteeditor.xml index d9b17c03e5..37659c63a4 100644 --- a/chrome/content/zotero/bindings/noteeditor.xml +++ b/chrome/content/zotero/bindings/noteeditor.xml @@ -417,48 +417,41 @@ 0) { - var x = this.boxObject.screenX; - var y = this.boxObject.screenY; - this.id('relatedPopup').openPopupAtScreen(x, y, false); - } - else { - this.id('related').add(); - } - }, this); + var relatedList = this.item.relatedItems; + if (relatedList.length > 0) { + var x = this.boxObject.screenX; + var y = this.boxObject.screenY; + this.id('relatedPopup').openPopupAtScreen(x, y, false); + } + else { + this.id('related').add(); + } ]]> diff --git a/chrome/content/zotero/bindings/relatedbox.xml b/chrome/content/zotero/bindings/relatedbox.xml index aa8cc30efe..3989dbbeaf 100644 --- a/chrome/content/zotero/bindings/relatedbox.xml +++ b/chrome/content/zotero/bindings/relatedbox.xml @@ -74,25 +74,20 @@ Zotero.Promise.check(this.item)); - var keys = this.item.relatedItems; - if (keys.length) { - let items = yield Zotero.Items.getAsync(keys) - .tap(() => Zotero.Promise.check(this.item)); - for (let item of items) { - r = r + item.getDisplayTitle() + ", "; - } - r = r.substr(0,r.length-2); + var r = ""; + + if (this.item) { + var keys = this.item.relatedItems; + if (keys.length) { + for (let key of keys) { + let item = Zotero.Items.getByLibraryAndKey(this.item.libraryID, key); + r = r + item.getDisplayTitle() + ", "; } + r = r.substr(0,r.length-2); } - - return r; - }, this); + } + + return r; ]]> @@ -129,89 +124,79 @@ - - Zotero.Promise.check(this.item)); - var relatedKeys = this.item.relatedItems; - for (var i = 0; i < relatedKeys.length; i++) { - let key = relatedKeys[i]; - let relatedItem = - yield Zotero.Items.getByLibraryAndKeyAsync( - this.item.libraryID, key - ) - .tap(() => Zotero.Promise.check(this.item)); - let id = relatedItem.id; - yield relatedItem.loadItemData() - .tap(() => Zotero.Promise.check(this.item)); - let icon = document.createElement("image"); - icon.className = "zotero-box-icon"; - let type = Zotero.ItemTypes.getName(relatedItem.itemTypeID); - if (type=='attachment') - { - switch (relatedItem.attaachmentLinkMode) { - case Zotero.Attachments.LINK_MODE_LINKED_URL: - type += '-web-link'; - break; - - case Zotero.Attachments.LINK_MODE_IMPORTED_URL: - type += '-snapshot'; - break; - - case Zotero.Attachments.LINK_MODE_LINKED_FILE: - type += '-link'; - break; - - case Zotero.Attachments.LINK_MODE_IMPORTED_FILE: - type += '-file'; - break; - } + - + this.updateCount(relatedKeys.length); + } + ]]> Zotero.Promise.check(this.mode)); - var tags = this.item.getTags(); - if (tags) { - for(var i = 0; i < tags.length; i++) - { - r = r + tags[i].tag + ", "; - } - r = r.substr(0,r.length-2); - } - } + var r = ""; - return r; - }, this); + if (this.item) { + var tags = this.item.getTags(); + if (tags) { + for(var i = 0; i < tags.length; i++) + { + r = r + tags[i].tag + ", "; + } + r = r.substr(0,r.length-2); + } + } + + return r; ]]> @@ -211,9 +207,6 @@ return Zotero.spawn(function* () { Zotero.debug('Reloading tags box'); - yield this.item.loadTags() - .tap(() => Zotero.Promise.check(this.mode)); - // Cancel field focusing while we're updating this._reloading = true; diff --git a/chrome/content/zotero/duplicatesMerge.js b/chrome/content/zotero/duplicatesMerge.js index 27bf6ec582..f6682aae73 100644 --- a/chrome/content/zotero/duplicatesMerge.js +++ b/chrome/content/zotero/duplicatesMerge.js @@ -131,9 +131,9 @@ var Zotero_Duplicates_Pane = new function () { // alternative values so that they're still available if the item box // modifies the item Zotero.spawn(function* () { - var diff = yield item.multiDiff(_otherItems, _ignoreFields); + var diff = item.multiDiff(_otherItems, _ignoreFields); if (diff) { - let itemValues = yield item.toJSON(); + let itemValues = item.toJSON(); for (let i in diff) { diff[i].unshift(itemValues[i] !== undefined ? itemValues[i] : ''); } @@ -141,8 +141,6 @@ var Zotero_Duplicates_Pane = new function () { } var newItem = yield item.copy(); - yield newItem.loadItemData(); - yield newItem.loadCreators(); itembox.item = newItem; }); } diff --git a/chrome/content/zotero/itemPane.js b/chrome/content/zotero/itemPane.js index 8c6dd2e1a3..45bfad1324 100644 --- a/chrome/content/zotero/itemPane.js +++ b/chrome/content/zotero/itemPane.js @@ -94,13 +94,11 @@ var ZoteroItemPane = new function() { _notesList.removeChild(_notesList.firstChild); } - yield item.loadChildItems(); let notes = yield Zotero.Items.getAsync(item.getNotes()); if (notes.length) { for (var i = 0; i < notes.length; i++) { let note = notes[i]; let id = notes[i].id; - yield note.loadItemData(); var icon = document.createElement('image'); icon.className = "zotero-box-icon"; @@ -148,7 +146,6 @@ var ZoteroItemPane = new function() { box.mode = 'edit'; } - yield Zotero.Promise.all([item.loadItemData(), item.loadCreators()]); box.item = item; }); diff --git a/chrome/content/zotero/locateMenu.js b/chrome/content/zotero/locateMenu.js index 836d101dff..9d960f443a 100644 --- a/chrome/content/zotero/locateMenu.js +++ b/chrome/content/zotero/locateMenu.js @@ -400,7 +400,6 @@ var Zotero_LocateMenu = new function() { } if(item.isRegularItem()) { - yield item.loadChildItems(); var attachments = item.getAttachments(); if(attachments) { // look through url fields for non-file:/// attachments diff --git a/chrome/content/zotero/recognizePDF.js b/chrome/content/zotero/recognizePDF.js index 316e089e13..4846282aa0 100644 --- a/chrome/content/zotero/recognizePDF.js +++ b/chrome/content/zotero/recognizePDF.js @@ -395,7 +395,6 @@ var Zotero_RecognizePDF = new function() { } // put new item in same collections as the old one - yield item.loadCollections(); let itemCollections = item.getCollections(); for (let i = 0; i < itemCollections.length; i++) { let collection = yield Zotero.Collections.getAsync(itemCollections[i]); diff --git a/chrome/content/zotero/xpcom/api.js b/chrome/content/zotero/xpcom/api.js index 81229692f3..fa5ec440e5 100644 --- a/chrome/content/zotero/xpcom/api.js +++ b/chrome/content/zotero/xpcom/api.js @@ -56,7 +56,6 @@ Zotero.API = { if (!col) { throw new Error('Invalid collection ID or key'); } - yield col.loadChildItems(); results = col.getChildItems(); break; diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 012170d9c7..24ddfedbbe 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -1090,8 +1090,7 @@ Zotero.Attachments = new function(){ Zotero.DB.requireTransaction(); - attachment.loadItemData(); - var newAttachment = yield attachment.clone(libraryID); + var newAttachment = attachment.clone(libraryID); if (attachment.isImportedAttachment()) { // Attachment path isn't copied over by clone() if libraryID is different newAttachment.attachmentPath = attachment.attachmentPath; diff --git a/chrome/content/zotero/xpcom/cite.js b/chrome/content/zotero/xpcom/cite.js index c542a99ef5..467164b951 100644 --- a/chrome/content/zotero/xpcom/cite.js +++ b/chrome/content/zotero/xpcom/cite.js @@ -529,7 +529,6 @@ Zotero.Cite.System.prototype = { throw "Zotero.Cite.System.retrieveItem called on non-item "+item; } - throw new Error("Unimplemented"); var cslItem = Zotero.Utilities.itemToCSLJSON(zoteroItem); // TEMP: citeproc-js currently expects the id property to be the item DB id diff --git a/chrome/content/zotero/xpcom/collectionTreeRow.js b/chrome/content/zotero/xpcom/collectionTreeRow.js index d3a7734758..f54a09045c 100644 --- a/chrome/content/zotero/xpcom/collectionTreeRow.js +++ b/chrome/content/zotero/xpcom/collectionTreeRow.js @@ -295,6 +295,7 @@ Zotero.CollectionTreeRow.prototype.getSearchObject = Zotero.Promise.coroutine(fu // Create the outer (filter) search var s2 = new Zotero.Search(); + if (this.isTrash()) { s2.addCondition('deleted', 'true'); } diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index ced95a6827..fc35fb239a 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -526,7 +526,7 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun ); } else if (objectType == 'search') { - let search = yield Zotero.Searches.getAsync(id); + let search = Zotero.Searches.get(id); let libraryID = search.libraryID; let startRow = this._rowMap['L' + libraryID]; @@ -545,7 +545,6 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun var inSearches = false; for (let i = startRow; i < this.rowCount; i++) { let treeRow = this.getRow(i); - Zotero.debug(treeRow.id); beforeRow = i; // If we've reached something other than collections, stop @@ -1504,10 +1503,6 @@ Zotero.CollectionTreeView.prototype.canDropCheckAsync = Zotero.Promise.coroutine } if (dataType == 'zotero/item') { - if (treeRow.isCollection()) { - yield treeRow.ref.loadChildItems(); - } - var ids = data; var items = Zotero.Items.get(ids); var skip = true; @@ -1627,7 +1622,6 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r // If linked item is in the trash, undelete it and remove it from collections // (since it shouldn't be restored to previous collections) if (linkedItem.deleted) { - yield linkedItem.loadCollections(); linkedItem.setCollections(); linkedItem.deleted = false; yield linkedItem.save({ @@ -1693,7 +1687,7 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r } // Create new clone item in target library - var newItem = yield item.clone(targetLibraryID, false, !options.tags); + var newItem = item.clone(targetLibraryID, false, !options.tags); // Set Rights field for My Publications if (options.license) { @@ -1717,11 +1711,10 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r // Child notes if (options.childNotes) { - yield item.loadChildItems(); var noteIDs = item.getNotes(); - var notes = yield Zotero.Items.getAsync(noteIDs); + var notes = Zotero.Items.get(noteIDs); for each(var note in notes) { - let newNote = yield note.clone(targetLibraryID); + let newNote = note.clone(targetLibraryID); newNote.parentID = newItemID; yield newNote.save({ skipSelect: true @@ -1733,9 +1726,8 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r // Child attachments if (options.childLinks || options.childFileAttachments) { - yield item.loadChildItems(); var attachmentIDs = item.getAttachments(); - var attachments = yield Zotero.Items.getAsync(attachmentIDs); + var attachments = Zotero.Items.get(attachmentIDs); for each(var attachment in attachments) { var linkMode = attachment.attachmentLinkMode; @@ -1864,8 +1856,8 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r if (targetTreeRow.isPublications()) { let items = yield Zotero.Items.getAsync(ids); - let io = yield this._treebox.treeBody.ownerDocument.defaultView.ZoteroPane - .showPublicationsWizard(items); + let io = this._treebox.treeBody.ownerDocument.defaultView + .ZoteroPane.showPublicationsWizard(items); if (!io) { return; } diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 7205c4211a..b68b90e161 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -28,11 +28,8 @@ Zotero.Collection = function(params = {}) { this._name = null; - this._hasChildCollections = null; - this._childCollections = []; - - this._hasChildItems = false; - this._childItems = []; + this._childCollections = new Set(); + this._childItems = new Set(); Zotero.Utilities.assignProps(this, params, ['name', 'libraryID', 'parentID', 'parentKey', 'lastSync']); @@ -162,19 +159,13 @@ Zotero.Collection.prototype.loadFromRow = function(row) { Zotero.Collection.prototype.hasChildCollections = function() { - if (this._hasChildCollections !== null) { - return this._hasChildCollections; - } - this._requireData('primaryData'); - return false; + this._requireData('childCollections'); + return this._childCollections.size > 0; } Zotero.Collection.prototype.hasChildItems = function() { - if (this._hasChildItems !== null) { - return this._hasChildItems; - } - this._requireData('primaryData'); - return false; + this._requireData('childItems'); + return this._childItems.size > 0; } @@ -189,19 +180,11 @@ Zotero.Collection.prototype.getChildCollections = function (asIDs) { // Return collectionIDs if (asIDs) { - var ids = []; - for each(var col in this._childCollections) { - ids.push(col.id); - } - return ids; + return this._childCollections.values(); } // Return Zotero.Collection objects - var objs = []; - for each(var col in this._childCollections) { - objs.push(col); - } - return objs; + return Array.from(this._childCollections).map(id => this.ObjectsClass.get(id)); } @@ -215,13 +198,14 @@ Zotero.Collection.prototype.getChildCollections = function (asIDs) { Zotero.Collection.prototype.getChildItems = function (asIDs, includeDeleted) { this._requireData('childItems'); - if (this._childItems.length == 0) { + if (this._childItems.size == 0) { return []; } // Remove deleted items if necessary var childItems = []; - for each(var item in this._childItems) { + for (let itemID of this._childItems) { + let item = this.ChildObjects.get(itemID); if (includeDeleted || !item.deleted) { childItems.push(item); } @@ -229,19 +213,11 @@ Zotero.Collection.prototype.getChildItems = function (asIDs, includeDeleted) { // Return itemIDs if (asIDs) { - var ids = []; - for each(var item in childItems) { - ids.push(item.id); - } - return ids; + return childItems.map(item => item.id); } // Return Zotero.Item objects - var objs = []; - for each(var item in childItems) { - objs.push(item); - } - return objs; + return childItems.slice(); } Zotero.Collection.prototype._initSave = Zotero.Promise.coroutine(function* (env) { @@ -388,7 +364,6 @@ Zotero.Collection.prototype.addItems = Zotero.Promise.coroutine(function* (itemI return; } - yield this.loadChildItems(); var current = this.getChildItems(true); Zotero.DB.requireTransaction(); @@ -400,15 +375,14 @@ Zotero.Collection.prototype.addItems = Zotero.Promise.coroutine(function* (itemI continue; } - let item = yield this.ChildObjects.getAsync(itemID); - yield item.loadCollections(); + let item = this.ChildObjects.get(itemID); item.addToCollection(this.id); yield item.save({ skipDateModifiedUpdate: true }); } - yield this.loadChildItems(true); + yield this._loadDataType('childItems'); }); /** @@ -434,7 +408,6 @@ Zotero.Collection.prototype.removeItems = Zotero.Promise.coroutine(function* (it return; } - yield this.loadChildItems(); var current = this.getChildItems(true); return Zotero.DB.executeTransaction(function* () { @@ -447,7 +420,6 @@ Zotero.Collection.prototype.removeItems = Zotero.Promise.coroutine(function* (it } let item = yield this.ChildObjects.getAsync(itemID); - yield item.loadCollections(); item.removeFromCollection(this.id); yield item.save({ skipDateModifiedUpdate: true @@ -455,7 +427,7 @@ Zotero.Collection.prototype.removeItems = Zotero.Promise.coroutine(function* (it } }.bind(this)); - yield this.loadChildItems(true); + yield this._loadDataType('childItems'); }); @@ -464,13 +436,7 @@ Zotero.Collection.prototype.removeItems = Zotero.Promise.coroutine(function* (it **/ Zotero.Collection.prototype.hasItem = function(itemID) { this._requireData('childItems'); - - for (let i=0; i 0; + this._childCollections.delete(collectionID); } } @@ -971,8 +861,7 @@ Zotero.Collection.prototype._registerChildItem = function (itemID) { if (this._loaded.childItems) { let item = this.ChildObjects.get(itemID); if (item) { - this._hasChildItems = true; - this._childItems.push(item); + this._childItems.add(itemID); } } } @@ -983,12 +872,6 @@ Zotero.Collection.prototype._registerChildItem = function (itemID) { */ Zotero.Collection.prototype._unregisterChildItem = function (itemID) { if (this._loaded.childItems) { - for (let i = 0; i < this._childItems.length; i++) { - if (this._childItems[i].id == itemID) { - this._childItems.splice(i, 1); - break; - } - } - this._hasChildItems = this._childItems.length > 0; + this._childItems.delete(itemID); } } diff --git a/chrome/content/zotero/xpcom/data/collections.js b/chrome/content/zotero/xpcom/data/collections.js index a27263b108..75b185a8e8 100644 --- a/chrome/content/zotero/xpcom/data/collections.js +++ b/chrome/content/zotero/xpcom/data/collections.js @@ -85,8 +85,7 @@ Zotero.Collections = function() { let children; if (parentID) { - let parent = yield Zotero.Collections.getAsync(parentID); - yield parent.loadChildCollections(); + let parent = Zotero.Collections.get(parentID); children = parent.getChildCollections(); } else if (libraryID) { let sql = "SELECT collectionID AS id FROM collections " @@ -156,6 +155,103 @@ Zotero.Collections = function() { } + this._loadChildCollections = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { + var sql = "SELECT C1.collectionID, C2.collectionID AS childCollectionID " + + "FROM collections C1 LEFT JOIN collections C2 ON (C1.collectionID=C2.parentCollectionID) " + + "WHERE C1.libraryID=?" + + (ids.length ? " AND C1.collectionID IN (" + ids.map(id => parseInt(id)).join(", ") + ")" : ""); + var params = [libraryID]; + var lastID; + var rows = []; + var setRows = function (collectionID, rows) { + var collection = this._objectCache[collectionID]; + if (!collection) { + throw new Error("Collection " + collectionID + " not found"); + } + + collection._childCollections = new Set(rows); + collection._loaded.childCollections = true; + collection._clearChanged('childCollections'); + }.bind(this); + + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + let collectionID = row.getResultByIndex(0); + + if (lastID && collectionID !== lastID) { + setRows(lastID, rows); + rows = []; + } + + lastID = collectionID; + + let childCollectionID = row.getResultByIndex(1); + // No child collections + if (childCollectionID === null) { + return; + } + rows.push(childCollectionID); + } + } + ); + if (lastID) { + setRows(lastID, rows); + } + }); + + + this._loadChildItems = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { + var sql = "SELECT collectionID, itemID FROM collections " + + "LEFT JOIN collectionItems USING (collectionID) " + + "WHERE libraryID=?" + idSQL; + var params = [libraryID]; + var lastID; + var rows = []; + var setRows = function (collectionID, rows) { + var collection = this._objectCache[collectionID]; + if (!collection) { + throw new Error("Collection " + collectionID + " not found"); + } + + collection._childItems = new Set(rows); + collection._loaded.childItems = true; + collection._clearChanged('childItems'); + }.bind(this); + + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + let collectionID = row.getResultByIndex(0); + + if (lastID && collectionID !== lastID) { + setRows(lastID, rows); + rows = []; + } + + lastID = collectionID; + + let itemID = row.getResultByIndex(1); + // No child items + if (itemID === null) { + return; + } + rows.push(itemID); + } + } + ); + if (lastID) { + setRows(lastID, rows); + } + }); + + this.registerChildCollection = function (collectionID, childCollectionID) { if (this._objectCache[collectionID]) { this._objectCache[collectionID]._registerChildCollection(childCollectionID); diff --git a/chrome/content/zotero/xpcom/data/creators.js b/chrome/content/zotero/xpcom/data/creators.js index e65d6e0507..909de38c59 100644 --- a/chrome/content/zotero/xpcom/data/creators.js +++ b/chrome/content/zotero/xpcom/data/creators.js @@ -30,29 +30,35 @@ Zotero.Creators = new function() { var _cache = {}; + this.init = Zotero.Promise.coroutine(function* (libraryID) { + var sql = "SELECT * FROM creators"; + var rows = yield Zotero.DB.queryAsync(sql); + for (let i = 0; i < rows.length; i++) { + let row = rows[i]; + _cache[row.creatorID] = this.cleanData({ + // Avoid "DB column 'name' not found" warnings from the DB row Proxy + firstName: row.firstName, + lastName: row.lastName, + fieldMode: row.fieldMode + }); + } + }); + /* * Returns creator data in internal format for a given creatorID */ - this.getAsync = Zotero.Promise.coroutine(function* (creatorID) { + this.get = function (creatorID) { if (!creatorID) { throw new Error("creatorID not provided"); } - if (_cache[creatorID]) { - return this.cleanData(_cache[creatorID]); - } - - var sql = "SELECT * FROM creators WHERE creatorID=?"; - var row = yield Zotero.DB.rowQueryAsync(sql, creatorID); - if (!row) { + if (!_cache[creatorID]) { throw new Error("Creator " + creatorID + " not found"); } - return _cache[creatorID] = this.cleanData({ - firstName: row.firstName, // avoid "DB column 'name' not found" warnings from the DB row Proxy - lastName: row.lastName, - fieldMode: row.fieldMode - }); - }); + + // Return copy of data + return this.cleanData(_cache[creatorID]); + }; this.getItemsWithCreator = function (creatorID) { @@ -87,12 +93,10 @@ Zotero.Creators = new function() { id = yield Zotero.ID.get('creators'); let sql = "INSERT INTO creators (creatorID, firstName, lastName, fieldMode) " + "VALUES (?, ?, ?, ?)"; - let insertID = yield Zotero.DB.queryAsync( + yield Zotero.DB.queryAsync( sql, [id, data.firstName, data.lastName, data.fieldMode] ); - if (!id) { - id = insertID; - } + _cache[id] = data; } return id; }); diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index 183544f585..05d5b59c26 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -401,7 +401,7 @@ Zotero.DataObject.prototype.setRelations = function (newRelations) { // Relations are stored internally as a flat array with individual predicate-object pairs, // so convert the incoming relations to that - var newRelationsFlat = this._flattenRelations(newRelations); + var newRelationsFlat = this.ObjectsClass.flattenRelations(newRelations); var changed = false; if (oldRelations.length != newRelationsFlat.length) { @@ -457,8 +457,6 @@ Zotero.DataObject.prototype._getLinkedObject = Zotero.Promise.coroutine(function throw new Error(this._ObjectType + " is already in library " + libraryID); } - yield this.loadRelations(); - var predicate = Zotero.Relations.linkedObjectPredicate; var libraryObjectPrefix = Zotero.URI.getLibraryURI(libraryID) + "/" + this._objectTypePlural + "/"; @@ -514,8 +512,6 @@ Zotero.DataObject.prototype._addLinkedObject = Zotero.Promise.coroutine(function throw new Error("Can't add linked " + this._objectType + " in same library"); } - yield this.loadRelations(); - var predicate = Zotero.Relations.linkedObjectPredicate; var thisURI = Zotero.URI['get' + this._ObjectType + 'URI'](this); var objectURI = Zotero.URI['get' + this._ObjectType + 'URI'](object); @@ -539,7 +535,6 @@ Zotero.DataObject.prototype._addLinkedObject = Zotero.Promise.coroutine(function }); } else { - yield object.loadRelations(); object.addRelation(predicate, thisURI); yield object.save({ skipDateModifiedUpdate: true, @@ -551,9 +546,11 @@ Zotero.DataObject.prototype._addLinkedObject = Zotero.Promise.coroutine(function }); -/* - * Build object from database - */ +// +// Bulk data loading functions +// +// These are called by Zotero.DataObjects.prototype._loadDataType(). +// Zotero.DataObject.prototype.loadPrimaryData = Zotero.Promise.coroutine(function* (reload, failOnMissing) { if (this._loaded.primaryData && !reload) return; @@ -610,65 +607,6 @@ Zotero.DataObject.prototype.loadPrimaryData = Zotero.Promise.coroutine(function* }); -Zotero.DataObject.prototype.loadRelations = Zotero.Promise.coroutine(function* (reload) { - if (!this.ObjectsClass._relationsTable) { - throw new Error("Relations not supported for " + this._objectTypePlural); - } - - if (this._loaded.relations && !reload) { - return; - } - - Zotero.debug("Loading relations for " + this._objectType + " " + this.libraryKey); - - this._requireData('primaryData'); - - var sql = "SELECT predicate, object FROM " + this.ObjectsClass._relationsTable + " " - + "JOIN relationPredicates USING (predicateID) " - + "WHERE " + this.ObjectsClass.idColumn + "=?"; - var rows = yield Zotero.DB.queryAsync(sql, this.id); - - var relations = {}; - function addRel(predicate, object) { - if (!relations[predicate]) { - relations[predicate] = []; - } - relations[predicate].push(object); - } - - for (let i = 0; i < rows.length; i++) { - let row = rows[i]; - addRel(row.predicate, row.object); - } - - /*if (this._objectType == 'item') { - let getURI = Zotero.URI["get" + this._ObjectType + "URI"].bind(Zotero.URI); - let objectURI = getURI(this); - - // Related items are bidirectional, so include any pointing to this object - let objects = yield Zotero.Relations.getByPredicateAndObject( - Zotero.Relations.relatedItemPredicate, objectURI - ); - for (let i = 0; i < objects.length; i++) { - addRel(Zotero.Relations.relatedItemPredicate, getURI(objects[i])); - } - - // Also include any owl:sameAs relations pointing to this object - objects = yield Zotero.Relations.getByPredicateAndObject( - Zotero.Relations.linkedObjectPredicate, objectURI - ); - for (let i = 0; i < objects.length; i++) { - addRel(Zotero.Relations.linkedObjectPredicate, getURI(objects[i])); - } - }*/ - - // Relations are stored as predicate-object pairs - this._relations = this._flattenRelations(relations); - this._loaded.relations = true; - this._clearChanged('relations'); -}); - - /** * Reloads loaded, changed data * @@ -735,7 +673,7 @@ Zotero.DataObject.prototype._requireData = function (dataType) { * @param {Boolean} reload */ Zotero.DataObject.prototype._loadDataType = function (dataType, reload) { - return this["load" + dataType[0].toUpperCase() + dataType.substr(1)](reload); + return this._ObjectsClass._loadDataType(dataType, this.libraryID, [this.id]); } Zotero.DataObject.prototype.loadAllData = function (reload) { @@ -868,6 +806,16 @@ Zotero.DataObject.prototype.save = Zotero.Promise.coroutine(function* (options) Zotero.debug('Updating database with new ' + this._objectType + ' data', 4); } + if (env.options.skipAll) { + [ + 'skipDateModifiedUpdate', + 'skipClientDateModifiedUpdate', + 'skipSyncedUpdate', + 'skipEditCheck', + 'skipSelect' + ].forEach(x => env.options[x] = true); + } + try { if (Zotero.DataObject.prototype._finalizeSave == this._finalizeSave) { throw new Error("_finalizeSave not implemented for Zotero." + this._ObjectType); @@ -1214,16 +1162,16 @@ Zotero.DataObject.prototype._finalizeErase = Zotero.Promise.coroutine(function* }); -Zotero.DataObject.prototype.toResponseJSON = Zotero.Promise.coroutine(function* (options) { +Zotero.DataObject.prototype.toResponseJSON = function (options) { // TODO: library block? return { key: this.key, version: this.version, meta: {}, - data: yield this.toJSON(options) + data: this.toJSON(options) }; -}); +} Zotero.DataObject.prototype._preToJSON = function (options) { @@ -1272,32 +1220,3 @@ Zotero.DataObject.prototype._disabledCheck = function () { + "use Zotero." + this._ObjectTypePlural + ".getAsync()"); } } - - -/** - * Flatten API JSON relations object into an array of unique predicate-object pairs - * - * @param {Object} relations - Relations object in API JSON format, with predicates as keys - * and arrays of URIs as objects - * @return {Array[]} - Predicate-object pairs - */ -Zotero.DataObject.prototype._flattenRelations = function (relations) { - var relationsFlat = []; - for (let predicate in relations) { - let object = relations[predicate]; - if (Array.isArray(object)) { - object = Zotero.Utilities.arrayUnique(object); - for (let i = 0; i < object.length; i++) { - relationsFlat.push([predicate, object[i]]); - } - } - else if (typeof object == 'string') { - relationsFlat.push([predicate, object]); - } - else { - Zotero.debug(object, 1); - throw new Error("Invalid relation value"); - } - } - return relationsFlat; -} diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index 7626944352..0ce1a73e53 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -336,6 +336,253 @@ Zotero.DataObjects.prototype.getNewer = Zotero.Promise.method(function (libraryI }); +/** + * Loads data for a given data type + * @param {String} dataType + * @param {Integer} libraryID + * @param {Integer[]} [ids] + */ +Zotero.DataObjects.prototype._loadDataType = Zotero.Promise.coroutine(function* (dataType, libraryID, ids) { + var funcName = "_load" + dataType[0].toUpperCase() + dataType.substr(1) + // Single data types need an 's' (e.g., 'note' -> 'loadNotes()') + + ((dataType.endsWith('s') || dataType.endsWith('Data') ? '' : 's')); + if (!this[funcName]) { + throw new Error(`Zotero.${this._ZDO_Objects}.${funcName} is not a function`); + } + + if (ids && ids.length == 0) { + return; + } + + var t = new Date; + var libraryName = Zotero.Libraries.get(libraryID).name; + + var idSQL = ""; + if (ids) { + idSQL = " AND " + this.idColumn + " IN (" + ids.map(id => parseInt(id)).join(", ") + ")"; + } + + Zotero.debug("Loading " + dataType + + (ids + ? " for " + ids.length + " " + (ids.length == 1 ? this._ZDO_object : this._ZDO_objects) + : '') + + " in " + libraryName); + + yield this[funcName](libraryID, ids ? ids : [], idSQL); + + Zotero.debug(`Loaded ${dataType} in ${libraryName} in ${new Date() - t} ms`); +}); + +Zotero.DataObjects.prototype.loadAllData = Zotero.Promise.coroutine(function* (libraryID, ids) { + var t = new Date(); + var libraryName = Zotero.Libraries.get(libraryID).name; + + Zotero.debug("Loading all data" + + (ids ? " for " + ids.length + " " + this._ZDO_objects : '') + + " in " + libraryName); + + let dataTypes = this.ObjectClass.prototype._dataTypes; + for (let i = 0; i < dataTypes.length; i++) { + yield this._loadDataType(dataTypes[i], libraryID, ids); + } + + Zotero.debug(`Loaded all data in ${libraryName} in ${new Date() - t} ms`); +}); + + +Zotero.DataObjects.prototype._loadPrimaryData = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL, options) { + var loaded = {}; + + // If library isn't an integer (presumably false or null), skip it + if (parseInt(libraryID) != libraryID) { + libraryID = false; + } + + var sql = this.primaryDataSQL; + var params = []; + if (libraryID !== false) { + sql += ' AND O.libraryID=?'; + params.push(libraryID); + } + if (ids.length) { + sql += ' AND O.' + this._ZDO_id + ' IN (' + ids.join(',') + ')'; + } + + yield Zotero.DB.queryAsync( + sql, + params, + { + onRow: function (row) { + var id = row.getResultByName(this._ZDO_id); + var columns = Object.keys(this._primaryDataSQLParts); + var rowObj = {}; + for (let i=0; i} */ -Zotero.Item.prototype.clone = Zotero.Promise.coroutine(function* (libraryID, skipTags) { +Zotero.Item.prototype.clone = function (libraryID, skipTags) { Zotero.debug('Cloning item ' + this.id); if (libraryID !== undefined && libraryID !== null && typeof libraryID !== 'number') { throw new Error("libraryID must be null or an integer"); } - yield this.loadPrimaryData(); - if (libraryID === undefined || libraryID === null) { libraryID = this.libraryID; } @@ -3579,7 +3668,6 @@ Zotero.Item.prototype.clone = Zotero.Promise.coroutine(function* (libraryID, ski newItem.libraryID = libraryID; newItem.setType(this.itemTypeID); - yield this.loadItemData(); var fieldIDs = this.getUsedFields(); for (let i = 0; i < fieldIDs.length; i++) { let fieldID = fieldIDs[i]; @@ -3588,11 +3676,9 @@ Zotero.Item.prototype.clone = Zotero.Promise.coroutine(function* (libraryID, ski // Regular item if (this.isRegularItem()) { - yield this.loadCreators(); newItem.setCreators(this.getCreators()); } else { - yield this.loadNote(); newItem.setNote(this.getNote()); if (sameLibrary) { var parent = this.parentKey; @@ -3614,18 +3700,16 @@ Zotero.Item.prototype.clone = Zotero.Promise.coroutine(function* (libraryID, ski } if (!skipTags) { - yield this.loadTags(); newItem.setTags(this.getTags()); } if (sameLibrary) { // DEBUG: this will add reverse-only relateds too - yield this.loadRelations(); newItem.setRelations(this.getRelations()); } return newItem; -}); +} /** @@ -3721,8 +3805,6 @@ Zotero.Item.prototype.isCollection = function() { /** * Populate the object's data from an API JSON data object - * - * If this object is identified (has an id or library/key), loadAllData() must have been called. */ Zotero.Item.prototype.fromJSON = function (json) { if (!json.itemType && !this._itemTypeID) { @@ -3867,7 +3949,7 @@ Zotero.Item.prototype.fromJSON = function (json) { /** * @param {Object} options */ -Zotero.Item.prototype.toJSON = Zotero.Promise.coroutine(function* (options = {}) { +Zotero.Item.prototype.toJSON = function (options = {}) { var env = this._preToJSON(options); var mode = env.mode; @@ -3877,7 +3959,6 @@ Zotero.Item.prototype.toJSON = Zotero.Promise.coroutine(function* (options = {}) obj.itemType = Zotero.ItemTypes.getName(this.itemTypeID); // Fields - yield this.loadItemData(); for (let i in this._itemData) { let val = this.getField(i) + ''; if (val !== '' || mode == 'full') { @@ -3887,7 +3968,6 @@ Zotero.Item.prototype.toJSON = Zotero.Promise.coroutine(function* (options = {}) // Creators if (this.isRegularItem()) { - yield this.loadCreators() obj.creators = this.getCreatorsJSON(); } else { @@ -3912,18 +3992,18 @@ Zotero.Item.prototype.toJSON = Zotero.Promise.coroutine(function* (options = {}) if (this.isFileAttachment()) { if (options.syncedStorageProperties) { - obj.mtime = yield Zotero.Sync.Storage.Local.getSyncedModificationTime(this.id); - obj.md5 = yield Zotero.Sync.Storage.Local.getSyncedHash(this.id); + obj.mtime = this.attachmentSyncedModificationTime; + obj.md5 = this.attachmentSyncedHash; } else { - obj.mtime = (yield this.attachmentModificationTime) || null; - obj.md5 = (yield this.attachmentHash) || null; + // TEMP + //obj.mtime = (yield this.attachmentModificationTime) || null; + //obj.md5 = (yield this.attachmentHash) || null; } } } // Notes and embedded attachment notes - yield this.loadNote(); let note = this.getNote(); if (note !== "" || mode == 'full' || (mode == 'new' && this.isNote())) { obj.note = note; @@ -3932,7 +4012,6 @@ Zotero.Item.prototype.toJSON = Zotero.Promise.coroutine(function* (options = {}) // Tags obj.tags = []; - yield this.loadTags() var tags = this.getTags(); for (let i=0; i/)) { - note = Zotero.Utilities.htmlSpecialChars(note); - note = Zotero.Notes.notePrefix + '

' - + note.replace(/\n/g, '

') - .replace(/\t/g, '    ') - .replace(/ /g, '  ') - + '

' + Zotero.Notes.noteSuffix; - note = note.replace(/

\s*<\/p>/g, '

 

'); - let sql = "UPDATE itemNotes SET note=? WHERE itemID=?"; - yield Zotero.DB.queryAsync(sql, [note, this.id]); - } - - // Don't include
wrapper when returning value - let startLen = note.substr(0, 36).match(/^
/)[0].length; - let endLen = 6; // "
".length - note = note.substr(startLen, note.length - startLen - endLen); - } - - this._noteText = note ? note : ''; - } - - this._loaded.note = true; - this._clearChanged('note'); -}); - - Zotero.Item.prototype.loadDisplayTitle = Zotero.Promise.coroutine(function* (reload) { if (this._displayTitle !== null && !reload) { return; @@ -4097,7 +4082,6 @@ Zotero.Item.prototype.loadDisplayTitle = Zotero.Promise.coroutine(function* (rel var itemTypeName = Zotero.ItemTypes.getName(itemTypeID); if (title === "" && (itemTypeID == 8 || itemTypeID == 10)) { // 'letter' and 'interview' itemTypeIDs - yield this.loadCreators(); var creatorsData = this.getCreators(); var authors = []; var participants = []; @@ -4175,7 +4159,6 @@ Zotero.Item.prototype.loadDisplayTitle = Zotero.Promise.coroutine(function* (rel strParts.push(part); } - yield this.loadCreators() var creatorData = this.getCreator(0); if (creatorData && creatorData.creatorTypeID === 1) { // author strParts.push(creatorData.lastName); @@ -4189,156 +4172,6 @@ Zotero.Item.prototype.loadDisplayTitle = Zotero.Promise.coroutine(function* (rel }); -/* - * Load in the creators from the database - */ -Zotero.Item.prototype.loadCreators = Zotero.Promise.coroutine(function* (reload) { - if (this._loaded.creators && !reload) { - return; - } - - Zotero.debug("Loading creators for item " + this.libraryKey); - - if (!this.id) { - throw new Error('ItemID not set for item before attempting to load creators'); - } - - var sql = 'SELECT creatorID, creatorTypeID, orderIndex FROM itemCreators ' - + 'WHERE itemID=? ORDER BY orderIndex'; - var rows = yield Zotero.DB.queryAsync(sql, this.id); - - this._creators = []; - this._creatorIDs = []; - this._loaded.creators = true; - this._clearChanged('creators'); - - if (!rows) { - return true; - } - - var maxOrderIndex = -1; - for (var i=0; i maxOrderIndex) { - maxOrderIndex = row.orderIndex; - } - let creatorData = yield Zotero.Creators.getAsync(row.creatorID); - creatorData.creatorTypeID = row.creatorTypeID; - this._creators[i] = creatorData; - this._creatorIDs[i] = row.creatorID; - } - if (i <= maxOrderIndex) { - Zotero.debug("Fixing incorrect creator indexes for item " + this.libraryKey - + " (" + i + ", " + maxOrderIndex + ")", 2); - while (i <= maxOrderIndex) { - this._changed.creators[i] = true; - i++; - } - } - - return true; -}); - - -Zotero.Item.prototype.loadChildItems = Zotero.Promise.coroutine(function* (reload) { - if (this._loaded.childItems && !reload) { - return; - } - - if (this.isNote() || this.isAttachment()) { - return; - } - - // Attachments - this._attachments = { - rows: null, - chronologicalWithTrashed: null, - chronologicalWithoutTrashed: null, - alphabeticalWithTrashed: null, - alphabeticalWithoutTrashed: null - }; - var sql = "SELECT A.itemID, value AS title, CASE WHEN DI.itemID IS NULL THEN 0 ELSE 1 END AS trashed " - + "FROM itemAttachments A " - + "NATURAL JOIN items I " - + "LEFT JOIN itemData ID ON (fieldID=110 AND A.itemID=ID.itemID) " - + "LEFT JOIN itemDataValues IDV USING (valueID) " - + "LEFT JOIN deletedItems DI USING (itemID) " - + "WHERE parentItemID=?"; - // Since we do the sort here and cache these results, a restart will be required - // if this pref (off by default) is turned on, but that's OK - if (Zotero.Prefs.get('sortAttachmentsChronologically')) { - sql += " ORDER BY dateAdded"; - } - this._attachments.rows = yield Zotero.DB.queryAsync(sql, this.id); - - // - // Notes - // - this._notes = { - rows: null, - rowsEmbedded: null, - chronologicalWithTrashed: null, - chronologicalWithoutTrashed: null, - alphabeticalWithTrashed: null, - alphabeticalWithoutTrashed: null, - numWithTrashed: null, - numWithoutTrashed: null, - numWithTrashedWithEmbedded: null, - numWithoutTrashedWithoutEmbedded: null - }; - var sql = "SELECT N.itemID, title, CASE WHEN DI.itemID IS NULL THEN 0 ELSE 1 END AS trashed " - + "FROM itemNotes N " - + "NATURAL JOIN items I " - + "LEFT JOIN deletedItems DI USING (itemID) " - + "WHERE parentItemID=?"; - if (Zotero.Prefs.get('sortAttachmentsChronologically')) { - sql += " ORDER BY dateAdded"; - } - this._notes.rows = yield Zotero.DB.queryAsync(sql, this.id); - - this._loaded.childItems = true; - this._clearChanged('childItems'); -}); - - -Zotero.Item.prototype.loadTags = Zotero.Promise.coroutine(function* (reload) { - if (this._loaded.tags && !reload) { - return; - } - - if (!this._id) { - return; - } - var sql = "SELECT tagID AS id, name AS tag, type FROM itemTags " - + "JOIN tags USING (tagID) WHERE itemID=?"; - var rows = yield Zotero.DB.queryAsync(sql, this.id); - - this._tags = []; - for (let i=0; i 0 ? ',\n' : ''; let item = yield this.getAsync(ids[i], { noCache: true }); - var json = yield item.toResponseJSON(); + var json = item.toResponseJSON(); yield prefix + JSON.stringify(json, null, 4); } @@ -212,105 +215,24 @@ Zotero.Items = function() { }; - this._cachedFields = {}; - this.cacheFields = Zotero.Promise.coroutine(function* (libraryID, fields, items) { - if (items && items.length == 0) { - return; - } - - var t = new Date; - - fields = fields.concat(); - - // Needed for display titles for some item types - if (fields.indexOf('title') != -1) { - fields.push('reporter', 'court'); - } - - Zotero.debug("Caching fields [" + fields.join() + "]" - + (items ? " for " + items.length + " items" : '') - + " in library " + libraryID); - - if (items && items.length > 0) { - yield this._load(libraryID, items); - } - else { - yield this._load(libraryID); - } - - var primaryFields = []; - var fieldIDs = []; - for each(var field in fields) { - // Check if field already cached - if (this._cachedFields[libraryID] && this._cachedFields[libraryID].indexOf(field) != -1) { - continue; - } - - if (!this._cachedFields[libraryID]) { - this._cachedFields[libraryID] = []; - } - this._cachedFields[libraryID].push(field); - - if (this.isPrimaryField(field)) { - primaryFields.push(field); - } - else { - fieldIDs.push(Zotero.ItemFields.getID(field)); - if (Zotero.ItemFields.isBaseField(field)) { - fieldIDs = fieldIDs.concat(Zotero.ItemFields.getTypeFieldsFromBase(field)); - } - } - } - - if (primaryFields.length) { - var sql = "SELECT O.itemID, " - + primaryFields.map((val) => this.getPrimaryDataSQLPart(val)).join(', ') - + this.primaryDataSQLFrom + " AND O.libraryID=?"; - var params = [libraryID]; - if (items) { - sql += " AND O.itemID IN (" + items.join() + ")"; - } - yield Zotero.DB.queryAsync( - sql, - params, - { - onRow: function (row) { - let obj = { - itemID: row.getResultByIndex(0) - }; - for (let i=0; i maxOrderIndex) { + maxOrderIndex = row.orderIndex; + } + + let creatorData = Zotero.Creators.get(row.creatorID); + creatorData.creatorTypeID = row.creatorTypeID; + item._creators[index] = creatorData; + item._creatorIDs[index] = row.creatorID; + index++; } - Zotero.debug("Cached fields in " + ((new Date) - t) + "ms"); + if (index <= maxOrderIndex) { + fixIncorrectIndexes(item, index, maxOrderIndex); + } + }); + + + this._loadNotes = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { + var notesToUpdate = []; + + var sql = "SELECT itemID, note FROM items " + + "JOIN itemNotes USING (itemID) " + + "WHERE libraryID=?" + idSQL; + var params = [libraryID]; + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + let itemID = row.getResultByIndex(0); + let item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not found"); + } + let note = row.getResultByIndex(1); + + // Convert non-HTML notes on-the-fly + if (note !== "") { + if (!note.substr(0, 36).match(/^
/)) { + note = Zotero.Utilities.htmlSpecialChars(note); + note = Zotero.Notes.notePrefix + '

' + + note.replace(/\n/g, '

') + .replace(/\t/g, '    ') + .replace(/ /g, '  ') + + '

' + Zotero.Notes.noteSuffix; + note = note.replace(/

\s*<\/p>/g, '

 

'); + notesToUpdate.push([item.id, note]); + } + + // Don't include
wrapper when returning value + let startLen = note.substr(0, 36).match(/^
/)[0].length; + let endLen = 6; // "
".length + note = note.substr(startLen, note.length - startLen - endLen); + } + + item._noteText = note ? note : ''; + item._loaded.note = true; + item._clearChanged('note'); + }.bind(this) + } + ); + + if (notesToUpdate.length) { + yield Zotero.DB.executeTransaction(function* () { + for (let i = 0; i < notesToUpdate.length; i++) { + let row = notesToUpdate[i]; + let sql = "UPDATE itemNotes SET note=? WHERE itemID=?"; + yield Zotero.DB.queryAsync(sql, [row[1], row[0]]); + } + }.bind(this)); + } + + // Mark notes and attachments without notes as loaded + sql = "SELECT itemID FROM items WHERE libraryID=?" + idSQL + + " AND itemTypeID IN (?, ?) AND itemID NOT IN (SELECT itemID FROM itemNotes)"; + params = [libraryID, Zotero.ItemTypes.getID('note'), Zotero.ItemTypes.getID('attachment')]; + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + let itemID = row.getResultByIndex(0); + let item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not loaded"); + } + + item._noteText = ''; + item._loaded.note = true; + item._clearChanged('note'); + }.bind(this) + } + ); + }); + + + this._loadChildItems = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { + var params = [libraryID]; + var rows = []; + var onRow = function (row, setFunc) { + var itemID = row.getResultByIndex(0); + + if (lastItemID && itemID !== lastItemID) { + setFunc(lastItemID, rows); + rows = []; + } + + lastItemID = itemID; + rows.push({ + itemID: row.getResultByIndex(1), + title: row.getResultByIndex(2), + trashed: row.getResultByIndex(3) + }); + }; + + var sql = "SELECT parentItemID, A.itemID, value AS title, " + + "CASE WHEN DI.itemID IS NULL THEN 0 ELSE 1 END AS trashed " + + "FROM itemAttachments A " + + "JOIN items I ON (A.parentItemID=I.itemID) " + + "LEFT JOIN itemData ID ON (fieldID=110 AND A.itemID=ID.itemID) " + + "LEFT JOIN itemDataValues IDV USING (valueID) " + + "LEFT JOIN deletedItems DI USING (itemID) " + + "WHERE libraryID=?" + + (ids.length ? " AND parentItemID IN (" + ids.map(id => parseInt(id)).join(", ") + ")" : ""); + // Since we do the sort here and cache these results, a restart will be required + // if this pref (off by default) is turned on, but that's OK + if (Zotero.Prefs.get('sortAttachmentsChronologically')) { + sql += " ORDER BY parentItemID, dateAdded"; + } + var setAttachmentItem = function (itemID, rows) { + var item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not loaded"); + } + + item._attachments = { + rows, + chronologicalWithTrashed: null, + chronologicalWithoutTrashed: null, + alphabeticalWithTrashed: null, + alphabeticalWithoutTrashed: null + }; + }.bind(this); + var lastItemID = null; + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + onRow(row, setAttachmentItem); + } + } + ); + if (lastItemID) { + setAttachmentItem(lastItemID, rows); + } + + // + // Notes + // + sql = "SELECT parentItemID, N.itemID, title, " + + "CASE WHEN DI.itemID IS NULL THEN 0 ELSE 1 END AS trashed " + + "FROM itemNotes N " + + "JOIN items I ON (N.parentItemID=I.itemID) " + + "LEFT JOIN deletedItems DI USING (itemID) " + + "WHERE libraryID=?" + + (ids.length ? " AND parentItemID IN (" + ids.map(id => parseInt(id)).join(", ") + ")" : ""); + if (Zotero.Prefs.get('sortNotesChronologically')) { + sql += " ORDER BY parentItemID, dateAdded"; + } + var setNoteItem = function (itemID, rows) { + var item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not loaded"); + } + + item._notes = { + rows, + rowsEmbedded: null, + chronologicalWithTrashed: null, + chronologicalWithoutTrashed: null, + alphabeticalWithTrashed: null, + alphabeticalWithoutTrashed: null, + numWithTrashed: null, + numWithoutTrashed: null, + numWithTrashedWithEmbedded: null, + numWithoutTrashedWithoutEmbedded: null + }; + }.bind(this); + lastItemID = null; + rows = []; + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + onRow(row, setNoteItem); + } + } + ); + if (lastItemID) { + setNoteItem(lastItemID, rows); + } + + // Mark all top-level items as having child items loaded + sql = "SELECT itemID FROM items I WHERE libraryID=?" + idSQL + " AND itemID NOT IN " + + "(SELECT itemID FROM itemAttachments UNION SELECT itemID FROM itemNotes)"; + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + var itemID = row.getResultByIndex(0); + var item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not loaded"); + } + item._loaded.childItems = true; + item._clearChanged('childItems'); + }.bind(this) + } + ); + }); + + + this._loadTags = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { + var sql = "SELECT itemID, name, type FROM items " + + "LEFT JOIN itemTags USING (itemID) " + + "LEFT JOIN tags USING (tagID) WHERE libraryID=?" + idSQL; + var params = [libraryID]; + + var lastItemID; + var rows = []; + var setRows = function (itemID, rows) { + var item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not found"); + } + + item._tags = []; + for (let i = 0; i < rows.length; i++) { + let row = rows[i]; + item._tags.push(Zotero.Tags.cleanData(row)); + } + + item._loaded.tags = true; + item._clearChanged('tags'); + }.bind(this); + + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + let itemID = row.getResultByIndex(0); + + if (lastItemID && itemID !== lastItemID) { + setRows(lastItemID, rows); + rows = []; + } + + lastItemID = itemID; + + // Item has no tags + let tag = row.getResultByIndex(1); + if (tag === null) { + return; + } + + rows.push({ + tag: tag, + type: row.getResultByIndex(2) + }); + }.bind(this) + } + ); + if (lastItemID) { + setRows(lastItemID, rows); + } + }); + + + this._loadCollections = Zotero.Promise.coroutine(function* (libraryID, ids, idSQL) { + var sql = "SELECT itemID, collectionID FROM items " + + "LEFT JOIN collectionItems USING (itemID) " + + "WHERE libraryID=?" + idSQL; + var params = [libraryID]; + + var lastItemID; + var rows = []; + var setRows = function (itemID, rows) { + var item = this._objectCache[itemID]; + if (!item) { + throw new Error("Item " + itemID + " not found"); + } + + item._collections = rows; + item._loaded.collections = true; + item._clearChanged('collections'); + }.bind(this); + + yield Zotero.DB.queryAsync( + sql, + params, + { + noCache: ids.length != 1, + onRow: function (row) { + let itemID = row.getResultByIndex(0); + + if (lastItemID && itemID !== lastItemID) { + setRows(lastItemID, rows); + rows = []; + } + + lastItemID = itemID; + let collectionID = row.getResultByIndex(1); + // No collections + if (collectionID === null) { + return; + } + rows.push(collectionID); + }.bind(this) + } + ); + if (lastItemID) { + setRows(lastItemID, rows); + } }); @@ -409,17 +725,11 @@ Zotero.Items = function() { var otherItemIDs = []; var itemURI = Zotero.URI.getItemURI(item); - yield item.loadTags(); - yield item.loadRelations(); var replPred = Zotero.Relations.replacedItemPredicate; var toSave = {}; toSave[this.id]; for each(var otherItem in otherItems) { - yield otherItem.loadChildItems(); - yield otherItem.loadCollections(); - yield otherItem.loadTags(); - yield otherItem.loadRelations(); let otherItemURI = Zotero.URI.getItemURI(otherItem); // Move child items to master @@ -632,16 +942,6 @@ Zotero.Items = function() { }); - this._postLoad = function (libraryID, ids) { - if (!ids) { - if (!this._cachedFields[libraryID]) { - this._cachedFields[libraryID] = []; - } - this._cachedFields[libraryID] = this.primaryFields.concat(); - } - } - - /* * Generate SQL to retrieve firstCreator field * diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js index 5b0ab097ce..4eff4f0169 100644 --- a/chrome/content/zotero/xpcom/db.js +++ b/chrome/content/zotero/xpcom/db.js @@ -630,7 +630,13 @@ Zotero.DBConnection.prototype.queryAsync = Zotero.Promise.coroutine(function* (s } } } - let rows = yield conn.executeCached(sql, params, onRow); + let rows; + if (options && options.noCache) { + rows = yield conn.execute(sql, params, onRow); + } + else { + rows = yield conn.executeCached(sql, params, onRow); + } // Parse out the SQL command being used let op = sql.match(/^[^a-z]*[^ ]+/i); if (op) { diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index 920776eb16..ace81c4fe9 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -82,7 +82,7 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree if (this._treebox) { if (this._needsSort) { - yield this.sort(); + this.sort(); } return; } @@ -133,11 +133,11 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree if (self._treebox.view.selection.count > 1) { switch (event.keyCode) { case 39: - self.expandSelectedRows().done(); + self.expandSelectedRows(); break; case 37: - self.collapseSelectedRows().done(); + self.collapseSelectedRows(); break; } @@ -148,7 +148,7 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree var key = String.fromCharCode(event.which); if (key == '+' && !(event.ctrlKey || event.altKey || event.metaKey)) { - self.expandAllRows().done(); + self.expandAllRows(); event.preventDefault(); return; } @@ -230,8 +230,8 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree // handleKeyPress() in zoteroPane.js. tree._handleEnter = function () {}; - yield this.sort(); - yield this.expandMatchParents(); + this.sort(); + this.expandMatchParents(); if (this._ownerDocument.defaultView.ZoteroPane_Local) { this._ownerDocument.defaultView.ZoteroPane_Local.clearItemsPaneMessage(); @@ -266,13 +266,10 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree */ Zotero.ItemTreeView.prototype.refresh = Zotero.serial(Zotero.Promise.coroutine(function* () { Zotero.debug('Refreshing items list for ' + this.id); - //if(!Zotero.ItemTreeView._haveCachedFields) yield Zotero.Promise.resolve(); - var cacheFields = ['title', 'date']; - - // Cache the visible fields so they don't load individually + // DEBUG: necessary? try { - var visibleFields = this.getVisibleFields(); + this._treebox.columns.count } // If treebox isn't ready, skip refresh catch (e) { @@ -286,33 +283,6 @@ Zotero.ItemTreeView.prototype.refresh = Zotero.serial(Zotero.Promise.coroutine(f }); try { - for (let i=0; i=0; i--) { - yield this.toggleOpenState(rowsToOpen[i], true); + this.toggleOpenState(rowsToOpen[i], true); } this._refreshItemRowMap(); if (unsuppress) { //this._treebox.endUpdateBatch(); this.selection.selectEventsSuppressed = false; } -}); +} -Zotero.ItemTreeView.prototype.expandMatchParents = Zotero.Promise.coroutine(function* () { +Zotero.ItemTreeView.prototype.expandMatchParents = function () { // Expand parents of child matches if (!this._searchMode) { return; @@ -2001,7 +1952,7 @@ Zotero.ItemTreeView.prototype.expandMatchParents = Zotero.Promise.coroutine(func for (var i=0; i} - File hash, null if never synced, if false if - * file doesn't exist - */ - getSyncedHash: Zotero.Promise.coroutine(function* (itemID) { - var sql = "SELECT storageHash FROM itemAttachments WHERE itemID=?"; - var hash = yield Zotero.DB.valueQueryAsync(sql, itemID); - if (hash === false) { - throw new Error("Item " + itemID + " not found"); - } - return hash; - }), - - - /** - * @param {Integer} itemID - * @param {String} hash File hash - * @param {Boolean} [updateItem=FALSE] - Mark attachment item as unsynced - */ - setSyncedHash: Zotero.Promise.coroutine(function* (itemID, hash, updateItem) { - if (hash !== null && hash.length != 32) { - throw new Error("Invalid file hash '" + hash + "'"); - } - - Zotero.DB.requireTransaction(); - - var sql = "UPDATE itemAttachments SET storageHash=? WHERE itemID=?"; - yield Zotero.DB.queryAsync(sql, [hash, itemID]); - - if (updateItem) { - let item = yield Zotero.Items.getAsync(itemID); - yield item.updateSynced(false); - } + sql = "UPDATE itemAttachments SET syncState=? WHERE itemID IN (" + sql + ")"; + yield Zotero.DB.queryAsync(sql, [this.SYNC_STATE_TO_UPLOAD].concat(params)); }), @@ -678,11 +577,10 @@ Zotero.Sync.Storage.Local = { // Set the file mtime to the time from the server yield OS.File.setDates(path, null, new Date(parseInt(mtime))); - yield Zotero.DB.executeTransaction(function* () { - yield this.setSyncedHash(item.id, md5); - yield this.setSyncState(item.id, this.SYNC_STATE_IN_SYNC); - yield this.setSyncedModificationTime(item.id, mtime); - }.bind(this)); + item.attachmentSyncState = this.SYNC_STATE_IN_SYNC; + item.attachmentSyncedModificationTime = mtime; + item.attachmentSyncedHash = md5; + yield item.saveTx(); return new Zotero.Sync.Storage.Result({ localChanges: true @@ -1040,7 +938,7 @@ Zotero.Sync.Storage.Local = { for (let localItem of localItems) { // Use the mtime for the dateModified field, since that's all that's shown in the // CR window at the moment - let localItemJSON = yield localItem.toJSON(); + let localItemJSON = localItem.toJSON(); localItemJSON.dateModified = Zotero.Date.dateToISO( new Date(yield localItem.attachmentModificationTime) ); @@ -1101,8 +999,9 @@ Zotero.Sync.Storage.Local = { else { syncState = this.SYNC_STATE_FORCE_DOWNLOAD; } - let itemID = Zotero.Items.getIDFromLibraryAndKey(libraryID, conflict.left.key); - yield Zotero.Sync.Storage.Local.setSyncState(itemID, syncState); + let item = Zotero.Items.getByLibraryAndKey(libraryID, conflict.left.key); + item.attachmentSyncState = syncState; + yield item.save({ skipAll: true }); } }.bind(this)); return true; diff --git a/chrome/content/zotero/xpcom/storage/webdav.js b/chrome/content/zotero/xpcom/storage/webdav.js index 13f64bc866..49c133c6bd 100644 --- a/chrome/content/zotero/xpcom/storage/webdav.js +++ b/chrome/content/zotero/xpcom/storage/webdav.js @@ -288,15 +288,14 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { Zotero.debug("File mod time matches remote file -- skipping download of " + item.libraryKey); - yield Zotero.DB.executeTransaction(function* () { - var syncState = Zotero.Sync.Storage.Local.getSyncState(item.id); - // DEBUG: Necessary to update item? - var updateItem = syncState != 1; - yield Zotero.Sync.Storage.Local.setSyncedModificationTime( - item.id, metadata.mtime, updateItem - ); - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - }); + var updateItem = item.attachmentSyncState != 1 + item.attachmentSyncedModificationTime = metadata.mtime; + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); + // DEBUG: Necessary? + if (updateItem) { + yield item.updateSynced(false); + } return new Zotero.Sync.Storage.Result({ localChanges: true, // ? @@ -416,7 +415,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { } // Check if file already exists on WebDAV server - if ((yield Zotero.Sync.Storage.Local.getSyncState(item.id)) + if (item.attachmentSyncState != Zotero.Sync.Storage.Local.SYNC_STATE_FORCE_UPLOAD) { if (metadata.mtime) { // Local file time @@ -438,15 +437,14 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { // If WebDAV server already has file, update synced properties if (!changed) { - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncedModificationTime( - item.id, fmtime, true - ); - if (hash) { - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, hash); - } - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - }); + item.attachmentSyncedModificationTime = fmtime; + if (hash) { + item.attachmentSyncedHash = hash; + } + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); + // skipAll doesn't mark as unsynced, so do that separately + yield item.updateSynced(false); return new Zotero.Sync.Storage.Result; } } @@ -460,9 +458,9 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { // API would ever be updated with the correct values, so we can't just wait for // the API to change.) If a conflict is found, we flag the item as in conflict // and require another file sync, which will trigger conflict resolution. - let smtime = yield Zotero.Sync.Storage.Local.getSyncedModificationTime(item.id); + let smtime = item.attachmentSyncedModificationTime; if (smtime != mtime) { - let shash = yield Zotero.Sync.Storage.Local.getSyncedHash(item.id); + let shash = item.attachmentSyncedHash; if (shash && metadata.md5 && shash == metadata.md5) { Zotero.debug("Last synced mod time for item " + item.libraryKey + " doesn't match time on storage server but hash does -- ignoring"); @@ -472,12 +470,13 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { Zotero.logError("Conflict -- last synced file mod time for item " + item.libraryKey + " does not match time on storage server" + " (" + smtime + " != " + mtime + ")"); - yield Zotero.DB.executeTransaction(function* () { - // Conflict resolution uses the synced mtime as the remote value, so set - // that to the WebDAV value, since that's the one in conflict. - yield Zotero.Sync.Storage.Local.setSyncedModificationTime(item.id, mtime); - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_conflict"); - }); + + // Conflict resolution uses the synced mtime as the remote value, so set + // that to the WebDAV value, since that's the one in conflict. + item.attachmentSyncedModificationTime = mtime; + item.attachmentSyncState = "in_conflict"; + yield item.saveTx({ skipAll: true }); + return new Zotero.Sync.Storage.Result({ fileSyncRequired: true }); @@ -1191,7 +1190,10 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { throw new Error(Zotero.Sync.Storage.Mode.WebDAV.defaultError); } - return { mtime, md5 }; + return { + mtime: parseInt(mtime), + md5 + }; }), @@ -1243,11 +1245,12 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { // Update .prop file on WebDAV server yield this._setStorageFileMetadata(item); - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - yield Zotero.Sync.Storage.Local.setSyncedModificationTime(item.id, params.mtime, true); - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, params.md5); - }); + item.attachmentSyncedModificationTime = params.mtime; + item.attachmentSyncedHash = params.md5; + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); + // skipAll doesn't mark as unsynced, so do that separately + yield item.updateSynced(false); try { yield OS.File.remove( diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js index 43ee3744b0..e8f2b4bfa0 100644 --- a/chrome/content/zotero/xpcom/storage/zfs.js +++ b/chrome/content/zotero/xpcom/storage/zfs.js @@ -131,15 +131,13 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } // Update local metadata and stop request, skipping file download - yield Zotero.DB.executeTransaction(function* () { - if (updateHash) { - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, requestData.md5); - } - yield Zotero.Sync.Storage.Local.setSyncedModificationTime( - item.id, requestData.mtime - ); - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - }); + item.attachmentSyncedModificationTime = requestData.mtime; + if (updateHash) { + item.attachmentSyncedHash = requestData.md5; + } + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); + return false; }), onProgress: function (a, b, c) { @@ -261,7 +259,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { var sql = "SELECT value FROM settings WHERE setting=? AND key=?"; var values = yield Zotero.DB.columnQueryAsync(sql, ['storage', 'zfsPurge']); - if (!values) { + if (!values.length) { return false; } @@ -353,7 +351,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { var headers = { "Content-Type": "application/x-www-form-urlencoded" }; - var storedHash = yield Zotero.Sync.Storage.Local.getSyncedHash(item.id); + var storedHash = item.attachmentSyncedHash; //var storedModTime = yield Zotero.Sync.Storage.getSyncedModificationTime(item.id); if (storedHash) { headers["If-Match"] = storedHash; @@ -538,17 +536,17 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { Zotero.debug(fileHash); if (json.data.md5 == fileHash) { - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncedModificationTime( - item.id, fileModTime - ); - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, fileHash); - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - }); + item.attachmentSyncedModificationTime = fileModTime; + item.attachmentSyncedHash = fileHash; + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); + return new Zotero.Sync.Storage.Result; } - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_conflict"); + item.attachmentSyncState = "in_conflict"; + yield item.saveTx({ skipAll: true }); + return new Zotero.Sync.Storage.Result({ fileSyncRequired: true }); @@ -767,11 +765,12 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { _updateItemFileInfo: Zotero.Promise.coroutine(function* (item, params) { // Mark as in-sync yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); + // Store file mod time and hash + item.attachmentSyncedModificationTime = params.mtime; + item.attachmentSyncedHash = params.md5; + item.attachmentSyncState = "in_sync"; + yield item.save({ skipAll: true }); - // Store file mod time and hash - yield Zotero.Sync.Storage.Local.setSyncedModificationTime(item.id, params.mtime); - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, params.md5); // Update sync cache with new file metadata and version from server var json = yield Zotero.Sync.Data.Local.getCacheObject( 'item', item.libraryID, item.key, item.version @@ -933,7 +932,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } // Check for conflict - if ((yield Zotero.Sync.Storage.Local.getSyncState(item.id)) + if (item.attachmentSyncState != Zotero.Sync.Storage.Local.SYNC_STATE_FORCE_UPLOAD) { if (info) { // Local file time diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 23b1242ae2..10d65e5e69 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -316,7 +316,7 @@ Zotero.Sync.Data.Engine.prototype._startDownload = Zotero.Promise.coroutine(func // Conflict resolution else if (objectType == 'item') { conflicts.push({ - left: yield obj.toJSON(), + left: obj.toJSON(), right: { deleted: true } diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 997f026c79..a2df6ce3c6 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -512,7 +512,7 @@ Zotero.Sync.Data.Local = { objectType, obj.libraryID, obj.key, obj.version ); - let jsonDataLocal = yield obj.toJSON(); + let jsonDataLocal = obj.toJSON(); // For items, check if mtime or file hash changed in metadata, // which would indicate that a remote storage sync took place and @@ -780,7 +780,8 @@ Zotero.Sync.Data.Local = { markToDownload = true; } if (markToDownload) { - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "to_download"); + item.attachmentSyncState = "to_download"; + yield item.save({ skipAll: true }); } }), @@ -870,7 +871,6 @@ Zotero.Sync.Data.Local = { _saveObjectFromJSON: Zotero.Promise.coroutine(function* (obj, json, options) { try { - yield obj.loadAllData(); obj.fromJSON(json); if (!options.saveAsChanged) { obj.version = json.version; diff --git a/chrome/content/zotero/xpcom/timeline.js b/chrome/content/zotero/xpcom/timeline.js index ed473c9a00..fdf27afb33 100644 --- a/chrome/content/zotero/xpcom/timeline.js +++ b/chrome/content/zotero/xpcom/timeline.js @@ -31,7 +31,6 @@ Zotero.Timeline = { yield '\n'; for (let i=0; i items.add(item)); } @@ -720,7 +718,7 @@ Zotero.Translate.ItemGetter.prototype = { * Converts an attachment to array format and copies it to the export folder if desired */ "_attachmentToArray":Zotero.Promise.coroutine(function* (attachment) { - var attachmentArray = yield Zotero.Utilities.Internal.itemToExportFormat(attachment, this.legacy); + var attachmentArray = Zotero.Utilities.Internal.itemToExportFormat(attachment, this.legacy); var linkMode = attachment.attachmentLinkMode; if(linkMode != Zotero.Attachments.LINK_MODE_LINKED_URL) { var attachFile = attachment.getFile(); @@ -864,13 +862,13 @@ Zotero.Translate.ItemGetter.prototype = { var returnItemArray = yield this._attachmentToArray(returnItem); if(returnItemArray) return returnItemArray; } else { - var returnItemArray = yield Zotero.Utilities.Internal.itemToExportFormat(returnItem, this.legacy); + var returnItemArray = Zotero.Utilities.Internal.itemToExportFormat(returnItem, this.legacy); // get attachments, although only urls will be passed if exportFileData is off returnItemArray.attachments = []; var attachments = returnItem.getAttachments(); for each(var attachmentID in attachments) { - var attachment = yield Zotero.Items.getAsync(attachmentID); + var attachment = Zotero.Items.get(attachmentID); var attachmentInfo = yield this._attachmentToArray(attachment); if(attachmentInfo) { diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js index 001099194b..d623696f24 100644 --- a/chrome/content/zotero/xpcom/utilities.js +++ b/chrome/content/zotero/xpcom/utilities.js @@ -1591,8 +1591,9 @@ Zotero.Utilities = { */ "itemToCSLJSON":function(zoteroItem) { if (zoteroItem instanceof Zotero.Item) { - return Zotero.Utilities.Internal.itemToExportFormat(zoteroItem). - then(Zotero.Utilities.itemToCSLJSON); + return this.itemToCSLJSON( + Zotero.Utilities.Internal.itemToExportFormat(zoteroItem) + ); } var cslType = CSL_TYPE_MAPPINGS[zoteroItem.itemType]; diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js index 5aca232684..e5d6e21dc4 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -610,44 +610,7 @@ Zotero.Utilities.Internal = { * @param {Boolean} legacy Add mappings for legacy (pre-4.0.27) translators * @return {Promise} */ - "itemToExportFormat": new function() { - return Zotero.Promise.coroutine(function* (zoteroItem, legacy) { - var item = yield zoteroItem.toJSON(); - - item.uri = Zotero.URI.getItemURI(zoteroItem); - delete item.key; - - if (!zoteroItem.isAttachment() && !zoteroItem.isNote()) { - yield zoteroItem.loadChildItems(); - - // Include attachments - item.attachments = []; - let attachments = zoteroItem.getAttachments(); - for (let i=0; i x.libraryID); + for (let libraryID of libraryIDs) { + yield Zotero.Collections.loadAllData(libraryID); + yield Zotero.Searches.loadAllData(libraryID); + yield Zotero.Items.loadAllData(libraryID); + } + yield Zotero.QuickCopy.init(); Zotero.Items.startEmptyTrashTimer(); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 6c835677e5..dace7b126c 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -1294,7 +1294,6 @@ var ZoteroPane = new function() var clearUndo = noteEditor.item ? noteEditor.item.id != item.id : false; noteEditor.parent = null; - yield item.loadNote(); noteEditor.item = item; // If loading new or different note, disable undo while we repopulate the text field @@ -1325,8 +1324,6 @@ var ZoteroPane = new function() else if (item.isAttachment()) { var attachmentBox = document.getElementById('zotero-attachment-box'); attachmentBox.mode = this.collectionsView.editable ? 'edit' : 'view'; - yield item.loadItemData(); - yield item.loadNote(); attachmentBox.item = item; document.getElementById('zotero-item-pane-content').selectedIndex = 3; @@ -1588,7 +1585,7 @@ var ZoteroPane = new function() var newItem; yield Zotero.DB.executeTransaction(function* () { - newItem = yield item.clone(null, !Zotero.Prefs.get('groups.copyTags')); + newItem = item.clone(null, !Zotero.Prefs.get('groups.copyTags')); yield newItem.save(); if (self.collectionsView.selectedTreeRow.isCollection() && newItem.isTopLevelItem()) { @@ -3641,7 +3638,6 @@ var ZoteroPane = new function() // Fall back to first attachment link if (!uri) { - yield item.loadChildItems(); let attachmentID = item.getAttachments()[0]; if (attachmentID) { let attachment = yield Zotero.Items.getAsync(attachmentID); @@ -3851,7 +3847,7 @@ var ZoteroPane = new function() }); - this.showPublicationsWizard = Zotero.Promise.coroutine(function* (items) { + this.showPublicationsWizard = function (items) { var io = { hasFiles: false, hasNotes: false, @@ -3863,14 +3859,12 @@ var ZoteroPane = new function() for (let i = 0; i < items.length; i++) { let item = items[i]; - yield item.loadItemData(); - yield item.loadChildItems(); - // Files if (!io.hasFiles && item.numAttachments()) { - let attachments = item.getAttachments(); - attachments = yield Zotero.Items.getAsync(attachments); - io.hasFiles = attachments.some(attachment => attachment.isFileAttachment()); + let attachmentIDs = item.getAttachments(); + io.hasFiles = Zotero.Items.get(attachmentIDs).some( + attachment => attachment.isFileAttachment() + ); } // Notes if (!io.hasNotes && item.numNotes()) { @@ -3887,7 +3881,7 @@ var ZoteroPane = new function() io.hasRights = allItemsHaveRights ? 'all' : (noItemsHaveRights ? 'none' : 'some'); window.openDialog('chrome://zotero/content/publicationsDialog.xul','','chrome,modal', io); return io.license ? io : false; - }); + }; /** diff --git a/components/zotero-protocol-handler.js b/components/zotero-protocol-handler.js index a0ce44e088..36c3d16d5f 100644 --- a/components/zotero-protocol-handler.js +++ b/components/zotero-protocol-handler.js @@ -214,7 +214,7 @@ function ZoteroProtocolHandler() { else if (combineChildItems || !results[i].isRegularItem() || results[i].numChildren() == 0) { itemsHash[results[i].id] = [items.length]; - items.push(yield results[i].toJSON({ mode: 'full' })); + items.push(results[i].toJSON({ mode: 'full' })); // Flag item as a search match items[items.length - 1].reportSearchMatch = true; } @@ -241,7 +241,6 @@ function ZoteroProtocolHandler() { } } }; - yield item.loadChildItems(); func(item.getNotes()); func(item.getAttachments()); } @@ -252,7 +251,7 @@ function ZoteroProtocolHandler() { else { for (var i in unhandledParents) { itemsHash[results[i].id] = [items.length]; - items.push(yield results[i].toJSON({ mode: 'full' })); + items.push(results[i].toJSON({ mode: 'full' })); // Flag item as a search match items[items.length - 1].reportSearchMatch = true; } @@ -264,7 +263,7 @@ function ZoteroProtocolHandler() { if (!searchItemIDs[id] && !itemsHash[id]) { var item = yield Zotero.Items.getAsync(id); itemsHash[id] = items.length; - items.push(yield item.toJSON({ mode: 'full' })); + items.push(item.toJSON({ mode: 'full' })); } } @@ -279,10 +278,10 @@ function ZoteroProtocolHandler() { }; } if (item.isNote()) { - items[itemsHash[parentID]].reportChildren.notes.push(yield item.toJSON({ mode: 'full' })); + items[itemsHash[parentID]].reportChildren.notes.push(item.toJSON({ mode: 'full' })); } if (item.isAttachment()) { - items[itemsHash[parentID]].reportChildren.attachments.push(yield item.toJSON({ mode: 'full' })); + items[itemsHash[parentID]].reportChildren.attachments.push(item.toJSON({ mode: 'full' })); } } } @@ -299,7 +298,7 @@ function ZoteroProtocolHandler() { // add on its own if (searchItemIDs[parentID]) { itemsHash[parentID] = [items.length]; - items.push(yield parentItem.toJSON({ mode: 'full' })); + items.push(parentItem.toJSON({ mode: 'full' })); items[items.length - 1].reportSearchMatch = true; } else { @@ -312,14 +311,14 @@ function ZoteroProtocolHandler() { items.push(parentItem.toJSON({ mode: 'full' })); if (item.isNote()) { items[items.length - 1].reportChildren = { - notes: [yield item.toJSON({ mode: 'full' })], + notes: [item.toJSON({ mode: 'full' })], attachments: [] }; } else if (item.isAttachment()) { items[items.length - 1].reportChildren = { notes: [], - attachments: [yield item.toJSON({ mode: 'full' })] + attachments: [item.toJSON({ mode: 'full' })] }; } } @@ -609,7 +608,6 @@ function ZoteroProtocolHandler() { if (params.controller == 'data') { switch (params.scopeObject) { case 'collections': - yield collection.loadChildItems(); var results = collection.getChildItems(); break; diff --git a/test/content/support.js b/test/content/support.js index c4af200b25..38da3e99c5 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -352,10 +352,9 @@ function getNameProperty(objectType) { return objectType == 'item' ? 'title' : 'name'; } -var modifyDataObject = Zotero.Promise.coroutine(function* (obj, params = {}, saveOptions) { +var modifyDataObject = function (obj, params = {}, saveOptions) { switch (obj.objectType) { case 'item': - yield obj.loadItemData(); obj.setField( 'title', params.title !== undefined ? params.title : Zotero.Utilities.randomString() @@ -366,7 +365,7 @@ var modifyDataObject = Zotero.Promise.coroutine(function* (obj, params = {}, sav obj.name = params.name !== undefined ? params.name : Zotero.Utilities.randomString(); } return obj.saveTx(saveOptions); -}); +}; /** * Return a promise for the error thrown by a promise, or false if none @@ -584,7 +583,7 @@ var generateItemJSONData = Zotero.Promise.coroutine(function* generateItemJSONDa for (let itemName in items) { let zItem = yield Zotero.Items.getAsync(items[itemName].id); - jsonData[itemName] = yield zItem.toJSON(options); + jsonData[itemName] = zItem.toJSON(options); // Don't replace some fields that _always_ change (e.g. item keys) // as long as it follows expected format diff --git a/test/resource/chai b/test/resource/chai index b369f25243..775281e138 160000 --- a/test/resource/chai +++ b/test/resource/chai @@ -1 +1 @@ -Subproject commit b369f252432c3486a66a0e93f441e4abb133d229 +Subproject commit 775281e138df26101fba1e554c516f47438851b5 diff --git a/test/resource/mocha b/test/resource/mocha index 2a8594424c..44b0045463 160000 --- a/test/resource/mocha +++ b/test/resource/mocha @@ -1 +1 @@ -Subproject commit 2a8594424c73ffeca41ef1668446372160528b4a +Subproject commit 44b0045463907b1d7963a2e9560c24d9552aac5d diff --git a/test/tests/collectionTest.js b/test/tests/collectionTest.js index 269f7ab15e..41f975efc6 100644 --- a/test/tests/collectionTest.js +++ b/test/tests/collectionTest.js @@ -152,7 +152,6 @@ describe("Zotero.Collection", function() { var collection2 = yield createDataObject('collection', { parentID: collection1.id }); yield collection1.saveTx(); - yield collection1.loadChildCollections(); var childCollections = collection1.getChildCollections(); assert.lengthOf(childCollections, 1); assert.equal(childCollections[0].id, collection2.id); @@ -163,8 +162,6 @@ describe("Zotero.Collection", function() { var collection2 = yield createDataObject('collection', { parentID: collection1.id }); yield collection1.saveTx(); - yield collection1.loadChildCollections(); - collection2.parentID = false; yield collection2.save() @@ -180,7 +177,6 @@ describe("Zotero.Collection", function() { item.addToCollection(collection.key); yield item.saveTx(); - yield collection.loadChildItems(); assert.lengthOf(collection.getChildItems(), 1); }) @@ -191,7 +187,6 @@ describe("Zotero.Collection", function() { item.addToCollection(collection.key); yield item.saveTx(); - yield collection.loadChildItems(); assert.lengthOf(collection.getChildItems(), 0); }) @@ -202,7 +197,6 @@ describe("Zotero.Collection", function() { item.addToCollection(collection.key); yield item.saveTx(); - yield collection.loadChildItems(); assert.lengthOf(collection.getChildItems(false, true), 1); }) }) diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 742c66faca..f8a77e41bd 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -390,12 +390,6 @@ describe("Zotero.CollectionTreeView", function() { parentItemID: item.id }); - // Hack to unload relations to test proper loading - // - // Probably need a better method for this - item._loaded.relations = false; - attachment._loaded.relations = false; - var ids = (yield drop('item', 'L' + group.libraryID, [item.id])).ids; yield cv.selectLibrary(group.libraryID); @@ -413,7 +407,7 @@ describe("Zotero.CollectionTreeView", function() { // Check attachment assert.isTrue(itemsView.isContainer(0)); - yield itemsView.toggleOpenState(0); + itemsView.toggleOpenState(0); assert.equal(itemsView.rowCount, 2); treeRow = itemsView.getRow(1); assert.equal(treeRow.ref.id, ids[1]); diff --git a/test/tests/creatorsTest.js b/test/tests/creatorsTest.js new file mode 100644 index 0000000000..90ccf202a1 --- /dev/null +++ b/test/tests/creatorsTest.js @@ -0,0 +1,21 @@ +"use strict"; + +describe("Zotero.Creators", function() { + describe("#getIDFromData()", function () { + it("should create creator and cache data", function* () { + var data1 = { + firstName: "First", + lastName: "Last" + }; + var creatorID; + yield Zotero.DB.executeTransaction(function* () { + creatorID = yield Zotero.Creators.getIDFromData(data1, true); + }); + assert.typeOf(creatorID, 'number'); + var data2 = Zotero.Creators.get(creatorID); + assert.isObject(data2); + assert.propertyVal(data2, "firstName", data1.firstName); + assert.propertyVal(data2, "lastName", data1.lastName); + }); + }); +}); diff --git a/test/tests/dataObjectTest.js b/test/tests/dataObjectTest.js index 18b4f62d4b..13f178d6cb 100644 --- a/test/tests/dataObjectTest.js +++ b/test/tests/dataObjectTest.js @@ -56,7 +56,6 @@ describe("Zotero.DataObject", function() { yield obj.saveTx(); if (type == 'item') { - yield obj.loadItemData(); obj.setField('title', Zotero.Utilities.randomString()); } else { @@ -131,7 +130,6 @@ describe("Zotero.DataObject", function() { yield obj.saveTx(); if (type == 'item') { - yield obj.loadItemData(); obj.setField('title', Zotero.Utilities.randomString()); } else { @@ -294,7 +292,7 @@ describe("Zotero.DataObject", function() { let obj = yield createDataObject(type); let libraryID = obj.libraryID; let key = obj.key; - let json = yield obj.toJSON(); + let json = obj.toJSON(); yield Zotero.Sync.Data.Local.saveCacheObjects(type, libraryID, [json]); yield obj.eraseTx(); let versions = yield Zotero.Sync.Data.Local.getCacheObjectVersions( diff --git a/test/tests/dataObjectUtilitiesTest.js b/test/tests/dataObjectUtilitiesTest.js index 565f7d66ce..bf425c1a2c 100644 --- a/test/tests/dataObjectUtilitiesTest.js +++ b/test/tests/dataObjectUtilitiesTest.js @@ -25,11 +25,11 @@ describe("Zotero.DataObjectUtilities", function() { yield Zotero.DB.executeTransaction(function* () { var item = new Zotero.Item('book'); id1 = yield item.save(); - json1 = yield item.toJSON(); + json1 = item.toJSON(); var item = new Zotero.Item('book'); id2 = yield item.save(); - json2 = yield item.toJSON(); + json2 = item.toJSON(); }); var changes = Zotero.DataObjectUtilities.diff(json1, json2); diff --git a/test/tests/fileInterfaceTest.js b/test/tests/fileInterfaceTest.js index d098c29140..b3435593b7 100644 --- a/test/tests/fileInterfaceTest.js +++ b/test/tests/fileInterfaceTest.js @@ -21,7 +21,7 @@ describe("Zotero_File_Interface", function() { let childItems = importedCollection[0].getChildItems(); let savedItems = {}; for (let i=0; i o.toResponseJSON())); + objectResponseJSON[type] = objects[type].map(o => o.toResponseJSON()); } server.respond(function (req) { @@ -457,12 +457,11 @@ describe("Zotero.Sync.Data.Engine", function () { var mtime = new Date().getTime(); var md5 = '57f8a4fda823187b91e1191487b87fe6'; - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncedModificationTime(item.id, mtime); - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, md5); - }); + item.attachmentSyncedModificationTime = mtime; + item.attachmentSyncedHash = md5; + yield item.saveTx({ skipAll: true }); - var itemResponseJSON = yield item.toResponseJSON(); + var itemResponseJSON = item.toResponseJSON(); itemResponseJSON.version = itemResponseJSON.data.version = lastLibraryVersion; itemResponseJSON.data.mtime = mtime; itemResponseJSON.data.md5 = md5; @@ -520,7 +519,7 @@ describe("Zotero.Sync.Data.Engine", function () { for (let type of types) { objects[type] = [yield createDataObject(type, { setTitle: true })]; objectNames[type] = {}; - objectResponseJSON[type] = yield Zotero.Promise.all(objects[type].map(o => o.toResponseJSON())); + objectResponseJSON[type] = objects[type].map(o => o.toResponseJSON()); } server.respond(function (req) { @@ -569,7 +568,6 @@ describe("Zotero.Sync.Data.Engine", function () { let version = o.version; let name = objectNames[type][key]; if (type == 'item') { - yield o.loadItemData(); assert.equal(name, o.getField('title')); } else { @@ -675,7 +673,7 @@ describe("Zotero.Sync.Data.Engine", function () { { key: obj.key, version: obj.version, - data: (yield obj.toJSON()) + data: obj.toJSON() } ] ); diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js index 611ae7abb3..3f5b82c7b5 100644 --- a/test/tests/syncLocalTest.js +++ b/test/tests/syncLocalTest.js @@ -105,7 +105,7 @@ describe("Zotero.Sync.Data.Local", function() { for (let type of types) { let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type); let obj = yield createDataObject(type); - let data = yield obj.toJSON(); + let data = obj.toJSON(); data.key = obj.key; data.version = 10; let json = { @@ -130,7 +130,7 @@ describe("Zotero.Sync.Data.Local", function() { var type = 'item'; let obj = yield createDataObject(type, { version: 5 }); - let data = yield obj.toJSON(); + let data = obj.toJSON(); yield Zotero.Sync.Data.Local.saveCacheObjects( type, libraryID, [data] ); @@ -165,7 +165,7 @@ describe("Zotero.Sync.Data.Local", function() { for (let type of types) { let obj = yield createDataObject(type, { version: 5 }); - let data = yield obj.toJSON(); + let data = obj.toJSON(); yield Zotero.Sync.Data.Local.saveCacheObjects( type, libraryID, [data] ); @@ -175,7 +175,7 @@ describe("Zotero.Sync.Data.Local", function() { let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type); let obj = yield createDataObject(type, { version: 10 }); - let data = yield obj.toJSON(); + let data = obj.toJSON(); yield Zotero.Sync.Data.Local.saveCacheObjects( type, libraryID, [data] ); @@ -222,11 +222,8 @@ describe("Zotero.Sync.Data.Local", function() { yield Zotero.Sync.Data.Local.processSyncCacheForObjectType( libraryID, 'item', { stopOnError: true } ); - var id = Zotero.Items.getIDFromLibraryAndKey(libraryID, key); - assert.equal( - (yield Zotero.Sync.Storage.Local.getSyncState(id)), - Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD - ); + var item = Zotero.Items.getByLibraryAndKey(libraryID, key); + assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD); }) it("should mark updated attachment items for download", function* () { @@ -239,18 +236,13 @@ describe("Zotero.Sync.Data.Local", function() { yield item.saveTx(); // Set file as synced - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncedModificationTime( - item.id, (yield item.attachmentModificationTime) - ); - yield Zotero.Sync.Storage.Local.setSyncedHash( - item.id, (yield item.attachmentHash) - ); - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - }); + item.attachmentSyncedModificationTime = yield item.attachmentModificationTime; + item.attachmentSyncedHash = yield item.attachmentHash; + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); // Simulate download of version with updated attachment - var json = yield item.toResponseJSON(); + var json = item.toResponseJSON(); json.version = 10; json.data.version = 10; json.data.md5 = '57f8a4fda823187b91e1191487b87fe6'; @@ -263,10 +255,7 @@ describe("Zotero.Sync.Data.Local", function() { libraryID, 'item', { stopOnError: true } ); - assert.equal( - (yield Zotero.Sync.Storage.Local.getSyncState(item.id)), - Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD - ); + assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD); }) it("should ignore attachment metadata when resolving metadata conflict", function* () { @@ -276,19 +265,14 @@ describe("Zotero.Sync.Data.Local", function() { var item = yield importFileAttachment('test.png'); item.version = 5; yield item.saveTx(); - var json = yield item.toResponseJSON(); + var json = item.toResponseJSON(); yield Zotero.Sync.Data.Local.saveCacheObjects('item', libraryID, [json]); // Set file as synced - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncedModificationTime( - item.id, (yield item.attachmentModificationTime) - ); - yield Zotero.Sync.Storage.Local.setSyncedHash( - item.id, (yield item.attachmentHash) - ); - yield Zotero.Sync.Storage.Local.setSyncState(item.id, "in_sync"); - }); + item.attachmentSyncedModificationTime = yield item.attachmentModificationTime; + item.attachmentSyncedHash = yield item.attachmentHash; + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); // Modify title locally, leaving item unsynced var newTitle = Zotero.Utilities.randomString(); @@ -307,10 +291,7 @@ describe("Zotero.Sync.Data.Local", function() { ); assert.equal(item.getField('title'), newTitle); - assert.equal( - (yield Zotero.Sync.Storage.Local.getSyncState(item.id)), - Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD - ); + assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD); }) }) @@ -348,7 +329,7 @@ describe("Zotero.Sync.Data.Local", function() { ) } ); - let jsonData = yield obj.toJSON(); + let jsonData = obj.toJSON(); jsonData.key = obj.key; jsonData.version = 10; let json = { @@ -426,7 +407,7 @@ describe("Zotero.Sync.Data.Local", function() { ) } ); - let jsonData = yield obj.toJSON(); + let jsonData = obj.toJSON(); jsonData.key = obj.key; jsonData.version = 10; let json = { @@ -496,7 +477,7 @@ describe("Zotero.Sync.Data.Local", function() { // Create object, generate JSON, and delete var obj = yield createDataObject(type, { version: 10 }); - var jsonData = yield obj.toJSON(); + var jsonData = obj.toJSON(); var key = jsonData.key = obj.key; jsonData.version = 10; let json = { @@ -544,7 +525,7 @@ describe("Zotero.Sync.Data.Local", function() { // Create object, generate JSON, and delete var obj = yield createDataObject(type, { version: 10 }); - var jsonData = yield obj.toJSON(); + var jsonData = obj.toJSON(); var key = jsonData.key = obj.key; jsonData.version = 10; let json = { @@ -576,7 +557,6 @@ describe("Zotero.Sync.Data.Local", function() { obj = objectsClass.getByLibraryAndKey(libraryID, key); assert.ok(obj); - yield obj.loadItemData(); assert.equal(obj.getField('title'), jsonData.title); }) @@ -594,7 +574,7 @@ describe("Zotero.Sync.Data.Local", function() { obj.setNote(""); obj.version = 10; yield obj.saveTx(); - var jsonData = yield obj.toJSON(); + var jsonData = obj.toJSON(); var key = jsonData.key = obj.key; let json = { key: obj.key, @@ -626,7 +606,6 @@ describe("Zotero.Sync.Data.Local", function() { obj = objectsClass.getByLibraryAndKey(libraryID, key); assert.ok(obj); - yield obj.loadNote(); assert.equal(obj.getNote(), noteText2); }) }) diff --git a/test/tests/translateTest.js b/test/tests/translateTest.js index 195413593d..c2d51f2237 100644 --- a/test/tests/translateTest.js +++ b/test/tests/translateTest.js @@ -175,7 +175,7 @@ describe("Zotero.Translate", function() { let newItems = yield saveItemsThroughTranslator("import", saveItems); let savedItems = {}; for (let i=0; i Date: Mon, 7 Mar 2016 17:13:30 -0500 Subject: [PATCH 03/31] Fix test failures --- test/tests/itemTest.js | 14 +++++--------- test/tests/utilitiesTest.js | 1 + 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index a12b5804e3..43108376a9 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -609,11 +609,8 @@ describe("Zotero.Item", function () { // File should be flagged for upload // DEBUG: Is this necessary? - assert.equal( - (yield Zotero.Sync.Storage.Local.getSyncState(item.id)), - Zotero.Sync.Storage.Local.SYNC_STATE_TO_UPLOAD - ); - assert.isNull(yield Zotero.Sync.Storage.Local.getSyncedHash(item.id)); + assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_UPLOAD); + assert.isNull(item.attachmentSyncedHash); }) }) @@ -897,10 +894,9 @@ describe("Zotero.Item", function () { var mtime = new Date().getTime(); var md5 = 'b32e33f529942d73bea4ed112310f804'; - yield Zotero.DB.executeTransaction(function* () { - yield Zotero.Sync.Storage.Local.setSyncedModificationTime(item.id, mtime); - yield Zotero.Sync.Storage.Local.setSyncedHash(item.id, md5); - }); + item.attachmentSyncedModificationTime = mtime; + item.attachmentSyncedHash = md5; + yield item.saveTx({ skipAll: true }); var json = item.toJSON({ syncedStorageProperties: true diff --git a/test/tests/utilitiesTest.js b/test/tests/utilitiesTest.js index 61290ed8d5..a72d9c1e37 100644 --- a/test/tests/utilitiesTest.js +++ b/test/tests/utilitiesTest.js @@ -359,6 +359,7 @@ describe("Zotero.Utilities", function() { }); describe("itemFromCSLJSON", function () { it("should stably perform itemToCSLJSON -> itemFromCSLJSON -> itemToCSLJSON", function* () { + this.timeout(10000); let data = loadSampleData('citeProcJSExport'); for (let i in data) { From ffb3823b4cf40b9a4183264ea86ce8f6c93e79ac Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:08:40 -0500 Subject: [PATCH 04/31] Properly include creators of last item when looking for duplicates --- chrome/content/zotero/xpcom/duplicates.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/chrome/content/zotero/xpcom/duplicates.js b/chrome/content/zotero/xpcom/duplicates.js index 096234ff1b..87021fbe2c 100644 --- a/chrome/content/zotero/xpcom/duplicates.js +++ b/chrome/content/zotero/xpcom/duplicates.js @@ -303,13 +303,13 @@ Zotero.Duplicates.prototype._findDuplicates = Zotero.Promise.coroutine(function* itemCreators = []; } } - else { - itemCreators.push({ - lastName: normalizeString(row.lastName), - firstInitial: row.fieldMode == 0 ? normalizeString(row.firstName).charAt(0) : false - }); - } + lastItemID = row.itemID; + + itemCreators.push({ + lastName: normalizeString(row.lastName), + firstInitial: row.fieldMode == 0 ? normalizeString(row.firstName).charAt(0) : false + }); } // Add final item creators if (itemCreators.length) { From 39f7d0f3332aee3e976484b280a4d6874778a97f Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:12:23 -0500 Subject: [PATCH 05/31] Don't require objects to be saved before calling toJSON() --- chrome/content/zotero/xpcom/data/dataObject.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index 05d5b59c26..c94b5ddf31 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -1175,10 +1175,6 @@ Zotero.DataObject.prototype.toResponseJSON = function (options) { Zotero.DataObject.prototype._preToJSON = function (options) { - if (!this._id) { - throw new Error(`${this._ObjectType} must be saved before running toJSON()`); - } - var env = { options }; env.mode = options.mode || 'new'; if (env.mode == 'patch') { From b9b769db51f2f7f84186b14b1609e25fff7d80a9 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:13:34 -0500 Subject: [PATCH 06/31] Trim repotime before passing to sqlToDate() --- chrome/content/zotero/xpcom/schema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/schema.js b/chrome/content/zotero/xpcom/schema.js index 4d47167924..7bfc9b040a 100644 --- a/chrome/content/zotero/xpcom/schema.js +++ b/chrome/content/zotero/xpcom/schema.js @@ -542,7 +542,7 @@ Zotero.Schema = new function(){ var Mode = Zotero.Utilities.capitalize(mode); var repotime = yield Zotero.File.getContentsFromURLAsync("resource://zotero/schema/repotime.txt"); - var date = Zotero.Date.sqlToDate(repotime, true); + var date = Zotero.Date.sqlToDate(repotime.trim(), true); repotime = Zotero.Date.toUnixTimestamp(date); var fileNameRE = new RegExp("^[^\.].+\\" + fileExt + "$"); From edcd2a16d29077b8578d16f616c1ae9a4a4620b0 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:15:12 -0500 Subject: [PATCH 07/31] Replace obsolete collectionTreeView::getLastViewedRow() call --- chrome/content/zotero/zoteroPane.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index dace7b126c..90df62fbac 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -926,8 +926,7 @@ var ZoteroPane = new function() // Select new row if (show) { Zotero.Prefs.set('lastViewedFolder', lastViewedFolderID); - var row = this.collectionsView.getLastViewedRow(); - this.collectionsView.selection.select(row); + this.collectionsView.selectByID(lastViewedFolderID); // async } } From 28eaaaf2bf5e0e268efb32e16a38aeb42766ee1a Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:16:24 -0500 Subject: [PATCH 08/31] Don't try to parse non-SQL dates in Date.sqlToDate() --- chrome/content/zotero/xpcom/date.js | 8 ++++++-- test/tests/dateTest.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/date.js b/chrome/content/zotero/xpcom/date.js index f62f0e29a7..d89a709930 100644 --- a/chrome/content/zotero/xpcom/date.js +++ b/chrome/content/zotero/xpcom/date.js @@ -87,6 +87,10 @@ Zotero.Date = new function(){ **/ function sqlToDate(sqldate, isUTC){ try { + if (!this.isSQLDate(sqldate) && !this.isSQLDateTime(sqldate)) { + throw new Error("Invalid date"); + } + var datetime = sqldate.split(' '); var dateparts = datetime[0].split('-'); if (datetime[1]){ @@ -98,7 +102,7 @@ Zotero.Date = new function(){ // Invalid date part if (dateparts.length==1){ - return false; + throw new Error("Invalid date part"); } if (isUTC){ @@ -699,7 +703,7 @@ Zotero.Date = new function(){ function toUnixTimestamp(date) { if (date === null || typeof date != 'object' || date.constructor.name != 'Date') { - throw ('Not a valid date in Zotero.Date.toUnixTimestamp()'); + throw new Error(`'${date}' is not a valid date`); } return Math.round(date.getTime() / 1000); } diff --git a/test/tests/dateTest.js b/test/tests/dateTest.js index dd6d6ff60c..7098c7193e 100644 --- a/test/tests/dateTest.js +++ b/test/tests/dateTest.js @@ -1,4 +1,19 @@ describe("Zotero.Date", function() { + describe("#sqlToDate()", function () { + it("should convert an SQL local date into a JS Date object", function* () { + var date = "2016-02-27 22:00:00"; + var offset = new Date().getTimezoneOffset() * 60 * 1000; + date = Zotero.Date.sqlToDate(date); + assert.equal(date.getTime(), 1456610400000 + offset); + }) + + it("should convert an SQL UTC date into a JS Date object", function* () { + var date = "2016-02-27 22:00:00"; + date = Zotero.Date.sqlToDate(date, true); + assert.equal(date.getTime(), 1456610400000); + }) + }) + describe("#isISODate()", function () { it("should determine whether a date is an ISO 8601 date", function () { assert.ok(Zotero.Date.isISODate("2015")); From 7a03b1e527b16a8bd4386a6614426ccdcd3620ea Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:28:38 -0500 Subject: [PATCH 09/31] Accept ISO dates in Item::setField() --- chrome/content/zotero/xpcom/data/item.js | 25 +++++++++++---- test/tests/itemTest.js | 39 ++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 2438f7d325..ab60603b70 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -663,10 +663,16 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) { switch (field) { case 'itemTypeID': - case 'dateAdded': break; + case 'dateAdded': case 'dateModified': + // Accept ISO dates + if (Zotero.Date.isISODate(value)) { + let d = Zotero.Date.isoToDate(value); + value = Zotero.Date.dateToSQL(d, true); + } + // Make sure it's valid let date = Zotero.Date.sqlToDate(value, true); if (!date) throw new Error("Invalid SQL date: " + value); @@ -790,11 +796,18 @@ Zotero.Item.prototype.setField = function(field, value, loadIn) { } // Validate access date else if (fieldID == Zotero.ItemFields.getID('accessDate')) { - if (value && (!Zotero.Date.isSQLDate(value) && - !Zotero.Date.isSQLDateTime(value) && - value != 'CURRENT_TIMESTAMP')) { - Zotero.debug("Discarding invalid accessDate '" + value + "' in Item.setField()"); - return false; + if (value && value != 'CURRENT_TIMESTAMP') { + // Accept ISO dates + if (Zotero.Date.isISODate(value)) { + let d = Zotero.Date.isoToDate(value); + value = Zotero.Date.dateToSQL(d, true); + } + + if (!Zotero.Date.isSQLDate(value) && !Zotero.Date.isSQLDateTime(value)) { + Zotero.logError(`Discarding invalid ${field} '${value}' for ` + + `item ${this.libraryKey} in setField()`); + return false; + } } } diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index 43108376a9..de34829056 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -135,6 +135,45 @@ describe("Zotero.Item", function () { item = yield Zotero.Items.getAsync(id); assert.equal(item.getField("versionNumber"), "1.0"); }); + + it("should accept ISO 8601 dates", function* () { + var fields = { + accessDate: "2015-06-07T20:56:00Z", + dateAdded: "2015-06-07T20:57:00Z", + dateModified: "2015-06-07T20:58:00Z", + }; + var item = createUnsavedDataObject('item'); + for (let i in fields) { + item.setField(i, fields[i]); + } + assert.equal(item.getField('accessDate'), '2015-06-07 20:56:00'); + assert.equal(item.dateAdded, '2015-06-07 20:57:00'); + assert.equal(item.dateModified, '2015-06-07 20:58:00'); + }) + + it("should accept SQL dates", function* () { + var fields = { + accessDate: "2015-06-07 20:56:00", + dateAdded: "2015-06-07 20:57:00", + dateModified: "2015-06-07 20:58:00", + }; + var item = createUnsavedDataObject('item'); + for (let i in fields) { + item.setField(i, fields[i]); + item.getField(i, fields[i]); + } + }) + + it("should ignore unknown accessDate values", function* () { + var fields = { + accessDate: "foo" + }; + var item = createUnsavedDataObject('item'); + for (let i in fields) { + item.setField(i, fields[i]); + } + assert.strictEqual(item.getField('accessDate'), ''); + }) }) describe("#dateAdded", function () { From 277ddc39f892dd4e8a6a7c5130d4fbfb54347f67 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:24:00 -0500 Subject: [PATCH 10/31] Don't include dateAdded/dateModified in item JSON if not set --- chrome/content/zotero/xpcom/data/item.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index ab60603b70..360b6bd66b 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -4046,10 +4046,15 @@ Zotero.Item.prototype.toJSON = function (options = {}) { obj.deleted = deleted ? 1 : 0; } - obj.dateAdded = Zotero.Date.sqlToISO8601(this.dateAdded); - obj.dateModified = Zotero.Date.sqlToISO8601(this.dateModified); if (obj.accessDate) obj.accessDate = Zotero.Date.sqlToISO8601(obj.accessDate); + if (this.dateAdded) { + obj.dateAdded = Zotero.Date.sqlToISO8601(this.dateAdded); + } + if (this.dateModified) { + obj.dateModified = Zotero.Date.sqlToISO8601(this.dateModified); + } + return this._postToJSON(env); } From ae6d560a66a4d8e8280b369b272446f0499cc014 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:30:28 -0500 Subject: [PATCH 11/31] Fix Item::multiDiff() --- chrome/content/zotero/xpcom/data/item.js | 36 +++++++++++------------- test/tests/itemTest.js | 32 +++++++++++++++++++++ 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 360b6bd66b..cee972116b 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3623,27 +3623,23 @@ Zotero.Item.prototype.multiDiff = function (otherItems, ignoreFields) { var hasDiffs = false; for (let i = 0; i < otherItems.length; i++) { - let otherItem = otherItems[i]; - let diff = []; - let otherData = otherItem.toJSON(); - let numDiffs = this.ObjectsClass.diff(thisData, otherData, diff); + let otherData = otherItems[i].toJSON(); + let changeset = Zotero.DataObjectUtilities.diff(thisData, otherData, ignoreFields); - if (numDiffs) { - for (let field in diff[1]) { - if (ignoreFields && ignoreFields.indexOf(field) != -1) { - continue; - } - - var value = diff[1][field]; - - if (!alternatives[field]) { - hasDiffs = true; - alternatives[field] = [value]; - } - else if (alternatives[field].indexOf(value) == -1) { - hasDiffs = true; - alternatives[field].push(value); - } + for (let i = 0; i < changeset.length; i++) { + let change = changeset[i]; + + if (change.op == 'delete') { + continue; + } + + if (!alternatives[change.field]) { + hasDiffs = true; + alternatives[change.field] = [change.value]; + } + else if (alternatives[change.field].indexOf(change.value) == -1) { + hasDiffs = true; + alternatives[change.field].push(change.value); } } } diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index de34829056..1a05fcbd54 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -853,6 +853,38 @@ describe("Zotero.Item", function () { }) }) + + describe("#multiDiff", function () { + it("should return set of alternatives for differing fields in other items", function* () { + var type = 'item'; + + var dates = ['2016-03-08 17:44:45']; + var accessDates = ['2016-03-08T18:44:45Z']; + var urls = ['http://www.example.com', 'http://example.net']; + + var obj1 = createUnsavedDataObject(type); + obj1.setField('date', '2016-03-07 12:34:56'); // different in 1 and 3, not in 2 + obj1.setField('url', 'http://example.com'); // different in all three + obj1.setField('title', 'Test'); // only in 1 + + var obj2 = createUnsavedDataObject(type); + obj2.setField('url', urls[0]); + obj2.setField('accessDate', accessDates[0]); // only in 2 + + var obj3 = createUnsavedDataObject(type); + obj3.setField('date', dates[0]); + obj3.setField('url', urls[1]); + + var alternatives = obj1.multiDiff([obj2, obj3]); + + assert.sameMembers(Object.keys(alternatives), ['url', 'date', 'accessDate']); + assert.sameMembers(alternatives.url, urls); + assert.sameMembers(alternatives.date, dates); + assert.sameMembers(alternatives.accessDate, accessDates); + }); + }); + + describe("#clone()", function () { // TODO: Expand to other data it("should copy creators", function* () { From f310c391623af6d94e8b0e8ddf3cf2e921b48fcb Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 03:33:25 -0500 Subject: [PATCH 12/31] Remove Item::copy() Instead of creating a duplicate copy of the item with the same primary data and saving that, it's safer just to use clone() (which doesn't preserve ids) and apply changes to the main object. --- chrome/content/zotero/xpcom/data/item.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index cee972116b..c0bd83fcbd 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3721,17 +3721,6 @@ Zotero.Item.prototype.clone = function (libraryID, skipTags) { } -/** - * @return {Promise} - A copy of the item with primary data loaded - */ -Zotero.Item.prototype.copy = Zotero.Promise.coroutine(function* () { - var newItem = new Zotero.Item; - newItem.id = this.id; - yield newItem.loadPrimaryData(); - return newItem; -});; - - Zotero.Item.prototype._eraseData = Zotero.Promise.coroutine(function* (env) { Zotero.DB.requireTransaction(); From 394aa8ddedd007885744d93a082b832093b1f4ab Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 04:50:55 -0500 Subject: [PATCH 13/31] Update display title after item edit --- chrome/content/zotero/xpcom/data/item.js | 205 +++++++++++----------- chrome/content/zotero/xpcom/data/items.js | 3 +- test/tests/itemTreeViewTest.js | 11 ++ test/tests/translateTest.js | 14 +- 4 files changed, 121 insertions(+), 112 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index c0bd83fcbd..8fb769dbfa 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -849,6 +849,105 @@ Zotero.Item.prototype.getDisplayTitle = function (includeAuthorAndDate) { } +/** + * Update the generated display title from the loaded data + */ +Zotero.Item.prototype.updateDisplayTitle = function () { + var title = this.getField('title', false, true); + var itemTypeID = this.itemTypeID; + var itemTypeName = Zotero.ItemTypes.getName(itemTypeID); + + if (title === "" && (itemTypeID == 8 || itemTypeID == 10)) { // 'letter' and 'interview' itemTypeIDs + var creatorsData = this.getCreators(); + var authors = []; + var participants = []; + for (let i=0; i 0) { + let names = []; + let max = Math.min(4, participants.length); + for (let i=0; i 0) { - let names = []; - let max = Math.min(4, participants.length); - for (let i=0; i Date: Fri, 11 Mar 2016 07:34:57 -0500 Subject: [PATCH 14/31] Fix display of Duplicate/Unfiled Items rows --- .../zotero/xpcom/collectionTreeView.js | 41 ++++++++++--------- chrome/content/zotero/zoteroPane.js | 21 ++++------ test/tests/collectionTreeViewTest.js | 22 ++++++++++ 3 files changed, 52 insertions(+), 32 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index fc35fb239a..015d9763e0 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -156,25 +156,28 @@ Zotero.CollectionTreeView.prototype.refresh = Zotero.Promise.coroutine(function* this._containerState = {}; } - if (this.hideSources.indexOf('duplicates') == -1) { - try { - this._duplicateLibraries = Zotero.Prefs.get('duplicateLibraries').split(',').map(function (val) parseInt(val)); - } - catch (e) { - // Add to personal library by default - Zotero.Prefs.set('duplicateLibraries', '0'); - this._duplicateLibraries = [0]; - } - } + var userLibraryID = Zotero.Libraries.userLibraryID; - try { - this._unfiledLibraries = Zotero.Prefs.get('unfiledLibraries').split(',').map(function (val) parseInt(val)); - } - catch (e) { - // Add to personal library by default - Zotero.Prefs.set('unfiledLibraries', '0'); - this._unfiledLibraries = [0]; + var readPref = function (pref) { + let ids = Zotero.Prefs.get(pref); + if (ids === "") { + this["_" + pref] = []; + } + else { + if (ids === undefined || typeof ids != 'string') { + ids = "" + userLibraryID; + Zotero.Prefs.set(pref, "" + userLibraryID); + } + this["_" + pref] = ids.split(',') + // Convert old id and convert to int + .map(id => id === "0" ? userLibraryID : parseInt(id)); + } + }.bind(this); + + if (this.hideSources.indexOf('duplicates') == -1) { + readPref('duplicateLibraries'); } + readPref('unfiledLibraries'); var oldCount = this.rowCount || 0; var newRows = []; @@ -916,10 +919,10 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi break; } - if (!found) { + var row = this._rowMap[type + id]; + if (!row) { return false; } - var row = this._rowMap[type + id]; this._treebox.ensureRowIsVisible(row); yield this.selectWait(row); diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 90df62fbac..914e8211b4 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -860,7 +860,7 @@ var ZoteroPane = new function() }); - this.setVirtual = function (libraryID, mode, show) { + this.setVirtual = Zotero.Promise.coroutine(function* (libraryID, mode, show) { switch (mode) { case 'duplicates': var prefKey = 'duplicateLibraries'; @@ -873,7 +873,7 @@ var ZoteroPane = new function() break; default: - throw ("Invalid virtual mode '" + mode + "' in ZoteroPane.setVirtual()"); + throw new Error("Invalid virtual mode '" + mode + "'"); } try { @@ -883,10 +883,6 @@ var ZoteroPane = new function() var ids = []; } - if (!libraryID) { - libraryID = Zotero.Libraries.userLibraryID; - } - var newids = []; for (let i = 0; i < ids.length; i++) { let id = ids[i]; @@ -898,8 +894,8 @@ var ZoteroPane = new function() if (id == libraryID && !show) { continue; } - // Remove libraryIDs that no longer exist - if (id != 0 && !Zotero.Libraries.exists(id)) { + // Remove libraries that no longer exist + if (!Zotero.Libraries.exists(id)) { continue; } newids.push(id); @@ -914,10 +910,10 @@ var ZoteroPane = new function() Zotero.Prefs.set(prefKey, newids.join()); - this.collectionsView.refresh(); + yield this.collectionsView.refresh(); // If group is closed, open it - this.collectionsView.selectLibrary(libraryID); + yield this.collectionsView.selectLibrary(libraryID); row = this.collectionsView.selection.currentIndex; if (!this.collectionsView.isContainerOpen(row)) { this.collectionsView.toggleOpenState(row); @@ -925,10 +921,9 @@ var ZoteroPane = new function() // Select new row if (show) { - Zotero.Prefs.set('lastViewedFolder', lastViewedFolderID); - this.collectionsView.selectByID(lastViewedFolderID); // async + yield this.collectionsView.selectByID(lastViewedFolderID); } - } + }); this.openLookupWindow = Zotero.Promise.coroutine(function* () { diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index f8a77e41bd..51ae7dd515 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -16,6 +16,28 @@ describe("Zotero.CollectionTreeView", function() { win.close(); }); + describe("#refresh()", function () { + it("should show Duplicate Items and Unfiled Items in My Library by default", function* () { + Zotero.Prefs.clear('duplicateLibraries'); + Zotero.Prefs.clear('unfiledLibraries'); + yield cv.refresh(); + yield assert.eventually.ok(cv.selectByID("D" + userLibraryID)); + yield assert.eventually.ok(cv.selectByID("U" + userLibraryID)); + assert.equal(Zotero.Prefs.get('duplicateLibraries'), "" + userLibraryID); + assert.equal(Zotero.Prefs.get('unfiledLibraries'), "" + userLibraryID); + }); + + it("shouldn't show Duplicate Items and Unfiled Items if hidden", function* () { + Zotero.Prefs.set('duplicateLibraries', ""); + Zotero.Prefs.set('unfiledLibraries', ""); + yield cv.refresh(); + yield assert.eventually.notOk(cv.selectByID("D" + userLibraryID)); + yield assert.eventually.notOk(cv.selectByID("U" + userLibraryID)); + assert.strictEqual(Zotero.Prefs.get('duplicateLibraries'), ""); + assert.strictEqual(Zotero.Prefs.get('unfiledLibraries'), ""); + }); + }); + describe("collapse/expand", function () { it("should close and open My Library repeatedly", function* () { var libraryID = Zotero.Libraries.userLibraryID; From 1982938bc71aa3b36a2fd93fa2c62e15dbe6a79a Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 07:36:36 -0500 Subject: [PATCH 15/31] Store userLibraryID in test --- test/tests/collectionTreeViewTest.js | 32 ++++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 51ae7dd515..f78c2d3e96 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -1,12 +1,13 @@ "use strict"; describe("Zotero.CollectionTreeView", function() { - var win, zp, cv; + var win, zp, cv, userLibraryID; before(function* () { win = yield loadZoteroPane(); zp = win.ZoteroPane; cv = zp.collectionsView; + userLibraryID = Zotero.Libraries.userLibraryID; }); beforeEach(function () { // TODO: Add a selectCollection() function and select a collection instead? @@ -40,29 +41,28 @@ describe("Zotero.CollectionTreeView", function() { describe("collapse/expand", function () { it("should close and open My Library repeatedly", function* () { - var libraryID = Zotero.Libraries.userLibraryID; - yield cv.selectLibrary(libraryID); + yield cv.selectLibrary(userLibraryID); var row = cv.selection.currentIndex; - cv.collapseLibrary(libraryID); + cv.collapseLibrary(userLibraryID); var nextRow = cv.getRow(row + 1); assert.equal(cv.selection.currentIndex, row); assert.ok(nextRow.isSeparator()); assert.isFalse(cv.isContainerOpen(row)); - yield cv.expandLibrary(libraryID); + yield cv.expandLibrary(userLibraryID); nextRow = cv.getRow(row + 1); assert.equal(cv.selection.currentIndex, row); assert.ok(!nextRow.isSeparator()); assert.ok(cv.isContainerOpen(row)); - cv.collapseLibrary(libraryID); + cv.collapseLibrary(userLibraryID); nextRow = cv.getRow(row + 1); assert.equal(cv.selection.currentIndex, row); assert.ok(nextRow.isSeparator()); assert.isFalse(cv.isContainerOpen(row)); - yield cv.expandLibrary(libraryID); + yield cv.expandLibrary(userLibraryID); nextRow = cv.getRow(row + 1); assert.equal(cv.selection.currentIndex, row); assert.ok(!nextRow.isSeparator()); @@ -96,13 +96,13 @@ describe("Zotero.CollectionTreeView", function() { var row = cv.selection.currentIndex; var treeRow = cv.getRow(row); assert.ok(treeRow.isTrash()); - assert.equal(treeRow.ref.libraryID, Zotero.Libraries.userLibraryID); + assert.equal(treeRow.ref.libraryID, userLibraryID); }) }) describe("#selectWait()", function () { it("shouldn't hang if row is already selected", function* () { - var row = cv.getRowIndexByID("T" + Zotero.Libraries.userLibraryID); + var row = cv.getRowIndexByID("T" + userLibraryID); cv.selection.select(row); yield Zotero.Promise.delay(50); yield cv.selectWait(row); @@ -130,7 +130,7 @@ describe("Zotero.CollectionTreeView", function() { }); // Library should still be selected - assert.equal(cv.getSelectedLibraryID(), Zotero.Libraries.userLibraryID); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); }); it("shouldn't select a new collection if skipSelect is passed", function* () { @@ -142,7 +142,7 @@ describe("Zotero.CollectionTreeView", function() { }); // Library should still be selected - assert.equal(cv.getSelectedLibraryID(), Zotero.Libraries.userLibraryID); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); }); it("shouldn't select a modified collection", function* () { @@ -157,7 +157,7 @@ describe("Zotero.CollectionTreeView", function() { yield collection.saveTx(); // Modified collection should not be selected - assert.equal(cv.getSelectedLibraryID(), Zotero.Libraries.userLibraryID); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); }); it("should maintain selection on a selected modified collection", function* () { @@ -239,8 +239,8 @@ describe("Zotero.CollectionTreeView", function() { var collectionRow = cv._rowMap["C" + collectionID]; var searchRow = cv._rowMap["S" + searchID]; - var duplicatesRow = cv._rowMap["D" + Zotero.Libraries.userLibraryID]; - var unfiledRow = cv._rowMap["U" + Zotero.Libraries.userLibraryID]; + var duplicatesRow = cv._rowMap["D" + userLibraryID]; + var unfiledRow = cv._rowMap["U" + userLibraryID]; assert.isAbove(searchRow, collectionRow); // If there's a duplicates row or an unfiled row, add before those. @@ -252,7 +252,7 @@ describe("Zotero.CollectionTreeView", function() { assert.isBelow(searchRow, unfiledRow); } else { - var trashRow = cv._rowMap["T" + Zotero.Libraries.userLibraryID]; + var trashRow = cv._rowMap["T" + userLibraryID]; assert.isBelow(searchRow, trashRow); } }) @@ -260,7 +260,7 @@ describe("Zotero.CollectionTreeView", function() { it("shouldn't select a new group", function* () { var group = yield createGroup(); // Library should still be selected - assert.equal(cv.getSelectedLibraryID(), Zotero.Libraries.userLibraryID); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); }) it("should remove a group and all children", function* () { From c4ca22ca62c7db21dad49c7825c738ec5f0f2a6c Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 11 Mar 2016 07:37:48 -0500 Subject: [PATCH 16/31] Fix duplicate merging Fixes #919 --- chrome/content/zotero/duplicatesMerge.js | 49 ++++++------ .../content/zotero/xpcom/libraryTreeView.js | 4 +- test/tests/duplicatesTest.js | 78 +++++++++++++++++++ 3 files changed, 105 insertions(+), 26 deletions(-) create mode 100644 test/tests/duplicatesTest.js diff --git a/chrome/content/zotero/duplicatesMerge.js b/chrome/content/zotero/duplicatesMerge.js index f6682aae73..0d951cde22 100644 --- a/chrome/content/zotero/duplicatesMerge.js +++ b/chrome/content/zotero/duplicatesMerge.js @@ -23,10 +23,13 @@ ***** END LICENSE BLOCK ***** */ +"use strict"; + var Zotero_Duplicates_Pane = new function () { - _items = []; - _otherItems = []; - _ignoreFields = ['dateAdded', 'dateModified', 'accessDate']; + var _masterItem; + var _items = []; + var _otherItems = []; + var _ignoreFields = ['dateAdded', 'dateModified', 'accessDate']; this.setItems = function (items, displayNumItemsOnTypeError) { var itemTypeID, oldestItem, otherItems = []; @@ -77,15 +80,13 @@ var Zotero_Duplicates_Pane = new function () { // Update the UI // - var diff = oldestItem.multiDiff(otherItems, _ignoreFields); - var button = document.getElementById('zotero-duplicates-merge-button'); var versionSelect = document.getElementById('zotero-duplicates-merge-version-select'); var itembox = document.getElementById('zotero-duplicates-merge-item-box'); var fieldSelect = document.getElementById('zotero-duplicates-merge-field-select'); - versionSelect.hidden = !diff; - if (diff) { + var alternatives = oldestItem.multiDiff(otherItems, _ignoreFields); + if (alternatives) { // Populate menulist with Date Added values from all items var dateList = document.getElementById('zotero-duplicates-merge-original-date'); @@ -111,8 +112,8 @@ var Zotero_Duplicates_Pane = new function () { } button.label = Zotero.getString('pane.item.duplicates.mergeItems', (otherItems.length + 1)); - itembox.hiddenFields = diff ? [] : ['dateAdded', 'dateModified']; - fieldSelect.hidden = !diff; + versionSelect.hidden = fieldSelect.hidden = !alternatives; + itembox.hiddenFields = alternatives ? [] : ['dateAdded', 'dateModified']; this.setMaster(0); @@ -130,25 +131,25 @@ var Zotero_Duplicates_Pane = new function () { // Add master item's values to the beginning of each set of // alternative values so that they're still available if the item box // modifies the item - Zotero.spawn(function* () { - var diff = item.multiDiff(_otherItems, _ignoreFields); - if (diff) { - let itemValues = item.toJSON(); - for (let i in diff) { - diff[i].unshift(itemValues[i] !== undefined ? itemValues[i] : ''); - } - itembox.fieldAlternatives = diff; + var alternatives = item.multiDiff(_otherItems, _ignoreFields); + if (alternatives) { + let itemValues = item.toJSON(); + for (let i in alternatives) { + alternatives[i].unshift(itemValues[i] !== undefined ? itemValues[i] : ''); } - - var newItem = yield item.copy(); - itembox.item = newItem; - }); + itembox.fieldAlternatives = alternatives; + } + + _masterItem = item; + itembox.item = item.clone(); } - this.merge = function () { + this.merge = Zotero.Promise.coroutine(function* () { var itembox = document.getElementById('zotero-duplicates-merge-item-box'); Zotero.CollectionTreeCache.clear(); - Zotero.Items.merge(itembox.item, _otherItems); - } + // Update master item with any field alternatives from the item box + _masterItem.fromJSON(itembox.item.toJSON()); + Zotero.Items.merge(_masterItem, _otherItems); + }); } diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js index 18e96855ab..4d1f29c172 100644 --- a/chrome/content/zotero/xpcom/libraryTreeView.js +++ b/chrome/content/zotero/xpcom/libraryTreeView.js @@ -79,7 +79,7 @@ Zotero.LibraryTreeView.prototype = { * Return the index of the row with a given ID (e.g., "C123" for collection 123) * * @param {String} - Row id - * @return {Integer} + * @return {Integer|false} */ getRowIndexByID: function (id) { var type = ""; @@ -87,7 +87,7 @@ Zotero.LibraryTreeView.prototype = { var type = id[0]; id = ('' + id).substr(1); } - return this._rowMap[type + id]; + return this._rowMap[type + id] !== undefined ? this._rowMap[type + id] : false; }, diff --git a/test/tests/duplicatesTest.js b/test/tests/duplicatesTest.js new file mode 100644 index 0000000000..bcb3290d86 --- /dev/null +++ b/test/tests/duplicatesTest.js @@ -0,0 +1,78 @@ +"use strict"; + +describe("Duplicate Items", function () { + var win, zp, cv; + + beforeEach(function* () { + Zotero.Prefs.clear('duplicateLibraries'); + win = yield loadZoteroPane(); + zp = win.ZoteroPane; + cv = zp.collectionsView; + + return selectLibrary(win); + }) + after(function () { + if (win) { + win.close(); + } + }); + + describe("Merging", function () { + it("should merge two items in duplicates view", function* () { + var item1 = yield createDataObject('item', { setTitle: true }); + var item2 = item1.clone(); + yield item2.saveTx(); + var uri2 = Zotero.URI.getItemURI(item2); + + var userLibraryID = Zotero.Libraries.userLibraryID; + + var selected = yield cv.selectByID('D' + userLibraryID); + assert.ok(selected); + yield waitForItemsLoad(win); + + // Select the first item, which should select both + var iv = zp.itemsView; + var index = iv.getRowIndexByID(item1.id); + assert.isNumber(index); + + var x = {}; + var y = {}; + var width = {}; + var height = {}; + iv._treebox.getCoordsForCellItem( + index, + iv._treebox.columns.getNamedColumn('zotero-items-column-title'), + 'text', + x, y, width, height + ); + + // Select row to trigger multi-select + var tree = iv._treebox.treeBody; + var rect = tree.getBoundingClientRect(); + var x = rect.left + x.value; + var y = rect.top + y.value; + tree.dispatchEvent(new MouseEvent("mousedown", { + clientX: x, + clientY: y, + detail: 1 + })); + + assert.equal(iv.selection.count, 2); + + // Click merge button + var button = win.document.getElementById('zotero-duplicates-merge-button'); + button.click(); + + yield waitForNotifierEvent('refresh', 'trash'); + + // Items should be gone + assert.isFalse(iv.getRowIndexByID(item1.id)); + assert.isFalse(iv.getRowIndexByID(item2.id)); + assert.isTrue(item2.deleted); + var rels = item1.getRelations(); + var pred = Zotero.Relations.replacedItemPredicate; + assert.property(rels, pred); + assert.equal(rels[pred], uri2); + }); + }); +}); From 36db3c98b3c731b894f4e646b877db1964a63ff7 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 12 Mar 2016 05:03:14 -0500 Subject: [PATCH 17/31] Fix selecting last selected collection --- chrome/content/zotero/xpcom/collectionTreeView.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 015d9763e0..b19407eeac 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -919,6 +919,10 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi break; } + if (found) { + return true; + } + var row = this._rowMap[type + id]; if (!row) { return false; From 0fc91a4ef281b9cf2ff0bc89cb33c7d175f715e6 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 12 Mar 2016 05:03:47 -0500 Subject: [PATCH 18/31] Tests for showing/hiding virtual folders --- test/tests/collectionTreeViewTest.js | 8 ++-- test/tests/zoteroPaneTest.js | 71 +++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index f78c2d3e96..e5b8e9c4a8 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -22,8 +22,8 @@ describe("Zotero.CollectionTreeView", function() { Zotero.Prefs.clear('duplicateLibraries'); Zotero.Prefs.clear('unfiledLibraries'); yield cv.refresh(); - yield assert.eventually.ok(cv.selectByID("D" + userLibraryID)); - yield assert.eventually.ok(cv.selectByID("U" + userLibraryID)); + assert.isTrue(cv.getRowIndexByID("D" + userLibraryID)); + assert.isTrue(cv.getRowIndexByID("U" + userLibraryID)); assert.equal(Zotero.Prefs.get('duplicateLibraries'), "" + userLibraryID); assert.equal(Zotero.Prefs.get('unfiledLibraries'), "" + userLibraryID); }); @@ -32,8 +32,8 @@ describe("Zotero.CollectionTreeView", function() { Zotero.Prefs.set('duplicateLibraries', ""); Zotero.Prefs.set('unfiledLibraries', ""); yield cv.refresh(); - yield assert.eventually.notOk(cv.selectByID("D" + userLibraryID)); - yield assert.eventually.notOk(cv.selectByID("U" + userLibraryID)); + assert.isFalse(cv.getRowIndexByID("D" + userLibraryID)); + assert.isFalse(cv.getRowIndexByID("U" + userLibraryID)); assert.strictEqual(Zotero.Prefs.get('duplicateLibraries'), ""); assert.strictEqual(Zotero.Prefs.get('unfiledLibraries'), ""); }); diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 5d768bfaaf..1b88d040c3 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1,13 +1,14 @@ "use strict"; describe("ZoteroPane", function() { - var win, doc, zp; + var win, doc, zp, userLibraryID; // Load Zotero pane and select library before(function* () { win = yield loadZoteroPane(); doc = win.document; zp = win.ZoteroPane; + userLibraryID = Zotero.Libraries.userLibraryID; }); after(function () { @@ -201,4 +202,72 @@ describe("ZoteroPane", function() { assert.equal((yield Zotero.File.getContentsAsync(path)), text); }) }) + + + describe("#setVirtual()", function () { + var cv; + + before(function* () { + cv = zp.collectionsView; + }); + beforeEach(function () { + Zotero.Prefs.clear('duplicateLibraries'); + Zotero.Prefs.clear('unfiledLibraries'); + return selectLibrary(win); + }) + + it("should show a hidden virtual folder", function* () { + // Start hidden + Zotero.Prefs.set('duplicateLibraries', ""); + Zotero.Prefs.set('unfiledLibraries', ""); + yield cv.refresh(); + + // Show Duplicate Items + var id = "D" + userLibraryID; + assert.isFalse(cv.getRowIndexByID(id)); + yield zp.setVirtual(userLibraryID, 'duplicates', true); + assert.ok(cv.getRowIndexByID(id)); + + // Show Unfiled Items + id = "U" + userLibraryID; + assert.isFalse(cv.getRowIndexByID(id)); + yield zp.setVirtual(userLibraryID, 'unfiled', true); + assert.ok(cv.getRowIndexByID(id)); + }); + + it("should hide a virtual folder shown by default", function* () { + yield cv.refresh(); + + // Hide Duplicate Items + var id = "D" + userLibraryID; + assert.ok(yield cv.selectByID(id)); + yield zp.setVirtual(userLibraryID, 'duplicates', false); + assert.isFalse(cv.getRowIndexByID(id)); + + // Hide Unfiled Items + id = "U" + userLibraryID; + assert.ok(yield cv.selectByID(id)); + yield zp.setVirtual(userLibraryID, 'unfiled', false); + assert.isFalse(cv.getRowIndexByID(id)); + }); + + it("should hide an explicitly shown virtual folder", function* () { + // Start shown + Zotero.Prefs.set('duplicateLibraries', "" + userLibraryID); + Zotero.Prefs.set('unfiledLibraries' "" + userLibraryID); + yield cv.refresh(); + + // Hide Duplicate Items + var id = "D" + userLibraryID; + assert.ok(yield cv.selectByID(id)); + yield zp.setVirtual(userLibraryID, 'duplicates', false); + assert.isFalse(cv.getRowIndexByID(id)); + + // Hide Unfiled Items + id = "U" + userLibraryID; + assert.ok(yield cv.selectByID(id)); + yield zp.setVirtual(userLibraryID, 'unfiled', false); + assert.isFalse(cv.getRowIndexByID(id)); + }); + }); }) From c18675801f25e584d61f0375d7bc2e693e7e84c0 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 04:57:45 -0400 Subject: [PATCH 19/31] Fix breakage in collection selection from 36db3c98 --- chrome/content/zotero/xpcom/collectionTreeView.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index b19407eeac..43de285b21 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -902,8 +902,7 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi switch (type) { case 'L': - var found = yield this.selectLibrary(id); - break; + return yield this.selectLibrary(id); case 'C': var found = yield this.expandToCollection(id); @@ -915,12 +914,7 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi break; case 'T': - var found = yield this.selectTrash(id); - break; - } - - if (found) { - return true; + return yield this.selectTrash(id); } var row = this._rowMap[type + id]; From 75bf69526c081a424549dbb3aa3a768887750c4f Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 20:00:29 -0400 Subject: [PATCH 20/31] Daylight saving time: it's a thing! --- test/tests/dateTest.js | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/test/tests/dateTest.js b/test/tests/dateTest.js index 7098c7193e..2b4588f671 100644 --- a/test/tests/dateTest.js +++ b/test/tests/dateTest.js @@ -1,10 +1,24 @@ describe("Zotero.Date", function() { describe("#sqlToDate()", function () { it("should convert an SQL local date into a JS Date object", function* () { - var date = "2016-02-27 22:00:00"; - var offset = new Date().getTimezoneOffset() * 60 * 1000; - date = Zotero.Date.sqlToDate(date); - assert.equal(date.getTime(), 1456610400000 + offset); + var d1 = new Date(); + var sqlDate = d1.getFullYear() + + '-' + + Zotero.Utilities.lpad(d1.getMonth() + 1, '0', 2) + + '-' + + Zotero.Utilities.lpad(d1.getDate(), '0', 2) + + ' ' + + Zotero.Utilities.lpad(d1.getHours(), '0', 2) + + ':' + + Zotero.Utilities.lpad(d1.getMinutes(), '0', 2) + + ':' + + Zotero.Utilities.lpad(d1.getSeconds(), '0', 2); + var offset = d1.getTimezoneOffset() * 60 * 1000; + var d2 = Zotero.Date.sqlToDate(sqlDate); + assert.equal( + Zotero.Date.sqlToDate(sqlDate).getTime(), + Math.floor(new Date().getTime() / 1000) * 1000 + ); }) it("should convert an SQL UTC date into a JS Date object", function* () { From d86b622c0a99037bceef382c0ec43b64c52ef9e4 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 20:27:18 -0400 Subject: [PATCH 21/31] More daylight saving time --- chrome/content/zotero/xpcom/storage/webdav.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/storage/webdav.js b/chrome/content/zotero/xpcom/storage/webdav.js index 49c133c6bd..67c9531b62 100644 --- a/chrome/content/zotero/xpcom/storage/webdav.js +++ b/chrome/content/zotero/xpcom/storage/webdav.js @@ -1072,7 +1072,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = { response, ".//D:getlastmodified", { D: 'DAV:' } ); lastModified = Zotero.Date.strToISO(lastModified); - lastModified = Zotero.Date.sqlToDate(lastModified); + lastModified = Zotero.Date.sqlToDate(lastModified, true); // Delete files older than a day before last sync time var days = (lastSyncDate - lastModified) / 1000 / 60 / 60 / 24; From 4fcf0c3c15c3c78f39d8ad4d872fc66c043c101e Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 20:30:24 -0400 Subject: [PATCH 22/31] nsITreeSelection can be 0 incorrectly even if count is 0 --- chrome/content/zotero/xpcom/collectionTreeView.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 43de285b21..6c11d803b7 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -67,6 +67,9 @@ Zotero.CollectionTreeView.prototype.type = 'collection'; Object.defineProperty(Zotero.CollectionTreeView.prototype, "selectedTreeRow", { get: function () { + if (!this.selection || !this.selection.count) { + return false; + } return this.getRow(this.selection.currentIndex); } }); @@ -940,7 +943,7 @@ Zotero.CollectionTreeView.prototype.selectLibrary = Zotero.Promise.coroutine(fun } // Check if library is already selected - if (this.selection.currentIndex != -1) { + if (this.selection && this.selection.count && this.selection.currentIndex != -1) { var treeRow = this.getRow(this.selection.currentIndex); if (treeRow.isLibrary(true) && treeRow.ref.libraryID == libraryID) { this._treebox.ensureRowIsVisible(this.selection.currentIndex); @@ -972,7 +975,7 @@ Zotero.CollectionTreeView.prototype.selectSearch = function (id) { Zotero.CollectionTreeView.prototype.selectTrash = Zotero.Promise.coroutine(function* (libraryID) { // Check if trash is already selected - if (this.selection.currentIndex != -1) { + if (this.selection && this.selection.count && this.selection.currentIndex != -1) { let itemGroup = this.getRow(this.selection.currentIndex); if (itemGroup.isTrash() && itemGroup.ref.libraryID == libraryID) { this._treebox.ensureRowIsVisible(this.selection.currentIndex); @@ -1196,6 +1199,8 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi * Returns libraryID or FALSE if not a library */ Zotero.CollectionTreeView.prototype.getSelectedLibraryID = function() { + if (!this.selection || !this.selection.count || this.selection.currentIndex == -1) return false; + var treeRow = this.getRow(this.selection.currentIndex); return treeRow && treeRow.ref && treeRow.ref.libraryID !== undefined && treeRow.ref.libraryID; From 6b509820b35837b686d9b022fb91e631f430a5f9 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 20:31:15 -0400 Subject: [PATCH 23/31] Fixes #918, Enabling "Show Unfiled Items" or "Show Duplicates" breaks UI --- chrome/content/zotero/zoteroPane.js | 13 ++++++------- test/content/support.js | 27 +++++++++++++++++++++++++++ test/tests/collectionTreeViewTest.js | 4 ++-- test/tests/duplicatesTest.js | 28 +++------------------------- test/tests/zoteroPaneTest.js | 24 ++++++++++++++++++++++-- 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 914e8211b4..5bef9a5adc 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -912,17 +912,16 @@ var ZoteroPane = new function() yield this.collectionsView.refresh(); - // If group is closed, open it - yield this.collectionsView.selectLibrary(libraryID); - row = this.collectionsView.selection.currentIndex; - if (!this.collectionsView.isContainerOpen(row)) { - this.collectionsView.toggleOpenState(row); - } - // Select new row if (show) { yield this.collectionsView.selectByID(lastViewedFolderID); } + // Select library root when hiding + else { + yield this.collectionsView.selectLibrary(libraryID); + } + + this.collectionsView.selection.selectEventsSuppressed = false; }); diff --git a/test/content/support.js b/test/content/support.js index 38da3e99c5..d9d9ddb35b 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -251,6 +251,33 @@ function waitForCallback(cb, interval, timeout) { } +function clickOnItemsRow(itemsView, row, button = 0) { + var x = {}; + var y = {}; + var width = {}; + var height = {}; + itemsView._treebox.getCoordsForCellItem( + row, + itemsView._treebox.columns.getNamedColumn('zotero-items-column-title'), + 'text', + x, y, width, height + ); + + // Select row to trigger multi-select + var tree = itemsView._treebox.treeBody; + var rect = tree.getBoundingClientRect(); + var x = rect.left + x.value; + var y = rect.top + y.value; + tree.dispatchEvent(new MouseEvent("mousedown", { + clientX: x, + clientY: y, + button, + detail: 1 + })); +} + + + /** * Get a default group used by all tests that want one, creating one if necessary */ diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index e5b8e9c4a8..25706b8394 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -22,8 +22,8 @@ describe("Zotero.CollectionTreeView", function() { Zotero.Prefs.clear('duplicateLibraries'); Zotero.Prefs.clear('unfiledLibraries'); yield cv.refresh(); - assert.isTrue(cv.getRowIndexByID("D" + userLibraryID)); - assert.isTrue(cv.getRowIndexByID("U" + userLibraryID)); + assert.ok(cv.getRowIndexByID("D" + userLibraryID)); + assert.ok(cv.getRowIndexByID("U" + userLibraryID)); assert.equal(Zotero.Prefs.get('duplicateLibraries'), "" + userLibraryID); assert.equal(Zotero.Prefs.get('unfiledLibraries'), "" + userLibraryID); }); diff --git a/test/tests/duplicatesTest.js b/test/tests/duplicatesTest.js index bcb3290d86..d7d84a1d83 100644 --- a/test/tests/duplicatesTest.js +++ b/test/tests/duplicatesTest.js @@ -32,31 +32,9 @@ describe("Duplicate Items", function () { // Select the first item, which should select both var iv = zp.itemsView; - var index = iv.getRowIndexByID(item1.id); - assert.isNumber(index); - - var x = {}; - var y = {}; - var width = {}; - var height = {}; - iv._treebox.getCoordsForCellItem( - index, - iv._treebox.columns.getNamedColumn('zotero-items-column-title'), - 'text', - x, y, width, height - ); - - // Select row to trigger multi-select - var tree = iv._treebox.treeBody; - var rect = tree.getBoundingClientRect(); - var x = rect.left + x.value; - var y = rect.top + y.value; - tree.dispatchEvent(new MouseEvent("mousedown", { - clientX: x, - clientY: y, - detail: 1 - })); - + var row = iv.getRowIndexByID(item1.id); + assert.isNumber(row); + clickOnItemsRow(iv, row); assert.equal(iv.selection.count, 2); // Click merge button diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 1b88d040c3..a86c23df97 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -217,6 +217,11 @@ describe("ZoteroPane", function() { }) it("should show a hidden virtual folder", function* () { + // Create unfiled, duplicate items + var title = Zotero.Utilities.randomString(); + var item1 = yield createDataObject('item', { title }); + var item2 = yield createDataObject('item', { title }); + // Start hidden Zotero.Prefs.set('duplicateLibraries', ""); Zotero.Prefs.set('unfiledLibraries', ""); @@ -226,7 +231,17 @@ describe("ZoteroPane", function() { var id = "D" + userLibraryID; assert.isFalse(cv.getRowIndexByID(id)); yield zp.setVirtual(userLibraryID, 'duplicates', true); - assert.ok(cv.getRowIndexByID(id)); + + // Clicking should select both items + var row = cv.getRowIndexByID(id); + assert.ok(row); + assert.equal(cv.selection.currentIndex, row); + yield waitForItemsLoad(win); + var iv = zp.itemsView; + row = iv.getRowIndexByID(item1.id); + assert.isNumber(row); + clickOnItemsRow(iv, row); + assert.equal(iv.selection.count, 2); // Show Unfiled Items id = "U" + userLibraryID; @@ -254,20 +269,25 @@ describe("ZoteroPane", function() { it("should hide an explicitly shown virtual folder", function* () { // Start shown Zotero.Prefs.set('duplicateLibraries', "" + userLibraryID); - Zotero.Prefs.set('unfiledLibraries' "" + userLibraryID); + Zotero.Prefs.set('unfiledLibraries', "" + userLibraryID); yield cv.refresh(); // Hide Duplicate Items var id = "D" + userLibraryID; assert.ok(yield cv.selectByID(id)); + yield waitForItemsLoad(win); yield zp.setVirtual(userLibraryID, 'duplicates', false); assert.isFalse(cv.getRowIndexByID(id)); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); + // Hide Unfiled Items id = "U" + userLibraryID; assert.ok(yield cv.selectByID(id)); + yield waitForItemsLoad(win); yield zp.setVirtual(userLibraryID, 'unfiled', false); assert.isFalse(cv.getRowIndexByID(id)); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); }); }); }) From 60830c27eebc7fffbd37fe3736277328b4c7b288 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 22:59:19 -0400 Subject: [PATCH 24/31] Remove items from open Unfiled Items view when added to collection --- chrome/content/zotero/xpcom/itemTreeView.js | 15 ++++++++++---- test/tests/itemTreeViewTest.js | 23 ++++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index ace81c4fe9..f30cde7bfb 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -590,7 +590,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio // If no quicksearch, process modifications manually else if (!quicksearch || quicksearch.value == '') { - var items = yield Zotero.Items.getAsync(ids); + var items = Zotero.Items.get(ids); for (let i = 0; i < items.length; i++) { let item = items[i]; @@ -609,9 +609,16 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio let parentItemID = this.getRow(row).ref.parentItemID; let parentIndex = this.getParentIndex(row); - // Top-level items just have to be resorted + // Top-level item if (this.isContainer(row)) { - sort = id; + // If Unfiled Items and itm was added to a collection, remove from view + if (collectionTreeRow.isUnfiled() && item.getCollections().length) { + this._removeRow(row); + } + // Otherwise just resort + else { + sort = id; + } } // If item moved from top-level to under another item, remove the old row. else if (parentIndex == -1 && parentItemID) { @@ -1585,7 +1592,7 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i // Clear the quicksearch and tag selection and try again (once) if (!noRecurse && this._ownerDocument.defaultView.ZoteroPane_Local) { let cleared1 = yield this._ownerDocument.defaultView.ZoteroPane_Local.clearQuicksearch(); - let cleared2 = yield this._ownerDocument.defaultView.ZoteroPane_Local.clearTagSelection(); + let cleared2 = this._ownerDocument.defaultView.ZoteroPane_Local.clearTagSelection(); if (cleared1 || cleared2) { return this.selectItem(id, expand, true); } diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js index f711e69a72..e60476261b 100644 --- a/test/tests/itemTreeViewTest.js +++ b/test/tests/itemTreeViewTest.js @@ -1,31 +1,31 @@ "use strict"; describe("Zotero.ItemTreeView", function() { - var win, zp, itemsView, existingItemID; + var win, zp, cv, itemsView, existingItemID; // Load Zotero pane and select library before(function* () { win = yield loadZoteroPane(); zp = win.ZoteroPane; + cv = zp.collectionsView; var item = new Zotero.Item('book'); existingItemID = yield item.saveTx(); }); beforeEach(function* () { - yield zp.collectionsView.selectLibrary(); - yield waitForItemsLoad(win) + yield selectLibrary(win); itemsView = zp.itemsView; }) after(function () { win.close(); }); - it("shouldn't show items in trash", function* () { + it("shouldn't show items in trash in library root", function* () { var item = yield createDataObject('item', { title: "foo" }); var itemID = item.id; item.deleted = true; yield item.saveTx(); - assert.notOk(itemsView.getRowIndexByID(itemID)); + assert.isFalse(itemsView.getRowIndexByID(itemID)); }) describe("#selectItem()", function () { @@ -231,6 +231,19 @@ describe("Zotero.ItemTreeView", function() { yield Zotero.Items.erase(items.map(item => item.id)); }) + + + it("should remove items from Unfiled Items when added to a collection", function* () { + var collection = yield createDataObject('collection'); + var item = yield createDataObject('item', { title: "Unfiled Item" }); + yield cv.selectByID("U" + Zotero.Libraries.userLibraryID); + yield waitForItemsLoad(win); + assert.isNumber(zp.itemsView.getRowIndexByID(item.id)); + yield Zotero.DB.executeTransaction(function* () { + yield collection.addItem(item.id); + }); + assert.isFalse(zp.itemsView.getRowIndexByID(item.id)); + }); }) describe("#drop()", function () { From 8e5016ae4d5ca745c537aab2a58fd35a832fda96 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 14 Mar 2016 17:10:18 -0400 Subject: [PATCH 25/31] Load synced settings (incl. tag colors) at startup --- chrome/content/zotero/bindings/tagsbox.xml | 66 +++++---- .../content/zotero/bindings/tagselector.xml | 93 ++++++------- chrome/content/zotero/xpcom/data/creators.js | 2 +- .../content/zotero/xpcom/data/dataObjects.js | 7 +- chrome/content/zotero/xpcom/data/item.js | 2 +- chrome/content/zotero/xpcom/data/library.js | 12 ++ chrome/content/zotero/xpcom/data/tags.js | 47 +++---- chrome/content/zotero/xpcom/itemTreeView.js | 5 +- chrome/content/zotero/xpcom/search.js | 2 +- .../content/zotero/xpcom/sync/syncEngine.js | 2 +- chrome/content/zotero/xpcom/syncedSettings.js | 126 +++++++++++++----- chrome/content/zotero/xpcom/zotero.js | 5 +- test/tests/libraryTest.js | 5 + test/tests/syncEngineTest.js | 12 +- test/tests/tagSelectorTest.js | 2 +- test/tests/tagsTest.js | 34 +++++ test/tests/tagsboxTest.js | 14 -- 17 files changed, 252 insertions(+), 184 deletions(-) diff --git a/chrome/content/zotero/bindings/tagsbox.xml b/chrome/content/zotero/bindings/tagsbox.xml index a1ba2c0485..bfc7c0737e 100644 --- a/chrome/content/zotero/bindings/tagsbox.xml +++ b/chrome/content/zotero/bindings/tagsbox.xml @@ -137,7 +137,8 @@ return Zotero.spawn(function* () { if (type == 'setting') { if (ids.some(function (val) val.split("/")[1] == 'tagColors') && this.item) { - return this.reload(); + this.reload(); + return; } } else if (type == 'item-tag') { @@ -194,7 +195,8 @@ } else if (type == 'tag') { if (event == 'modify') { - return this.reload(); + this.reload(); + return; } } }.bind(this)); @@ -204,38 +206,32 @@ Zotero.Promise.check(this.mode)); - - var rows = this.id('tagRows'); - while(rows.hasChildNodes()) { - rows.removeChild(rows.firstChild); - } - var tags = this.item.getTags(); - - // Sort tags alphabetically - var collation = Zotero.getLocaleCollation(); - tags.sort(function (a, b) collation.compareString(1, a.tag, b.tag)); - - for (let i=0; i @@ -711,7 +707,7 @@ this._lastTabIndex = this.item.getTags().length; } - yield this.reload(); + this.reload(); } // Single tag at end else { diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml index 7c355e6f18..e2334fc018 100644 --- a/chrome/content/zotero/bindings/tagselector.xml +++ b/chrome/content/zotero/bindings/tagselector.xml @@ -236,8 +236,7 @@ var emptyRegular = true; var tagsBox = this.id('tags-box'); - var tagColors = yield Zotero.Tags.getColors(this.libraryID) - .tap(() => Zotero.Promise.check(this.mode)); + var tagColors = Zotero.Tags.getColors(this.libraryID); // If new data, rebuild boxes if (fetch || this._dirty) { @@ -375,45 +374,43 @@ @@ -512,7 +509,7 @@ }.bind(this)); if (tagObjs.length) { - yield this.insertSorted(tagObjs); + this.insertSorted(tagObjs); } } // Don't add anything for item or collection-item; just update scope @@ -671,7 +668,7 @@ // Colored tags don't need to exist, so in that case // just rename the color setting else { - let color = yield Zotero.Tags.getColor(this.libraryID, oldName); + let color = Zotero.Tags.getColor(this.libraryID, oldName); if (!color) { throw new Error("Can't rename missing tag"); } @@ -715,18 +712,6 @@ ]]> - - - - - @@ -877,7 +862,7 @@ name: name }; - var tagColors = yield Zotero.Tags.getColors(this.libraryID); + var tagColors = Zotero.Tags.getColors(this.libraryID); if (tagColors.size >= Zotero.Tags.MAX_COLORED_TAGS && !tagColors.has(io.name)) { var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] .getService(Components.interfaces.nsIPromptService); diff --git a/chrome/content/zotero/xpcom/data/creators.js b/chrome/content/zotero/xpcom/data/creators.js index 909de38c59..177596a89a 100644 --- a/chrome/content/zotero/xpcom/data/creators.js +++ b/chrome/content/zotero/xpcom/data/creators.js @@ -30,7 +30,7 @@ Zotero.Creators = new function() { var _cache = {}; - this.init = Zotero.Promise.coroutine(function* (libraryID) { + this.init = Zotero.Promise.coroutine(function* () { var sql = "SELECT * FROM creators"; var rows = yield Zotero.DB.queryAsync(sql); for (let i = 0; i < rows.length; i++) { diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index 0ce1a73e53..6db8053e73 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -373,12 +373,13 @@ Zotero.DataObjects.prototype._loadDataType = Zotero.Promise.coroutine(function* Zotero.debug(`Loaded ${dataType} in ${libraryName} in ${new Date() - t} ms`); }); -Zotero.DataObjects.prototype.loadAllData = Zotero.Promise.coroutine(function* (libraryID, ids) { +Zotero.DataObjects.prototype.loadAll = Zotero.Promise.coroutine(function* (libraryID, ids) { var t = new Date(); var libraryName = Zotero.Libraries.get(libraryID).name; - Zotero.debug("Loading all data" - + (ids ? " for " + ids.length + " " + this._ZDO_objects : '') + Zotero.debug("Loading " + + (ids ? ids.length : "all") + " " + + (ids && ids.length == 1 ? this._ZDO_object : this._ZDO_objects) + " in " + libraryName); let dataTypes = this.ObjectClass.prototype._dataTypes; diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 8fb769dbfa..972186b409 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3548,7 +3548,7 @@ Zotero.Item.prototype.getImageSrcWithTags = Zotero.Promise.coroutine(function* ( return uri; } - var tagColors = yield Zotero.Tags.getColors(this.libraryID); + var tagColors = Zotero.Tags.getColors(this.libraryID); var colorData = []; for (let i=0; i} - A promise for a Map with tag names as keys and - * objects containing 'color' and 'position' as values + * @return {Map} - A Map with tag names as keys and objects containing 'color' and 'position' + * as values */ - this.getColors = Zotero.Promise.coroutine(function* (libraryID) { + this.getColors = function (libraryID) { + if (!libraryID) { + throw new Error("libraryID not provided"); + } + if (_libraryColorsByName[libraryID]) { return _libraryColorsByName[libraryID]; } - var tagColors = yield Zotero.SyncedSettings.get(libraryID, 'tagColors'); - - // If the colors became available from another run - if (_libraryColorsByName[libraryID]) { - return _libraryColorsByName[libraryID]; - } + var tagColors = Zotero.SyncedSettings.get(libraryID, 'tagColors'); tagColors = tagColors || []; @@ -452,7 +449,7 @@ Zotero.Tags = new function() { } return _libraryColorsByName[libraryID]; - }); + }; /** @@ -465,7 +462,7 @@ Zotero.Tags = new function() { throw new Error("libraryID must be an integer"); } - yield this.getColors(libraryID); + this.getColors(libraryID); var tagColors = _libraryColors[libraryID]; @@ -541,7 +538,7 @@ Zotero.Tags = new function() { delete _libraryColorsByName[libraryID]; // Get the tag colors for each library in which they were modified - let tagColors = yield Zotero.SyncedSettings.get(libraryID, 'tagColors'); + let tagColors = Zotero.SyncedSettings.get(libraryID, 'tagColors'); if (!tagColors) { tagColors = []; } diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index f30cde7bfb..e12ca1133f 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -170,12 +170,11 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree if (coloredTagsRE.test(key)) { let libraryID = self.collectionTreeRow.ref.libraryID; let position = parseInt(key) - 1; - let colorData = yield Zotero.Tags.getColorByPosition(libraryID, position); + let colorData = Zotero.Tags.getColorByPosition(libraryID, position); // If a color isn't assigned to this number or any // other numbers, allow key navigation if (!colorData) { - let colors = yield Zotero.Tags.getColors(libraryID); - return !colors.size; + return !Zotero.Tags.getColors(libraryID).size; } var items = self.getSelectedItems(); diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js index db9a0a1520..923ef61d1a 100644 --- a/chrome/content/zotero/xpcom/search.js +++ b/chrome/content/zotero/xpcom/search.js @@ -815,7 +815,7 @@ Zotero.Search.prototype.search = Zotero.Promise.coroutine(function* (asTempTable /** * Populate the object's data from an API JSON data object * - * If this object is identified (has an id or library/key), loadAllData() must have been called. + * If this object is identified (has an id or library/key), loadAll() must have been called. */ Zotero.Search.prototype.fromJSON = function (json) { if (!json.name) { diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index 10d65e5e69..b71845bb56 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -290,7 +290,7 @@ Zotero.Sync.Data.Engine.prototype._startDownload = Zotero.Promise.coroutine(func } if (objectType == 'setting') { - let meta = yield Zotero.SyncedSettings.getMetadata(this.libraryID, key); + let meta = Zotero.SyncedSettings.getMetadata(this.libraryID, key); if (!meta) { continue; } diff --git a/chrome/content/zotero/xpcom/syncedSettings.js b/chrome/content/zotero/xpcom/syncedSettings.js index 9ba3d0104e..74afbe26ae 100644 --- a/chrome/content/zotero/xpcom/syncedSettings.js +++ b/chrome/content/zotero/xpcom/syncedSettings.js @@ -27,6 +27,8 @@ * @namespace */ Zotero.SyncedSettings = (function () { + var _cache = {}; + // // Public methods // @@ -34,47 +36,95 @@ Zotero.SyncedSettings = (function () { idColumn: "setting", table: "syncedSettings", - get: Zotero.Promise.coroutine(function* (libraryID, setting) { - var sql = "SELECT value FROM syncedSettings WHERE setting=? AND libraryID=?"; - var json = yield Zotero.DB.valueQueryAsync(sql, [setting, libraryID]); - if (!json) { - return false; + loadAll: Zotero.Promise.coroutine(function* (libraryID) { + Zotero.debug("Loading synced settings for library " + libraryID); + + if (!_cache[libraryID]) { + _cache[libraryID] = {}; } - return JSON.parse(json); + + var invalid = []; + + var sql = "SELECT setting, value, synced, version FROM syncedSettings " + + "WHERE libraryID=?"; + yield Zotero.DB.queryAsync( + sql, + libraryID, + { + onRow: function (row) { + var setting = row.getResultByIndex(0); + + var value = row.getResultByIndex(1); + try { + value = JSON.parse(value); + } + catch (e) { + invalid.push([libraryID, setting]); + return; + } + + _cache[libraryID][setting] = { + value, + synced: !!row.getResultByIndex(2), + version: row.getResultByIndex(3) + }; + } + } + ); + + // TODO: Delete invalid settings }), + /** + * Return settings object + * + * @return {Object|null} + */ + get: function (libraryID, setting) { + if (!_cache[libraryID]) { + throw new Zotero.Exception.UnloadedDataException( + "Synced settings not loaded for library " + libraryID, + "syncedSettings" + ); + } + + if (!_cache[libraryID][setting]) { + return null; + } + + return JSON.parse(JSON.stringify(_cache[libraryID][setting].value)); + }, + /** * Used by sync and tests * * @return {Object} - Object with 'synced' and 'version' properties */ - getMetadata: Zotero.Promise.coroutine(function* (libraryID, setting) { - var sql = "SELECT * FROM syncedSettings WHERE setting=? AND libraryID=?"; - var row = yield Zotero.DB.rowQueryAsync(sql, [setting, libraryID]); - if (!row) { - return false; + getMetadata: function (libraryID, setting) { + if (!_cache[libraryID]) { + throw new Zotero.Exception.UnloadedDataException( + "Synced settings not loaded for library " + libraryID, + "syncedSettings" + ); + } + + var o = _cache[libraryID][setting]; + if (!o) { + return null; } return { - synced: !!row.synced, - version: row.version + synced: o.synced, + version: o.version }; - }), + }, set: Zotero.Promise.coroutine(function* (libraryID, setting, value, version = 0, synced) { if (typeof value == undefined) { throw new Error("Value not provided"); } - // TODO: get rid of this once we have proper affected rows handling - var sql = "SELECT value FROM syncedSettings WHERE setting=? AND libraryID=?"; - var currentValue = yield Zotero.DB.valueQueryAsync(sql, [setting, libraryID]); - - // Make sure we can tell the difference between a - // missing setting (FALSE as returned by valueQuery()) - // and a FALSE setting (FALSE as returned by JSON.parse()) - var hasCurrentValue = currentValue !== false; - - currentValue = JSON.parse(currentValue); + var currentValue = this.get(libraryID, setting); + var hasCurrentValue = currentValue !== null; // Value hasn't changed if (value === currentValue) { @@ -93,7 +143,7 @@ Zotero.SyncedSettings = (function () { }; } - if (currentValue === false) { + if (!hasCurrentValue) { var event = 'add'; var extraData = {}; } @@ -102,6 +152,7 @@ Zotero.SyncedSettings = (function () { } synced = synced ? 1 : 0; + version = parseInt(version); if (hasCurrentValue) { var sql = "UPDATE syncedSettings SET value=?, version=?, synced=? " @@ -117,6 +168,13 @@ Zotero.SyncedSettings = (function () { sql, [setting, libraryID, JSON.stringify(value), version, synced] ); } + + _cache[libraryID][setting] = { + value, + synced: !!synced, + version + } + yield Zotero.Notifier.trigger(event, 'setting', [id], extraData); return true; }), @@ -124,22 +182,16 @@ Zotero.SyncedSettings = (function () { clear: Zotero.Promise.coroutine(function* (libraryID, setting, options) { options = options || {}; - // TODO: get rid of this once we have proper affected rows handling - var sql = "SELECT value FROM syncedSettings WHERE setting=? AND libraryID=?"; - var currentValue = yield Zotero.DB.valueQueryAsync(sql, [setting, libraryID]); - if (currentValue === false) { - return false; - } - currentValue = JSON.parse(currentValue); + var currentValue = this.get(libraryID, setting); + var hasCurrentValue = currentValue !== null; var id = libraryID + '/' + setting; var extraData = {}; extraData[id] = { - changed: {} - }; - extraData[id].changed = { - value: currentValue + changed: { + value: currentValue + } }; if (options.skipDeleteLog) { extraData[id].skipDeleteLog = true; @@ -148,6 +200,8 @@ Zotero.SyncedSettings = (function () { var sql = "DELETE FROM syncedSettings WHERE setting=? AND libraryID=?"; yield Zotero.DB.queryAsync(sql, [setting, libraryID]); + delete _cache[libraryID][setting]; + yield Zotero.Notifier.trigger('delete', 'setting', [id], extraData); return true; }) diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index f1a56391d0..e3b493657e 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -629,9 +629,8 @@ Components.utils.import("resource://gre/modules/osfile.jsm"); let libraryIDs = Zotero.Libraries.getAll().map(x => x.libraryID); for (let libraryID of libraryIDs) { - yield Zotero.Collections.loadAllData(libraryID); - yield Zotero.Searches.loadAllData(libraryID); - yield Zotero.Items.loadAllData(libraryID); + let library = Zotero.Libraries.get(libraryID); + yield library.loadAllDataTypes(); } yield Zotero.QuickCopy.init(); diff --git a/test/tests/libraryTest.js b/test/tests/libraryTest.js index 047aa3fe50..193641c451 100644 --- a/test/tests/libraryTest.js +++ b/test/tests/libraryTest.js @@ -150,6 +150,11 @@ describe("Zotero.Library", function() { yield library.saveTx(); assert.isFalse(Zotero.Libraries.isEditable(library.libraryID)); }); + + it("should initialize library after creation", function* () { + let library = yield createGroup({}); + Zotero.SyncedSettings.get(library.libraryID, "tagColors"); + }); }); describe("#erase()", function() { it("should erase a group library", function* () { diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 6f8a2a4284..019060b7d1 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -239,10 +239,10 @@ describe("Zotero.Sync.Data.Engine", function () { assert.equal(Zotero.Libraries.getVersion(userLibraryID), 3); // Make sure local objects exist - var setting = yield Zotero.SyncedSettings.get(userLibraryID, "tagColors"); + var setting = Zotero.SyncedSettings.get(userLibraryID, "tagColors"); assert.lengthOf(setting, 1); assert.equal(setting[0].name, 'A'); - var settingMetadata = yield Zotero.SyncedSettings.getMetadata(userLibraryID, "tagColors"); + var settingMetadata = Zotero.SyncedSettings.getMetadata(userLibraryID, "tagColors"); assert.equal(settingMetadata.version, 2); assert.isTrue(settingMetadata.synced); @@ -812,7 +812,7 @@ describe("Zotero.Sync.Data.Engine", function () { yield engine._startDownload(); // Make sure objects were deleted - assert.isFalse(yield Zotero.SyncedSettings.get(userLibraryID, 'tagColors')); + assert.isNull(Zotero.SyncedSettings.get(userLibraryID, 'tagColors')); assert.isFalse(Zotero.Collections.exists(collectionID)); assert.isFalse(Zotero.Searches.exists(searchID)); assert.isFalse(Zotero.Items.exists(itemID)); @@ -901,7 +901,7 @@ describe("Zotero.Sync.Data.Engine", function () { yield engine._startDownload(); // Make sure objects weren't deleted - assert.ok(yield Zotero.SyncedSettings.get(userLibraryID, 'tagColors')); + assert.ok(Zotero.SyncedSettings.get(userLibraryID, 'tagColors')); assert.ok(Zotero.Collections.exists(collectionID)); assert.ok(Zotero.Searches.exists(searchID)); }) @@ -1212,10 +1212,10 @@ describe("Zotero.Sync.Data.Engine", function () { yield engine._fullSync(); // Check settings - var setting = yield Zotero.SyncedSettings.get(userLibraryID, "tagColors"); + var setting = Zotero.SyncedSettings.get(userLibraryID, "tagColors"); assert.lengthOf(setting, 1); assert.equal(setting[0].name, 'A'); - var settingMetadata = yield Zotero.SyncedSettings.getMetadata(userLibraryID, "tagColors"); + var settingMetadata = Zotero.SyncedSettings.getMetadata(userLibraryID, "tagColors"); assert.equal(settingMetadata.version, 2); assert.isTrue(settingMetadata.synced); diff --git a/test/tests/tagSelectorTest.js b/test/tests/tagSelectorTest.js index f03e2d3d49..4a8f09d0aa 100644 --- a/test/tests/tagSelectorTest.js +++ b/test/tests/tagSelectorTest.js @@ -4,7 +4,7 @@ describe("Tag Selector", function () { var win, doc, collectionsView; var clearTagColors = Zotero.Promise.coroutine(function* (libraryID) { - var tagColors = yield Zotero.Tags.getColors(libraryID); + var tagColors = Zotero.Tags.getColors(libraryID); for (let name of tagColors.keys()) { yield Zotero.Tags.setColor(libraryID, name, false); } diff --git a/test/tests/tagsTest.js b/test/tests/tagsTest.js index 7c2f1de769..de223fa55b 100644 --- a/test/tests/tagsTest.js +++ b/test/tests/tagsTest.js @@ -64,4 +64,38 @@ describe("Zotero.Tags", function () { assert.isFalse(yield Zotero.Tags.getName(tagID)); }) }) + + + describe("#setColor()", function () { + var libraryID; + + before(function* () { + libraryID = Zotero.Libraries.userLibraryID; + + // Clear library tag colors + var colors = Zotero.Tags.getColors(libraryID); + for (let color of colors.keys()) { + yield Zotero.Tags.setColor(libraryID, color); + } + }); + + it("should set color for a tag", function* () { + var aColor = '#ABCDEF'; + var bColor = '#BCDEF0'; + yield Zotero.Tags.setColor(libraryID, "A", aColor); + yield Zotero.Tags.setColor(libraryID, "B", bColor); + + var o = Zotero.Tags.getColor(libraryID, "A") + assert.equal(o.color, aColor); + assert.equal(o.position, 0); + var o = Zotero.Tags.getColor(libraryID, "B") + assert.equal(o.color, bColor); + assert.equal(o.position, 1); + + var o = Zotero.SyncedSettings.get(libraryID, 'tagColors'); + assert.isArray(o); + assert.lengthOf(o, 2); + assert.sameMembers(o.map(c => c.color), [aColor, bColor]); + }); + }); }) diff --git a/test/tests/tagsboxTest.js b/test/tests/tagsboxTest.js index 3c10710fc2..631a65aea9 100644 --- a/test/tests/tagsboxTest.js +++ b/test/tests/tagsboxTest.js @@ -14,17 +14,6 @@ describe("Item Tags Box", function () { win.close(); }); - function waitForTagsBox() { - var deferred = Zotero.Promise.defer(); - var tagsbox = doc.getElementById('zotero-editpane-tags'); - var onRefresh = function (event) { - tagsbox.removeEventListener('refresh', onRefresh); - deferred.resolve(); - } - tagsbox.addEventListener('refresh', onRefresh); - return deferred.promise; - } - describe("#notify()", function () { it("should update an existing tag on rename", function* () { var tag = Zotero.Utilities.randomString(); @@ -43,7 +32,6 @@ describe("Item Tags Box", function () { var tabbox = doc.getElementById('zotero-view-tabbox'); tabbox.selectedIndex = 2; - yield waitForTagsBox(); var tagsbox = doc.getElementById('zotero-editpane-tags'); var rows = tagsbox.id('tagRows').getElementsByTagName('row'); assert.equal(rows.length, 1); @@ -77,7 +65,6 @@ describe("Item Tags Box", function () { var tabbox = doc.getElementById('zotero-view-tabbox'); tabbox.selectedIndex = 2; - yield waitForTagsBox(); var tagsbox = doc.getElementById('zotero-editpane-tags'); var rows = tagsbox.id('tagRows').getElementsByTagName('row'); @@ -108,7 +95,6 @@ describe("Item Tags Box", function () { var tabbox = doc.getElementById('zotero-view-tabbox'); tabbox.selectedIndex = 2; - yield waitForTagsBox(); var tagsbox = doc.getElementById('zotero-editpane-tags'); var rows = tagsbox.id('tagRows').getElementsByTagName('row'); assert.equal(rows.length, 1); From 9ffcd8930354ea1fbe0d9806ea938d01f86ae2a7 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 15 Mar 2016 01:15:40 -0400 Subject: [PATCH 26/31] Fix error selecting from creator autocomplete --- chrome/content/zotero/bindings/itembox.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chrome/content/zotero/bindings/itembox.xml b/chrome/content/zotero/bindings/itembox.xml index d004a9cc07..c89cd2dfb6 100644 --- a/chrome/content/zotero/bindings/itembox.xml +++ b/chrome/content/zotero/bindings/itembox.xml @@ -1569,7 +1569,7 @@ this._lastTabIndex = parseInt(textbox.getAttribute('ztabindex')) - 1; this._tabDirection = 1; - var creator = yield Zotero.Creators.getAsync(creatorID); + var creator = Zotero.Creators.get(creatorID); var otherField = creatorField == 'lastName' ? 'firstName' : 'lastName'; From be04f3d33cae3564f2f2e26dc0b77fa2ce4a1998 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 15 Mar 2016 01:16:36 -0400 Subject: [PATCH 27/31] Restore colored tags at top of tag selector when not linked to item --- chrome/content/zotero/bindings/tagselector.xml | 7 +++++++ test/tests/tagSelectorTest.js | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml index e2334fc018..03bb70a42b 100644 --- a/chrome/content/zotero/bindings/tagselector.xml +++ b/chrome/content/zotero/bindings/tagselector.xml @@ -244,6 +244,13 @@ .tap(() => Zotero.Promise.check(this.mode)); tagsBox.textContent = ""; + // Add colored tags that aren't already real tags + let regularTags = new Set(this._tags.map(tag => tag.tag)); + let coloredTags = new Set(tagColors.keys()); + [for (x of coloredTags) if (!regularTags.has(x)) x].forEach(x => + this._tags.push(Zotero.Tags.cleanData({ tag: x })) + ); + // Sort by name let collation = Zotero.getLocaleCollation(); this._tags.sort(function (a, b) { diff --git a/test/tests/tagSelectorTest.js b/test/tests/tagSelectorTest.js index 4a8f09d0aa..957b0af7f0 100644 --- a/test/tests/tagSelectorTest.js +++ b/test/tests/tagSelectorTest.js @@ -155,6 +155,18 @@ describe("Tag Selector", function () { assert.equal(getRegularTags().length, 1); }) + it("should show a colored tag at the top of the list even when linked to no items", function* () { + var libraryID = Zotero.Libraries.userLibraryID; + + var tagSelector = doc.getElementById('zotero-tag-selector'); + var tagElems = tagSelector.id('tags-box').childNodes; + var count = tagElems.length; + + yield Zotero.Tags.setColor(libraryID, "Top", '#AAAAAA'); + + assert.equal(tagElems.length, count + 1); + }); + it("shouldn't re-insert a new tag that matches an existing color", function* () { var libraryID = Zotero.Libraries.userLibraryID; From 5cd3ab22ba810abfb1c63224b6f69c0e16da86d4 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 15 Mar 2016 01:17:07 -0400 Subject: [PATCH 28/31] Make sure "Unfiled Items" is showing before test --- test/tests/itemTreeViewTest.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js index e60476261b..8625e3a031 100644 --- a/test/tests/itemTreeViewTest.js +++ b/test/tests/itemTreeViewTest.js @@ -234,9 +234,12 @@ describe("Zotero.ItemTreeView", function() { it("should remove items from Unfiled Items when added to a collection", function* () { + var userLibraryID = Zotero.Libraries.userLibraryID; var collection = yield createDataObject('collection'); var item = yield createDataObject('item', { title: "Unfiled Item" }); - yield cv.selectByID("U" + Zotero.Libraries.userLibraryID); + yield zp.setVirtual(userLibraryID, 'unfiled', true); + var selected = yield cv.selectByID("U" + userLibraryID); + assert.ok(selected); yield waitForItemsLoad(win); assert.isNumber(zp.itemsView.getRowIndexByID(item.id)); yield Zotero.DB.executeTransaction(function* () { From b9444892a068eb94107acfc1a8f106c3521979fc Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 15 Mar 2016 01:18:01 -0400 Subject: [PATCH 29/31] Ring bell on test error if interactive shell --- test/content/runtests.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/content/runtests.js b/test/content/runtests.js index 266b7f93e7..73d90ec34f 100644 --- a/test/content/runtests.js +++ b/test/content/runtests.js @@ -137,6 +137,8 @@ function Reporter(runner) { dump("\r" + indentStr // Dark red X for errors + "\033[31;40m" + Mocha.reporters.Base.symbols.err + " [FAIL]\033[0m" + // Trigger bell if interactive + + (Zotero.noUserInput ? "" : "\007") + " " + test.title + "\n" + indentStr + " " + err.toString() + " at\n" + err.stack.replace(/^/gm, indentStr + " ")); From 8df6b4bbd3a02efff98442d8b6e384a754a6c42b Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 16 Mar 2016 01:36:59 -0400 Subject: [PATCH 30/31] Don't modify Date Modified when updating downloaded attachment state --- chrome/content/zotero/xpcom/storage/storageLocal.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/storage/storageLocal.js b/chrome/content/zotero/xpcom/storage/storageLocal.js index be1ac435e8..354a2c6a7c 100644 --- a/chrome/content/zotero/xpcom/storage/storageLocal.js +++ b/chrome/content/zotero/xpcom/storage/storageLocal.js @@ -580,7 +580,10 @@ Zotero.Sync.Storage.Local = { item.attachmentSyncState = this.SYNC_STATE_IN_SYNC; item.attachmentSyncedModificationTime = mtime; item.attachmentSyncedHash = md5; - yield item.saveTx(); + yield item.saveTx({ + skipDateModifiedUpdate: true, + skipSelect: true + }); return new Zotero.Sync.Storage.Result({ localChanges: true From 28dc7d17e24a8b8b1a551990061954bfb4bad3cf Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 16 Mar 2016 02:01:51 -0400 Subject: [PATCH 31/31] Fix setting of local mtime when remote file change matches local file --- .../zotero/xpcom/storage/storageLocal.js | 7 +-- .../zotero/xpcom/storage/streamListener.js | 2 + chrome/content/zotero/xpcom/storage/zfs.js | 7 ++- test/tests/zfsTest.js | 54 +++++++++++++++++++ 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/chrome/content/zotero/xpcom/storage/storageLocal.js b/chrome/content/zotero/xpcom/storage/storageLocal.js index 354a2c6a7c..f74d0f74a4 100644 --- a/chrome/content/zotero/xpcom/storage/storageLocal.js +++ b/chrome/content/zotero/xpcom/storage/storageLocal.js @@ -577,13 +577,10 @@ Zotero.Sync.Storage.Local = { // Set the file mtime to the time from the server yield OS.File.setDates(path, null, new Date(parseInt(mtime))); - item.attachmentSyncState = this.SYNC_STATE_IN_SYNC; item.attachmentSyncedModificationTime = mtime; item.attachmentSyncedHash = md5; - yield item.saveTx({ - skipDateModifiedUpdate: true, - skipSelect: true - }); + item.attachmentSyncState = "in_sync"; + yield item.saveTx({ skipAll: true }); return new Zotero.Sync.Storage.Result({ localChanges: true diff --git a/chrome/content/zotero/xpcom/storage/streamListener.js b/chrome/content/zotero/xpcom/storage/streamListener.js index 136a8f8958..8b1323e24c 100644 --- a/chrome/content/zotero/xpcom/storage/streamListener.js +++ b/chrome/content/zotero/xpcom/storage/streamListener.js @@ -156,6 +156,8 @@ Zotero.Sync.Storage.StreamListener.prototype = { if (!result) { oldChannel.cancel(Components.results.NS_BINDING_ABORTED); newChannel.cancel(Components.results.NS_BINDING_ABORTED); + Zotero.debug("Cancelling redirect"); + // TODO: Prevent onStateChange error return false; } } diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js index e8f2b4bfa0..df5a6989f8 100644 --- a/chrome/content/zotero/xpcom/storage/zfs.js +++ b/chrome/content/zotero/xpcom/storage/zfs.js @@ -99,7 +99,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { var header; try { header = "Zotero-File-Modification-Time"; - requestData.mtime = oldChannel.getResponseHeader(header); + requestData.mtime = parseInt(oldChannel.getResponseHeader(header)); header = "Zotero-File-MD5"; requestData.md5 = oldChannel.getResponseHeader(header); header = "Zotero-File-Compressed"; @@ -131,6 +131,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } // Update local metadata and stop request, skipping file download + yield OS.File.setDates(path, null, new Date(requestData.mtime)); item.attachmentSyncedModificationTime = requestData.mtime; if (updateHash) { item.attachmentSyncedHash = requestData.md5; @@ -138,6 +139,10 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { item.attachmentSyncState = "in_sync"; yield item.saveTx({ skipAll: true }); + deferred.resolve(new Zotero.Sync.Storage.Result({ + localChanges: true + })); + return false; }), onProgress: function (a, b, c) { diff --git a/test/tests/zfsTest.js b/test/tests/zfsTest.js index 7c26982ad7..6178bd0bba 100644 --- a/test/tests/zfsTest.js +++ b/test/tests/zfsTest.js @@ -561,6 +561,60 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { assert.equal(item2.version, 15); }) + it("should update local info for remotely updated file that matches local file", function* () { + var { engine, client, caller } = yield setup(); + + var library = Zotero.Libraries.userLibrary; + library.libraryVersion = 5; + yield library.saveTx(); + library.storageDownloadNeeded = true; + + var file = getTestDataDirectory(); + file.append('test.txt'); + var item = yield Zotero.Attachments.importFromFile({ file }); + item.version = 5; + item.attachmentSyncState = "to_download"; + yield item.saveTx(); + var path = yield item.getFilePathAsync(); + yield OS.File.setDates(path, null, new Date() - 100000); + + var json = item.toJSON(); + yield Zotero.Sync.Data.Local.saveCacheObject('item', item.libraryID, json); + + var mtime = (Math.floor(new Date().getTime() / 1000) * 1000) + ""; + var md5 = Zotero.Utilities.Internal.md5(file) + + var s3Path = `pretend-s3/${item.key}`; + this.httpd.registerPathHandler( + `/users/1/items/${item.key}/file`, + { + handle: function (request, response) { + if (!request.hasHeader('Zotero-API-Key')) { + response.setStatusLine(null, 403, "Forbidden"); + return; + } + var key = request.getHeader('Zotero-API-Key'); + if (key != apiKey) { + response.setStatusLine(null, 403, "Invalid key"); + return; + } + response.setStatusLine(null, 302, "Found"); + response.setHeader("Zotero-File-Modification-Time", mtime, false); + response.setHeader("Zotero-File-MD5", md5, false); + response.setHeader("Zotero-File-Compressed", "No", false); + response.setHeader("Location", baseURL + s3Path, false); + } + } + ); + var result = yield engine.start(); + + assert.equal(item.attachmentSyncedModificationTime, mtime); + yield assert.eventually.equal(item.attachmentModificationTime, mtime); + assert.isTrue(result.localChanges); + assert.isFalse(result.remoteChanges); + assert.isFalse(result.syncRequired); + }) + it("should update local info for file that already exists on the server", function* () { var { engine, client, caller } = yield setup();