www

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

commit 5c94119c70e1a07f48ff7af3489a8156a0bfe720
parent 15d28014ed7d89aa34c39808880b65da52afd0e1
Author: Dan Stillman <dstillman@zotero.org>
Date:   Fri, 10 Oct 2014 04:35:45 -0400

Fixes duplicates view for async DB

It's way too slow, though, since the whole list is regenerated after
merging.

Fixes #519

Also:

- The arguments to Zotero.Item.prototype.clone() have changed, and it no
  longer takes an existing item or copies primary data. To create an
  in-memory copy of an item, use the new Zotero.Item.prototype.copy().

- Zotero.Item.prototype.getUsedFields() now gets in-memory fields rather
  than the fields in the database

Diffstat:
Mchrome/content/zotero/bindings/merge.xml | 4++--
Mchrome/content/zotero/duplicatesMerge.js | 23++++++++++++++---------
Mchrome/content/zotero/xpcom/attachments.js | 15++++-----------
Mchrome/content/zotero/xpcom/collectionTreeView.js | 27+++++++--------------------
Mchrome/content/zotero/xpcom/data/dataObjects.js | 109++++++++++++++++++++++++++++++++++++-------------------------------------------
Mchrome/content/zotero/xpcom/data/item.js | 221++++++++++++++++++++++---------------------------------------------------------
Mchrome/content/zotero/xpcom/data/items.js | 121+++++++++++++++++++++++++++++++++++++++++++------------------------------------
Mchrome/content/zotero/xpcom/data/relations.js | 5++++-
Mchrome/content/zotero/xpcom/duplicates.js | 153+++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
Mchrome/content/zotero/xpcom/itemTreeView.js | 2+-
Mchrome/content/zotero/zoteroPane.js | 2+-
11 files changed, 305 insertions(+), 377 deletions(-)

diff --git a/chrome/content/zotero/bindings/merge.xml b/chrome/content/zotero/bindings/merge.xml @@ -71,7 +71,7 @@ } // Clone object so changes in merge pane don't affect it - this._leftpane.ref = val.clone(true); + this._leftpane.ref = val.copy(); this._leftpane.original = val; ]]> </setter> @@ -97,7 +97,7 @@ } // Clone object so changes in merge pane don't affect it - this._rightpane.ref = val.clone(true); + this._rightpane.ref = val.copy(); this._rightpane.original = val; } diff --git a/chrome/content/zotero/duplicatesMerge.js b/chrome/content/zotero/duplicatesMerge.js @@ -130,16 +130,21 @@ 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 - var diff = item.multiDiff(_otherItems, _ignoreFields); - if (diff) { - var itemValues = item.serialize() - for (var i in diff) { - diff[i].unshift(itemValues.fields[i]); + Zotero.spawn(function* () { + var diff = yield item.multiDiff(_otherItems, _ignoreFields); + if (diff) { + let itemValues = yield item.toJSON(); + for (let i in diff) { + diff[i].unshift(itemValues[i] !== undefined ? itemValues[i] : ''); + } + itembox.fieldAlternatives = diff; } - itembox.fieldAlternatives = diff; - } - - itembox.item = item.clone(true); + + var newItem = yield item.copy(); + yield newItem.loadItemData(); + yield newItem.loadCreators(); + itembox.item = newItem; + }); } diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js @@ -1067,22 +1067,15 @@ Zotero.Attachments = new function(){ throw ("Attachment is already in library " + libraryID); } - var newAttachment = new Zotero.Item('attachment'); - newAttachment.libraryID = libraryID; - // Link mode needs to be set when saving new attachment - newAttachment.attachmentLinkMode = linkMode; + var newAttachment = attachment.clone(libraryID); if (attachment.isImportedAttachment()) { // Attachment path isn't copied over by clone() if libraryID is different newAttachment.attachmentPath = attachment.attachmentPath; } - // DEBUG: save here because clone() doesn't currently work on unsaved tagged items - var id = newAttachment.save(); - newAttachment = Zotero.Items.get(id); - attachment.clone(false, newAttachment); if (parentItemID) { - newAttachment.setSource(parentItemID); + newAttachment.parentID = parentItemID; } - newAttachment.save(); + yield newAttachment.save(); // Copy over files if they exist if (newAttachment.isImportedAttachment() && attachment.getFile()) { @@ -1091,7 +1084,7 @@ Zotero.Attachments = new function(){ Zotero.File.copyDirectory(dir, newDir); } - newAttachment.addLinkedItem(attachment); + yield newAttachment.addLinkedItem(attachment); return newAttachment.id; }); diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -1567,23 +1567,15 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r return Zotero.Attachments.copyAttachmentToLibrary(item, targetLibraryID); } - // Create new unsaved clone item in target library - var newItem = new Zotero.Item(item.itemTypeID); - newItem.libraryID = targetLibraryID; - // DEBUG: save here because clone() doesn't currently work on unsaved tagged items - var id = yield newItem.save(); - newItem = yield Zotero.Items.getAsync(id); - yield item.clone(false, newItem, false, !Zotero.Prefs.get('groups.copyTags')); - yield newItem.save(); - //var id = newItem.save(); - //var newItem = Zotero.Items.get(id); + // Create new clone item in target library + var newItem = item.clone(targetLibraryID, false, !Zotero.Prefs.get('groups.copyTags')); + var newItemID = yield newItem.save(); // Record link yield newItem.addLinkedItem(item); - var newID = id; if (item.isNote()) { - return newID; + return newItemID; } // For regular items, add child items if prefs and permissions allow @@ -1593,14 +1585,9 @@ Zotero.CollectionTreeView.prototype.drop = Zotero.Promise.coroutine(function* (r var noteIDs = item.getNotes(); var notes = yield Zotero.Items.getAsync(noteIDs); for each(var note in notes) { - var newNote = new Zotero.Item('note'); - newNote.libraryID = targetLibraryID; - // DEBUG: save here because clone() doesn't currently work on unsaved tagged items - var id = yield newNote.save(); - newNote = yield Zotero.Items.getAsync(id); - yield note.clone(false, newNote); - newNote.parentID = newItem.id; - yield newNote.save(); + let newNote = note.clone(targetLibraryID); + newNote.parentID = newItemID; + yield newNote.save() yield newNote.addLinkedItem(note); } diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -421,73 +421,64 @@ Zotero.DataObjects = function (object, objectPlural, id, table) { /** - * @param {Object} data1 Serialized copy of first object - * @param {Object} data2 Serialized copy of second object - * @param {Array} diff Empty array to put diff data in - * @param {Boolean} [includeMatches=false] Include all fields, even those - * that aren't different + * @param {Object} data1 - JSON of first object + * @param {Object} data2 - JSON of second object + * @param {Array} diff - Empty array to put diff data in + * @param {Boolean} [includeMatches=false] - Include all fields, even those + * that aren't different */ this.diff = function (data1, data2, diff, includeMatches) { diff.push({}, {}); var numDiffs = 0; - var subs = ['primary', 'fields']; - var skipFields = ['collectionID', 'creatorID', 'itemID', 'searchID', 'tagID', 'libraryID', 'key']; + var skipFields = ['collectionKey', 'itemKey', 'searchKey']; - for each(var sub in subs) { - diff[0][sub] = {}; - diff[1][sub] = {}; - for (var field in data1[sub]) { - if (skipFields.indexOf(field) != -1) { - continue; - } - - if (!data1[sub][field] && !data2[sub][field]) { - continue; - } - - var changed = !data1[sub][field] || !data2[sub][field] || - data1[sub][field] != data2[sub][field]; - - if (includeMatches || changed) { - diff[0][sub][field] = data1[sub][field] ? - data1[sub][field] : ''; - diff[1][sub][field] = data2[sub][field] ? - data2[sub][field] : ''; - } - - if (changed) { - numDiffs++; - } + for (var field in data1) { + if (skipFields.indexOf(field) != -1) { + continue; } - // DEBUG: some of this is probably redundant - for (var field in data2[sub]) { - if (skipFields.indexOf(field) != -1) { - continue; - } - - if (diff[0][sub][field] != undefined) { - continue; - } - - if (!data1[sub][field] && !data2[sub][field]) { - continue; - } - - var changed = !data1[sub][field] || !data2[sub][field] || - data1[sub][field] != data2[sub][field]; - - if (includeMatches || changed) { - diff[0][sub][field] = data1[sub][field] ? - data1[sub][field] : ''; - diff[1][sub][field] = data2[sub][field] ? - data2[sub][field] : ''; - } - - if (changed) { - numDiffs++; - } + if (data1[field] === false && (data2[field] === false || data2[field] === undefined)) { + continue; + } + + var changed = data1[field] !== data2[field]; + + if (includeMatches || changed) { + diff[0][field] = data1[field] !== false ? data1[field] : ''; + diff[1][field] = (data2[field] !== false && data2[field] !== undefined) + ? data2[field] : ''; + } + + if (changed) { + numDiffs++; + } + } + + // DEBUG: some of this is probably redundant + for (var field in data2) { + if (skipFields.indexOf(field) != -1) { + continue; + } + + if (diff[0][field] !== undefined) { + continue; + } + + if (data2[field] === false && (data1[field] === false || data1[field] === undefined)) { + continue; + } + + var changed = data1[field] !== data2[field]; + + if (includeMatches || changed) { + diff[0][field] = (data1[field] !== false && data1[field] !== undefined) + ? data1[field] : ''; + diff[1][field] = data2[field] !== false ? data2[field] : ''; + } + + if (changed) { + numDiffs++; } } diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js @@ -232,7 +232,7 @@ Zotero.Item.prototype.getField = function(field, unformatted, includeBaseMapped) ); } - var value = value ? value : ''; + value = value ? value : ''; if (!unformatted) { // Multipart date fields @@ -251,18 +251,11 @@ Zotero.Item.prototype.getField = function(field, unformatted, includeBaseMapped) * @return {Integer{}|String[]} */ Zotero.Item.prototype.getUsedFields = Zotero.Promise.coroutine(function* (asNames) { - if (!this.id) { - return []; - } - var sql = "SELECT fieldID FROM itemData WHERE itemID=?"; - if (asNames) { - sql = "SELECT fieldName FROM fields WHERE fieldID IN (" + sql + ")"; - } - var fields = yield Zotero.DB.columnQueryAsync(sql, this._id); - if (!fields) { - return []; - } - return fields; + this._requireData('itemData'); + + return Object.keys(this._itemData) + .filter(id => this._itemData[id] !== false) + .map(id => asNames ? Zotero.ItemFields.getName(id) : parseInt(id)); }); @@ -1581,7 +1574,8 @@ Zotero.Item.prototype.save = Zotero.Promise.coroutine(function* (options) { for (let i=0; i<toAdd.length; i++) { let tag = toAdd[i]; let tagID = yield Zotero.Tags.getIDFromName(this.libraryID, tag.tag, true); - let sql = "INSERT INTO itemTags (itemID, tagID, type) VALUES (?, ?, ?)"; + // "OR REPLACE" allows changing type + let sql = "INSERT OR REPLACE INTO itemTags (itemID, tagID, type) VALUES (?, ?, ?)"; yield Zotero.DB.queryAsync(sql, [this.id, tagID, tag.type ? tag.type : 0]); Zotero.Notifier.trigger('add', 'item-tag', this.id + '-' + tag.tag); } @@ -3248,31 +3242,35 @@ Zotero.Item.prototype.setTags = function (tags) { /** - * Add a single manual tag to the item. If an automatic tag with the same name already exists, - * replace it with a manual one. + * Add a single tag to the item. If type is 1 and an automatic tag with the same name already + * exists, replace it with a manual one. * * A separate save() is required to update the database. * * @param {String} name + * @param {Number} [type=0] */ -Zotero.Item.prototype.addTag = function (name) { +Zotero.Item.prototype.addTag = function (name, type) { + type = type ? parseInt(type) : 0; + var changed = false; var tags = this.getTags(); for (let i=0; i<tags.length; i++) { let tag = tags[i]; if (tag.tag === name) { - if (!tag.type) { + if (tag.type == type) { Zotero.debug("Tag '" + name + "' already exists on item " + this.libraryKey); return false; } - tag.type = 0; + tag.type = type; changed = true; break; } } if (!changed) { tags.push({ - tag: name + tag: name, + type: type }); } this.setTags(tags); @@ -3706,24 +3704,25 @@ Zotero.Item.prototype.diff = function (item, includeMatches, ignoreFields) { * * Currently compares only item data, not primary fields */ -Zotero.Item.prototype.multiDiff = function (otherItems, ignoreFields) { - var thisData = this.serialize(); +Zotero.Item.prototype.multiDiff = Zotero.Promise.coroutine(function* (otherItems, ignoreFields) { + var thisData = yield this.toJSON(); var alternatives = {}; var hasDiffs = false; - for each(var otherItem in otherItems) { - var diff = []; - var otherData = otherItem.serialize(); - var numDiffs = Zotero.Items.diff(thisData, otherData, diff); + for (let i = 0; i < otherItems.length; i++) { + let otherItem = otherItems[i]; + let diff = []; + let otherData = yield otherItem.toJSON(); + let numDiffs = Zotero.Items.diff(thisData, otherData, diff); if (numDiffs) { - for (var field in diff[1].fields) { + for (let field in diff[1]) { if (ignoreFields && ignoreFields.indexOf(field) != -1) { continue; } - var value = diff[1].fields[field]; + var value = diff[1][field]; if (!alternatives[field]) { hasDiffs = true; @@ -3742,143 +3741,41 @@ Zotero.Item.prototype.multiDiff = function (otherItems, ignoreFields) { } return alternatives; -} +}); /** * Returns an unsaved copy of the item * - * @param {Boolean} [includePrimary=false] - * @param {Zotero.Item} [newItem=null] Target item for clone (used to pass a saved - * item for duplicating items with tags) - * @param {Boolean} [unsaved=false] Skip properties that require a saved object (e.g., tags) - * @param {Boolean} [skipTags=false] Skip tags (implied by 'unsaved') + * @param {Number} [libraryID] - libraryID of the new item, or the same as original if omitted + * @param {Boolean} [skipTags=false] - Skip tags */ -Zotero.Item.prototype.clone = function(includePrimary, newItem, unsaved, skipTags) { +Zotero.Item.prototype.clone = function(libraryID, skipTags) { Zotero.debug('Cloning item ' + this.id); - if (includePrimary && newItem) { - throw ("includePrimary and newItem parameters are mutually exclusive in Zotero.Item.clone()"); + if (libraryID !== undefined && libraryID !== null && typeof libraryID !== 'number') { + throw new Error("libraryID must be null or an integer"); } - if (unsaved) { - skipTags = true; - } - - Zotero.DB.beginTransaction(); - - // TODO: get rid of serialize() call - var obj = this.serialize(); - - var itemTypeID = this.itemTypeID; + this._requireData('primaryData'); - if (newItem) { - var sameLibrary = newItem.libraryID == this.libraryID; - } - else { - var newItem = new Zotero.Item; - var sameLibrary = true; - - if (includePrimary) { - newItem.id = this.id; - newItem.libraryID = this.libraryID; - newItem.key = this.key; - newItem.setType(itemTypeID); - for (var field in obj.primary) { - switch (field) { - case 'itemID': - case 'itemType': - case 'libraryID': - case 'key': - continue; - } - newItem.setField(field, obj.primary[field]); - } - } - else { - newItem.setType(itemTypeID); - } + if (libraryID === undefined || libraryID === null) { + libraryID = this.libraryID; } + var sameLibrary = libraryID == this.libraryID; - var changedFields = {}; - for (var field in obj.fields) { - var fieldID = Zotero.ItemFields.getID(field); - if (fieldID && Zotero.ItemFields.isValidForType(fieldID, itemTypeID)) { - newItem.setField(field, obj.fields[field]); - changedFields[field] = true; - } - } - // If modifying an existing item, clear other fields not in the cloned item - if (newItem) { - var previousFields = this.getUsedFields(true); - for each(var field in previousFields) { - if (!changedFields[field] && Zotero.ItemFields.isValidForType(field, itemTypeID)) { - newItem.setField(field, false); - } - } + var newItem = new Zotero.Item; + newItem.setType(this.itemTypeID); + + var fieldIDs = this.getUsedFields(); + for (let i = 0; i < fieldIDs.length; i++) { + let fieldID = fieldIDs[i]; + newItem.setField(fieldID, this.getField(fieldID)); } // Regular item if (this.isRegularItem()) { - if (includePrimary) { - // newItem = loaded from db - // obj = in-memory - var max = Math.max(newItem.numCreators(), this.numCreators()); - var deleteOffset = 0; - for (var i=0; i<max; i++) { - var newIndex = i - deleteOffset; - - // Remove existing creators (loaded because we set the itemID - // above) not in the in-memory version - if (!obj.creators[i]) { - if (newItem.getCreator(newIndex)) { - newItem.removeCreator(newIndex); - deleteOffset++; - } - continue; - } - // Add in-memory creators - newItem.setCreator( - newIndex, this.getCreator(i).ref, obj.creators[i].creatorTypeID - ); - } - } - else { - // If overwriting an existing item, clear existing creators - if (newItem) { - for (var i=newItem.numCreators()-1; i>=0; i--) { - if (newItem.getCreator(i)) { - newItem.removeCreator(i); - } - } - } - - var i = 0; - for (var c in obj.creators) { - var creator = this.getCreator(c).ref; - var creatorTypeID = this.getCreator(c).creatorTypeID; - - if (!sameLibrary) { - var creatorDataID = Zotero.Creators.getDataID(this.getCreator(c).ref); - var creatorIDs = Zotero.Creators.getCreatorsWithData(creatorDataID, newItem.libraryID); - if (creatorIDs) { - // TODO: support multiple creators? - var creator = Zotero.Creators.get(creatorIDs[0]); - } - else { - var newCreator = new Zotero.Creator; - newCreator.libraryID = newItem.libraryID; - newCreator.setFields(creator); - var creator = newCreator; - } - - var creatorTypeID = this.getCreator(c).creatorTypeID; - } - - newItem.setCreator(i, creator, creatorTypeID); - i++; - } - } + newItem.setCreators(newItem.getCreators()); } else { newItem.setNote(this.getNote()); @@ -3904,29 +3801,31 @@ Zotero.Item.prototype.clone = function(includePrimary, newItem, unsaved, skipTag } } - if (!skipTags && obj.tags) { - for each(var tag in obj.tags) { - if (sameLibrary) { - newItem.addTagByID(tag.primary.tagID); - } - else { - newItem.addTag(tag.fields.name, tag.fields.type); - } - } + if (!skipTags) { + newItem.setTags(this.getTags()); } - if (obj.related && sameLibrary) { + if (sameLibrary) { // DEBUG: this will add reverse-only relateds too - newItem.relatedItems = obj.related; + newItem.setRelations(this.getRelations()); } - Zotero.DB.commitTransaction(); - return newItem; } /** + * @return {Promise<Zotero.Item>} - 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; +});; + + +/** * Delete item from database and clear from Zotero.Items internal array * * Items.erase() should be used for multiple items diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js @@ -481,65 +481,76 @@ Zotero.Items = new function() { this.merge = function (item, otherItems) { - Zotero.DB.beginTransaction(); - - var otherItemIDs = []; - var itemURI = Zotero.URI.getItemURI(item); - - for each(var otherItem in otherItems) { - // Move child items to master - var ids = otherItem.getAttachments(true).concat(otherItem.getNotes(true)); - for each(var id in ids) { - var attachment = Zotero.Items.get(id); - - // TODO: Skip identical children? - - attachment.parentID = item.id; - attachment.save(); - } - - // All other operations are additive only and do not affect the, - // old item, which will be put in the trash - - // Add collections to master - var collectionIDs = otherItem.getCollections(); - for each(var collectionID in collectionIDs) { - var collection = Zotero.Collections.get(collectionID); - collection.addItem(item.id); - } + return Zotero.DB.executeTransaction(function* () { + var otherItemIDs = []; + var itemURI = Zotero.URI.getItemURI(item); - // Add tags to master - var tags = otherItem.getTags(); - for each(var tag in tags) { - item.addTagByID(tag.id); - } + yield item.loadTags(); + yield item.loadRelations(); - // Related items - var relatedItems = otherItem.relatedItems; - for each(var relatedItemID in relatedItems) { - item.addRelatedItem(relatedItemID); + for each(var otherItem in otherItems) { + yield otherItem.loadChildItems(); + yield otherItem.loadCollections(); + yield otherItem.loadTags(); + yield otherItem.loadRelations(); + + // Move child items to master + var ids = otherItem.getAttachments(true).concat(otherItem.getNotes(true)); + for each(var id in ids) { + var attachment = yield Zotero.Items.getAsync(id); + + // TODO: Skip identical children? + + attachment.parentID = item.id; + yield attachment.save(); + } + + // All other operations are additive only and do not affect the, + // old item, which will be put in the trash + + // Add collections to master + var collectionIDs = otherItem.getCollections(); + for each(var collectionID in collectionIDs) { + let collection = yield Zotero.Collections.getAsync(collectionID); + yield collection.addItem(item.id); + } + + // Add tags to master + var tags = otherItem.getTags(); + for (let j = 0; j < tags.length; j++) { + item.addTag(tags[j].tag); + } + + // Related items + var relatedItems = otherItem.relatedItems; + for each(var relatedItemID in relatedItems) { + yield item.addRelatedItem(relatedItemID); + } + + // Relations + yield Zotero.Relations.copyURIs( + item.libraryID, + Zotero.URI.getItemURI(otherItem), + Zotero.URI.getItemURI(item) + ); + + // Add relation to track merge + var otherItemURI = Zotero.URI.getItemURI(otherItem); + yield Zotero.Relations.add( + item.libraryID, + otherItemURI, + Zotero.Relations.deletedItemPredicate, + itemURI + ); + + // Trash other item + otherItem.deleted = true; + yield otherItem.save(); } - // Relations - Zotero.Relations.copyURIs( - item.libraryID, - Zotero.URI.getItemURI(otherItem), - Zotero.URI.getItemURI(item) - ); - - // Add relation to track merge - var otherItemURI = Zotero.URI.getItemURI(otherItem); - Zotero.Relations.add(item.libraryID, otherItemURI, Zotero.Relations.deletedItemPredicate, itemURI); - - // Trash other item - otherItem.deleted = true; - otherItem.save(); - } - - item.save(); - - Zotero.DB.commitTransaction(); - } + yield item.save(); + }); + }; this.trash = function (ids) { diff --git a/chrome/content/zotero/xpcom/data/relations.js b/chrome/content/zotero/xpcom/data/relations.js @@ -79,11 +79,14 @@ Zotero.Relations = new function () { } var toReturn = []; + var loads = []; for (let i=0; i<rows.length; i++) { var relation = new Zotero.Relation; - relation.id = rows[i]; + relation.id = rows[i]; + loads.push(relation.load()); toReturn.push(relation); } + yield Zotero.Promise.all(loads); return toReturn; }); diff --git a/chrome/content/zotero/xpcom/duplicates.js b/chrome/content/zotero/xpcom/duplicates.js @@ -54,13 +54,15 @@ Zotero.Duplicates.prototype.getSearchObject = Zotero.Promise.coroutine(function* + "(id INTEGER PRIMARY KEY)"; yield Zotero.DB.queryAsync(sql); - this._findDuplicates(); + yield this._findDuplicates(); var ids = this._sets.findAll(true); + Zotero.debug("Inserting rows into temp table"); sql = "INSERT INTO tmpDuplicates VALUES (?)"; for (let i=0; i<ids.length; i++) { yield Zotero.DB.queryAsync(sql, [ids[i]], { debug: false }) } + Zotero.debug("Done"); }.bind(this)); var s = new Zotero.Search; @@ -88,7 +90,7 @@ Zotero.Duplicates.prototype._getObjectFromID = function (id) { } -Zotero.Duplicates.prototype._findDuplicates = function () { +Zotero.Duplicates.prototype._findDuplicates = Zotero.Promise.coroutine(function* () { var start = Date.now(); var self = this; @@ -136,8 +138,8 @@ Zotero.Duplicates.prototype._findDuplicates = function () { * set and the next start row would be a * different title. */ - function processRows(compareRows, reprocessMatches) { - if (!rows) { + function processRows(rows, compareRows, reprocessMatches) { + if (!rows.length) { return; } @@ -182,7 +184,7 @@ Zotero.Duplicates.prototype._findDuplicates = function () { + "JOIN itemDataValues USING (valueID) " + "WHERE libraryID=? AND itemTypeID=? AND fieldID=? " + "AND itemID NOT IN (SELECT itemID FROM deletedItems)"; - var rows = Zotero.DB.query( + var rows = yield Zotero.DB.queryAsync( sql, [ this._libraryID, @@ -191,29 +193,43 @@ Zotero.Duplicates.prototype._findDuplicates = function () { ] ); var isbnCache = {}; - if (rows) { - for each(var row in rows) { - row.value = (row.value+'').replace(/[^\dX]+/ig, '').toUpperCase(); //ignore formatting - isbnCache[row.itemID] = row.value; + if (rows.length) { + let newRows = []; + for (let i = 0; i < rows.length; i++) { + let row = rows[i]; + // Ignore formatting + let newVal = (row.value + '').replace(/[^\dX]+/ig, '').toUpperCase(); + isbnCache[row.itemID] = newVal; + newRows.push({ + itemID: row.itemID, + value: newVal + }); } - rows.sort(sortByValue); - processRows(); + newRows.sort(sortByValue); + processRows(newRows); } // DOI var sql = "SELECT itemID, value FROM items JOIN itemData USING (itemID) " + "JOIN itemDataValues USING (valueID) " - + "WHERE libraryID=? AND fieldID=? AND REGEXP('^10\\.', value) " + + "WHERE libraryID=? AND fieldID=? AND value LIKE '10\\.%' " + "AND itemID NOT IN (SELECT itemID FROM deletedItems)"; - var rows = Zotero.DB.query(sql, [this._libraryID, Zotero.ItemFields.getID('DOI')]); + var rows = yield Zotero.DB.queryAsync(sql, [this._libraryID, Zotero.ItemFields.getID('DOI')]); var doiCache = {}; - if (rows) { - for each(var row in rows) { - row.value = (row.value+'').trim().toUpperCase(); //DOIs are case insensitive - doiCache[row.itemID] = row.value; + if (rows.length) { + let newRows = []; + for (let i = 0; i < rows.length; i++) { + let row = rows[i]; + // DOIs are case insensitive + let newVal = (row.value + '').trim().toUpperCase(); + doiCache[row.itemID] = newVal; + newRows.push({ + itemID: row.itemID, + value: newVal + }); } - rows.sort(sortByValue); - processRows(); + newRows.sort(sortByValue); + processRows(newRows); } // Get years @@ -228,33 +244,70 @@ Zotero.Duplicates.prototype._findDuplicates = function () { + "AND SUBSTR(value, 1, 4) != '0000' " + "AND itemID NOT IN (SELECT itemID FROM deletedItems) " + "ORDER BY value"; - var rows = Zotero.DB.query(sql, [this._libraryID].concat(dateFields)); + var rows = yield Zotero.DB.queryAsync(sql, [this._libraryID].concat(dateFields)); var yearCache = {}; - if (rows) { - for each(var row in rows) { - yearCache[row.itemID] = row.year; - } + for (let i = 0; i < rows.length; i++) { + let row = rows[i]; + yearCache[row.itemID] = row.year; } - var creatorRowsCache = {}; - // Match on normalized title + var titleIDs = Zotero.ItemFields.getTypeFieldsFromBase('title'); + titleIDs.push(Zotero.ItemFields.getID('title')); var sql = "SELECT itemID, value FROM items JOIN itemData USING (itemID) " + "JOIN itemDataValues USING (valueID) " - + "WHERE libraryID=? AND fieldID BETWEEN 110 AND 113 " + + "WHERE libraryID=? AND fieldID IN " + + "(" + titleIDs.join(', ') + ") " + "AND itemTypeID NOT IN (1, 14) " + "AND itemID NOT IN (SELECT itemID FROM deletedItems)"; - var rows = Zotero.DB.query(sql, [this._libraryID]); - if(rows) { + var rows = yield Zotero.DB.queryAsync(sql, [this._libraryID]); + if (rows.length) { //normalize all values ahead of time rows = rows.map(function(row) { - row.value = normalizeString(row.value); - return row; + return { + itemID: row.itemID, + value: normalizeString(row.value) + }; }); //sort rows by normalized values rows.sort(sortByValue); - processRows(function (a, b) { + // Get all creators and separate by itemID + // + // We won't need all of these, but otherwise we would have to make processRows() + // asynchronous, which would be too slow + let creatorRowsCache = {}; + let sql = "SELECT itemID, lastName, firstName, fieldMode FROM items " + + "JOIN itemCreators USING (itemID) " + + "JOIN creators USING (creatorID) " + + "WHERE libraryID=? AND itemTypeID NOT IN (1, 14) AND " + + "itemID NOT IN (SELECT itemID FROM deletedItems)" + + "ORDER BY itemID, orderIndex"; + let creatorRows = yield Zotero.DB.queryAsync(sql, this._libraryID); + let lastItemID; + let itemCreators = []; + for (let i = 0; i < creatorRows.length; i++) { + let row = creatorRows[i]; + if (lastItemID && row.itemID != lastItemID) { + if (itemCreators.length) { + creatorRowsCache[lastItemID] = itemCreators; + itemCreators = []; + } + } + else { + itemCreators.push({ + lastName: normalizeString(row.lastName), + firstInitial: row.fieldMode == 0 ? normalizeString(row.firstName).charAt(0) : false + }); + } + lastItemID = row.itemID; + } + // Add final item creators + if (itemCreators.length) { + creatorRowsCache[lastItemID] = itemCreators; + } + + processRows(rows, function (a, b) { var aTitle = a.value; var bTitle = b.value; @@ -293,25 +346,9 @@ Zotero.Duplicates.prototype._findDuplicates = function () { if (typeof creatorRowsCache[a.itemID] != 'undefined') { aCreatorRows = creatorRowsCache[a.itemID]; } - else { - var sql = "SELECT lastName, firstName, fieldMode FROM itemCreators " - + "JOIN creators USING (creatorID) " - + "WHERE itemID=? ORDER BY orderIndex LIMIT 10"; - aCreatorRows = Zotero.DB.query(sql, a.itemID); - creatorRowsCache[a.itemID] = aCreatorRows; - } - - // Check for at least one match on last name + first initial of first name if (typeof creatorRowsCache[b.itemID] != 'undefined') { bCreatorRows = creatorRowsCache[b.itemID]; } - else { - var sql = "SELECT lastName, firstName, fieldMode FROM itemCreators " - + "JOIN creators USING (creatorID) " - + "WHERE itemID=? ORDER BY orderIndex LIMIT 10"; - bCreatorRows = Zotero.DB.query(sql, b.itemID); - creatorRowsCache[b.itemID] = bCreatorRows; - } // Match if no creators if (!aCreatorRows && !bCreatorRows) { @@ -322,13 +359,15 @@ Zotero.Duplicates.prototype._findDuplicates = function () { return 0; } - for each(var aCreatorRow in aCreatorRows) { - var aLastName = normalizeString(aCreatorRow.lastName); - var aFirstInitial = aCreatorRow.fieldMode == 0 ? normalizeString(aCreatorRow.firstName).charAt(0) : false; + for (let i = 0; i < aCreatorRows.length; i++) { + let aCreatorRow = aCreatorRows[i]; + let aLastName = aCreatorRow.lastName; + let aFirstInitial = aCreatorRow.firstInitial; - for each(var bCreatorRow in bCreatorRows) { - var bLastName = normalizeString(bCreatorRow.lastName); - var bFirstInitial = bCreatorRow.fieldMode == 0 ? normalizeString(bCreatorRow.firstName).charAt(0) : false; + for (let j = 0; j < bCreatorRows.length; j++) { + let bCreatorRow = bCreatorRows[j]; + let bLastName = bCreatorRow.lastName; + let bFirstInitial = bCreatorRow.firstInitial; if (aLastName === bLastName && aFirstInitial === bFirstInitial) { return 1; @@ -348,12 +387,12 @@ Zotero.Duplicates.prototype._findDuplicates = function () { + "WHERE libraryID=? AND fieldID=? " + "AND itemID NOT IN (SELECT itemID FROM deletedItems) " + "ORDER BY value"; - var rows = Zotero.DB.query(sql, [this._libraryID, Zotero.ItemFields.getID(field)]); - processRows(); + var rows = yield Zotero.DB.queryAsync(sql, [this._libraryID, Zotero.ItemFields.getID(field)]); + processRows(rows); }*/ Zotero.debug("Found duplicates in " + (Date.now() - start) + " ms"); -} +}); diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js @@ -869,7 +869,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio // Mirror ZoteroPane.onTreeMouseDown behavior var itemID = this._rows[previousRow].ref.id; var setItemIDs = collectionTreeRow.ref.getSetItemsByItemID(itemID); - yield this.selectItems(setItemIDs); + this.selectItems(setItemIDs); } } else { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js @@ -1387,7 +1387,7 @@ var ZoteroPane = new function() this.itemsView._itemSelectedPromiseResolver.reject(e); } throw e; - }.bind(this));; + }.bind(this)); };