commit e83bbbad4eda7ff29c334da48cb9eddf38f52c8f
parent 38df733a376f451226ef7128174876c9e875c392
Author: Adomas Venčkauskas <adomas.ven@gmail.com>
Date: Sun, 18 Sep 2016 12:24:55 +0300
Fix getTranslatorCode bugs
- /connector/getTranslatorCode used to respond with empty content
- CodeGetter froze connectors by issuing hundreds of simultanious XHR
requests
Diffstat:
9 files changed, 95 insertions(+), 43 deletions(-)
diff --git a/chrome/content/zotero/tools/testTranslators/testTranslators.js b/chrome/content/zotero/tools/testTranslators/testTranslators.js
@@ -24,8 +24,9 @@
*/
const NUM_CONCURRENT_TESTS = 6;
-const TRANSLATOR_TYPES = ["Web", "Import", "Export", "Search"];
const TABLE_COLUMNS = ["Translator", "Supported", "Status", "Pending", "Succeeded", "Failed", "Mismatch", "Issues"];
+// Not using const to prevent const collisions in connectors
+var TRANSLATOR_TYPES = ["Web", "Import", "Export", "Search"];
var translatorTables = {},
translatorTestViews = {},
translatorTestViewsToRun = {},
diff --git a/chrome/content/zotero/xpcom/connector/repo.js b/chrome/content/zotero/xpcom/connector/repo.js
@@ -56,7 +56,7 @@ Zotero.Repo = new function() {
* Get translator code from repository
* @param {String} translatorID ID of the translator to retrieve code for
*/
- this.getTranslatorCode = Zotero.Promise.method(function (translatorID) {
+ this.getTranslatorCode = Zotero.Promise.method(function (translatorID, debugMode) {
var deferred = Zotero.Promise.defer();
// try standalone
@@ -72,6 +72,11 @@ Zotero.Repo = new function() {
);
return;
}
+ // Don't fetch from repo in debug mode
+ if (debugMode) {
+ deferred.resolve([false, Zotero.Repo.SOURCE_ZOTERO_STANDALONE]);
+ return;
+ }
// then try repo
diff --git a/chrome/content/zotero/xpcom/connector/translator.js b/chrome/content/zotero/xpcom/connector/translator.js
@@ -130,7 +130,9 @@ Zotero.Translators = new function() {
if(!_initialized) Zotero.Translators.init()
var translators = _cache[type].slice(0);
var codeGetter = new Zotero.Translators.CodeGetter(translators, debugMode);
- return codeGetter.getAll();
+ return codeGetter.getAll().then(function() {
+ return translators;
+ });;
});
/**
@@ -326,25 +328,37 @@ Zotero.Translators = new function() {
Zotero.Translators.CodeGetter = function(translators, debugMode) {
this._translators = translators;
this._debugMode = debugMode;
-}
+ this._concurrency = 1;
+};
-Zotero.Translators.CodeGetter.prototype.getAll = Zotero.Promise.method(function () {
- var translators = [];
- for (let translator of this._translators) {
- // retrieve code if no code and translator is supported locally
- if((translator.runMode === Zotero.Translator.RUN_MODE_IN_BROWSER && !translator.hasOwnProperty("code"))
- // or if debug mode is enabled (even if unsupported locally)
- || (this._debugMode && (!translator.hasOwnProperty("code")
- // or if in debug mode and the code we have came from the repo (which doesn't
- // include test cases)
- || (Zotero.Repo && translator.codeSource === Zotero.Repo.SOURCE_REPO)))) {
- // get next translator
- translators.push(translator.getCode());
- }
+Zotero.Translators.CodeGetter.prototype.getCodeFor = Zotero.Promise.method(function(i) {
+ let translator = this._translators[i];
+ // retrieve code if no code and translator is supported locally
+ if((translator.runMode === Zotero.Translator.RUN_MODE_IN_BROWSER && !translator.hasOwnProperty("code"))
+ // or if debug mode is enabled (even if unsupported locally)
+ || (this._debugMode && (!translator.hasOwnProperty("code")
+ // or if in debug mode and the code we have came from the repo (which doesn't
+ // include test cases)
+ || (Zotero.Repo && translator.codeSource === Zotero.Repo.SOURCE_REPO)))) {
+ // get code
+ return translator.getCode();
}
- return Zotero.Promise.all(translators);
});
+Zotero.Translators.CodeGetter.prototype.getAll = function () {
+ var codes = [];
+ // Chain promises with some level of concurrency. If unchained, fires
+ // off hundreds of xhttprequests on connectors and crashes the extension
+ for (let i = 0; i < this._translators.length; i++) {
+ if (i < this._concurrency) {
+ codes.push(this.getCodeFor(i));
+ } else {
+ codes.push(codes[i-this._concurrency].then(() => this.getCodeFor(i)));
+ }
+ }
+ return Promise.all(codes);
+};
+
var TRANSLATOR_REQUIRED_PROPERTIES = ["translatorID", "translatorType", "label", "creator", "target",
"priority", "lastUpdated"];
var TRANSLATOR_PASSING_PROPERTIES = TRANSLATOR_REQUIRED_PROPERTIES
@@ -428,8 +442,8 @@ Zotero.Translator.prototype.init = function(info) {
*
* @return {Promise<String|false>} - Promise for translator code or false if none
*/
-Zotero.Translator.prototype.getCode = function () {
- return Zotero.Repo.getTranslatorCode(this.translatorID)
+Zotero.Translator.prototype.getCode = function (debugMode) {
+ return Zotero.Repo.getTranslatorCode(this.translatorID, debugMode)
.then(function (args) {
var code = args[0];
var source = args[1];
diff --git a/chrome/content/zotero/xpcom/server_connector.js b/chrome/content/zotero/xpcom/server_connector.js
@@ -618,7 +618,9 @@ Zotero.Server.Connector.GetTranslatorCode.prototype = {
*/
"init":function(postData, sendResponseCallback) {
var translator = Zotero.Translators.get(postData.translatorID);
- sendResponseCallback(200, "application/javascript", translator.code);
+ translator.getCode().then(function(code) {
+ sendResponseCallback(200, "application/javascript", code);
+ });
}
}
diff --git a/chrome/content/zotero/xpcom/translation/translate.js b/chrome/content/zotero/xpcom/translation/translate.js
@@ -1264,7 +1264,7 @@ Zotero.Translate.Base.prototype = {
// Zotero.Translators.get() returns a promise in the connectors, but we don't expect it to
// otherwise
- if (!this.isConnector && this.translator[0].then) {
+ if (!Zotero.isConnector && this.translator[0].then) {
throw new Error("Translator should not be a promise in non-connector mode");
}
diff --git a/chrome/content/zotero/xpcom/translation/translators.js b/chrome/content/zotero/xpcom/translation/translators.js
@@ -26,7 +26,7 @@
"use strict";
// Enumeration of types of translators
-const TRANSLATOR_TYPES = {"import":1, "export":2, "web":4, "search":8};
+var TRANSLATOR_TYPES = {"import":1, "export":2, "web":4, "search":8};
/**
* Singleton to handle loading and caching of translators
diff --git a/test/content/support.js b/test/content/support.js
@@ -753,6 +753,28 @@ var generateTranslatorExportData = Zotero.Promise.coroutine(function* generateTr
return translatorExportData;
});
+
+/**
+ * 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;
+}
+
+
/**
* Imports an attachment from a test file.
* @param {string} filename - The filename to import (in data directory)
diff --git a/test/tests/server_connectorTest.js b/test/tests/server_connectorTest.js
@@ -33,6 +33,34 @@ describe("Connector Server", function () {
after(function () {
win.close();
});
+
+
+ describe('/connector/getTranslatorCode', function() {
+ it('should respond with translator code', function* () {
+ var code = 'function detectWeb() {}\nfunction doImport() {}';
+ var translator = buildDummyTranslator(4, code);
+ sinon.stub(Zotero.Translators, 'get').returns(translator);
+
+ var response = yield Zotero.HTTP.request(
+ 'POST',
+ connectorServerPath + "/connector/getTranslatorCode",
+ {
+ headers: {
+ "Content-Type": "application/json"
+ },
+ body: JSON.stringify({
+ translatorID: "dummy-translator",
+ })
+ }
+ );
+
+ assert.isTrue(Zotero.Translators.get.calledWith('dummy-translator'));
+ assert.equal(response.response, code);
+
+ Zotero.Translators.get.restore();
+ })
+ });
+
describe("/connector/saveItems", function () {
// TODO: Test cookies
diff --git a/test/tests/translateTest.js b/test/tests/translateTest.js
@@ -2,26 +2,6 @@ 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
