From 3fc38d750bc0b9efe3273f157019103ba3d52300 Mon Sep 17 00:00:00 2001 From: Simon Kornblith Date: Thu, 4 Jun 2015 00:47:58 -0400 Subject: [PATCH] Use Zotero.Item.fromJSON() for saving from translators Also: - Move some canonicalization of items returned by translators to Zotero.Translate - Make Zotero.Translate#translate return a promise - Add tests --- .../zotero/xpcom/translation/translate.js | 148 +++-- .../xpcom/translation/translate_item.js | 542 ++++++++--------- test/runtests.sh | 1 + test/tests/data/snapshot/index.html | 16 + test/tests/translateTest.js | 552 +++++++++++++++++- 5 files changed, 921 insertions(+), 338 deletions(-) create mode 100644 test/tests/data/snapshot/index.html diff --git a/chrome/content/zotero/xpcom/translation/translate.js b/chrome/content/zotero/xpcom/translation/translate.js index 60b7672122..6ae3e08b0f 100644 --- a/chrome/content/zotero/xpcom/translation/translate.js +++ b/chrome/content/zotero/xpcom/translation/translate.js @@ -90,14 +90,13 @@ Zotero.Translate.Sandbox = { const allowedObjects = ["complete", "attachments", "seeAlso", "creators", "tags", "notes"]; - delete item.complete; + // Create a new object here, so that we strip the "complete" property + // (But don't create a new object if we're in a child translator, since that + // would be a problem for the sandbox) + var newItem = translate._parentTranslator ? item : {}; for(var i in item) { var val = item[i]; - if(!val && val !== 0) { - // remove null, undefined, and false properties, and convert objects to strings - delete item[i]; - continue; - } + if(i === "complete" || (!val && val !== 0)) continue; var type = typeof val; var isObject = type === "object" || type === "xml" || type === "function", @@ -105,15 +104,18 @@ Zotero.Translate.Sandbox = { if(isObject && !shouldBeObject) { // Convert things that shouldn't be objects to objects translate._debug("Translate: WARNING: typeof "+i+" is "+type+"; converting to string"); - item[i] = val.toString(); + newItem[i] = val.toString(); } else if(shouldBeObject && !isObject) { translate._debug("Translate: WARNING: typeof "+i+" is "+type+"; converting to array"); - item[i] = [val]; + newItem[i] = [val]; } else if(type === "string") { // trim strings - item[i] = val.trim(); + newItem[i] = val.trim(); + } else { + newItem[i] = val; } } + item = newItem; // Clean empty creators if (item.creators) { @@ -125,6 +127,9 @@ Zotero.Translate.Sandbox = { } } } + + // Canonicalize tags + 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 @@ -137,15 +142,35 @@ Zotero.Translate.Sandbox = { // We use this within the connector to keep track of items as they are saved if(!item.id) item.id = Zotero.Utilities.randomString(); - // don't save documents as documents in connector, since we can't pass them around - if(Zotero.isConnector) { + if(item.attachments) { var attachments = item.attachments; - var nAttachments = attachments.length; - for(var j=0; jlibraryID - ID of library in which items should be saved + *
  • attachmentMode - One of Zotero.Translate.ItemSaver.ATTACHMENT_* specifying how attachments should be saved
    • + *
  • forceTagType - Force tags to specified tag type
    • + *
  • cookieSandbox - Cookie sandbox for attachment requests
    • + *
  • baseURI - URI to which attachment paths should be relative
    • + */ +Zotero.Translate.ItemSaver = function(options) { // initialize constants - this.newItems = []; - this.newCollections = []; this._IDMap = {}; // determine library ID - if(libraryID === false) { - this._libraryID = false; - } else if(libraryID === true || libraryID == undefined) { - this._libraryID = null; + if(!options.libraryID) { + this._libraryID = Zotero.Libraries.userLibraryID; } else { - this._libraryID = libraryID; + this._libraryID = options.libraryID; } - this.attachmentMode = attachmentMode; - // If group filesEditable==false, don't save attachments - if (typeof this._libraryID == 'number') { - var type = Zotero.Libraries.getType(this._libraryID); - switch (type) { - case 'group': - var groupID = Zotero.Groups.getGroupIDFromLibraryID(this._libraryID); - var group = Zotero.Groups.get(groupID); - if (!group.filesEditable) { - this.attachmentMode = Zotero.Translate.ItemSaver.ATTACHMENT_MODE_IGNORE; - } - break; - } - } - - // force tag types if requested - this._forceTagType = forceTagType; - // to set cookies on downloaded files - this._cookieSandbox = cookieSandbox; + this.attachmentMode = Zotero.Libraries.isFilesEditable(this._libraryID) ? options.attachmentMode : + Zotero.Translate.ItemSaver.ATTACHMENT_MODE_IGNORE; + this._forceTagType = options.forceTagType; + this._cookieSandbox = options.cookieSandbox; // the URI to which other URIs are assumed to be relative if(typeof baseURI === "object" && baseURI instanceof Components.interfaces.nsIURI) { - this._baseURI = baseURI; + this._baseURI = options.baseURI; } else { // try to convert to a URI - this._baseURI = null; try { this._baseURI = Components.classes["@mozilla.org/network/io-service;1"]. - getService(Components.interfaces.nsIIOService).newURI(baseURI, null, null); + getService(Components.interfaces.nsIIOService).newURI(options.baseURI, null, null); } catch(e) {}; } }; @@ -83,130 +73,146 @@ Zotero.Translate.ItemSaver.prototype = { * @param items Items in Zotero.Item.toArray() format * @param {Function} callback A callback to be executed when saving is complete. If saving * succeeded, this callback will be passed true as the first argument and a list of items - * saved as the second. If - saving failed, the callback will be passed false as the first + * saved as the second. If saving failed, the callback will be passed false as the first * argument and an error object as the second * @param {Function} [attachmentCallback] A callback that receives information about attachment * save progress. The callback will be called as attachmentCallback(attachment, false, error) * on failure or attachmentCallback(attachment, progressPercent) periodically during saving. */ "saveItems":function(items, callback, attachmentCallback) { - Zotero.DB.executeTransaction(function* () { + Zotero.spawn(function* () { try { - var newItems = []; - for each(var item in items) { - // 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; - if(item.note) newItem.setNote(item.note); - var myID = newItem.save(); - newItem = Zotero.Items.get(myID); - } else { - if(type == "attachment") { // handle attachments differently - newItem = this._saveAttachment(item, null, attachmentCallback); - if(!newItem) continue; - var myID = newItem.id; + let newItems = [], standaloneAttachments = []; + yield (Zotero.DB.executeTransaction(function* () { + for (let iitem=0; iitem"); - } else if (!attachment.url && asUrl) { + } + + if (!attachment.url && asUrl) { Zotero.debug("Translate: attachment path looks like a URI: " + attachment.path); attachment.url = asUrl; delete attachment.path; } - } else { - if (attachment.url) { - attachment.linkMode = "imported_url"; - newItem = yield Zotero.Attachments.importSnapshotFromFile({ - file: file, - url: attachment.url, - title: attachment.title, - contentType: attachment.mimeType, - charset: attachment.charset, - parentItemID: parentID - }); - } - else { - attachment.linkMode = "imported_file"; - newItem = yield Zotero.Attachments.importFromFile({ - file: file, - parentItemID: parentID - }); - } - done = true; } - } - - if(!done) { + let url = Zotero.Attachments.cleanAttachmentURI(attachment.url); if (!url) { throw new Error("Translate: Invalid attachment.url specified <" + attachment.url + ">"); } - + attachment.url = url; url = Components.classes["@mozilla.org/network/io-service;1"] .getService(Components.interfaces.nsIIOService) .newURI(url, null, null); // This cannot fail, since we check above - + // see if this is actually a file URL if(url.scheme == "file") { throw new Error("Translate: Local file attachments cannot be specified in attachment.url"); } else if(url.scheme != "http" && url.scheme != "https") { throw new Error("Translate: " + url.scheme + " protocol is not allowed for attachments from translators."); } - + // At this point, must be a valid HTTP/HTTPS url attachment.linkMode = "linked_file"; newItem = yield Zotero.Attachments.linkFromURL({ @@ -339,7 +318,27 @@ Zotero.Translate.ItemSaver.prototype = { parentItemID: parentID, contentType: attachment.mimeType || undefined, title: attachment.title || undefined - }) + }); + } else { + if (attachment.url) { + attachment.linkMode = "imported_url"; + newItem = yield Zotero.Attachments.importSnapshotFromFile({ + file: file, + url: attachment.url, + title: attachment.title, + contentType: attachment.mimeType, + charset: attachment.charset, + parentItemID: parentID + }); + } + else { + attachment.linkMode = "imported_file"; + newItem = yield Zotero.Attachments.importFromFile({ + file: file, + parentItemID: parentID + }); + if (attachment.title) newItem.setField("title", attachment.title); + } } return newItem; @@ -562,8 +561,11 @@ Zotero.Translate.ItemSaver.prototype = { // Import from URL let mimeType = attachment.mimeType ? attachment.mimeType : null; - let parentItem = yield Zotero.Items.getAsync(parentID); - let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem); + let fileBaseName; + if (parentID) { + let parentItem = yield Zotero.Items.getAsync(parentID); + fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem); + } Zotero.debug('Importing attachment from URL'); attachment.linkMode = "imported_url"; @@ -578,125 +580,61 @@ Zotero.Translate.ItemSaver.prototype = { fileBaseName: fileBaseName, contentType: mimeType, cookieSandbox: this._cookieSandbox - }) + }); }), - "_saveFields":function(item, newItem) { - // fields that should be handled differently - const skipFields = ["note", "notes", "itemID", "attachments", "tags", "seeAlso", - "itemType", "complete", "creators"]; - - var typeID = Zotero.ItemTypes.getID(item.itemType); - var fieldID; - for(var field in item) { - // loop through item fields - if(item[field] && skipFields.indexOf(field) === -1 && (fieldID = Zotero.ItemFields.getID(field))) { - // if field is in db and shouldn't be skipped - - // try to map from base field - if(Zotero.ItemFields.isBaseField(fieldID)) { - fieldID = Zotero.ItemFields.getFieldIDFromTypeAndBase(typeID, fieldID); - - // Skip mapping if item field already exists - var fieldName = Zotero.ItemFields.getName(fieldID); - if(fieldName !== field && item[fieldName]) continue; - - if(fieldID) { - Zotero.debug("Translate: Mapping "+field+" to "+fieldName, 5); - } - } - - // if field is valid for this type, set field - if(fieldID && Zotero.ItemFields.isValidForType(fieldID, typeID)) { - newItem.setField(fieldID, item[field]); - } else { - Zotero.debug("Translate: Discarded field "+field+" for item: field not valid for type "+item.itemType, 3); - } - } + "_saveNote":Zotero.Promise.coroutine(function* (note, parentID) { + var myNote = new Zotero.Item('note'); + myNote.libraryID = this._libraryID; + if(parentID) { + myNote.parentID = parentID; } + + if(typeof note == "object") { + myNote.setNote(note.note); + if(note.tags) myNote.setTags(this._cleanTags(note.tags)); + this._handleRelated(note, myNote); + } else { + myNote.setNote(note); + } + yield myNote.save(); + return myNote; + }), + + /** + * Remove automatic tags if automatic tags pref is on, and set type + * to automatic if forced + */ + "_cleanTags":function(tags) { + // If all tags are automatic and automatic tags pref is on, return immediately + let tagPref = Zotero.Prefs.get("automaticTags"); + if(this._forceTagType == 1 && !tagPref) return []; + + let newTags = []; + for(let i=0; i 1) { // Attachment is a snapshot with supporting files. Check if any of the diff --git a/test/runtests.sh b/test/runtests.sh index 16d0cc960e..4a36c237a0 100755 --- a/test/runtests.sh +++ b/test/runtests.sh @@ -105,6 +105,7 @@ user_pref("extensions.zotero.debug.time", $DEBUG); user_pref("extensions.zotero.firstRunGuidance", false); user_pref("extensions.zotero.firstRun2", false); user_pref("extensions.zotero.reportTranslationFailure", false); +user_pref("extensions.zotero.httpServer.enabled", true); EOF # -v flag on Windows makes Firefox process hang diff --git a/test/tests/data/snapshot/index.html b/test/tests/data/snapshot/index.html new file mode 100644 index 0000000000..6ae791eae3 --- /dev/null +++ b/test/tests/data/snapshot/index.html @@ -0,0 +1,16 @@ + + + + + + + + + + +

    It works!

    +

    This is the default web page for this server.

    +

    The web server software is running but no content has been added, yet.

    + + + diff --git a/test/tests/translateTest.js b/test/tests/translateTest.js index 0a5e7985ad..7a47b4769f 100644 --- a/test/tests/translateTest.js +++ b/test/tests/translateTest.js @@ -1,5 +1,551 @@ +new function() { Components.utils.import("resource://gre/modules/osfile.jsm"); +/** + * Build a dummy translator that can be passed to Zotero.Translate + */ +function buildDummyTranslator(translatorType, code) { + let info = { + "translatorID":"dummy-translator", + "translatorType":1, // import + "label":"Dummy Translator", + "creator":"Simon Kornblith", + "target":"", + "priority":100, + "browserSupport":"g", + "inRepository":false, + "lastUpdated":"0000-00-00 00:00:00", + }; + let translator = new Zotero.Translator(info); + translator.code = code; + return translator; +} + +/** + * Create a new translator that saves the specified items + * @param {String} translatorType - "import" or "web" + * @param {Object} items - items as translator JSON + */ +function saveItemsThroughTranslator(translatorType, items) { + let tyname; + if (translatorType == "web") { + tyname = "Web"; + } else if (translatorType == "import") { + tyname = "Import"; + } else { + throw new Error("invalid translator type "+translatorType); + } + + let translate = new Zotero.Translate[tyname](); + let browser; + if (translatorType == "web") { + browser = Zotero.Browser.createHiddenBrowser(); + translate.setDocument(browser.contentDocument); + } else if (translatorType == "import") { + translate.setString(""); + } + translate.setTranslator(buildDummyTranslator(translatorType == "web" ? 4 : 1, + "function detectWeb() {}\n"+ + "function do"+tyname+"() {\n"+ + " var json = JSON.parse('"+JSON.stringify(items).replace(/'/g, "\'")+"');\n"+ + " for (var i=0; i