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.
This commit is contained in:
Dan Stillman 2017-02-08 14:44:21 -05:00
parent 3e35764405
commit 27cb099c82
4 changed files with 201 additions and 136 deletions

View file

@ -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);
});
/*

View file

@ -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;
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;
// 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);
}
}
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--;
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;
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;
}
}
}
// 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;
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--;
}
}
}
translate._runHandler("itemDone", item, item);
// 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;
}
let maybePromise = translate._runHandler("itemDone", item, item);
// DEBUG: Is this ever necessary?
if (maybePromise) {
return resolve ? maybePromise.then(resolve) : maybePromise;
}
if (resolve) {
resolve();
}
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 in a child translator, canonicalize tags
if (!translate._parentTranslator) {
if(attachment.tags !== undefined) attachment.tags = translate._cleanTags(attachment.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);
}
}
// 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;
}
// 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);
}
return new translate._sandboxManager.sandbox.Promise(function (resolve, reject) {
try {
run(resolve);
}
}
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);
}
catch (e) {
reject(e);
}
}
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);
// 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++;
});
},
/**
@ -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);

View file

@ -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;

View file

@ -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…