commit 27cb099c8251b902f3dc7dafc06da48da1c0cf1e
parent 3e35764405a1a9ff823d24a3c0909e17657fcf45
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 8 Feb 2017 14:44:21 -0500
Allow asynchronous item saving in import translators
This allows for imported items to be saved individually instead of being
saved in a batch after processing the entire imported file (which for
large imports would hang the UI, even if the actual saving was
asynchronous). This also fixes the progress meter during asynchronous
saves.
To take advantage of this, import translators will need to return a
promise when available (using the native Promise object) from doImport()
and wait for optional promises from item.complete().
The logic here can probably be streamlined further. (E.g., we might be
able to say that item.complete() always returns a promise.) It's
complicated by the fact that, at the moment, Promise isn't available in
child sandboxes, though this can probably be fixed.
Tests forthcoming, but they require a translator that supports this,
which needs to be committed separately.
View with -w for a cleaner diff.
Diffstat:
4 files changed, 196 insertions(+), 131 deletions(-)
diff --git a/chrome/content/zotero/fileInterface.js b/chrome/content/zotero/fileInterface.js
@@ -329,12 +329,21 @@ var Zotero_File_Interface = new function() {
importCollection.name = collectionName;
yield importCollection.saveTx();
}
-
+
translation.setTranslator(translators[0]);
- // TODO: Restore a progress meter
- /*translation.setHandler("itemDone", function () {
- Zotero.updateZoteroPaneProgressMeter(translation.getProgress());
- });*/
+
+ // Show progress popup
+ var progressWin = new Zotero.ProgressWindow({
+ closeOnClick: false
+ });
+ progressWin.changeHeadline(Zotero.getString('fileInterface.importing'));
+ var icon = 'chrome://zotero/skin/treesource-unfiled' + (Zotero.hiDPI ? "@2x" : "") + '.png';
+ let progress = new progressWin.ItemProgress(icon, OS.Path.basename(translation.path));
+ progressWin.show();
+
+ translation.setHandler("itemDone", function () {
+ progress.setProgress(translation.getProgress());
+ });
yield Zotero.Promise.delay(0);
@@ -352,7 +361,6 @@ var Zotero_File_Interface = new function() {
// Show popup on completion
var numItems = translation.newItems.length;
- var progressWin = new Zotero.ProgressWindow();
progressWin.changeHeadline(Zotero.getString('fileInterface.importComplete'));
if (numItems == 1) {
var icon = translation.newItems[0].getImageSrc();
@@ -360,10 +368,12 @@ var Zotero_File_Interface = new function() {
else {
var icon = 'chrome://zotero/skin/treesource-unfiled' + (Zotero.hiDPI ? "@2x" : "") + '.png';
}
- var title = Zotero.getString(`fileInterface.itemsWereImported`, numItems, numItems);
- progressWin.addLines(title, icon)
- progressWin.show();
- progressWin.startCloseTimer();
+ var text = Zotero.getString(`fileInterface.itemsWereImported`, numItems, numItems);
+ progress.setIcon(icon);
+ progress.setText(text);
+ // For synchronous translators, which don't update progress
+ progress.setProgress(100);
+ progressWin.startCloseTimer(5000);
});
/*
diff --git a/chrome/content/zotero/xpcom/translation/translate.js b/chrome/content/zotero/xpcom/translation/translate.js
@@ -80,129 +80,165 @@ Zotero.Translate.Sandbox = {
* @param {Zotero.Translate} translate
* @param {SandboxItem} An item created using the Zotero.Item class from the sandbox
*/
- "_itemDone":function(translate, item) {
- //Zotero.debug("Translate: Saving item");
-
- // warn if itemDone called after translation completed
- if(translate._complete) {
- Zotero.debug("Translate: WARNING: Zotero.Item#complete() called after Zotero.done(); please fix your code", 2);
- }
+ _itemDone: function (translate, item) {
+ var run = function (resolve) {
+ Zotero.debug("Translate: Saving item");
- const allowedObjects = ["complete", "attachments", "seeAlso", "creators", "tags", "notes"];
-
- // Create a new object here, so that we strip the "complete" property
- var newItem = {};
- var oldItem = item;
- for(var i in item) {
- var val = item[i];
- if(i === "complete" || (!val && val !== 0)) continue;
+ // warn if itemDone called after translation completed
+ if(translate._complete) {
+ Zotero.debug("Translate: WARNING: Zotero.Item#complete() called after Zotero.done(); please fix your code", 2);
+ }
+
+ const allowedObjects = ["complete", "attachments", "seeAlso", "creators", "tags", "notes"];
- var type = typeof val;
- var isObject = type === "object" || type === "xml" || type === "function",
- shouldBeObject = allowedObjects.indexOf(i) !== -1;
- if(isObject && !shouldBeObject) {
- // Convert things that shouldn't be objects to objects
- translate._debug("Translate: WARNING: typeof "+i+" is "+type+"; converting to string");
- newItem[i] = val.toString();
- } else if(shouldBeObject && !isObject) {
- translate._debug("Translate: WARNING: typeof "+i+" is "+type+"; converting to array");
- newItem[i] = [val];
- } else if(type === "string") {
- // trim strings
- newItem[i] = val.trim();
- } else {
- newItem[i] = val;
+ // Create a new object here, so that we strip the "complete" property
+ var newItem = {};
+ var oldItem = item;
+ for(var i in item) {
+ var val = item[i];
+ if(i === "complete" || (!val && val !== 0)) continue;
+
+ var type = typeof val;
+ var isObject = type === "object" || type === "xml" || type === "function",
+ shouldBeObject = allowedObjects.indexOf(i) !== -1;
+ if(isObject && !shouldBeObject) {
+ // Convert things that shouldn't be objects to objects
+ translate._debug("Translate: WARNING: typeof "+i+" is "+type+"; converting to string");
+ newItem[i] = val.toString();
+ } else if(shouldBeObject && !isObject) {
+ translate._debug("Translate: WARNING: typeof "+i+" is "+type+"; converting to array");
+ newItem[i] = [val];
+ } else if(type === "string") {
+ // trim strings
+ newItem[i] = val.trim();
+ } else {
+ newItem[i] = val;
+ }
}
- }
- item = newItem;
-
- // Clean empty creators
- if (item.creators) {
- for (var i=0; i<item.creators.length; i++) {
- var creator = item.creators[i];
- if (!creator.firstName && !creator.lastName) {
- item.creators.splice(i, 1);
- i--;
+ item = newItem;
+
+ // Clean empty creators
+ if (item.creators) {
+ for (var i=0; i<item.creators.length; i++) {
+ var creator = item.creators[i];
+ if (!creator.firstName && !creator.lastName) {
+ item.creators.splice(i, 1);
+ i--;
+ }
}
}
- }
-
- // If we're not in a child translator, canonicalize tags
- if (!translate._parentTranslator) {
- if(item.tags) item.tags = translate._cleanTags(item.tags);
- }
-
- // if we're not supposed to save the item or we're in a child translator,
- // just return the item array
- if(translate._libraryID === false || translate._parentTranslator) {
- translate.newItems.push(item);
- if(translate._parentTranslator && Zotero.isFx && !Zotero.isBookmarklet) {
- // Copy object so it is accessible to child translator
- item = translate._sandboxManager.copyObject(item);
- item.complete = oldItem.complete;
+
+ // If we're not in a child translator, canonicalize tags
+ if (!translate._parentTranslator) {
+ if(item.tags) item.tags = translate._cleanTags(item.tags);
}
- translate._runHandler("itemDone", item, item);
- return;
- }
-
- // We use this within the connector to keep track of items as they are saved
- if(!item.id) item.id = Zotero.Utilities.randomString();
-
- if(item.attachments) {
- var attachments = item.attachments;
- for(var j=0; j<attachments.length; j++) {
- var attachment = attachments[j];
-
- // Don't save documents as documents in connector, since we can't pass them around
- if(Zotero.isConnector && attachment.document) {
- attachment.url = attachment.document.documentURI || attachment.document.URL;
- attachment.mimeType = "text/html";
- delete attachment.document;
+
+ // if we're not supposed to save the item or we're in a child translator,
+ // just return the item array
+ if(translate._libraryID === false || translate._parentTranslator) {
+ translate.newItems.push(item);
+ if(translate._parentTranslator && Zotero.isFx && !Zotero.isBookmarklet) {
+ // Copy object so it is accessible to child translator
+ item = translate._sandboxManager.copyObject(item);
+ item.complete = oldItem.complete;
}
-
- // If we're not in a child translator, canonicalize tags
- if (!translate._parentTranslator) {
- if(attachment.tags !== undefined) attachment.tags = translate._cleanTags(attachment.tags);
+ let maybePromise = translate._runHandler("itemDone", item, item);
+ // DEBUG: Is this ever necessary?
+ if (maybePromise) {
+ return resolve ? maybePromise.then(resolve) : maybePromise;
}
+ if (resolve) {
+ resolve();
+ }
+ return;
}
- }
-
- if(item.notes) {
- var notes = item.notes;
- for(var j=0; j<notes.length; j++) {
- var note = notes[j];
- if(!note) {
- notes.splice(j--, 1);
- } else if(typeof(note) != "object") {
- // Convert to object
- notes[j] = {"note":note.toString()}
+
+ // We use this within the connector to keep track of items as they are saved
+ if(!item.id) item.id = Zotero.Utilities.randomString();
+
+ if(item.attachments) {
+ var attachments = item.attachments;
+ for(var j=0; j<attachments.length; j++) {
+ var attachment = attachments[j];
+
+ // Don't save documents as documents in connector, since we can't pass them around
+ if(Zotero.isConnector && attachment.document) {
+ attachment.url = attachment.document.documentURI || attachment.document.URL;
+ attachment.mimeType = "text/html";
+ delete attachment.document;
+ }
+
+ // If we're not in a child translator, canonicalize tags
+ if (!translate._parentTranslator) {
+ if(attachment.tags !== undefined) attachment.tags = translate._cleanTags(attachment.tags);
+ }
}
- // If we're not in a child translator, canonicalize tags
- if (!translate._parentTranslator) {
- if(note.tags !== undefined) note.tags = translate._cleanTags(note.tags);
+ }
+
+ if(item.notes) {
+ var notes = item.notes;
+ for(var j=0; j<notes.length; j++) {
+ var note = notes[j];
+ if(!note) {
+ notes.splice(j--, 1);
+ } else if(typeof(note) != "object") {
+ // Convert to object
+ notes[j] = {"note":note.toString()}
+ }
+ // If we're not in a child translator, canonicalize tags
+ if (!translate._parentTranslator) {
+ if(note.tags !== undefined) note.tags = translate._cleanTags(note.tags);
+ }
}
}
- }
-
- if (item.version) {
- translate._debug("Translate: item.version is deprecated; set item.versionNumber instead");
- item.versionNumber = item.version;
- }
-
- if (item.accessDate) {
- if (Zotero.Date.isSQLDateTime(item.accessDate)) {
- translate._debug("Translate: Passing accessDate as SQL is deprecated; pass an ISO 8601 date instead");
- item.accessDate = Zotero.Date.sqlToISO8601(item.accessDate);
+
+ if (item.version) {
+ translate._debug("Translate: item.version is deprecated; set item.versionNumber instead");
+ item.versionNumber = item.version;
+ }
+
+ if (item.accessDate) {
+ if (Zotero.Date.isSQLDateTime(item.accessDate)) {
+ translate._debug("Translate: Passing accessDate as SQL is deprecated; pass an ISO 8601 date instead");
+ item.accessDate = Zotero.Date.sqlToISO8601(item.accessDate);
+ }
+ }
+
+ // Fire itemSaving event
+ translate._runHandler("itemSaving", item);
+
+ translate._savingItems++;
+
+ // For synchronous import (when Promise isn't available in the sandbox) and web
+ // translators, we queue saves
+ if (!resolve || translate instanceof Zotero.Translate.Web) {
+ Zotero.debug("Translate: Saving via queue");
+ translate.saveQueue.push(item);
+ if (resolve) {
+ resolve();
+ }
}
+ // For async import, save items immediately
+ else {
+ Zotero.debug("Translate: Saving now");
+ translate._saveItems([item]).then(resolve);
+ }
+ };
+
+ if (!translate._sandboxManager.sandbox.Promise) {
+ Zotero.debug("Translate: Promise not available in sandbox in _itemDone()");
+ run();
+ return;
}
-
- // Fire itemSaving event
- translate._runHandler("itemSaving", item);
- // TODO: This used to only be used for some modes. Since it's now used for everything with
- // async saving, there's probably a bunch of code for the non-queued mode that can be removed.
- translate.saveQueue.push(item);
- translate._savingItems++;
+ return new translate._sandboxManager.sandbox.Promise(function (resolve, reject) {
+ try {
+ run(resolve);
+ }
+ catch (e) {
+ reject(e);
+ }
+ });
},
/**
@@ -1320,7 +1356,23 @@ Zotero.Translate.Base.prototype = {
// translate
try {
- Function.prototype.apply.call(this._sandboxManager.sandbox["do" + this._entryFunctionSuffix], null, this._getParameters());
+ let maybePromise = Function.prototype.apply.call(
+ this._sandboxManager.sandbox["do" + this._entryFunctionSuffix],
+ null,
+ this._getParameters()
+ );
+ // doImport can return a promise to allow for incremental saves (via promise-returning
+ // item.complete() calls)
+ if (maybePromise) {
+ maybePromise
+ .then(
+ () => this.decrementAsyncProcesses("Zotero.Translate#translate()")
+ )
+ .catch((e) => {
+ this.complete(false, e)
+ });
+ return;
+ }
} catch (e) {
this.complete(false, e);
return false;
@@ -1579,11 +1631,14 @@ Zotero.Translate.Base.prototype = {
}
}
- // Trigger itemDone events
+ // Trigger itemDone events, waiting for them if they return promises
+ var maybePromises = [];
for(var i=0, nItems = items.length; i<nItems; i++) {
- this._runHandler("itemDone", newItems[i], items[i]);
+ maybePromises.push(this._runHandler("itemDone", newItems[i], items[i]));
}
-
+ return Zotero.Promise.all(maybePromises).then(() => newItems);
+ }.bind(this))
+ .then(function (newItems) {
// Specify that itemDone event was dispatched, so that we don't defer
// attachmentProgress notifications anymore
itemDoneEventsDispatched = true;
@@ -1612,10 +1667,12 @@ Zotero.Translate.Base.prototype = {
if(!this._savingItems && !this._savingAttachments.length && (!this._currentState || this._waitingForSave)) {
if(this.newCollections && this._itemSaver.saveCollections) {
var me = this;
- this._itemSaver.saveCollections(this.newCollections).then(function (newCollections) {
+ this._itemSaver.saveCollections(this.newCollections)
+ .then(function (newCollections) {
me.newCollections = newCollections;
me._runHandler("done", true);
- }, function (err) {
+ })
+ .catch(function (err) {
me._runHandler("error", err);
me._runHandler("done", false);
});
@@ -1767,10 +1824,10 @@ Zotero.Translate.Base.prototype = {
if(this instanceof Zotero.Translate.Export || this instanceof Zotero.Translate.Import) {
src += "Zotero.Collection = function () {};"+
- "Zotero.Collection.prototype.complete = function() { Zotero._collectionDone(this); };";
+ "Zotero.Collection.prototype.complete = function() { return Zotero._collectionDone(this); };";
}
- src += "Zotero.Item.prototype.complete = function() { Zotero._itemDone(this); }";
+ src += "Zotero.Item.prototype.complete = function() { return Zotero._itemDone(this); }";
this._sandboxManager.eval(src);
this._sandboxManager.importObject(this.Sandbox, this);
diff --git a/chrome/content/zotero/xpcom/translation/translate_item.js b/chrome/content/zotero/xpcom/translation/translate_item.js
@@ -158,18 +158,15 @@ Zotero.Translate.ItemSaver.prototype = {
}
}.bind(this));
- // Handle standalone attachments outside of the transaction, because they can involve downloading
+ // Handle attachments outside of the transaction, because they can involve downloading
for (let item of standaloneAttachments) {
let newItem = yield this._saveAttachment(item, null, attachmentCallback);
if (newItem) newItems.push(newItem);
}
- // Save child attachments afterwards, since we want to signal completion as soon as the main
- // items are saved
- var promise = Zotero.Promise.delay(1);
for (let a of childAttachments) {
// Workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=449811 (fixed in Fx51?)
let [item, parentItemID] = a;
- promise = promise.then(() => this._saveAttachment(item, parentItemID, attachmentCallback));
+ yield this._saveAttachment(item, parentItemID, attachmentCallback);
}
return newItems;
diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties
@@ -655,6 +655,7 @@ zotero.preferences.advanced.debug.error = An error occurred sending debug output
dragAndDrop.existingFiles = The following files already existed in the destination directory and were not copied:
dragAndDrop.filesNotFound = The following files were not found and could not be copied:
+fileInterface.importing = Importing…
fileInterface.importComplete = Import Complete
fileInterface.itemsWereImported = %1$S item was imported;%1$S items were imported
fileInterface.itemsExported = Exporting items…
