From 6db417e63e27da969c82bbedf523a9237fb68ccb Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Tue, 12 Aug 2008 07:10:50 +0000
Subject: [PATCH] Improved subcollection support -- fixes "Cannot set parent of
 collection [x] to invalid parent [x]" error, among other things

Removed child collections from XML -- now uses parent attribute exclusively -- and increased API version to 2
---
 .../zotero/xpcom/collectionTreeView.js        |  23 ++--
 .../content/zotero/xpcom/data/collection.js   |  91 +++++++++++----
 chrome/content/zotero/xpcom/sync.js           | 105 ++++++++++++++----
 3 files changed, 163 insertions(+), 56 deletions(-)

diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js
index cfc611d164..9cc25a6951 100644
--- a/chrome/content/zotero/xpcom/collectionTreeView.js
+++ b/chrome/content/zotero/xpcom/collectionTreeView.js
@@ -169,16 +169,7 @@ Zotero.CollectionTreeView.prototype.notify = function(action, type, ids)
 	
 	var madeChanges = false;
 	
-	if (action == 'refresh') {
-		switch (type) {
-			case 'share':
-				this.reload();
-				this.rememberSelection(savedSelection);
-				break;
-		}
-	}
-	
-	else if(action == 'delete') {
+	if (action == 'delete') {
 		//Since a delete involves shifting of rows, we have to do it in order
 		
 		//sort the ids by row
@@ -222,8 +213,9 @@ Zotero.CollectionTreeView.prototype.notify = function(action, type, ids)
 		for (var i=0; i<ids.length; i++) {
 			// Open the parent collection if closed
 			var collection = Zotero.Collections.get(ids[i]);
-			var parentID = collection.getParent();
-			if (parentID && !this.isContainerOpen(this._collectionRowMap[parentID])) {
+			var parentID = collection.parent;
+			if (parentID && this._collectionRowMap[parentID] &&
+					!this.isContainerOpen(this._collectionRowMap[parentID])) {
 				this.toggleOpenState(this._collectionRowMap[parentID]);
 			}
 		}
@@ -231,8 +223,7 @@ Zotero.CollectionTreeView.prototype.notify = function(action, type, ids)
 		this.reload();
 		this.rememberSelection(savedSelection);
 	}
-	else if(action == 'modify')
-	{
+	else if (action == 'modify' || action == 'refresh') {
 		this.reload();
 		this.rememberSelection(savedSelection);
 	}
@@ -550,6 +541,10 @@ Zotero.CollectionTreeView.prototype.saveSelection = function()
  */
 Zotero.CollectionTreeView.prototype.rememberSelection = function(selection)
 {
+	if (!selection) {
+		return;
+	}
+	
 	var id = selection.substr(1);
 	switch (selection.substr(0, 1)) {
 		// Library
diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js
index c79b67926e..8a182733c7 100644
--- a/chrome/content/zotero/xpcom/data/collection.js
+++ b/chrome/content/zotero/xpcom/data/collection.js
@@ -37,7 +37,7 @@ Zotero.Collection.prototype._init = function () {
 	this._changed = false;
 	this._previousData = false;
 	
-	this._hasChildCollections = false;
+	this._hasChildCollections;
 	this._childCollections = [];
 	this._childCollectionsLoaded = false;
 	
@@ -59,7 +59,7 @@ Zotero.Collection.prototype.__defineSetter__('dateModified', function (val) { th
 Zotero.Collection.prototype.__defineGetter__('key', function () { return this._get('key'); });
 Zotero.Collection.prototype.__defineSetter__('key', function (val) { this._set('key', val); });
 
-Zotero.Collection.prototype.__defineSetter__('childCollections', function (arr) { this._setChildCollections(arr); });
+//Zotero.Collection.prototype.__defineSetter__('childCollections', function (arr) { this._setChildCollections(arr); });
 Zotero.Collection.prototype.__defineSetter__('childItems', function (arr) { this._setChildItems(arr); });
 
 
@@ -150,8 +150,8 @@ Zotero.Collection.prototype.loadFromRow = function(row) {
 	this._parent = row.parentCollectionID;
 	this._dateModified = row.dateModified;
 	this._key = row.key;
-	this._hasChildCollections = row.hasChildCollections;
-	this._hasChildItems = row.hasChildItems;
+	this._hasChildCollections = !!row.hasChildCollections;
+	this._hasChildItems = !!row.hasChildItems;
 	this._loadChildItems();
 }
 
@@ -161,13 +161,29 @@ Zotero.Collection.prototype.isEmpty = function() {
 }
 
 Zotero.Collection.prototype.hasChildCollections = function() {
-	return !!(parseInt(this._hasChildCollections));
+	if (!this.id) {
+		throw ("Zotero.Collection.hasChildCollections cannot be called "
+			+ "on an unsaved collection");
+	}
+	
+	if (this._hasChildCollections == undefined) {
+		var sql = "SELECT COUNT(*) FROM collections WHERE "
+			+ "parentCollectionID=?";
+		this._hasChildCollections = !!Zotero.DB.valueQuery(sql, this.id);
+	}
+	
+	return this._hasChildCollections;
 }
 
 Zotero.Collection.prototype.hasChildItems = function() {
 	return !!(parseInt(this._hasChildItems));
 }
 
+Zotero.Collection.prototype.refreshChildCollections = function () {
+	this._hasChildCollections = undefined;
+	this._childCollectionsLoaded = false;
+}
+
 
 /**
  * Check if collection exists in the database
@@ -272,7 +288,7 @@ Zotero.Collection.prototype.save = function () {
 			throw ('Cannot move collection into itself!');
 		}
 		
-		if (this.hasDescendent('collection', this.parent)) {
+		if (this.id && this.hasDescendent('collection', this.parent)) {
 			throw ('Cannot move collection into one of its own descendents!', 2);
 		}
 	}
@@ -339,6 +355,21 @@ Zotero.Collection.prototype.save = function () {
 			collectionID = insertID;
 		}
 		
+		
+		if (this._changed.parent) {
+			var parentIDs = [];
+			if (this._previousData.parent) {
+				parentIDs.push(this._previousData.parent);
+			}
+			if (this.parent) {
+				parentIDs.push(this.parent);
+			}
+			
+			Zotero.Notifier.trigger('move', 'collection', this.id);
+		}
+		
+		
+		/*
 		// Subcollections
 		if (this._changed.childCollections) {
 			var removed = [];
@@ -381,6 +412,7 @@ Zotero.Collection.prototype.save = function () {
 			
 			// TODO: notifier
 		}
+		*/
 		
 		// Child items
 		if (this._changed.childItems) {
@@ -469,7 +501,7 @@ Zotero.Collection.prototype.save = function () {
 		this._key = key;
 	}
 	
-	Zotero.Collections.reloadAll();
+	Zotero.Collections.reload(this.id);
 	
 	if (isNew) {
 		Zotero.Notifier.trigger('add', 'collection', this.id);
@@ -478,15 +510,12 @@ Zotero.Collection.prototype.save = function () {
 		Zotero.Notifier.trigger('modify', 'collection', this.id, this._previousData);
 	}
 	
-	if (this._changed.parent) {
-		var notifyIDs = [this.id];
-		if (this._previousData.parent) {
-			notifyIDs.push(this._previousData.parent);
+	// Refresh child collection counts
+	if (parentIDs) {
+		for each(var id in parentIDs) {
+			var col = Zotero.Collections.get(id);
+			col.refreshChildCollections();
 		}
-		if (this.parent) {
-			notifyIDs.push(this.parent);
-		}
-		//Zotero.Notifier.trigger('move', 'collection', notifyIDs, notifierData);
 	}
 	
 	return this.id;
@@ -701,7 +730,7 @@ Zotero.Collection.prototype.serialize = function(nested) {
 		parent: this.parent,
 		childCollections: this.getChildCollections(true),
 		childItems: this.getChildItems(true),
-		descendents: this.getDescendents(nested)
+		descendents: this.id ? this.getDescendents(nested) : []
 	};
 	return obj;
 }
@@ -709,15 +738,20 @@ Zotero.Collection.prototype.serialize = function(nested) {
 
 /**
  * Returns an array of descendent collections and items
- *	(rows of 'id', 'type' ('item' or 'collection'), 'parent', and,
- * 	if collection, 'name' and the nesting 'level')
  *
  * @param	bool		recursive	Descend into subcollections
  * @param	bool		nested		Return multidimensional array with 'children'
  *									nodes instead of flat array
  * @param	string	type			'item', 'collection', or FALSE for both
+ * @return	{Object[]}			Array of objects with 'id',
+ *								'type' ('item' or 'collection'), 'parent',
+ *								and, if collection, 'name' and the nesting 'level'
  */
 Zotero.Collection.prototype.getChildren = function(recursive, nested, type, level) {
+	if (!this.id) {
+		throw ('Zotero.Collection.getChildren() cannot be called on an unsaved item');
+	}
+	
 	var toReturn = [];
 	
 	if (!level) {
@@ -813,9 +847,11 @@ Zotero.Collection.prototype._prepFieldChange = function (field) {
 }
 
 
+/*
 Zotero.Collection.prototype._setChildCollections = function (collectionIDs) {
 	this._setChildren('collection', collectionIDs);
 }
+*/
 
 
 Zotero.Collection.prototype._setChildItems = function (itemIDs) {
@@ -889,15 +925,18 @@ Zotero.Collection.prototype._setChildren = function (type, ids) {
 		this._childItems = Zotero.Items.get(newIDs);
 	}
 	else {
-		for (var id in newIDs) {
-			this['_child' + Types].push(Zotero[Types].get(id));
+		for each(var id in newIDs) {
+			var obj = Zotero[Types].get(id);
+			if (!obj) {
+				throw (type + ' ' + id + ' not found in Zotero.Collection._setChildren()');
+			}
+			this['_child' + Types].push(obj);
 		}
 	}
 	
 	return true;
 }
 
-
 Zotero.Collection.prototype._loadChildCollections = function () {
 	var sql = "SELECT collectionID FROM collections WHERE parentCollectionID=?";
 	var ids = Zotero.DB.columnQuery(sql, this.id);
@@ -906,8 +945,16 @@ Zotero.Collection.prototype._loadChildCollections = function () {
 	
 	if (ids) {
 		for each(var id in ids) {
-			this._childCollections.push(Zotero.Collections.get(id));
+			var col = Zotero.Collections.get(id);
+			if (!col) {
+				throw ('Collection ' + id + ' not found in Zotero.Collection._loadChildCollections()');
+			}
+			this._childCollections.push(col);
 		}
+		this._hasChildCollections = true;
+	}
+	else {
+		this._hasChildCollections = false;
 	}
 	
 	this._childCollectionsLoaded = true;
diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js
index 69e36cabb3..2ac12b8d08 100644
--- a/chrome/content/zotero/xpcom/sync.js
+++ b/chrome/content/zotero/xpcom/sync.js
@@ -480,7 +480,7 @@ Zotero.Sync.Server = new function () {
 	});
 	
 	this.nextLocalSyncDate = false;
-	this.apiVersion = 1;
+	this.apiVersion = 2;
 	
 	default xml namespace = '';
 	
@@ -1169,6 +1169,7 @@ Zotero.BufferedInputListener.prototype = {
 }
 
 
+// TODO: use prototype
 Zotero.Sync.Server.EventListener = {
 	init: function () {
 		Zotero.Notifier.registerObserver(this);
@@ -1207,12 +1208,82 @@ Zotero.Sync.Server.Data = new function() {
 	
 	default xml namespace = '';
 	
+	
+	/**
+	 * Reorder XML nodes for parent/child relationships, etc.
+	 *
+	 * @param {E4X}	xml
+	 */
+	function _preprocessUpdatedXML(xml) {
+		if (xml.collections.length()) {
+			var collections = xml.collections.children();
+			var orderedCollections = <collections/>;
+			var collectionIDHash = {};
+			
+			for (var i=0; i<collections.length(); i++) {
+				// Build a hash of all collection ids
+				collectionIDHash[collections[i].@id.toString()] = true;
+				
+				// Pull out top-level collections
+				if (!collections[i].@parent.toString()) {
+					orderedCollections.collection += collections[i];
+					delete collections[i];
+					i--;
+				}
+			}
+			
+			// Pull out all collections pointing to parents that
+			// aren't present, which we assume already exist
+			for (var i=0; i<collections.length(); i++) {
+				if (!collectionIDHash[collections[i].@parent]) {
+					orderedCollections.collection += collections[i]
+					delete collections[i];
+					i--;
+				}
+			}
+			
+			// Insert children directly under parents
+			for (var i=0; i<orderedCollections.children().length(); i++) {
+				for (var j=0; j<collections.length(); j++) {
+					if (collections[j].@parent.toString() ==
+							orderedCollections.children()[i].@id.toString()) {
+						// Make a clone of object, since otherwise
+						// delete below erases inserted item as well
+						// (which only seems to happen with
+						// insertChildBefore(), not += above)
+						var newChild = new XML(collections[j].toXMLString())
+						
+						// If last top-level, just append
+						if (i == orderedCollections.children().length() - 1) {
+							orderedCollections.appendChild(newChild);
+						}
+						else {
+							orderedCollections.insertChildBefore(
+								orderedCollections.children()[i+1],
+								newChild
+							);
+						}
+						delete collections[j];
+						j--;
+					}
+				}
+			}
+			
+			xml.collections = orderedCollections;
+		}
+		
+		return xml;
+	}
+	
+	
 	function processUpdatedXML(xml, lastLocalSyncDate, uploadIDs) {
 		if (xml.children().length() == 0) {
 			Zotero.debug('No changes received from server');
 			return Zotero.Sync.Server.Data.buildUploadXML(uploadIDs);
 		}
 		
+		xml = _preprocessUpdatedXML(xml);
+		
 		var remoteCreatorStore = {};
 		var relatedItemsStore = {};
 		
@@ -1564,27 +1635,13 @@ Zotero.Sync.Server.Data = new function() {
 				}
 			}
 			
+			/*
 			if (type == 'collection') {
-				// Sort collections in order of parent collections,
-				// so referenced parent collections always exist when saving
-				var cmp = function (a, b) {
-					var pA = a.parent;
-					var pB = b.parent;
-					if (pA == pB) {
-						return 0;
-					}
-					return (pA < pB) ? -1 : 1;
-				};
-				toSaveParents.sort(cmp);
-				
 				// Temporarily remove and store subcollections before saving
 				// since referenced collections may not exist yet
 				var collections = [];
 				for each(var obj in toSaveParents) {
 					var colIDs = obj.getChildCollections(true);
-					if (!colIDs.length) {
-						continue;
-					}
 					// TODO: use exist(), like related items above
 					obj.childCollections = [];
 					collections.push({
@@ -1593,6 +1650,7 @@ Zotero.Sync.Server.Data = new function() {
 					});
 				}
 			}
+			*/
 			
 			// Save objects
 			Zotero.debug('Saving merged ' + types);
@@ -1613,15 +1671,17 @@ Zotero.Sync.Server.Data = new function() {
 					item.save();
 				}
 			}
+			/*
 			// Add back subcollections
 			else if (type == 'collection') {
 				for each(var collection in collections) {
-					if (collection.collections) {
-						collection.obj.childCollections = collection.collections;
+					if (collection.childCollections) {
+						collection.obj.childCollections = collection.childCollections;
 						collection.obj.save();
 					}
 				}
 			}
+			*/
 			
 			
 			// Delete
@@ -1961,21 +2021,24 @@ Zotero.Sync.Server.Data = new function() {
 		
 		var children = collection.getChildren();
 		if (children) {
-			xml.collections = '';
+			//xml.collections = '';
 			xml.items = '';
 			for each(var child in children) {
+				/*
 				if (child.type == 'collection') {
 					xml.collections = xml.collections ?
 						xml.collections + ' ' + child.id : child.id;
 				}
-				else if (child.type == 'item') {
+				else */if (child.type == 'item') {
 					xml.items = xml.items ?
 						xml.items + ' ' + child.id : child.id;
 				}
 			}
+			/*
 			if (xml.collections == '') {
 				delete xml.collections;
 			}
+			*/
 			if (xml.items == '') {
 				delete xml.items;
 			}
@@ -2021,9 +2084,11 @@ Zotero.Sync.Server.Data = new function() {
 			collection.key = xmlCollection.@key.toString();
 		}
 		
+		/*
 		// Subcollections
 		var str = xmlCollection.collections.toString();
 		collection.childCollections = str == '' ? [] : str.split(' ');
+		*/
 		
 		// Child items
 		var str = xmlCollection.items.toString();