From e2cbfbd0fecfa2afee127f9e85382a03f0edd45a Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 21 Apr 2016 11:07:16 -0400
Subject: [PATCH] Deasyncify Zotero.Tags.getID()/getAsync(), and add
 Zotero.Tags.create()

---
 .../content/zotero/bindings/tagselector.xml   |  4 +-
 chrome/content/zotero/xpcom/data/item.js      |  8 +-
 chrome/content/zotero/xpcom/data/tags.js      | 87 ++++++++++++++++---
 chrome/content/zotero/xpcom/zotero.js         |  1 +
 test/tests/supportTest.js                     |  2 +-
 test/tests/tagSelectorTest.js                 |  2 +-
 test/tests/tagsTest.js                        | 14 +--
 test/tests/tagsboxTest.js                     |  4 +-
 8 files changed, 90 insertions(+), 32 deletions(-)

diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml
index 03bb70a42..c4cca673d 100644
--- a/chrome/content/zotero/bindings/tagselector.xml
+++ b/chrome/content/zotero/bindings/tagselector.xml
@@ -669,7 +669,7 @@
 							this.selection.delete(oldName);
 						}
 						
-						if (yield Zotero.Tags.getID(oldName)) {
+						if (Zotero.Tags.getID(oldName)) {
 							yield Zotero.Tags.rename(this.libraryID, oldName, newName.value);
 						}
 						// Colored tags don't need to exist, so in that case
@@ -707,7 +707,7 @@
 							return;
 						}
 						
-						var tagID = yield Zotero.Tags.getID(name);
+						var tagID = Zotero.Tags.getID(name);
 						if (tagID) {
 							yield Zotero.Tags.removeFromLibrary(this.libraryID, tagID);
 						}
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 43004ff6e..9544e3705 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -1621,7 +1621,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		
 		for (let i=0; i<toAdd.length; i++) {
 			let tag = toAdd[i];
-			let tagID = yield Zotero.Tags.getID(tag.tag, true);
+			let tagID = yield Zotero.Tags.create(tag.tag);
 			let tagType = tag.type ? tag.type : 0;
 			// "OR REPLACE" allows changing type
 			let sql = "INSERT OR REPLACE INTO itemTags (itemID, tagID, type) VALUES (?, ?, ?)";
@@ -1639,7 +1639,7 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		if (toRemove.length) {
 			for (let i=0; i<toRemove.length; i++) {
 				let tag = toRemove[i];
-				let tagID = yield Zotero.Tags.getID(tag.tag);
+				let tagID = Zotero.Tags.getID(tag.tag);
 				let sql = "DELETE FROM itemTags WHERE itemID=? AND tagID=? AND type=?";
 				yield Zotero.DB.queryAsync(sql, [this.id, tagID, tag.type ? tag.type : 0]);
 				let notifierData = {};
@@ -3319,6 +3319,8 @@ Zotero.Item.prototype.addTag = function (name, type) {
 /**
  * Replace an existing tag with a new manual tag
  *
+ * A separate save() is required to update the database.
+ *
  * @param {String} oldTag
  * @param {String} newTag
  */
@@ -3358,13 +3360,11 @@ Zotero.Item.prototype.replaceTag = function (oldTag, newTag) {
  */
 Zotero.Item.prototype.removeTag = function(tagName) {
 	this._requireData('tags');
-	Zotero.debug(this._tags);
 	var newTags = this._tags.filter(function (tagData) tagData.tag !== tagName);
 	if (newTags.length == this._tags.length) {
 		Zotero.debug('Cannot remove missing tag ' + tagName + ' from item ' + this.libraryKey);
 		return;
 	}
-	Zotero.debug(newTags);
 	this.setTags(newTags);
 }
 
diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js
index 450e751ee..dfaa31d34 100644
--- a/chrome/content/zotero/xpcom/data/tags.js
+++ b/chrome/content/zotero/xpcom/data/tags.js
@@ -30,12 +30,32 @@
 Zotero.Tags = new function() {
 	this.MAX_COLORED_TAGS = 6;
 	
+	var _initialized = false;
+	var _tagsByID = new Map();
+	var _idsByTag = new Map();
 	var _libraryColors = {};
 	var _libraryColorsByName = {};
 	var _itemsListImagePromises = {};
 	var _itemsListExtraImagePromises = {};
 	
 	
+	this.init = Zotero.Promise.coroutine(function* () {
+		yield Zotero.DB.queryAsync(
+			"SELECT tagID, name FROM tags",
+			false,
+			{
+				onRow: function (row) {
+					var tagID = row.getResultByIndex(0);
+					var name = row.getResultByIndex(1);
+					_tagsByID.set(tagID, name);
+					_idsByTag.set(name, tagID);
+				}
+			}
+		);
+		_initialized = true;
+	});
+	
+	
 	/**
 	 * Returns a tag for a given tagID
 	 *
@@ -43,31 +63,61 @@ Zotero.Tags = new function() {
 	 * @return {Promise<String|false>} - A tag name, or false if tag with id not found
 	 */
 	this.getName = function (tagID) {
-		return Zotero.DB.valueQueryAsync("SELECT name FROM tags WHERE tagID=?", tagID);
-	}
+		if (!_initialized) {
+			throw new Zotero.Exception.UnloadedDataException("Tags not yet loaded");
+		}
+		
+		var name = _tagsByID.get(tagID);
+		return name !== undefined ? name : false;
+	};
 	
 	
 	/**
-	 * Returns the tagID matching given fields, or creates a new tag and returns its id
+	 * Returns the tagID matching given fields, or false if none
 	 *
 	 * @param {String} name - Tag data in API JSON format
-	 * @param {Boolean} [create=false] - If no matching tag, create one;
-	 *                                   requires a wrapping transaction
-	 * @return {Promise<Integer>} tagID
+	 * @return {Integer} tagID
 	 */
-	this.getID = Zotero.Promise.coroutine(function* (name, create) {
-		if (create) {
-			Zotero.DB.requireTransaction();
+	this.getID = function (name) {
+		if (!_initialized) {
+			throw new Zotero.Exception.UnloadedDataException("Tags not yet loaded");
 		}
+		if (arguments.length > 1) {
+			throw new Error("Zotero.Tags.getID() no longer takes a second parameter -- use Zotero.Tags.create()");
+		}
+		
 		data = this.cleanData({
 			tag: name
 		});
-		var sql = "SELECT tagID FROM tags WHERE name=?";
-		var id = yield Zotero.DB.valueQueryAsync(sql, data.tag);
-		if (!id && create) {
+		var id = _idsByTag.get(data.tag);
+		return id !== undefined ? id : false;
+	};
+	
+	
+	/**
+	 * Returns the tagID matching given fields, or creates one and returns its id
+	 *
+	 * Requires a wrapping transaction
+	 *
+	 * @param {String} name - Tag data in API JSON format
+	 * @return {Promise<Integer>} tagID
+	 */
+	this.create = Zotero.Promise.coroutine(function* (name) {
+		if (!_initialized) {
+			throw new Zotero.Exception.UnloadedDataException("Tags not yet loaded");
+		}
+		
+		Zotero.DB.requireTransaction();
+		data = this.cleanData({
+			tag: name
+		});
+		var id = this.getID(data.tag);
+		if (!id) {
 			id = Zotero.ID.get('tags');
 			let sql = "INSERT INTO tags (tagID, name) VALUES (?, ?)";
 			yield Zotero.DB.queryAsync(sql, [id, data.tag]);
+			_tagsByID.set(id, data.tag);
+			_idsByTag.set(data.tag, id);
 		}
 		return id;
 	});
@@ -252,7 +302,7 @@ Zotero.Tags = new function() {
 			var notifierData = {};
 			for (let i=0; i<tagIDs.length; i++) {
 				let tagID = tagIDs[i];
-				let name = yield this.getName(tagID);
+				let name = this.getName(tagID);
 				if (name === false) {
 					continue;
 				}
@@ -314,11 +364,14 @@ Zotero.Tags = new function() {
 	/**
 	 * Delete obsolete tags from database
 	 *
-	 * @param {Number} libraryID
 	 * @param {Number|Number[]} [tagIDs] - tagID or array of tagIDs to purge
 	 * @return {Promise}
 	 */
 	this.purge = Zotero.Promise.coroutine(function* (tagIDs) {
+		if (!_initialized) {
+			throw new Zotero.Exception.UnloadedDataException("Tags not yet loaded");
+		}
+		
 		if (!tagIDs && !Zotero.Prefs.get('purge.tags')) {
 			return;
 		}
@@ -363,6 +416,12 @@ Zotero.Tags = new function() {
 		notifierData = {};
 		for (let i=0; i<toDelete.length; i++) {
 			let row = toDelete[i];
+			
+			Zotero.DB.addCurrentCallback('commit', () => {
+				_tagsByID.delete(row.id);
+				_idsByTag.delete(row.name);
+			});
+			
 			ids.push(row.id);
 			notifierData[row.id] = {
 				old: {
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
index f3362f1f5..d340c33f2 100644
--- a/chrome/content/zotero/xpcom/zotero.js
+++ b/chrome/content/zotero/xpcom/zotero.js
@@ -625,6 +625,7 @@ Components.utils.import("resource://gre/modules/osfile.jsm");
 				yield Zotero.Collections.init();
 				yield Zotero.Items.init();
 				yield Zotero.Searches.init();
+				yield Zotero.Tags.init();
 				yield Zotero.Creators.init();
 				yield Zotero.Groups.init();
 				yield Zotero.Relations.init();
diff --git a/test/tests/supportTest.js b/test/tests/supportTest.js
index 89a43c377..33a48d557 100644
--- a/test/tests/supportTest.js
+++ b/test/tests/supportTest.js
@@ -72,7 +72,7 @@ describe("Support Functions for Unit Testing", function() {
 
 			let tags = data.itemWithTags.tags;
 			for (let i=0; i<tags.length; i++) {
-				let tagID = yield Zotero.Tags.getID(tags[i].tag);
+				let tagID = Zotero.Tags.getID(tags[i].tag);
 				assert.ok(tagID, '"' + tags[i].tag + '" tag was inserted into the database');
 				assert.ok(zItem.hasTag(tags[i].tag), '"' + tags[i].tag + '" tag was assigned to item');
 			}
diff --git a/test/tests/tagSelectorTest.js b/test/tests/tagSelectorTest.js
index 957b0af7f..30bf1fba4 100644
--- a/test/tests/tagSelectorTest.js
+++ b/test/tests/tagSelectorTest.js
@@ -173,7 +173,7 @@ describe("Tag Selector", function () {
 			/*// Remove all tags in library
 			var tags = yield Zotero.Tags.getAll(libraryID);
 			tags.forEach(function (tag) {
-				var tagID = yield Zotero.Tags.getID(tag);
+				var tagID = Zotero.Tags.getID(tag);
 				yield Zotero.Tags.removeFromLibrary(libraryID, tagID);
 			});*/
 			
diff --git a/test/tests/tagsTest.js b/test/tests/tagsTest.js
index de223fa55..c0f9d482c 100644
--- a/test/tests/tagsTest.js
+++ b/test/tests/tagsTest.js
@@ -8,7 +8,7 @@ describe("Zotero.Tags", function () {
 			item.addTag(tagName);
 			yield item.saveTx();
 			
-			assert.typeOf((yield Zotero.Tags.getID(tagName)), "number");
+			assert.typeOf(Zotero.Tags.getID(tagName), "number");
 		})
 	})
 	
@@ -20,8 +20,8 @@ describe("Zotero.Tags", function () {
 			yield item.saveTx();
 			
 			var libraryID = Zotero.Libraries.userLibraryID;
-			var tagID = yield Zotero.Tags.getID(tagName);
-			assert.equal((yield Zotero.Tags.getName(tagID)), tagName);
+			var tagID = Zotero.Tags.getID(tagName);
+			assert.equal(Zotero.Tags.getName(tagID), tagName);
 		})
 	})
 	
@@ -35,7 +35,7 @@ describe("Zotero.Tags", function () {
 			yield item.saveTx();
 			assert.lengthOf(item.getTags(), 1);
 			
-			var tagID = yield Zotero.Tags.getID(tagName);
+			var tagID = Zotero.Tags.getID(tagName);
 			yield Zotero.Tags.removeFromLibrary(libraryID, tagID);
 			assert.lengthOf(item.getTags(), 0);
 		})
@@ -50,18 +50,18 @@ describe("Zotero.Tags", function () {
 			item.addTag(tagName);
 			yield item.saveTx();
 			
-			var tagID = yield Zotero.Tags.getID(tagName);
+			var tagID = Zotero.Tags.getID(tagName);
 			assert.typeOf(tagID, "number");
 			
 			yield item.eraseTx();
 			
-			assert.equal((yield Zotero.Tags.getName(tagID)), tagName);
+			assert.equal(Zotero.Tags.getName(tagID), tagName);
 			
 			yield Zotero.DB.executeTransaction(function* () {
 				yield Zotero.Tags.purge();
 			});
 			
-			assert.isFalse(yield Zotero.Tags.getName(tagID));
+			assert.isFalse(Zotero.Tags.getName(tagID));
 		})
 	})
 	
diff --git a/test/tests/tagsboxTest.js b/test/tests/tagsboxTest.js
index 631a65aea..dcefb05b7 100644
--- a/test/tests/tagsboxTest.js
+++ b/test/tests/tagsboxTest.js
@@ -100,9 +100,7 @@ describe("Item Tags Box", function () {
 			assert.equal(rows.length, 1);
 			assert.equal(rows[0].textContent, tag);
 			
-			yield Zotero.Tags.removeFromLibrary(
-				Zotero.Libraries.userLibraryID, (yield Zotero.Tags.getID(tag))
-			);
+			yield Zotero.Tags.removeFromLibrary(Zotero.Libraries.userLibraryID, Zotero.Tags.getID(tag));
 			
 			var rows = tagsbox.id('tagRows').getElementsByTagName('row');
 			assert.equal(rows.length, 0);