From 1774962de92df30805b05b6babf8d236e6f2f4fc Mon Sep 17 00:00:00 2001
From: Simon Kornblith <simon@simonster.com>
Date: Mon, 4 Jul 2011 09:08:49 +0000
Subject: [PATCH] - Refactor Zotero.Translate save process. There are now
 itemSaving and itemDone handlers. The itemSaving handler is called as soon as
 the translator returns an item, but before it's saved to the database. The
 itemDone handler is called when the item is saved. This allows us to show a
 grayed-out item icon in the connector while the item is saving to server, and
 then un-gray it when it is saved to the server. - Move Zotero.randomString()
 to Zotero.Utilities - Fix import of collections (broken by recent commits)

---
 chrome/content/zotero/fileInterface.js        |   2 +-
 .../zotero/xpcom/connector/translate_item.js  |  55 ++++----
 .../content/zotero/xpcom/server_connector.js  |  20 ++-
 .../zotero/xpcom/translation/translate.js     |  73 ++++++++--
 .../xpcom/translation/translate_item.js       | 129 +++++++++---------
 chrome/content/zotero/xpcom/utilities.js      |  18 +++
 chrome/content/zotero/xpcom/zotero.js         |  13 +-
 7 files changed, 189 insertions(+), 121 deletions(-)

diff --git a/chrome/content/zotero/fileInterface.js b/chrome/content/zotero/fileInterface.js
index ae2975f975..d92d15c1fa 100644
--- a/chrome/content/zotero/fileInterface.js
+++ b/chrome/content/zotero/fileInterface.js
@@ -330,7 +330,7 @@ var Zotero_File_Interface = new function() {
 	 */
 	function _importDone(obj, worked) {
 		// add items to import collection
-		_importCollection.addItems(obj.newItems);
+		_importCollection.addItems([item.id for each(item in obj.newItems)]);
 		
 		Zotero.DB.commitTransaction();
 		
diff --git a/chrome/content/zotero/xpcom/connector/translate_item.js b/chrome/content/zotero/xpcom/connector/translate_item.js
index c3ad2a9b71..3842d8e148 100644
--- a/chrome/content/zotero/xpcom/connector/translate_item.js
+++ b/chrome/content/zotero/xpcom/connector/translate_item.js
@@ -26,7 +26,6 @@
 Zotero.Translate.ItemSaver = function(libraryID, attachmentMode, forceTagType) {
 	this.newItems = [];
 	
-	this._itemsToSaveToServer = [];
 	this._timeoutID = null;
 }
 
@@ -35,37 +34,46 @@ Zotero.Translate.ItemSaver.ATTACHMENT_MODE_DOWNLOAD = 1;
 Zotero.Translate.ItemSaver.ATTACHMENT_MODE_FILE = 2;
 
 Zotero.Translate.ItemSaver.prototype = {
-	"saveItem":function(item) {
+	/**
+	 * Saves items to Standalone or the server
+	 */
+	"saveItems":function(items, callback) {
 		// don't save documents as documents, since we can't pass them around
-		for(var i in item.attachments) {
-			if(item.attachments[i].document) {
-				item.attachments[i].url = item.attachments[i].document.location.href;
-				delete item.attachments[i].document;
+		var nItems = items.length;
+		for(var i=0; i<nItems.length; i++) {
+			var attachments = item[i].attachments;
+			var nAttachments = attachments.length;
+			for(var j=0; j<nAttachments.length; j++) {
+				if(attachments[j].document) {
+					attachments[j].url = attachments[j].document.location.href;
+					delete attachments[j].document;
+				}
 			}
 		}
 		
-		// save items
-		this.newItems.push(item);
 		var me = this;
-		Zotero.Connector.callMethod("saveItems", {"items":[item]}, function(success) {
-			if(success === false && !Zotero.isFx) {
-				// attempt to save to server on a timer
-				if(me._timeoutID) clearTimeout(me._timeoutID);
-				me._itemsToSaveToServer.push(item);
-				me._timeoutID = setTimeout(function() { me._saveToServer() }, 2000);
+		// first try to save items via connector
+		Zotero.Connector.callMethod("saveItems", {"items":items}, function(success, status) {
+			if(success !== false) {
+				Zotero.debug("Translate: Save via Standalone succeeded");
+				callback(true, items);
+			} else if(Zotero.isFx) {
+				callback(false, new Error("Save via Standalone failed with "+status));
+			} else {
+				me._saveToServer(items, callback);
 			}
 		});
 	},
 	
-	"_saveToServer":function() {
+	/**
+	 * Saves items to server
+	 */
+	"_saveToServer":function(items, callback) {
 		const IGNORE_FIELDS = ["seeAlso", "attachments", "complete"];
 		
-		// clear timeout, since saving has begin
-		this._timeoutID = null;
-		
 		var newItems = [];
-		for(var i in this._itemsToSaveToServer) {
-			var item = this._itemsToSaveToServer[i];
+		for(var i in items) {
+			var item = items[i];
 			
 			var newItem = {};
 			newItems.push(newItem);
@@ -180,15 +188,14 @@ Zotero.Translate.ItemSaver.prototype = {
 		
 		var url = 'users/%%USERID%%/items?key=%%APIKEY%%';
 		var payload = JSON.stringify({"items":newItems}, null, "\t")
-		this._itemsToSaveToServer = [];
 		
 		Zotero.OAuth.doAuthenticatedPost(url, payload, function(status, message) {
 			if(!status) {
-				Zotero.Messaging.sendMessage("saveDialog_error", status);
-				Zotero.debug("Translate: Save to server payload:\n\n"+payload);
-				Zotero.logError(new Error("Translate: Save to server failed: "+message));
+				Zotero.debug("Translate: Save to server failed with message "+message+"; payload:\n\n"+payload);
+				callback(false, new Error("Save to server failed with "+message));
 			} else {
 				Zotero.debug("Translate: Save to server complete");
+				callback(true, newItems);
 			}
 		}, true);
 	}
diff --git a/chrome/content/zotero/xpcom/server_connector.js b/chrome/content/zotero/xpcom/server_connector.js
index 407f2a1604..8e0945d36c 100755
--- a/chrome/content/zotero/xpcom/server_connector.js
+++ b/chrome/content/zotero/xpcom/server_connector.js
@@ -465,11 +465,21 @@ Zotero.Server.Connector.SaveItem.prototype = {
 		// save items
 		var itemSaver = new Zotero.Translate.ItemSaver(libraryID,
 			Zotero.Translate.ItemSaver.ATTACHMENT_MODE_DOWNLOAD, 1);
-		for each(var item in data.items) {
-			var savedItem = itemSaver.saveItem(item);
-			if(collection) collection.addItem(savedItem.id);
-		}
-		sendResponseCallback(201);
+		itemSaver.saveItems(data.items, function(returnValue, data) {
+			if(returnValue) {
+				try {
+					for each(var item in data) {
+						if(collection) collection.addItem(savedItem.id);
+					}
+					sendResponseCallback(201);
+				} catch(e) {
+					sendResponseCallback(500);
+				}
+			} else {
+				sendResponseCallback(500);
+				throw data;
+			}
+		});
 	}
 }
 
diff --git a/chrome/content/zotero/xpcom/translation/translate.js b/chrome/content/zotero/xpcom/translation/translate.js
index 2974d01cbf..789aa478ac 100644
--- a/chrome/content/zotero/xpcom/translation/translate.js
+++ b/chrome/content/zotero/xpcom/translation/translate.js
@@ -105,17 +105,32 @@ Zotero.Translate.Sandbox = {
 				return;
 			}
 			
-			var newItem = translate._itemSaver.saveItem(item);
+			// We use this within the connector to keep track of items as they are saved
+			if(!item.id) item.id = Zotero.Utilities.randomString();
 			
-			// Allow progress meter to update
-			//
-			// This can probably be re-enabled for web translators once badly asynced ones are fixed
-			if(!translate.noWait && translate instanceof Zotero.Translate.Import) {
-				Zotero.wait();
+			if(translate instanceof Zotero.Translate.Web) {
+				// For web translators, we queue saves
+				translate.saveQueue.push(item);
+				translate._runHandler("itemSaving", item);
+			} else {
+				var newItem;
+				translate._itemSaver.saveItems([item], function(returnValue, data) {
+					if(returnValue) {
+						newItem = data[0];
+						translate.newItems.push(newItem);
+					} else {
+						translate.complete(false, data);
+						throw data;
+					}
+				});
+			
+				// Allow progress meter to update
+				if(!translate.noWait) Zotero.wait();
+				
+				translate._runHandler("itemSaving", item);
+				// pass both the saved item and the original JS array item
+				translate._runHandler("itemDone", newItem, item);
 			}
-			
-			// pass both the saved item and the original JS array item
-			translate._runHandler("itemDone", newItem, item);
 		},
 		
 		/**
@@ -948,6 +963,30 @@ Zotero.Translate.Base.prototype = {
 		this.decrementAsyncProcesses();
 	},
 	
+	/**
+	 * Executed when items have been saved (which may happen asynchronously, if in connector)
+	 *
+	 * @param {Boolean} returnValue Whether saving was successful
+	 * @param {Zotero.Item[]|Error} data If returnValue is true, this will be an array of
+	 *                                    Zotero.Item objects. If returnValue is false, this will
+	 *                                    be a string error message.
+	 */
+	"itemsSaved":function(returnValue, data) {
+		if(returnValue) {
+			// trigger deferred itemDone events
+			var nItems = data.length;
+			for(var i=0; i<nItems; i++) {
+				this._runHandler("itemDone", data[i], this.saveQueue[i]);
+			}
+			
+			this.saveQueue = [];
+		} else {
+			Zotero.logError(data);
+		}
+		
+		this._runHandler("done", returnValue);
+	},
+	
 	/**
 	 * Executed on translator completion, either automatically from a synchronous scraper or as
 	 * done() from an asynchronous scraper. Finishes things up and calls callback function(s).
@@ -1008,7 +1047,14 @@ Zotero.Translate.Base.prototype = {
 			if(returnValue === undefined) returnValue = true;
 			
 			if(returnValue) {
-				this._debug("Translation successful");
+				if(this.saveQueue.length) {
+					var me = this;
+					this._itemSaver.saveItems(this.saveQueue.slice(),
+						function(returnValue, data) { me.itemsSaved(returnValue, data) });
+					return;
+				} else {
+					this._debug("Translation successful");
+				}
 			} else {
 				if(error) {
 					// report error to console
@@ -1082,6 +1128,7 @@ Zotero.Translate.Base.prototype = {
 		this._runningAsyncProcesses = 0;
 		this._returnValue = undefined;
 		this._aborted = false;
+		this.saveQueue = [];
 		
 		Zotero.debug("Translate: Parsing code for "+translator.label, 4);
 		
@@ -1118,7 +1165,7 @@ Zotero.Translate.Base.prototype = {
 				"}"+
 		"};";
 		
-		if(this instanceof Zotero.Translate.Export) {
+		if(this instanceof Zotero.Translate.Export || this instanceof Zotero.Translate.Import) {
 			src += "Zotero.Collection = function () {};"+
 			"Zotero.Collection.prototype.complete = function() { Zotero._collectionDone(this); };";
 		} else if (this instanceof Zotero.Translate.Import) {
@@ -1312,7 +1359,7 @@ Zotero.Translate.Web.prototype._getParameters = function() { return [this.docume
 Zotero.Translate.Web.prototype._prepareTranslation = function() {
 	this._itemSaver = new Zotero.Translate.ItemSaver(this._libraryID,
 		Zotero.Translate.ItemSaver[(this._saveAttachments ? "ATTACHMENT_MODE_DOWNLOAD" : "ATTACHMENT_MODE_IGNORE")], 1);
-	this.newItems = this._itemSaver.newItems;
+	this.newItems = [];
 }
 
 /**
@@ -1507,7 +1554,7 @@ Zotero.Translate.Import.prototype._prepareTranslation = function() {
 	this._progress = undefined;
 	this._itemSaver = new Zotero.Translate.ItemSaver(this._libraryID,
 		Zotero.Translate.ItemSaver[(this._saveAttachments ? "ATTACHMENT_MODE_FILE" : "ATTACHMENT_MODE_IGNORE")]);
-	this.newItems = this._itemSaver.newItems;
+	this.newItems = [];
 	this.newCollections = this._itemSaver.newCollections;
 }
 
diff --git a/chrome/content/zotero/xpcom/translation/translate_item.js b/chrome/content/zotero/xpcom/translation/translate_item.js
index 7cacab00cc..34cd653dd1 100644
--- a/chrome/content/zotero/xpcom/translation/translate_item.js
+++ b/chrome/content/zotero/xpcom/translation/translate_item.js
@@ -72,73 +72,79 @@ Zotero.Translate.ItemSaver.ATTACHMENT_MODE_DOWNLOAD = 1;
 Zotero.Translate.ItemSaver.ATTACHMENT_MODE_FILE = 2;
 
 Zotero.Translate.ItemSaver.prototype = {
-	"saveItem":function(item) {
+	"saveItems":function(items, callback) {
 		// if no open transaction, open a transaction and add a timer call to close it
+		var openedTransaction = false;
 		if(!Zotero.DB.transactionInProgress()) {
-			Zotero.debug("Translate: Beginning transaction");
 			Zotero.DB.beginTransaction();
-			
-			var me = this;
-			Zotero.setTimeout(function() { me._closeTransaction() }, 0);
-			Zotero.showZoteroPaneProgressMeter(Zotero.getString("ingester.scraping"), false);
+			openedTransaction = true;
 		}
 		
-		// Get typeID, defaulting to "webpage"
-		var newItem;
-		var type = (item.itemType ? item.itemType : "webpage");
-		
-		if(type == "note") {			// handle notes differently
-			newItem = new Zotero.Item('note');
-			newItem.libraryID = this._libraryID;
-			newItem.setNote(item.note);
-			var myID = newItem.save();
-			newItem = Zotero.Items.get(myID);
-		} else {
-			if(type == "attachment") {	// handle attachments differently
-				newItem = this._saveAttachment(item);
-				if(!newItem) return;
-				var myID = newItem.id;
-			} else {
-				var typeID = Zotero.ItemTypes.getID(type);
-				newItem = new Zotero.Item(typeID);
-				newItem._libraryID = this._libraryID;
-			
-				this._saveFields(item, newItem);
+		try {
+			var newItems = [];
+			for each(var item in items) {
+				// Get typeID, defaulting to "webpage"
+				var newItem;
+				var type = (item.itemType ? item.itemType : "webpage");
 				
-				// handle creators
-				if(item.creators) {
-					this._saveCreators(item, newItem);
-				}
-				
-				// save item
-				var myID = newItem.save();
-				newItem = Zotero.Items.get(myID);
-				
-				// handle notes
-				if(item.notes) {
-					this._saveNotes(item, myID);
-				}
-			
-				// handle attachments
-				if(item.attachments) {
-					for(var i=0; i<item.attachments.length; i++) {
-						var newAttachment = this._saveAttachment(item.attachments[i], myID);
-						if(newAttachment) this._saveTags(item.attachments[i], newAttachment);
+				if(type == "note") {			// handle notes differently
+					newItem = new Zotero.Item('note');
+					newItem.libraryID = this._libraryID;
+					newItem.setNote(item.note);
+					var myID = newItem.save();
+					newItem = Zotero.Items.get(myID);
+				} else {
+					if(type == "attachment") {	// handle attachments differently
+						newItem = this._saveAttachment(item);
+						if(!newItem) return;
+						var myID = newItem.id;
+					} else {
+						var typeID = Zotero.ItemTypes.getID(type);
+						newItem = new Zotero.Item(typeID);
+						newItem._libraryID = this._libraryID;
+					
+						this._saveFields(item, newItem);
+						
+						// handle creators
+						if(item.creators) {
+							this._saveCreators(item, newItem);
+						}
+						
+						// save item
+						var myID = newItem.save();
+						newItem = Zotero.Items.get(myID);
+						
+						// handle notes
+						if(item.notes) {
+							this._saveNotes(item, myID);
+						}
+					
+						// handle attachments
+						if(item.attachments) {
+							for(var i=0; i<item.attachments.length; i++) {
+								var newAttachment = this._saveAttachment(item.attachments[i], myID);
+								if(newAttachment) this._saveTags(item.attachments[i], newAttachment);
+							}
+						}
 					}
 				}
+				
+				if(item.itemID) this._IDMap[item.itemID] = myID;
+				
+				// handle see also
+				this._saveTags(item, newItem);
+				
+				// add to new item list
+				newItem = Zotero.Items.get(myID);
+				newItems.push(newItem);
 			}
+			
+			if(openedTransaction) Zotero.DB.commitTransaction();
+			callback(true, newItems);
+		} catch(e) {
+			if(openedTransaction) Zotero.DB.rollbackTransaction();
+			callback(false, e);
 		}
-		
-		if(item.itemID) this._IDMap[item.itemID] = myID;
-		
-		// handle see also
-		this._saveTags(item, newItem);
-		
-		// add to new item list
-		this.newItems.push(myID);
-		
-		newItem = Zotero.Items.get(myID);
-		return newItem;
 	},
 	
 	"saveCollection":function(collection) {
@@ -509,15 +515,6 @@ Zotero.Translate.ItemSaver.prototype = {
 				}
 			}
 		}
-	},
-	
-	/**
-	 * Closes the transaction when the current code block finishes executing.
-	 */
-	"_closeTransaction":function() {
-		Zotero.debug("Translate: Closing transaction");
-		Zotero.hideZoteroPaneOverlay();
-		Zotero.DB.commitTransaction();
 	}
 }
 
diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js
index cfcaec6541..fb688f67fe 100644
--- a/chrome/content/zotero/xpcom/utilities.js
+++ b/chrome/content/zotero/xpcom/utilities.js
@@ -732,6 +732,24 @@ Zotero.Utilities = {
 		}
 		
 		return strings.join(delimiter ? delimiter : ", ");
+	},
+	
+	/**
+	* Generate a random string of length 'len' (defaults to 8)
+	**/
+	"randomString":function(len, chars) {
+		if (!chars) {
+			chars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXTZabcdefghiklmnopqrstuvwxyz";
+		}
+		if (!len) {
+			len = 8;
+		}
+		var randomstring = '';
+		for (var i=0; i<len; i++) {
+			var rnum = Math.floor(Math.random() * chars.length);
+			randomstring += chars.substring(rnum,rnum+1);
+		}
+		return randomstring;
 	}
 }
 
diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js
index 6f4ee0813e..efe91531ba 100644
--- a/chrome/content/zotero/xpcom/zotero.js
+++ b/chrome/content/zotero/xpcom/zotero.js
@@ -1471,18 +1471,7 @@ if(appInfo.platformVersion[0] >= 2) {
 	* Generate a random string of length 'len' (defaults to 8)
 	**/
 	function randomString(len, chars) {
-		if (!chars) {
-			chars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXTZabcdefghiklmnopqrstuvwxyz";
-		}
-		if (!len) {
-			len = 8;
-		}
-		var randomstring = '';
-		for (var i=0; i<len; i++) {
-			var rnum = Math.floor(Math.random() * chars.length);
-			randomstring += chars.substring(rnum,rnum+1);
-		}
-		return randomstring;
+		return Zotero.Utilities.randomString(len, chars);
 	}