From 4782c725e716c91ae9535c5c258c1ddcbb92bd0f Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 10 Oct 2008 01:48:19 +0000
Subject: [PATCH] Reworked and standarized data object caching -- this should
 fix problems of long-term references containing stale, orphaned versions of
 data objects

---
 .../content/zotero/xpcom/data/collection.js   |   1 -
 .../content/zotero/xpcom/data/collections.js  | 125 +++++++-----------
 chrome/content/zotero/xpcom/data/creator.js   |  43 ++++--
 chrome/content/zotero/xpcom/data/creators.js  | 102 +++++++-------
 .../content/zotero/xpcom/data/dataObjects.js  |  90 +++++++++++++
 chrome/content/zotero/xpcom/data/item.js      |   4 +-
 chrome/content/zotero/xpcom/data/items.js     |  97 +++++---------
 chrome/content/zotero/xpcom/data/tag.js       |  28 ++--
 chrome/content/zotero/xpcom/data/tags.js      |  79 +++++++----
 chrome/content/zotero/xpcom/zotero.js         |   4 +-
 10 files changed, 317 insertions(+), 256 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js
index 5defa0e4b..f822f30b6 100644
--- a/chrome/content/zotero/xpcom/data/collection.js
+++ b/chrome/content/zotero/xpcom/data/collection.js
@@ -310,7 +310,6 @@ Zotero.Collection.prototype.save = function () {
 		Zotero.DB.query("DELETE FROM collections WHERE collectionID=?", oldID);
 		Zotero.DB.query("UPDATE collections SET key=? WHERE collectionID=?", [row.key, this.id]);
 		
-		Zotero.Collections.unload(oldID);
 		Zotero.Notifier.trigger('id-change', 'collection', oldID + '-' + this.id);
 		
 		// Update child collections that have cached the previous id
diff --git a/chrome/content/zotero/xpcom/data/collections.js b/chrome/content/zotero/xpcom/data/collections.js
index f366926a0..f16698c5e 100644
--- a/chrome/content/zotero/xpcom/data/collections.js
+++ b/chrome/content/zotero/xpcom/data/collections.js
@@ -28,26 +28,20 @@ Zotero.Collections = new function() {
 	Zotero.DataObjects.apply(this, ['collection']);
 	this.constructor.prototype = new Zotero.DataObjects();
 	
-	var _collections = {};
-	var _collectionsLoaded = false;
-	
 	this.get = get;
 	this.add = add;
 	this.getUpdated = getUpdated;
 	this.getCollectionsContainingItems = getCollectionsContainingItems;
-	this.reload = reload;
-	this.reloadAll = reloadAll;
 	this.erase = erase;
-	this.unload = unload;
 	
 	/*
 	 * Returns a Zotero.Collection object for a collectionID
 	 */
 	function get(id) {
-		if (!_collectionsLoaded) {
+		if (this._reloadCache) {
 			this.reloadAll();
 		}
-		return (typeof _collections[id]!='undefined') ? _collections[id] : false;
+		return this._objectCache[id] ? this._objectCache[id] : false;
 	}
 	
 	
@@ -107,8 +101,8 @@ Zotero.Collections = new function() {
 		ids = Zotero.flattenArguments(ids);
 		
 		for each(var id in ids) {
-			if (_collections[id]) {
-				_collections[id]._refreshParent();
+			if (this._objectCache[id]) {
+				this._objectCache[id]._refreshParent();
 			}
 		}
 	}
@@ -124,68 +118,13 @@ Zotero.Collections = new function() {
 		ids = Zotero.flattenArguments(ids);
 		
 		for each(var id in ids) {
-			if (_collections[id]) {
-				_collections[id]._refreshChildCollections();
+			if (this._objectCache[id]) {
+				this._objectCache[id]._refreshChildCollections();
 			}
 		}
 	}
 	
 	
-	function reload(id) {
-		if (!_collectionsLoaded) {
-			this.reloadAll();
-			return;
-		}
-		
-		if (!_collections[id]) {
-			_collections[id] = new Zotero.Collection(id);
-		}
-		_collections[id].load();
-	}
-	
-	
-	/**
-	* Loads collection data from DB and adds to internal cache
-	**/
-	function reloadAll() {
-		Zotero.debug('Loading all collections');
-		
-		// This should be the same as the query in Zotero.Collection.load(),
-		// just without a specific collectionID
-		var sql = "SELECT C.*, "
-			+ "(SELECT COUNT(*) FROM collections WHERE "
-			+ "parentCollectionID=C.collectionID)!=0 AS hasChildCollections, "
-			+ "(SELECT COUNT(*) FROM collectionItems WHERE "
-			+ "collectionID=C.collectionID)!=0 AS hasChildItems "
-			+ "FROM collections C";
-		var result = Zotero.DB.query(sql);
-		
-		var collectionIDs = [];
-		
-		if (result) {
-			for (var i=0; i<result.length; i++) {
-				var collectionID = result[i].collectionID;
-				collectionIDs.push(collectionID);
-				
-				// If collection doesn't exist, create new object and stuff in array
-				if (!_collections[collectionID]) {
-					_collections[collectionID] = new Zotero.Collection;
-				}
-				_collections[collectionID].loadFromRow(result[i]);
-			}
-		}
-		
-		// Remove old collections that no longer exist
-		for each(var c in _collections) {
-			if (collectionIDs.indexOf(c.id) == -1) {
-				this.unload(c.id);
-			}
-		}
-		
-		_collectionsLoaded = true;
-	}
-	
-	
 	function erase(ids) {
 		ids = Zotero.flattenArguments(ids);
 		
@@ -204,18 +143,50 @@ Zotero.Collections = new function() {
 	}
 	
 	
-	/**
-	* Clear collection from internal cache (used by Zotero.Collection.erase())
-	*
-	* Can be passed ids as individual parameters or as an array of ids, or both
-	**/
-	function unload() {
-		var ids = Zotero.flattenArguments(arguments);
+	this._load = function () {
+		if (!arguments[0] && !this._reloadCache) {
+			return;
+		}
 		
-		for(var i=0; i<ids.length; i++) {
-			delete _collections[ids[i]];
+		this._reloadCache = false;
+		
+		// This should be the same as the query in Zotero.Collection.load(),
+		// just without a specific collectionID
+		var sql = "SELECT C.*, "
+			+ "(SELECT COUNT(*) FROM collections WHERE "
+			+ "parentCollectionID=C.collectionID)!=0 AS hasChildCollections, "
+			+ "(SELECT COUNT(*) FROM collectionItems WHERE "
+			+ "collectionID=C.collectionID)!=0 AS hasChildItems "
+			+ "FROM collections C WHERE 1";
+		if (arguments[0]) {
+			sql += " AND collectionID IN (" + Zotero.join(arguments[0], ",") + ")";
+		}
+		var rows = Zotero.DB.query(sql);
+		var ids = [];
+		for each(var row in rows) {
+			var id = row.collectionID;
+			ids.push(id);
+			
+			// Creator doesn't exist -- create new object and stuff in array
+			if (!this._objectCache[id]) {
+				//this.get(id);
+				this._objectCache[id] = new Zotero.Collection;
+				this._objectCache[id].loadFromRow(row);
+			}
+			// Existing creator -- reload in place
+			else {
+				this._objectCache[id].loadFromRow(row);
+			}
+		}
+		
+		// If loading all creators, remove old creators that no longer exist
+		if (!arguments[0]) {
+			for each(var c in this._objectCache) {
+				if (ids.indexOf(c.id) == -1) {
+					this.unload(c.id);
+				}
+			}
 		}
 	}
-
 }
 
diff --git a/chrome/content/zotero/xpcom/data/creator.js b/chrome/content/zotero/xpcom/data/creator.js
index fe67771c4..0091eabc1 100644
--- a/chrome/content/zotero/xpcom/data/creator.js
+++ b/chrome/content/zotero/xpcom/data/creator.js
@@ -166,9 +166,11 @@ Zotero.Creator.prototype.save = function () {
 		Zotero.DB.query("DELETE FROM creators WHERE creatorID=?", oldID);
 		Zotero.DB.query("UPDATE creators SET key=? WHERE creatorID=?", [row.key, this.id]);
 		
-		Zotero.Creators.unload(oldID);
 		Zotero.Notifier.trigger('id-change', 'creator', oldID + '-' + this.id);
 		
+		// Do this here because otherwise updateLinkedItems() below would
+		// load a duplicate copy in the new position
+		Zotero.Creators.reload(this.id);
 		// update caches
 	}
 	
@@ -230,6 +232,7 @@ Zotero.Creator.prototype.save = function () {
 		}
 		
 		if (this.id) {
+			Zotero.debug("Updating linked items");
 			this.updateLinkedItems();
 		}
 		
@@ -390,31 +393,43 @@ Zotero.Creator.prototype.erase = function () {
 }
 
 
-// Also called from Zotero.Creators.reload()
-Zotero.Creator.prototype.load = function () {
+Zotero.Creator.prototype.load = function (allowFail) {
 	Zotero.debug("Loading data for creator " + this.id + " in Zotero.Creator.load()");
 	
 	if (!this.id) {
 		throw ("creatorID not set in Zotero.Creator.load()");
 	}
 	
-	var sql = "SELECT key, dateModified, creatorDataID, CD.* "
-			+ "FROM creators C NATURAL JOIN creatorData CD WHERE creatorID=?";
-	var data = Zotero.DB.rowQuery(sql, this.id);
+	var sql = "SELECT C.*, CD.* FROM creators C NATURAL JOIN creatorData CD "
+				+ "WHERE creatorID=?";
+	var row = Zotero.DB.rowQuery(sql, this.id);
 	
-	this._init();
-	this._loaded = true;
-	
-	if (!data) {
-		return;
+	if (!row) {
+		if (allowFail) {
+			this._loaded = true;
+			return false;
+		}
+		throw ("Creator " + this.id + " not found in Zotero.Item.load()");
 	}
 	
-	for (var key in data) {
-		this['_' + key] = data[key];
-	}
+	this.loadFromRow(row);
+	return true;
 }
 
 
+Zotero.Creator.prototype.loadFromRow = function (row) {
+	this._init();
+	
+	for (var col in row) {
+		//Zotero.debug("Setting field '" + col + "' to '" + row[col] + "' for creator " + this.id);
+		this['_' + col] = row[col] ? row[col] : '';
+	}
+	
+	this._loaded = true;
+}
+
+
+
 Zotero.Creator.prototype._checkValue = function (field, value) {
 	if (this['_' + field] === undefined) {
 		throw ("Invalid property " + field + " in Zotero.Creator._checkValue()");
diff --git a/chrome/content/zotero/xpcom/data/creators.js b/chrome/content/zotero/xpcom/data/creators.js
index f0e0e995e..58cbd8f24 100644
--- a/chrome/content/zotero/xpcom/data/creators.js
+++ b/chrome/content/zotero/xpcom/data/creators.js
@@ -25,9 +25,6 @@ Zotero.Creators = new function() {
 	Zotero.DataObjects.apply(this, ['creator']);
 	this.constructor.prototype = new Zotero.DataObjects();
 	
-	var _creatorsByID = {}; // Zotero.Creator objects indexed by creatorID
-	var _creatorDataHash = {}; // creatorDataIDs indexed by md5 hash of data
-	
 	this.get = get;
 	this.getUpdated = getUpdated;
 	this.getDataID = getDataID;
@@ -35,21 +32,19 @@ Zotero.Creators = new function() {
 	this.countCreatorsWithData = countCreatorsWithData;
 	this.updateData = updateData;
 	this.deleteData = deleteData;
-	this.reload = reload;
 	this.erase = erase;
 	this.purge = purge;
-	this.unload = unload;
 	
 	this.fields = ['firstName', 'lastName', 'fieldMode', 'birthYear'];
 	
-	var self = this;
+	var _creatorDataHash = {}; // creatorDataIDs indexed by md5 hash of data
 	
 	/*
 	 * Returns a Zotero.Creator object for a given creatorID
 	 */
 	function get(creatorID) {
-		if (_creatorsByID[creatorID]) {
-			return _creatorsByID[creatorID];
+		if (this._objectCache[creatorID]) {
+			return this._objectCache[creatorID];
 		}
 		
 		var sql = 'SELECT COUNT(*) FROM creators WHERE creatorID=?';
@@ -59,8 +54,8 @@ Zotero.Creators = new function() {
 			return false;
 		}
 		
-		_creatorsByID[creatorID] = new Zotero.Creator(creatorID);
-		return _creatorsByID[creatorID];
+		this._objectCache[creatorID] = new Zotero.Creator(creatorID);
+		return this._objectCache[creatorID];
 	}
 	
 	
@@ -173,32 +168,6 @@ Zotero.Creators = new function() {
 	}
 	
 	
-	/*
-	 * Reloads data for specified creators into internal array
-	 *
-	 * Can be passed ids as individual parameters or as an array of ids, or both
-	 */
-	function reload() {
-		if (!arguments[0]) {
-			return false;
-		}
-		
-		var ids = Zotero.flattenArguments(arguments);
-		Zotero.debug('Reloading creators ' + ids);
-		
-		for each(var id in ids) {
-			if (!_creatorsByID[id]) {
-				this.get(id);
-			}
-			else {
-				_creatorsByID[id].load();
-			}
-		}
-		
-		return true;
-	}
-	
-	
 	/**
 	 * Remove creator(s) from all linked items and call this.purge()
 	 * to delete creator rows
@@ -249,7 +218,7 @@ Zotero.Creators = new function() {
 		if (toDelete) {
 			// Clear creator entries in internal array
 			for each(var creatorID in toDelete) {
-				delete _creatorsByID[creatorID];
+				delete this._objectCache[creatorID];
 			}
 			
 			var sql = "DELETE FROM creators WHERE creatorID NOT IN "
@@ -275,20 +244,47 @@ Zotero.Creators = new function() {
 	}
 	
 	
-	/**
-	 * Clear creator from internal array
-	 *
-	 * @param	int		id		creatorID
-	 */
-	function unload(id) {
-		delete _creatorsByID[id];
-	}
-	
-	
-	this.unloadAll = function () {
-		Zotero.debug("Unloading all creators");
-		_creatorsByID = {};
-		_creatorDataHash = {};
+	this._load = function () {
+		if (!arguments[0] && !this._reloadCache) {
+			return;
+		}
+		
+		if (this._reloadCache) {
+			Zotero.debug("Clearing creator data hash");
+			_creatorDataHash = {};
+		}
+		
+		var sql = "SELECT C.*, CD.* FROM creators C NATURAL JOIN creatorData CD "
+					+ "WHERE 1";
+		if (arguments[0]) {
+			sql += " AND creatorID IN (" + Zotero.join(arguments[0], ",") + ")";
+		}
+		var rows = Zotero.DB.query(sql);
+		var ids = [];
+		for each(var row in rows) {
+			var id = row.creatorID;
+			ids.push(id);
+			
+			// Creator doesn't exist -- create new object and stuff in array
+			if (!this._objectCache[id]) {
+				this.get(id);
+			}
+			// Existing creator -- reload in place
+			else {
+				this._objectCache[id].loadFromRow(row);
+			}
+		}
+		
+		// If loading all creators, remove old creators that no longer exist
+		if (!arguments[0]) {
+			for each(var c in this._objectCache) {
+				if (ids.indexOf(c.id) == -1) {
+					this.unload(c.id);
+				}
+			}
+			
+			this._reloadCache = false;
+		}
 	}
 	
 	
@@ -349,8 +345,8 @@ Zotero.Creators = new function() {
 		
 		var creators = getCreatorsWithData(creatorDataID);
 		for each(var creatorID in creators) {
-			if (_creatorsByID[creatorID]) {
-				_creatorsByID[creatorID].load();
+			if (this._objectCache[creatorID]) {
+				this._objectCache[creatorID].load();
 			}
 		}
 	}
diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js
index 6f765e581..6c330bc84 100644
--- a/chrome/content/zotero/xpcom/data/dataObjects.js
+++ b/chrome/content/zotero/xpcom/data/dataObjects.js
@@ -14,6 +14,9 @@ Zotero.DataObjects = function (object, objectPlural, id, table) {
 	this._ZDO_id = (id ? id : object) + 'ID';
 	this._ZDO_table = table ? table : this._ZDO_objects;
 	
+	this._objectCache = {};
+	this._reloadCache = true;
+	
 	
 	/**
 	 * Retrieves an object by its secondary lookup key
@@ -30,5 +33,92 @@ Zotero.DataObjects = function (object, objectPlural, id, table) {
 		}
 		return Zotero[this._ZDO_Objects].get(id);
 	}
+	
+	
+	/*
+	 * Reloads data for specified items into internal array
+	 *
+	 * Can be passed ids as individual parameters or as an array of ids, or both
+	 */
+	this.reload = function () {
+		if (!arguments[0]) {
+			return false;
+		}
+		
+		var ids = Zotero.flattenArguments(arguments);
+		Zotero.debug('Reloading ' + this._ZDO_objects + ' ' + ids);
+		
+		// Reset cache keys to itemIDs stored in database using object keys
+		var sql = "SELECT " + this._ZDO_id + " AS id, key FROM " + this._ZDO_table
+					+ " WHERE " + this._ZDO_id + " IN ("
+					+ ids.map(function () '?').join() + ")";
+		var rows = Zotero.DB.query(sql, ids);
+		
+		var keyIDs = {};
+		for each(var row in rows) {
+			keyIDs[row.key] = row.id
+		}
+		var store = {};
+		
+		for (var id in this._objectCache) {
+			var obj = this._objectCache[id];
+			var dbID = keyIDs[obj.key];
+			if (!dbID || id == dbID) {
+				continue;
+			}
+			store[dbID] = obj;
+			delete this._objectCache[id];
+		}
+		for (var id in store) {
+			if (this._objectCache[id]) {
+				throw("Existing " + this._ZDO_object + " " + id
+					+ " exists in cache in Zotero.DataObjects.reload()");
+			}
+			this._objectCache[id] = store[id];
+		}
+		
+		// Reload data
+		this._load(ids);
+		
+		return true;
+	}
+	
+	
+	this.reloadAll = function () {
+		Zotero.debug("Reloading all " + this._ZDO_objects);
+		
+		// Reset cache keys to itemIDs stored in database using object keys
+		var sql = "SELECT " + this._ZDO_id + " AS id, key FROM " + this._ZDO_table;
+		var rows = Zotero.DB.query(sql);
+		
+		var keyIDs = {};
+		for each(var row in rows) {
+			keyIDs[row.key] = row.id;
+		}
+		
+		var store = {};
+		for each(var obj in this._objectCache) {
+			store[keyIDs[obj.key]] = obj;
+		}
+		
+		this._objectCache = store;
+		
+		// Reload data
+		this._reloadCache = true;
+		this._load();
+	}
+	
+	
+	/**
+	 * Clear object from internal array
+	 *
+	 * @param	int[]	ids		objectIDs
+	 */
+	this.unload = function () {
+		var ids = Zotero.flattenArguments(arguments);
+		for (var i=0; i<ids.length; i++) {
+			delete _objectCache[ids[i]];
+		}
+	}
 }
 
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 5556dede6..a4e41a4a7 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -302,7 +302,6 @@ Zotero.Item.prototype.loadPrimaryData = function(allowFail) {
 	}
 	
 	this.loadFromRow(row);
-	
 	return true;
 }
 
@@ -1038,7 +1037,6 @@ Zotero.Item.prototype.save = function() {
 		Zotero.DB.query("DELETE FROM items WHERE itemID=?", oldID);
 		Zotero.DB.query("UPDATE items SET key=? WHERE itemID=?", [row.key, this.id]);
 		
-		Zotero.Items.unload(oldID);
 		Zotero.Notifier.trigger('id-change', 'item', oldID + '-' + this.id);
 		
 		// update caches
@@ -2792,7 +2790,7 @@ Zotero.Item.prototype.getTags = function() {
 	
 	var tagObjs = [];
 	for (var i=0; i<tags.length; i++) {
-		var tag = Zotero.Tags.get(tags[i].tagID, true);
+		var tag = Zotero.Tags.get(tags[i].tagID);
 		tagObjs.push(tag);
 	}
 	return tagObjs;
diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js
index 08e760851..363073d54 100644
--- a/chrome/content/zotero/xpcom/data/items.js
+++ b/chrome/content/zotero/xpcom/data/items.js
@@ -34,18 +34,13 @@ Zotero.Items = new function() {
 	this.getAll = getAll;
 	this.getUpdated = getUpdated;
 	this.add = add;
-	this.reload = reload;
-	this.reloadAll = reloadAll;
 	this.cacheFields = cacheFields;
 	this.erase = erase;
 	this.purge = purge;
-	this.unload = unload;
 	this.getFirstCreatorSQL = getFirstCreatorSQL;
 	this.getSortTitle = getSortTitle;
 	
 	// Private members
-	var _items = [];
-	var _itemsLoaded = false;
 	var _cachedFields = [];
 	var _firstCreatorSQL = '';
 	
@@ -71,33 +66,33 @@ Zotero.Items = new function() {
 		
 		for (var i=0; i<ids.length; i++) {
 			// Check if already loaded
-			if (!_items[ids[i]]) {
+			if (!this._objectCache[ids[i]]) {
 				toLoad.push(ids[i]);
 			}
 		}
 		
 		// New items to load
 		if (toLoad.length) {
-			_load(toLoad);
+			this._load(toLoad);
 		}
 		
 		// If single id, return the object directly
 		if (arguments[0] && typeof arguments[0]!='object'
 				&& typeof arguments[1]=='undefined') {
-			if (!_items[arguments[0]]) {
+			if (!this._objectCache[arguments[0]]) {
 				Zotero.debug("Item " + arguments[0] + " doesn't exist", 2);
 				return false;
 			}
-			return _items[arguments[0]];
+			return this._objectCache[arguments[0]];
 		}
 		
 		// Otherwise, build return array
 		for (i=0; i<ids.length; i++) {
-			if (!_items[ids[i]]) {
+			if (!this._objectCache[ids[i]]) {
 				Zotero.debug("Item " + ids[i] + " doesn't exist", 2);
 				continue;
 			}
-			loaded.push(_items[ids[i]]);
+			loaded.push(this._objectCache[ids[i]]);
 		}
 		
 		return loaded;
@@ -210,36 +205,19 @@ Zotero.Items = new function() {
 	}
 	
 	
-	/*
-	 * Reloads data for specified items into internal array
-	 *
-	 * Can be passed ids as individual parameters or as an array of ids, or both
-	 */
-	function reload() {
-		if (!arguments[0]) {
-			return false;
+	function cacheFields(fields, items) {
+		if (items && items.length == 0) {
+			return;
 		}
 		
-		var ids = Zotero.flattenArguments(arguments);
-		Zotero.debug('Reloading ' + ids);
-		_load(ids);
-		
-		return true;
-	}
-	
-	
-	function reloadAll() {
-		Zotero.debug("Loading all items");
-		_items = [];
-		_itemsLoaded = false;
-		_load();
-	}
-	
-	
-	function cacheFields(fields, items) {
 		Zotero.debug("Caching fields [" + fields.join() + "]"
-			+ (items ? " for " + items + " items" : ''));
-		_load(items);
+			+ (items ? " for " + items.length + " items" : ''));
+		if (items && items.length > 0) {
+			this._load(items);
+		}
+		else {
+			this._load();
+		}
 		
 		var primaryFields = [];
 		var fieldIDs = [];
@@ -270,7 +248,7 @@ Zotero.Items = new function() {
 			var rows = Zotero.DB.query(sql);
 			for each(var row in rows) {
 				//Zotero.debug('Calling loadFromRow for item ' + row.itemID);
-				_items[row.itemID].loadFromRow(row);
+				this._objectCache[row.itemID].loadFromRow(row);
 			}
 		}
 		
@@ -292,8 +270,8 @@ Zotero.Items = new function() {
 		var itemDataRows = Zotero.DB.query(sql);
 		for each(var row in itemDataRows) {
 			//Zotero.debug('Setting field ' + row.fieldID + ' for item ' + row.itemID);
-			if (_items[row.itemID]) {
-				_items[row.itemID].setField(row.fieldID, row.value, true);
+			if (this._objectCache[row.itemID]) {
+				this._objectCache[row.itemID].setField(row.fieldID, row.value, true);
 			}
 			else {
 				if (!missingItems) {
@@ -323,8 +301,8 @@ Zotero.Items = new function() {
 			
 			for each(var row in rows) {
 				//Zotero.debug('Setting title for note ' + row.itemID);
-				if (_items[row.itemID]) {
-					_items[row.itemID].setField(titleFieldID, row['title'], true);
+				if (this._objectCache[row.itemID]) {
+					this._objectCache[row.itemID].setField(titleFieldID, row.title, true);
 				}
 				else {
 					if (!missingItems) {
@@ -341,10 +319,10 @@ Zotero.Items = new function() {
 		// Set nonexistent fields in the cache list to false (instead of null)
 		for each(var itemID in allItemIDs) {
 			for each(var fieldID in fieldIDs) {
-				if (Zotero.ItemFields.isValidForType(fieldID, _items[itemID].itemTypeID)) {
+				if (Zotero.ItemFields.isValidForType(fieldID, this._objectCache[itemID].itemTypeID)) {
 					if (!itemFieldsCached[itemID] || !itemFieldsCached[itemID][fieldID]) {
 						//Zotero.debug('Setting field ' + fieldID + ' to false for item ' + itemID);
-						_items[itemID].setField(fieldID, false, true);
+						this._objectCache[itemID].setField(fieldID, false, true);
 					}
 				}
 			}
@@ -409,14 +387,6 @@ Zotero.Items = new function() {
 	}
 	
 	
-	/**
-	* Clear item from internal array (used by Zotero.Item.erase())
-	**/
-	function unload(id) {
-		delete _items[id];
-	}
-	
-	
 	/*
 	 * Generate SQL to retrieve firstCreator field
 	 *
@@ -541,8 +511,8 @@ Zotero.Items = new function() {
 	}
 	
 	
-	function _load() {
-		if (!arguments[0] && _itemsLoaded) {
+	this._load = function () {
+		if (!arguments[0] && !this._reloadCache) {
 			return;
 		}
 		
@@ -552,9 +522,8 @@ Zotero.Items = new function() {
 			+ "(SELECT COUNT(*) FROM itemNotes WHERE sourceItemID=I.itemID) AS numNotes, "
 			+ "(SELECT COUNT(*) FROM itemAttachments WHERE sourceItemID=I.itemID) AS numAttachments "
 			+ 'FROM items I WHERE 1';
-		
 		if (arguments[0]) {
-			sql += ' AND I.itemID IN (' + Zotero.join(arguments,',') + ')';
+			sql += ' AND I.itemID IN (' + Zotero.join(arguments[0], ',') + ')';
 		}
 		var itemsRows = Zotero.DB.query(sql);
 		var itemIDs = [];
@@ -564,30 +533,28 @@ Zotero.Items = new function() {
 			itemIDs.push(itemID);
 			
 			// Item doesn't exist -- create new object and stuff in array
-			if (!_items[row.itemID]) {
+			if (!this._objectCache[row.itemID]) {
 				var item = new Zotero.Item();
 				item.loadFromRow(row, true);
-				_items[row.itemID] = item;
+				this._objectCache[row.itemID] = item;
 			}
 			// Existing item -- reload in place
 			else {
-				_items[row.itemID].loadFromRow(row, true);
+				this._objectCache[row.itemID].loadFromRow(row, true);
 			}
 		}
 		
 		// If loading all items, remove old items that no longer exist
 		if (!arguments[0]) {
-			for each(var c in _items) {
+			for each(var c in this._objectCache) {
 				if (itemIDs.indexOf(c.id) == -1) {
 					this.unload(c.id);
 				}
 			}
-		}
-		
-		if (!arguments[0]) {
-			_itemsLoaded = true;
+			
 			_cachedFields = ['itemID', 'itemTypeID', 'dateAdded', 'dateModified',
 				'firstCreator', 'numNotes', 'numAttachments', 'numChildren'];
+			this._reloadCache = false;
 		}
 	}
 }
diff --git a/chrome/content/zotero/xpcom/data/tag.js b/chrome/content/zotero/xpcom/data/tag.js
index 46dd86f5d..8a946443e 100644
--- a/chrome/content/zotero/xpcom/data/tag.js
+++ b/chrome/content/zotero/xpcom/data/tag.js
@@ -118,21 +118,26 @@ Zotero.Tag.prototype.load = function() {
 	}
 	
 	var sql = "SELECT name, type, dateModified, key FROM tags WHERE tagID=?";
-	var data = Zotero.DB.rowQuery(sql, this.id);
+	var row = Zotero.DB.rowQuery(sql, this.id);
 	
-	this._init();
-	this._loaded = true;
-	
-	if (!data) {
-		return;
-	}
-	
-	for (var key in data) {
-		this['_' + key] = data[key];
-	}
+	this.loadFromRow(row);
+	return true;
 }
 
 
+Zotero.Tag.prototype.loadFromRow = function (row) {
+	this._init();
+	
+	for (var col in row) {
+		//Zotero.debug("Setting field '" + col + "' to '" + row[col] + "' for tag " + this.id);
+		this['_' + col] = row[col] ? row[col] : '';
+	}
+	
+	this._loaded = true;
+}
+
+
+
 /**
  * Returns items linked to this tag
  *
@@ -249,7 +254,6 @@ Zotero.Tag.prototype.save = function () {
 		
 		Zotero.DB.query("UPDATE tags SET key=? WHERE tagID=?", [row.key, this.id]);
 		
-		Zotero.Tags.unload(oldID);
 		Zotero.Notifier.trigger('id-change', 'tag', oldID + '-' + this.id);
 		
 		// update caches
diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js
index c9057a667..adb21fe6f 100644
--- a/chrome/content/zotero/xpcom/data/tags.js
+++ b/chrome/content/zotero/xpcom/data/tags.js
@@ -29,7 +29,6 @@ Zotero.Tags = new function() {
 	this.constructor.prototype = new Zotero.DataObjects();
 	
 	var _tags = {}; // indexed by tag text
-	var _tagsByID = {}; // indexed by tagID
 	
 	this.get = get;
 	this.getName = getName;
@@ -42,31 +41,18 @@ Zotero.Tags = new function() {
 	this.getTagItems = getTagItems;
 	this.search = search;
 	this.rename = rename;
-	this.reload = reload;
 	this.erase = erase;
 	this.purge = purge;
-	this.unload = unload;
 	
 	
 	/*
 	 * Returns a tag and type for a given tagID
 	 */
-	function get(tagID, skipCheck) {
-		if (_tagsByID[tagID]) {
-			return _tagsByID[tagID];
+	function get(id, skipCheck) {
+		if (this._reloadCache) {
+			this.reloadAll();
 		}
-		
-		if (!skipCheck) {
-			var sql = 'SELECT COUNT(*) FROM tags WHERE tagID=?';
-			var result = Zotero.DB.valueQuery(sql, tagID);
-			
-			if (!result) {
-				return false;
-			}
-		}
-		
-		_tagsByID[tagID] = new Zotero.Tag(tagID);
-		return _tagsByID[tagID];
+		return this._objectCache[id] ? this._objectCache[id] : false;
 	}
 	
 	
@@ -326,11 +312,6 @@ Zotero.Tags = new function() {
 	}
 	
 	
-	function reload(ids) {
-		this.unload(ids);
-	}
-	
-	
 	function erase(ids) {
 		ids = Zotero.flattenArguments(ids);
 		
@@ -415,12 +396,12 @@ Zotero.Tags = new function() {
 	 *
 	 * @param	int|array	ids	 	One or more tagIDs
 	 */
-	function unload() {
+	this.unload = function () {
 		var ids = Zotero.flattenArguments(arguments);
 		
 		for each(var id in ids) {
-			var tag = _tagsByID[id];
-			delete _tagsByID[id];
+			var tag = this._objectCache[id];
+			delete this._objectCache[id];
 			if (tag && _tags[tag.type]) {
 				delete _tags[tag.type]['_' + tag.name];
 			}
@@ -428,9 +409,49 @@ Zotero.Tags = new function() {
 	}
 	
 	
-	this.unloadAll = function (ids) {
-		_tags = {};
-		_tagsByID = {};
+	this._load = function () {
+		if (!arguments[0] && !this._reloadCache) {
+			return;
+		}
+		
+		if (this._reloadCache) {
+			_tags = {};
+		}
+		
+		this._reloadCache = false;
+		
+		// This should be the same as the query in Zotero.Tag.load(),
+		// just without a specific tagID
+		var sql = "SELECT * FROM tags WHERE 1";
+		if (arguments[0]) {
+			sql += " AND tagID IN (" + Zotero.join(arguments[0], ",") + ")";
+		}
+		var rows = Zotero.DB.query(sql);
+		var ids = [];
+		for each(var row in rows) {
+			var id = row.tagID;
+			ids.push(id);
+			
+			// Tag doesn't exist -- create new object and stuff in array
+			if (!this._objectCache[id]) {
+				//this.get(id);
+				this._objectCache[id] = new Zotero.Tag;
+				this._objectCache[id].loadFromRow(row);
+			}
+			// Existing tag -- reload in place
+			else {
+				this._objectCache[id].loadFromRow(row);
+			}
+		}
+		
+		if (!arguments[0]) {
+			// If loading all tags, remove old tags that no longer exist
+			for each(var c in this._objectCache) {
+				if (ids.indexOf(c.id) == -1) {
+					this.unload(c.id);
+				}
+			}
+		}
 	}
 }
 
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
index 33a4553e9..36a2e9119 100644
--- a/chrome/content/zotero/xpcom/zotero.js
+++ b/chrome/content/zotero/xpcom/zotero.js
@@ -934,9 +934,9 @@ var Zotero = new function(){
 	
 	
 	function reloadDataObjects() {
-		Zotero.Tags.unloadAll();
+		Zotero.Tags.reloadAll();
 		Zotero.Collections.reloadAll();
-		Zotero.Creators.unloadAll();
+		Zotero.Creators.reloadAll();
 		Zotero.Items.reloadAll();
 	}
 };