From fe31d4c7896c51f5db58b88c140039bc6df4e400 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Mon, 16 Apr 2007 22:06:49 +0000
Subject: [PATCH] - Fixed tag selector still displaying all tags for
 collections and saved searches - Added asTempTable parameter to
 Zotero.Search.search() to save results to a temporary table -- avoids large
 inserts with post-search filters - Added proper collation sort support for
 tags

---
 chrome/content/zotero/xpcom/data_access.js  | 106 ++++++++++----------
 chrome/content/zotero/xpcom/itemTreeView.js |   6 +-
 chrome/content/zotero/xpcom/search.js       |  95 ++++++++++++------
 chrome/content/zotero/xpcom/zotero.js       |  10 ++
 4 files changed, 127 insertions(+), 90 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data_access.js b/chrome/content/zotero/xpcom/data_access.js
index 8e1c88d43..16bff5205 100644
--- a/chrome/content/zotero/xpcom/data_access.js
+++ b/chrome/content/zotero/xpcom/data_access.js
@@ -1780,9 +1780,18 @@ Zotero.Item.prototype.getTags = function(){
 		return false;
 	}
 	var sql = "SELECT tagID AS id, tag, tagType AS type FROM tags WHERE tagID IN "
-		+ "(SELECT tagID FROM itemTags WHERE itemID=" + this.getID() + ") "
-		+ "ORDER BY tag COLLATE NOCASE";
-	return Zotero.DB.query(sql);
+		+ "(SELECT tagID FROM itemTags WHERE itemID=" + this.getID() + ")";
+	
+	var tags = Zotero.DB.query(sql);
+	if (!tags) {
+		return false;
+	}
+	
+	var collation = Zotero.getLocaleCollation();
+	tags.sort(function(a, b) {
+		return collation.compareString(0, a.tag, b.tag);
+	});
+	return tags;
 }
 
 Zotero.Item.prototype.getTagIDs = function(){
@@ -3635,13 +3644,21 @@ Zotero.Tags = new function(){
 		if (types) {
 			sql += "WHERE tagType IN (" + types.join() + ") ";
 		}
-		sql += "ORDER BY tag COLLATE NOCASE";
 		var tags = Zotero.DB.query(sql);
+		if (!tags) {
+			return {};
+		}
+		
+		var collation = Zotero.getLocaleCollation();
+		tags.sort(function(a, b) {
+			return collation.compareString(0, a.tag, b.tag);
+		});
+		
 		var indexed = {};
-		for each(var tag in tags) {
-			indexed[tag.tagID] = {
-				tag: tag.tag,
-				type: tag.tagType
+		for (var i=0; i<tags.length; i++) {
+			indexed[tags[i].tagID] = {
+				tag: tags[i].tag,
+				type: tags[i].tagType
 			};
 		}
 		return indexed;
@@ -3654,49 +3671,36 @@ Zotero.Tags = new function(){
 	 * _types_ is an optional array of tagTypes to fetch
 	 */
 	function getAllWithinSearch(search, types) {
-		// If search has post-search filters (e.g. fulltext content), run it
-		// and just include the ids
-		//
-		// DEBUG: it's possible there's a query length limit here
-		// or that this slows things down with large libraries
-		// -- should probably use a temporary table instead
-		if (search.hasPostSearchFilter()) {
-			var ids = search.search();
-			var sql = "SELECT DISTINCT tagID, tag, tagType FROM itemTags "
-				+ "NATURAL JOIN tags WHERE itemID IN (" + ids.join() + ") ";
-			if (types) {
-				sql += "AND tagType IN (" + types.join() + ") ";
-			}
-			sql += "ORDER BY tag COLLATE NOCASE";
-			var tags = Zotero.DB.query(sql);
-		}
-		else {
-			var sql = "CREATE TEMPORARY TABLE tmpSearchResults AS " + search.getSQL();
-			Zotero.DB.query(sql, search.getSQLParams());
-			sql = "CREATE INDEX tmpSearchResults_itemID ON tmpSearchResults(itemID)";
-			Zotero.DB.query(sql);
-			
-			var sql = "SELECT DISTINCT tagID, tag, tagType FROM itemTags "
-				+ "NATURAL JOIN tags WHERE ("
-				+ "itemID IN (SELECT itemID FROM tmpSearchResults) OR "
-				+ "itemID IN (SELECT itemID FROM itemNotes WHERE sourceItemID IN (SELECT itemID FROM tmpSearchResults)) OR "
-				+ "itemID IN (SELECT itemID FROM itemAttachments WHERE sourceItemID IN (SELECT itemID FROM tmpSearchResults))"
-				+ ") ";
-			if (types) {
-				sql += "AND tagType IN (" + types.join() + ") ";
-			}
-			sql += "ORDER BY tag COLLATE NOCASE";
-			var tags = Zotero.DB.query(sql);
-			
-			sql = "DROP TABLE tmpSearchResults";
-			Zotero.DB.query(sql);
+		// Save search results to temporary table
+		var tmpTable = search.search(true);
+		if (!tmpTable) {
+			return {};
 		}
 		
+		var sql = "SELECT DISTINCT tagID, tag, tagType FROM itemTags "
+			+ "NATURAL JOIN tags WHERE itemID IN "
+			+ "(SELECT itemID FROM " + tmpTable + ") ";
+		if (types) {
+			sql += "AND tagType IN (" + types.join() + ") ";
+		}
+		var tags = Zotero.DB.query(sql);
+		
+		Zotero.DB.query("DROP TABLE " + tmpTable);
+		
+		if (!tags) {
+			return {};
+		}
+		
+		var collation = Zotero.getLocaleCollation();
+		tags.sort(function(a, b) {
+			return collation.compareString(0, a.tag, b.tag);
+		});
+		
 		var indexed = {};
-		for each(var tag in tags) {
-			indexed[tag.tagID] = {
-				tag: tag.tag,
-				type: tag.tagType
+		for (var i=0; i<tags.length; i++) {
+			indexed[tags[i].tagID] = {
+				tag: tags[i].tag,
+				type: tags[i].tagType
 			};
 		}
 		return indexed;
@@ -4558,11 +4562,7 @@ Zotero.getCollections = function(parent, recursive){
 	}
 	
 	// Do proper collation sort
-	var localeService = Components.classes["@mozilla.org/intl/nslocaleservice;1"]
-		.getService(Components.interfaces.nsILocaleService);
-	var collationFactory = Components.classes["@mozilla.org/intl/collation-factory;1"]
-		.getService(Components.interfaces.nsICollationFactory);
-	var collation = collationFactory.CreateCollation(localeService.getApplicationLocale());
+	var collation = Zotero.getLocaleCollation();
 	children.sort(function (a, b) {
 		return collation.compareString(0, a.name, b.name);
 	});
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 6f2709cbf..27a49c844 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -718,11 +718,7 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 	var columnField = this.getSortField();
 	var order = this.getSortDirection() == 'ascending';
 	
-	var localeService = Components.classes["@mozilla.org/intl/nslocaleservice;1"]
-		.getService(Components.interfaces.nsILocaleService);
-	var collationFactory = Components.classes["@mozilla.org/intl/collation-factory;1"]
-		.getService(Components.interfaces.nsICollationFactory);
-	var collation = collationFactory.CreateCollation(localeService.getApplicationLocale());
+	var collation = Zotero.getLocaleCollation();
 	
 	// Year is really the date field truncated
 	if (columnField == 'year') {
diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js
index 16a770aeb..a1fc91c78 100644
--- a/chrome/content/zotero/xpcom/search.js
+++ b/chrome/content/zotero/xpcom/search.js
@@ -318,7 +318,7 @@ Zotero.Search.prototype.hasPostSearchFilter = function() {
 /*
  * Run the search and return an array of item ids for results
  */
-Zotero.Search.prototype.search = function(){
+Zotero.Search.prototype.search = function(asTempTable){
 	if (!this._sql){
 		this._buildQuery();
 	}
@@ -331,49 +331,47 @@ Zotero.Search.prototype.search = function(){
 				return false;
 			}
 			
-			Zotero.DB.query("DROP TABLE IF EXISTS tmpSearchResults");
-			
-			var sql = "CREATE TEMPORARY TABLE tmpSearchResults (itemID INT)";
-			Zotero.DB.query(sql);
-			var sql = "INSERT INTO tmpSearchResults VALUES (?)";
-			var insertStatement = Zotero.DB.getStatement(sql);
-			for (var i=0; i<ids.length; i++) {
-				insertStatement.bindInt32Parameter(1, ids[i]);
-				try {
-					insertStatement.execute();
-				}
-				catch (e) {
-					throw (Zotero.DB.getLastErrorString());
-				}
-			}
-			insertStatement.reset();
+			var tmpTable = this._idsToTempTable(ids);
 		}
 		// Otherwise, just copy to temp table directly
 		else {
-			var sql = "CREATE TEMPORARY TABLE tmpSearchResults AS "
+			var tmpTable = "tmpSearchResults_" + Zotero.randomString(8);
+			var sql = "CREATE TEMPORARY TABLE " + tmpTable + " AS "
 				+ this._scope.getSQL();
 			Zotero.DB.query(sql, this._scope.getSQLParams());
+			var sql = "CREATE INDEX " + tmpTable + "_itemID ON " + tmpTable + "(itemID)";
+			Zotero.DB.query(sql);
 		}
 		
-		var sql = "CREATE INDEX tmpSearchResults_itemID ON tmpSearchResults(itemID)";
-		Zotero.DB.query(sql);
-		
+		// Search ids in temp table and their child items
 		var sql = "SELECT itemID FROM items WHERE itemID IN (" + this._sql + ") "
 			+ "AND ("
-			+ "itemID IN (SELECT itemID FROM tmpSearchResults) OR "
+			+ "itemID IN (SELECT itemID FROM " + tmpTable + ") OR "
 			+ "itemID IN (SELECT itemID FROM itemAttachments"
-			+ " WHERE sourceItemID IN (SELECT itemID FROM tmpSearchResults)) OR "
+			+ " WHERE sourceItemID IN (SELECT itemID FROM " + tmpTable + ")) OR "
 			+ "itemID IN (SELECT itemID FROM itemNotes"
-			+ " WHERE sourceItemID IN (SELECT itemID FROM tmpSearchResults))"
+			+ " WHERE sourceItemID IN (SELECT itemID FROM " + tmpTable + "))"
 			+ ")";
-		var ids = Zotero.DB.columnQuery(sql, this._sqlParams);
 		
-		Zotero.DB.query("DROP TABLE tmpSearchResults");
+		if (asTempTable) {
+			var tmpTable2 = "tmpSearchResults_" + Zotero.randomString(8);
+			sql = "CREATE TEMPORARY TABLE " + tmpTable2 + " AS " + sql;
+			Zotero.DB.query(sql, this._sqlParams);
+			return tmpTable2;
+		}
+		else {
+			var ids = Zotero.DB.columnQuery(sql, this._sqlParams);
+			Zotero.DB.query("DROP TABLE " + tmpTable);
+			return ids;
+		}
 	}
-	else {
-		var ids = Zotero.DB.columnQuery(this._sql, this._sqlParams);
+	
+	if (this._scope) {
+		throw ("Fulltext content search with custom scope not currently supported in Zotero.Search.search()");
 	}
 	
+	var ids = Zotero.DB.columnQuery(this._sql, this._sqlParams);
+	
 	//Zotero.debug('IDs from main search: ');
 	//Zotero.debug(ids);
 	
@@ -406,10 +404,6 @@ Zotero.Search.prototype.search = function(){
 	// (a separate fulltext word search filtered by fulltext content)
 	for each(var condition in this._conditions){
 		if (condition['condition']=='fulltextContent'){
-			if (this._scope) {
-				throw ("Cannot perform fulltext content search with custom scope in Zotero.Search.search()");
-			}
-			
 			//Zotero.debug('Running subsearch against fulltext word index');
 			
 			// Run a new search against the fulltext word index
@@ -500,6 +494,15 @@ Zotero.Search.prototype.search = function(){
 	//Zotero.debug('Final result set');
 	//Zotero.debug(ids);
 	
+	if (asTempTable) {
+		var ids = this._scope.search();
+		if (!ids) {
+			return false;
+		}
+		
+		return this._idsToTempTable(ids);
+	}
+	
 	return ids;
 }
 
@@ -523,6 +526,34 @@ Zotero.Search.prototype.getSQLParams = function(){
 }
 
 
+/*
+ * Batch insert
+ */
+Zotero.Search.prototype._idsToTempTable = function (ids) {
+	var tmpTable = "tmpSearchResults_" + Zotero.randomString(8);
+	
+	var sql = "CREATE TEMPORARY TABLE " + tmpTable + " (itemID INT)";
+	Zotero.DB.query(sql);
+	var sql = "INSERT INTO " + tmpTable + " VALUES (?)";
+	var insertStatement = Zotero.DB.getStatement(sql);
+	for (var i=0; i<ids.length; i++) {
+		insertStatement.bindInt32Parameter(1, ids[i]);
+		try {
+			insertStatement.execute();
+		}
+		catch (e) {
+			throw (Zotero.DB.getLastErrorString());
+		}
+	}
+	insertStatement.reset();
+	
+	var sql = "CREATE INDEX " + tmpTable + "_itemID ON " + tmpTable + "(itemID)";
+	Zotero.DB.query(sql);
+	
+	return tmpTable;
+}
+
+
 /*
  * Build the SQL query for the search
  */
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
index aa60b4f8a..b5befecab 100644
--- a/chrome/content/zotero/xpcom/zotero.js
+++ b/chrome/content/zotero/xpcom/zotero.js
@@ -47,6 +47,7 @@ var Zotero = new function(){
 	this.varDump = varDump;
 	this.safeDebug = safeDebug;
 	this.getString = getString;
+	this.getLocaleCollation = getLocaleCollation;
 	this.setFontSize = setFontSize;
 	this.flattenArguments = flattenArguments;
 	this.getAncestorByTagName = getAncestorByTagName;
@@ -569,6 +570,15 @@ var Zotero = new function(){
 	}
 	
 	
+	function getLocaleCollation() {
+		var localeService = Components.classes["@mozilla.org/intl/nslocaleservice;1"]
+			.getService(Components.interfaces.nsILocaleService);
+		var collationFactory = Components.classes["@mozilla.org/intl/collation-factory;1"]
+			.getService(Components.interfaces.nsICollationFactory);
+		return collation = collationFactory.CreateCollation(localeService.getApplicationLocale());
+	}
+	
+	
 	/*
 	 * Sets font size based on prefs -- intended for use on root element
 	 *  (zotero-pane, note window, etc.)