From 5c94119c70e1a07f48ff7af3489a8156a0bfe720 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 10 Oct 2014 04:35:45 -0400 Subject: [PATCH] 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 --- chrome/content/zotero/bindings/merge.xml | 4 +- chrome/content/zotero/duplicatesMerge.js | 23 +- chrome/content/zotero/xpcom/attachments.js | 15 +- .../zotero/xpcom/collectionTreeView.js | 27 +-- .../content/zotero/xpcom/data/dataObjects.js | 109 ++++----- chrome/content/zotero/xpcom/data/item.js | 221 +++++------------- chrome/content/zotero/xpcom/data/items.js | 121 +++++----- chrome/content/zotero/xpcom/data/relations.js | 5 +- chrome/content/zotero/xpcom/duplicates.js | 153 +++++++----- chrome/content/zotero/xpcom/itemTreeView.js | 2 +- chrome/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 index 2fe1e3020..2e7d61c17 100644 --- 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; ]]> @@ -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 index e48fa47a7..27bf6ec58 100644 --- 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 index 4884b9377..abf5095a9 100644 --- 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 index 30a2eaa11..e6c832139 100644 --- 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 index b701142a1..9a6aded3d 100644 --- 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 index 2d210a6d6..bf4a9f43d 100644 --- 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=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,28 +3801,30 @@ 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} - 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 * diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index c60dd20ff..49fb6ee64 100644 --- 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); + return Zotero.DB.executeTransaction(function* () { + var otherItemIDs = []; + var itemURI = Zotero.URI.getItemURI(item); + + yield item.loadTags(); + yield item.loadRelations(); + + for each(var otherItem in otherItems) { + yield otherItem.loadChildItems(); + yield otherItem.loadCollections(); + yield otherItem.loadTags(); + yield otherItem.loadRelations(); - // TODO: Skip identical children? + // 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(); + } - 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) { + 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(); } - // 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); - } - - // Add tags to master - var tags = otherItem.getTags(); - for each(var tag in tags) { - item.addTagByID(tag.id); - } - - // Related items - var relatedItems = otherItem.relatedItems; - for each(var relatedItemID in relatedItems) { - item.addRelatedItem(relatedItemID); - } - - // 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 index 6d3995f89..310f39e97 100644 --- 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