- Fix some creator-related sync problems

- Don't flip out if server decides to include the default libraryID in an object node

chrome/content/zotero/xpcom/data/creator.js

@@ -249,11 +249,23 @@ Zotero.Creator.prototype.save = function () {
 			key
 		];
 		
-		var sql = "REPLACE INTO creators (" + columns.join(', ') + ") VALUES ("
+		if (isNew) {
-			+ placeholders.join(', ') + ")";
+			var sql = "INSERT INTO creators (" + columns.join(', ') + ") VALUES ("
-		var insertID = Zotero.DB.query(sql, sqlValues);
+						+ placeholders.join(', ') + ")";
-		if (!creatorID) {
+			var insertID = Zotero.DB.query(sql, sqlValues);
-			creatorID = insertID;
+			if (!creatorID) {
+				creatorID = insertID;
+			}
+		}
+		else {
+			// Remove tagID from beginning
+			columns.shift();
+			sqlValues.shift();
+			sqlValues.push(creatorID);
+			
+			var sql = "UPDATE creators SET " + columns.join("=?, ") + "=?"
+				+ " WHERE creatorID=?";
+			Zotero.DB.query(sql, sqlValues);
 		}
 		
 		if (deleteDataID) {

chrome/content/zotero/xpcom/sync.js

@@ -1329,7 +1329,7 @@ Zotero.Sync.Server = new function () {
 				// Reconcile and save updated data from server and
 				// prepare local data to upload
 				var xmlstr = Zotero.Sync.Server.Data.processUpdatedXML(
-					xml.updated, lastLocalSyncDate, syncSession
+					xml.updated, lastLocalSyncDate, syncSession, libraryID
 				);
 				
 				//Zotero.debug(xmlstr);
@@ -2112,7 +2112,7 @@ Zotero.BufferedInputListener.prototype = {
  * @property	{Zotero.Sync.ObjectKeySet}	uploadKeys.updated
  * @property	{Zotero.Sync.ObjectKeySet}	uploadKeys.deleted
  */
-Zotero.Sync.Server.Session = function () {
+Zotero.Sync.Server.Session = function (defaultLibraryID) {
 	this.uploadKeys = {};
 	this.uploadKeys.updated = new Zotero.Sync.ObjectKeySet;
 	this.uploadKeys.deleted = new Zotero.Sync.ObjectKeySet;
@@ -2229,12 +2229,16 @@ Zotero.Sync.Server.Data = new function() {
 	}
 	
 	
-	function processUpdatedXML(xml, lastLocalSyncDate, syncSession) {
+	function processUpdatedXML(xml, lastLocalSyncDate, syncSession, defaultLibraryID) {
 		if (xml.children().length() == 0) {
 			Zotero.debug('No changes received from server');
 			return Zotero.Sync.Server.Data.buildUploadXML(syncSession);
 		}
 		
+		function _libID(libraryID) {
+			return _getLibraryID(libraryID, defaultLibraryID);
+		}
+		
 		Zotero.DB.beginTransaction();
 		
 		var deletedCollectionKeys = _getDeletedCollectionKeys(xml);
@@ -2271,6 +2275,59 @@ Zotero.Sync.Server.Data = new function() {
 			}
 		}
 		
+		// Get unmodified creators embedded within items -- this is necessary if, say,
+		// a creator was deleted locally and appears in a new/modified item remotely
+		var embeddedCreators = {};
+		for each(var creatorNode in xml.items.item.creator.creator) {
+			var libraryID = _libID(creatorNode.@libraryID.toString());
+			var key = creatorNode.@key.toString();
+			
+			var creatorObj = Zotero.Creators.getByLibraryAndKey(libraryID, key);
+			// If creator exists locally, we don't need it
+			if (creatorObj) {
+				continue;
+			}
+			// Note which embedded creators are available
+			var lkh = Zotero.Creators.makeLibraryKeyHash(libraryID, key);
+			if (!embeddedCreators[lkh]) {
+				embeddedCreators[lkh] = true;
+			}
+		}
+		// Make sure embedded creators aren't already provided in the <creators> node
+		// This isn't necessary if the server data is correct
+		for each(var creatorNode in xml.creators.creator) {
+			var libraryID = _libID(creatorNode.@libraryID.toString());
+			var key = creatorNode.@key.toString();
+			var lkh = Zotero.Creators.makeLibraryKeyHash(libraryID, key);
+			if (embeddedCreators[lkh]) {
+				var msg = "Creator " + libraryID + "/" + key + " was unnecessarily embedded in server response "
+							+ "in Zotero.Sync.Server.Data.processUpdatedXML()";
+				Zotero.debug(msg, 2);
+				Components.utils.reportError(msg)
+				delete embeddedCreators[lkh];
+			}
+		}
+		// For any embedded creators that don't exist locally and aren't already
+		// included in the <creators> node, copy the node into <creators> for saving
+		var elementCreated = !!xml.creators.length();
+		for each(var creatorNode in xml.items.item.creator.creator) {
+			var libraryID = _libID(creatorNode.@libraryID.toString());
+			var key = creatorNode.@key.toString();
+			
+			var lkh = Zotero.Creators.makeLibraryKeyHash(libraryID, key);
+			if (embeddedCreators[lkh]) {
+				if (!elementCreated) {
+					xml.creators = new XML("<creators/>");
+					elementCreated = true;
+				}
+				
+				Zotero.debug("Adding embedded creator " + libraryID + "/" + key + " to <creators>");
+				
+				xml.creators.appendChild(creatorNode);
+				delete embeddedCreators[lkh];
+			}
+		}
+		
 		// Other objects
 		for each(var syncObject in Zotero.Sync.syncObjects) {
 			var Type = syncObject.singular; // 'Item'
@@ -2289,7 +2346,7 @@ Zotero.Sync.Server.Data = new function() {
 			
 			typeloop:
 			for each(var xmlNode in xml[types][type]) {
-				var libraryID = xmlNode.@libraryID.toString();
+				var libraryID = _libID(xmlNode.@libraryID.toString());
 				var key = xmlNode.@key.toString();
 				var objLibraryKeyHash = Zotero[Types].makeLibraryKeyHash(libraryID, key);
 				
@@ -2337,7 +2394,7 @@ Zotero.Sync.Server.Data = new function() {
 							Zotero.Sync.Server.Data.removeMissingRelatedItems(xmlNode);
 						}
 						
-						var remoteObj = Zotero.Sync.Server.Data['xmlTo' + Type](xmlNode);
+						var remoteObj = Zotero.Sync.Server.Data['xmlTo' + Type](xmlNode, null, null, defaultLibraryID);
 
 						// Some types we don't bother to reconcile
 						if (_noMergeTypes.indexOf(type) != -1) {
@@ -2499,7 +2556,7 @@ Zotero.Sync.Server.Data = new function() {
 				//
 				// If we skipped CR above, we already have an object to use
 				if (!skipCR) {
-					obj = Zotero.Sync.Server.Data['xmlTo' + Type](xmlNode, obj);
+					obj = Zotero.Sync.Server.Data['xmlTo' + Type](xmlNode, obj, null, defaultLibraryID);
 				}
 				
 				if (isNewObject && type == 'tag') {
@@ -2581,8 +2638,7 @@ Zotero.Sync.Server.Data = new function() {
 				Zotero.debug("Processing remotely deleted " + types);
 				
 				for each(var xmlNode in xml.deleted[types][type]) {
-					var libraryID = xmlNode.@libraryID.toString();
+					var libraryID = _libID(xmlNode.@libraryID.toString());
-					libraryID = libraryID ? parseInt(libraryID) : null;
 					var key = xmlNode.@key.toString();
 					var obj = Zotero[Types].getByLibraryAndKey(libraryID, key);
 					// Object can't be found
@@ -2592,8 +2648,8 @@ Zotero.Sync.Server.Data = new function() {
 						// caused its deletion during the sync
 						syncSession.removeFromDeleted({
 							objectType: type,
-							libraryID: parseInt(xmlNode.@libraryID),
+							libraryID: libraryID,
-							key: xmlNode.@key.toString()
+							key: key
 						});
 						continue;
 					}
@@ -3363,7 +3419,7 @@ Zotero.Sync.Server.Data = new function() {
 	 * @param	object	item			(Optional) Existing Zotero.Item to update
 	 * @param	bool	skipPrimary		(Optional) Ignore passed primary fields (except itemTypeID)
 	 */
-	function xmlToItem(xmlItem, item, skipPrimary) {
+	function xmlToItem(xmlItem, item, skipPrimary, defaultLibraryID) {
 		if (!item) {
 			item = new Zotero.Item;
 		}
@@ -3376,8 +3432,7 @@ Zotero.Sync.Server.Data = new function() {
 		
 		var data = {};
 		if (!skipPrimary) {
-			var libraryID = xmlItem.@libraryID.toString();
+			data.libraryID = _getLibraryID(xmlItem.@libraryID.toString(), defaultLibraryID);
-			data.libraryID = libraryID ? parseInt(libraryID) : null;
 			data.key = xmlItem.@key.toString();
 			data.dateAdded = xmlItem.@dateAdded.toString();
 			data.dateModified = xmlItem.@dateModified.toString();
@@ -3422,27 +3477,16 @@ Zotero.Sync.Server.Data = new function() {
 				throw ('No creator in position ' + i);
 			}
 			
-			var creatorObj = Zotero.Creators.getByLibraryAndKey(data.libraryID, creator.@key.toString());
+			var libraryID = data.libraryID;
-			// If creator doesn't exist locally (e.g., if it was deleted locally
+			var key = creator.@key.toString();
-			// and appears in a new/modified item remotely), get it from within
+			var creatorObj = Zotero.Creators.getByLibraryAndKey(libraryID, key);
-			// the item's creator block, where a copy should be provided
 			if (!creatorObj) {
-				if (creator.creator.length() == 0) {
+				var msg = "Data for missing local creator " + libraryID + "/" + key
-					var msg = "Data for missing local creator "
+					+ " not provided in Zotero.Sync.Server.Data.xmlToItem()";
-						+ data.libraryID + "/" + creator.@key.toString()
+				var e = new Zotero.Error(msg, "MISSING_OBJECT");
-						+ " not provided in Zotero.Sync.Server.Data.xmlToItem()";
+				throw (e);
-					var e = new Zotero.Error(msg, "MISSING_OBJECT");
-					throw (e);
-				}
-				
-				creator.creator.@libraryID = data.libraryID;
-				var creatorObj = Zotero.Sync.Server.Data.xmlToCreator(creator.creator);
-				
-				if (creator.@key.toString() != creatorObj.key) {
-					throw ("Creator does not match item creator in Zotero.Sync.Server.Data.xmlToItem() "
-						+ "(" + creator.@key.toString() + "!=" + creatorObj.key + ")");
-				}
 			}
+			
 			item.setCreator(
 				pos,
 				creatorObj,
@@ -3565,7 +3609,7 @@ Zotero.Sync.Server.Data = new function() {
 	 * @param	object	item		(Optional) Existing Zotero.Collection to update
 	 * @param	bool	skipPrimary		(Optional) Ignore passed primary fields (except itemTypeID)
 	 */
-	function xmlToCollection(xmlCollection, collection, skipPrimary) {
+	function xmlToCollection(xmlCollection, collection, skipPrimary, defaultLibraryID) {
 		if (!collection) {
 			collection = new Zotero.Collection;
 		}
@@ -3575,8 +3619,7 @@ Zotero.Sync.Server.Data = new function() {
 		}
 		
 		if (!skipPrimary) {
-			var libraryID = xmlCollection.@libraryID.toString();
+			collection.libraryID = _getLibraryID(xmlCollection.@libraryID.toString(), defaultLibraryID);
-			collection.libraryID = libraryID ? parseInt(libraryID) : null;
 			collection.key = xmlCollection.@key.toString();
 			var parentKey = xmlCollection.@parent.toString();
 			if (parentKey) {
@@ -3702,7 +3745,7 @@ Zotero.Sync.Server.Data = new function() {
 	 * @param	object	item		(Optional) Existing Zotero.Creator to update
 	 * @param	bool	skipPrimary	(Optional) Ignore passed primary fields (except itemTypeID)
 	 */
-	function xmlToCreator(xmlCreator, creator, skipPrimary) {
+	function xmlToCreator(xmlCreator, creator, skipPrimary, defaultLibraryID) {
 		if (!creator) {
 			creator = new Zotero.Creator;
 		}
@@ -3712,8 +3755,7 @@ Zotero.Sync.Server.Data = new function() {
 		}
 		
 		if (!skipPrimary) {
-			var libraryID = xmlCreator.@libraryID.toString();
+			creator.libraryID = _getLibraryID(xmlCreator.@libraryID.toString(), defaultLibraryID);
-			creator.libraryID = libraryID ? parseInt(libraryID) : null;
 			creator.key = xmlCreator.@key.toString();
 			creator.dateAdded = xmlCreator.@dateAdded.toString();
 			creator.dateModified = xmlCreator.@dateModified.toString();
@@ -3773,7 +3815,7 @@ Zotero.Sync.Server.Data = new function() {
 	 * @param	object	item		(Optional) Existing Zotero.Search to update
 	 * @param	bool	skipPrimary		(Optional) Ignore passed primary fields (except itemTypeID)
 	 */
-	function xmlToSearch(xmlSearch, search, skipPrimary) {
+	function xmlToSearch(xmlSearch, search, skipPrimary, defaultLibraryID) {
 		if (!search) {
 			search = new Zotero.Search;
 		}
@@ -3783,8 +3825,7 @@ Zotero.Sync.Server.Data = new function() {
 		}
 		
 		if (!skipPrimary) {
-			var libraryID = xmlSearch.@libraryID.toString();
+			search.libraryID = _getLibraryID(xmlSearch.@libraryID.toString(), defaultLibraryID);
-			search.libraryID = libraryID ? parseInt(libraryID) : null;
 			search.key = xmlSearch.@key.toString();
 			search.dateAdded = xmlSearch.@dateAdded.toString();
 			search.dateModified = xmlSearch.@dateModified.toString();
@@ -3863,7 +3904,7 @@ Zotero.Sync.Server.Data = new function() {
 	 * @param	object	tag				(Optional) Existing Zotero.Tag to update
 	 * @param	bool	skipPrimary		(Optional) Ignore passed primary fields
 	 */
-	function xmlToTag(xmlTag, tag, skipPrimary) {
+	function xmlToTag(xmlTag, tag, skipPrimary, defaultLibraryID) {
 		if (!tag) {
 			tag = new Zotero.Tag;
 		}
@@ -3873,8 +3914,7 @@ Zotero.Sync.Server.Data = new function() {
 		}
 		
 		if (!skipPrimary) {
-			var libraryID = xmlTag.@libraryID.toString();
+			tag.libraryID = _getLibraryID(xmlTag.@libraryID.toString(), defaultLibraryID);
-			tag.libraryID = libraryID ? parseInt(libraryID) : null;
 			tag.key = xmlTag.@key.toString();
 			tag.dateAdded = xmlTag.@dateAdded.toString();
 			tag.dateModified = xmlTag.@dateModified.toString();
@@ -3985,4 +4025,12 @@ Zotero.Sync.Server.Data = new function() {
 	function _xmlize(str) {
 		return str.replace(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\ud800-\udfff\ufffe\uffff]/g, '\u2B1A');
 	}
+	
+	
+	function _getLibraryID(libraryID, defaultLibraryID) {
+		if (!libraryID) {
+			return null;
+		}
+		return libraryID == defaultLibraryID ? null : parseInt(libraryID);
+	}
 }