From 013af36855fd9e412f8f6d2f296af348eaccc8fd Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 2 Jun 2006 19:48:22 +0000
Subject: [PATCH] Fixed some issues with object reloading -- because of the way
 JS closures work, simply overwriting the old object doesn't work, since the
 interface code retains a reference to the old object -- in other words, a
 memory leak and stale data. Existing item objects now reload in place
 instead. Same change made to collections, though I'll have to test that after
 I add Collection editing and save().

This behavior of JS has some implications for how interface code handles things like deletes. The following won't do what one might expect/desire (and my apologies if this is obvious or redundant, but I figure it's worth pointing out):

var item = Scholar.Items.get(1);
Scholar.debug(item);
Scholar.DB.query("DELETE FROM items WHERE itemID=1");
Scholar.Items.reloadAll();
Scholar.debug(item);

The last line will still display the old object, even though _items[1] has been deleted inside Scholar.Items. The following does work, however:

var item = Scholar.Items.get(1);
Scholar.debug(item);
Scholar.DB.query("DELETE FROM items WHERE itemID=1");
Scholar.Items.reloadAll();
var item = Scholar.Items.get(1);
Scholar.debug(item);

Now item is properly undefined. Moral of the story: object references need to be deleted manually after receiving delete notifications, for if external code still has references to deleted data objects, the data layer can't do much about it.
---
 .../content/scholar/xpcom/data_access.js      | 53 ++++++++++++-------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/chrome/chromeFiles/content/scholar/xpcom/data_access.js b/chrome/chromeFiles/content/scholar/xpcom/data_access.js
index 4b120330f7..160866c9bc 100644
--- a/chrome/chromeFiles/content/scholar/xpcom/data_access.js
+++ b/chrome/chromeFiles/content/scholar/xpcom/data_access.js
@@ -857,15 +857,11 @@ Scholar.Items = new function(){
 	
 	
 	/*
-	 * Reloads all currently cached items
+	 * Reloads all items
 	 */
 	function reloadAll(){
-		var ids = new Array();
-		for (itemID in _items){
-			ids.push(itemID);
-		}
-		_load(ids);
-		return true;
+		_items = new Array();
+		_load();
 	}
 	
 	
@@ -908,10 +904,6 @@ Scholar.Items = new function(){
 	
 	
 	function _load(){
-		if (!arguments){
-			return false;
-		}
-		
 		// Should be the same as query in Scholar.Item.loadFromID, just
 		// without itemID clause
 		var sql = 'SELECT I.*, lastName AS firstCreator '
@@ -920,7 +912,7 @@ Scholar.Items = new function(){
 			+ 'LEFT JOIN creators C ON (IC.creatorID=C.creatorID) '
 			+ 'WHERE (IC.orderIndex=0 OR IC.orderIndex IS NULL)';
 		
-		if (arguments[0]!='all'){
+		if (arguments[0]){
 			sql += ' AND I.itemID IN (' + Scholar.join(arguments,',') + ')';
 		}
 		
@@ -928,9 +920,16 @@ Scholar.Items = new function(){
 		
 		if (result){
 			for (var i=0,len=result.length; i<len; i++){
-				var obj = new Scholar.Item();
-				obj.loadFromRow(result[i]);
-				_items[result[i]['itemID']] = obj;
+				// Item doesn't exist -- create new object and stuff in array
+				if (!_items[result[i]['itemID']]){
+					var obj = new Scholar.Item();
+					obj.loadFromRow(result[i]);
+					_items[result[i]['itemID']] = obj;
+				}
+				// Existing item -- reload in place
+				else {
+					_items[result[i]['itemID']].loadFromRow(result[i]);
+				}
 			}
 		}
 		return true;
@@ -947,6 +946,13 @@ Scholar.Items = new function(){
  * Generally should be called from Scholar.Collection rather than directly
  */
 Scholar.Collection = function(){
+	 this._init();
+}
+
+Scholar.Collection.prototype._init = function(){
+	//
+	// Public members for access by public methods -- do not access directly
+	//
 	this._id;
 	this._name;
 	this._parent;
@@ -956,7 +962,6 @@ Scholar.Collection = function(){
 	this._childItemsLoaded;
 }
 
-
 /*
  * Build collection from database
  */
@@ -979,6 +984,7 @@ Scholar.Collection.prototype.loadFromID = function(id){
  * Populate collection data from a database row
  */
 Scholar.Collection.prototype.loadFromRow = function(row){
+	this._init();
 	this._id = row['collectionID'];
 	this._name = row['collectionName'];
 	this._parent = row['parentCollectionID'];
@@ -1248,9 +1254,18 @@ Scholar.Collections = new function(){
 		}
 		
 		for (var i=0; i<result.length; i++){
-			var collection = new Scholar.Collection();
-			collection.loadFromRow(result[i]);
-			_collections[collection.getID()] = collection;
+			var collectionID = result[i]['collectionID'];
+			
+			// If collection doesn't exist, create new object and stuff in array
+			if (!_collections[collectionID]){
+				var collection = new Scholar.Collection();
+				collection.loadFromRow(result[i]);
+				_collections[collectionID] = collection;
+			}
+			// If existing collection, reload in place
+			else {
+				_collections[collectionID].loadFromRow(result[i]);
+			}
 		}
 		_collectionsLoaded = true;
 	}