From 10181f7f56b1aa2b9f3d14a8603a09ad94fe8ade Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 30 Apr 2016 01:10:12 -0400 Subject: [PATCH] Fix sync error on missing full-text If a version is returned for an item's full-text content but a 404 is returned for the content itself (because it's missing in Elasticsearch for some reason), don't throw an error. Also remove legacy array comprehensions in fulltext and syncFullTextEngine test files, which apparently weren't being run. --- .../zotero/xpcom/sync/syncAPIClient.js | 5 +- .../zotero/xpcom/sync/syncFullTextEngine.js | 5 +- test/tests/fulltextTest.js | 2 +- test/tests/syncFullTextEngineTest.js | 48 +++++++++++++++++-- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/chrome/content/zotero/xpcom/sync/syncAPIClient.js b/chrome/content/zotero/xpcom/sync/syncAPIClient.js index 1b11c3623..e21f68f25 100644 --- a/chrome/content/zotero/xpcom/sync/syncAPIClient.js +++ b/chrome/content/zotero/xpcom/sync/syncAPIClient.js @@ -437,7 +437,10 @@ Zotero.Sync.APIClient.prototype = { target: `items/${itemKey}/fulltext` }; var uri = this.buildRequestURI(params); - var xmlhttp = yield this.makeRequest("GET", uri); + var xmlhttp = yield this.makeRequest("GET", uri, { successCodes: [200, 404] }); + if (xmlhttp.status == 404) { + return false; + } var version = xmlhttp.getResponseHeader('Last-Modified-Version'); if (!version) { throw new Error("Last-Modified-Version not provided"); diff --git a/chrome/content/zotero/xpcom/sync/syncFullTextEngine.js b/chrome/content/zotero/xpcom/sync/syncFullTextEngine.js index 66a20881c..72b7b155d 100644 --- a/chrome/content/zotero/xpcom/sync/syncFullTextEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncFullTextEngine.js @@ -71,7 +71,7 @@ Zotero.Sync.Data.FullTextEngine.prototype._download = Zotero.Promise.coroutine(f for (let key in results.versions) { let id = Zotero.Items.getIDFromLibraryAndKey(this.libraryID, key); if (!id) { - Zotero.debug(`Skipping full-text for missing item ${this.libraryID}/${key}`); + Zotero.debug(`Skipping full-text for missing item ${this.library.name}`); continue; } @@ -79,7 +79,7 @@ Zotero.Sync.Data.FullTextEngine.prototype._download = Zotero.Promise.coroutine(f // interrupted sync let version = yield Zotero.Fulltext.getItemVersion(id); if (version == results.versions[key]) { - Zotero.debug(`Skipping up-to-date full text for ${this.libraryKey}`); + Zotero.debug(`Skipping up-to-date full text for ${this.library.name}`); continue; } keys.push(key); @@ -94,6 +94,7 @@ Zotero.Sync.Data.FullTextEngine.prototype._download = Zotero.Promise.coroutine(f this.library.libraryType, this.library.libraryTypeID, key ) .then(function (results) { + if (!results) return; return Zotero.Fulltext.setItemContent( this.libraryID, tmpKey, results.data, results.version ) diff --git a/test/tests/fulltextTest.js b/test/tests/fulltextTest.js index 1bfedeba0..3f596a96f 100644 --- a/test/tests/fulltextTest.js +++ b/test/tests/fulltextTest.js @@ -167,7 +167,7 @@ describe("Zotero.Fulltext", function () { yield Zotero.Attachments.createDirectoryForItem(attachment); let path = attachment.getFilePath(); - let content = [Zotero.Utilities.randomString() for (x of new Array(10))].join(" "); + let content = new Array(10).fill("").map(x => Zotero.Utilities.randomString()).join(" "); yield Zotero.File.putContentsAsync(path, content); if (!options.skip) { diff --git a/test/tests/syncFullTextEngineTest.js b/test/tests/syncFullTextEngineTest.js index 115c0cd5c..7deb20bbc 100644 --- a/test/tests/syncFullTextEngineTest.js +++ b/test/tests/syncFullTextEngineTest.js @@ -39,6 +39,10 @@ describe("Zotero.Sync.Data.FullTextEngine", function () { setHTTPResponse(server, baseURL, response, responses); } + function generateContent() { + return new Array(10).fill("").map(x => Zotero.Utilities.randomString()).join(" "); + } + // // Tests // @@ -66,7 +70,7 @@ describe("Zotero.Sync.Data.FullTextEngine", function () { attachment.attachmentFilename = 'test.pdf'; yield attachment.saveTx(); - var content = [Zotero.Utilities.randomString() for (x of new Array(10))].join(" "); + var content = generateContent() var spy = sinon.spy(Zotero.Fulltext, "startContentProcessor") var itemFullTextVersion = 10; @@ -126,7 +130,7 @@ describe("Zotero.Sync.Data.FullTextEngine", function () { attachment.attachmentFilename = 'test.pdf'; yield attachment.saveTx(); - content = [Zotero.Utilities.randomString() for (x of new Array(10))].join(" "); + content = generateContent() spy = sinon.spy(Zotero.Fulltext, "startContentProcessor") itemFullTextVersion = 17; @@ -175,6 +179,42 @@ describe("Zotero.Sync.Data.FullTextEngine", function () { spy.restore(); }) + it("should handle remotely missing full-text content", function* () { + ({ engine, client, caller } = yield setup()); + + var item = yield createDataObject('item'); + var attachment = new Zotero.Item('attachment'); + attachment.parentItemID = item.id; + attachment.attachmentLinkMode = 'imported_file'; + attachment.attachmentContentType = 'application/pdf'; + attachment.attachmentFilename = 'test.pdf'; + yield attachment.saveTx(); + + var itemFullTextVersion = 10; + var libraryFullTextVersion = 15; + setResponse({ + method: "GET", + url: "users/1/fulltext", + status: 200, + headers: { + "Last-Modified-Version": libraryFullTextVersion + }, + json: { + [attachment.key]: itemFullTextVersion + } + }); + setResponse({ + method: "GET", + url: `users/1/items/${attachment.key}/fulltext`, + status: 404, + headers: { + "Last-Modified-Version": itemFullTextVersion + }, + text: "" + }); + yield engine.start(); + }) + it("should upload new full-text content and subsequent updates", function* () { // https://github.com/cjohansen/Sinon.JS/issues/607 var fixSinonBug = ";charset=utf-8"; @@ -196,7 +236,7 @@ describe("Zotero.Sync.Data.FullTextEngine", function () { yield Zotero.Attachments.createDirectoryForItem(attachment); var path = attachment.getFilePath(); - var content = [Zotero.Utilities.randomString() for (x of new Array(10))].join(" "); + var content = generateContent() var htmlContent = "" + content + ""; yield Zotero.File.putContentsAsync(path, content); yield Zotero.Fulltext.indexItems([attachment.id]); @@ -268,7 +308,7 @@ describe("Zotero.Sync.Data.FullTextEngine", function () { yield Zotero.Attachments.createDirectoryForItem(attachment); path = attachment.getFilePath(); - content = [Zotero.Utilities.randomString() for (x of new Array(10))].join(" "); + content = generateContent() htmlContent = "" + content + ""; yield Zotero.File.putContentsAsync(path, content); yield Zotero.Fulltext.indexItems([attachment.id]);