From efda43f6e39bfe16c2852fbf59a424fa69018f9d Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Wed, 1 May 2013 19:14:45 -0400
Subject: [PATCH] Speed up saving of tags with many items

Fixes #289, RIS import gets progressively slower
---
 chrome/content/zotero/xpcom/data/tag.js  | 150 +++++++++++------------
 chrome/content/zotero/xpcom/utilities.js |   9 +-
 2 files changed, 77 insertions(+), 82 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/tag.js b/chrome/content/zotero/xpcom/data/tag.js
index e2146e43be..fb67b9a904 100644
--- a/chrome/content/zotero/xpcom/data/tag.js
+++ b/chrome/content/zotero/xpcom/data/tag.js
@@ -47,7 +47,9 @@ Zotero.Tag.prototype._init = function () {
 	this._previousData = false;
 	
 	this._linkedItemsLoaded = false;
-	this._linkedItems = [];
+	this._linkedItems = {};
+	this._linkedItemIDsToAdd = [];
+	this._linkedItemIDsToRemove = [];
 }
 
 
@@ -212,66 +214,63 @@ Zotero.Tag.prototype.getLinkedItems = function (asIDs) {
 		this._loadLinkedItems();
 	}
 	
-	if (this._linkedItems.length == 0) {
-		return false;
+	if (Zotero.Utilities.isEmpty(this._linkedItems)) {
+		return [];
 	}
 	
 	// Return itemIDs
 	if (asIDs) {
-		var ids = [];
-		for each(var item in this._linkedItems) {
-			ids.push(item.id);
-		}
-		return ids;
+		return Object.keys(this._linkedItems);
 	}
 	
 	// Return Zotero.Item objects
-	var objs = [];
-	for each(var item in this._linkedItems) {
-		objs.push(item);
-	}
-	return objs;
+	return [item for each(item in this._linkedItems)];
 }
 
 
 Zotero.Tag.prototype.countLinkedItems = function () {
-	return this.getLinkedItems().length;
+	return Object.keys(this._linkedItems).length;
 }
 
 
 Zotero.Tag.prototype.addItem = function (itemID) {
-	var current = this.getLinkedItems(true);
-	if (current && current.indexOf(itemID) != -1) {
+	if (!this._linkedItemsLoaded) {
+		this._loadLinkedItems();
+	}
+	
+	if (this._linkedItems[itemID]) {
 		Zotero.debug("Item " + itemID + " already has tag "
 			+ this.id + " in Zotero.Tag.addItem()");
 		return false;
 	}
 	
-	this._prepFieldChange('linkedItems');
 	var item = Zotero.Items.get(itemID);
 	if (!item) {
-		throw ("Can't link invalid item " + itemID + " to tag " + this.id
-			+ " in Zotero.Tag.addItem()");
+		throw new Error("Can't link invalid item " + itemID + " to tag " + this.id);
 	}
-	this._linkedItems.push(item);
+	this._prepFieldChange('linkedItems');
+	this._linkedItems[itemID] = item;
+	this._linkedItemIDsToAdd.push(itemID);
+	this._linkedItemIDsToRemove = this._linkedItemIDsToRemove.filter(function (x) x != itemID);
 	return true;
 }
 
 
 Zotero.Tag.prototype.removeItem = function (itemID) {
-	var current = this.getLinkedItems(true);
-	if (current) {
-		var index = current.indexOf(itemID);
+	if (!this._linkedItemsLoaded) {
+		this._loadLinkedItems();
 	}
 	
-	if (!current || index == -1) {
+	if (!this._linkedItems[itemID]) {
 		Zotero.debug("Item " + itemID + " doesn't have tag "
 			+ this.id + " in Zotero.Tag.removeItem()");
 		return false;
 	}
 	
 	this._prepFieldChange('linkedItems');
-	this._linkedItems.splice(index, 1);
+	delete this._linkedItems[itemID];
+	this._linkedItemIDsToAdd = this._linkedItemIDsToAdd.filter(function (x) x != itemID);
+	this._linkedItemIDsToRemove.push(itemID);
 	return true;
 }
 
@@ -406,7 +405,6 @@ Zotero.Tag.prototype.save = function (full) {
 			}
 		}
 		
-		
 		// Linked items
 		if (full || this._changed.linkedItems) {
 			var removed = [];
@@ -430,17 +428,8 @@ Zotero.Tag.prototype.save = function (full) {
 				}
 			}
 			else {
-				if (this._previousData.linkedItems) {
-					removed = Zotero.Utilities.arrayDiff(
-						this._previousData.linkedItems, currentIDs
-					);
-					newids = Zotero.Utilities.arrayDiff(
-						currentIDs, this._previousData.linkedItems
-					);
-				}
-				else {
-					newids = currentIDs;
-				}
+				removed = this._linkedItemIDsToRemove;
+				newids = this._linkedItemIDsToAdd;
 			}
 			
 			if (removed.length) {
@@ -675,14 +664,13 @@ Zotero.Tag.prototype._loadLinkedItems = function() {
 	}
 	
 	var sql = "SELECT itemID FROM itemTags WHERE tagID=?";
-	var ids = Zotero.DB.columnQuery(sql, this.id);
+	var ids = Zotero.DB.columnQuery(sql, this.id) || [];
 	
-	this._linkedItems = [];
+	this._linkedItems = {};
 	
-	if (ids) {
-		for each(var id in ids) {
-			this._linkedItems.push(Zotero.Items.get(id));
-		}
+	for (let i=0; i<ids.length; i++) {
+		let id = ids[i];
+		this._linkedItems[id] = Zotero.Items.get(id);
 	}
 	
 	this._linkedItemsLoaded = true;
@@ -694,57 +682,63 @@ Zotero.Tag.prototype._setLinkedItems = function (itemIDs) {
 		this._loadLinkedItems();
 	}
 	
-	if (itemIDs.constructor.name != 'Array') {
-		throw ('ids must be an array in Zotero.Tag._setLinkedItems()');
+	if (!Array.isArray(itemIDs)) {
+		throw new Error('itemIDs must be an array');
 	}
 	
-	var currentIDs = this.getLinkedItems(true);
-	if (!currentIDs) {
-		currentIDs = [];
-	}
-	var oldIDs = []; // children being kept
-	var newIDs = []; // new children
+	var numCurrent = this.countLinkedItems();
 	
 	if (itemIDs.length == 0) {
-		if (currentIDs.length == 0) {
+		if (numCurrent == 0) {
 			Zotero.debug('No linked items added', 4);
 			return false;
 		}
+		
+		// No items passed, so remove all currently linked items
+		this._prepFieldChange('linkedItems');
+		this._linkedItems = {};
+		this._linkedItemIDsToAdd = [];
+		this._linkedItemIDsToRemove = this.getLinkedItems(true);
+		return true;
 	}
-	else {
-		for (var i in itemIDs) {
-			var id = parseInt(itemIDs[i]);
-			if (isNaN(id)) {
-				throw ("Invalid itemID '" + itemIDs[i]
-					+ "' in Zotero.Tag._setLinkedItems()");
-			}
-			
-			if (currentIDs.indexOf(id) != -1) {
-				Zotero.debug("Item " + itemIDs[i]
-					+ " is already linked to tag " + this.id);
-				if (oldIDs.indexOf(id) == -1) {
-					oldIDs.push(id);
-				}
-				continue;
-			}
-			
-			newIDs.push(id);
+	
+	// Get all new items
+	for (let i=0; i<itemIDs.length; i++) {
+		let id = itemIDs[i];
+		if (isNaN(parseInt(id))) {
+			throw new Error("Invalid itemID '" + itemIDs[i] + "'");
+		}
+		
+		if (this._linkedItemIDs[id]) {
+			Zotero.debug("Item " + id + " is already linked to tag " + this.id);
+		}
+		else {
+			this._linkedItemIDsToAdd.push(id);
 		}
 	}
 	
-	// Mark as changed if new or removed ids
-	if (newIDs.length > 0 || oldIDs.length != currentIDs.length) {
-		this._prepFieldChange('linkedItems');
-	}
-	else {
+	// If no new items and the old and new lengths match, none are being removed either
+	if (!this._linkedItemIDsToAdd.length && numCurrent == itemIDs.length) {
 		Zotero.debug('Linked items not changed in Zotero.Tag._setLinkedItems()', 4);
 		return false;
 	}
 	
-	newIDs = oldIDs.concat(newIDs);
+	this._prepFieldChange('linkedItems');
+	
+	// Rebuild linked items with items that exist
+	var items = Zotero.Items.get(itemIDs) || [];
+	this._linkedItems = {};
+	for (let i=0; i<items; i++) {
+		this._linkedItems[items[i].id] = items[i];
+	}
+	
+	// Mark items not found for removal
+	for (let i=0; i<itemIDs.length; i++) {
+		if (!this._linkedItemIDs[id]) {
+			this._linkedItemIDsToRemove.push(id);
+		}
+	}
 	
-	var items = Zotero.Items.get(itemIDs);
-	this._linkedItems = items ? items : [];
 	return true;
 }
 
diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js
index 87b28d13b4..8e0e830d6e 100644
--- a/chrome/content/zotero/xpcom/utilities.js
+++ b/chrome/content/zotero/xpcom/utilities.js
@@ -557,11 +557,11 @@ Zotero.Utilities = {
 	 *										otherwise return the values
 	 */
 	"arrayDiff":function(array1, array2, useIndex) {
-		if (array1.constructor.name != 'Array') {
-			throw ("array1 is not an array in Zotero.Utilities.arrayDiff() (" + array1 + ")");
+		if (!Array.isArray(array1)) {
+			throw ("array1 is not an array (" + array1 + ")");
 		}
-		if (array2.constructor.name != 'Array') {
-			throw ("array2 is not an array in Zotero.Utilities.arrayDiff() (" + array2 + ")");
+		if (!Array.isArray(array2)) {
+			throw ("array2 is not an array (" + array2 + ")");
 		}
 		
 		var val, pos, vals = [];
@@ -575,6 +575,7 @@ Zotero.Utilities = {
 		return vals;
 	},
 	
+	
 	/**
 	 * Return new array with duplicate values removed
 	 *