From 4d94810defe0c24f9bb82b43391ecc8ed26dac86 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 4 Sep 2026 13:02:37 -0400 Subject: [PATCH] Don't send a conditional request for an invalidated retractions cache The cached retractions list is saved with the current data version from the code, which we can bump to make clients discard their cached data and refetch it. We've never actually done that, but if we did, it would've hit a bug: init() would've started the refetch but still sent the cached ETag, so if the list itself hadn't changed, the server would've returned 304 and nothing would have been updated. The stale version then would have survived, so the refetch would have been retried on every startup. --- chrome/content/zotero/xpcom/retractions.js | 5 +- test/tests/retractionsTest.js | 58 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/retractions.js b/chrome/content/zotero/xpcom/retractions.js index 89f8a47cbf..32e4d561b9 100644 --- a/chrome/content/zotero/xpcom/retractions.js +++ b/chrome/content/zotero/xpcom/retractions.js @@ -525,9 +525,10 @@ Zotero.Retractions = { return; } - // Download list + // Download list. Skip the conditional request if the cached list was saved by a + // different version, so that a version bump forces a full refresh of stored data. var headers = {}; - if (this._cacheETag) { + if (this._cacheETag && this._cacheVersion == this._version) { headers["If-None-Match"] = this._cacheETag; } var req = await Zotero.HTTP.request( diff --git a/test/tests/retractionsTest.js b/test/tests/retractionsTest.js index 434e280c9f..a575bd54d7 100644 --- a/test/tests/retractionsTest.js +++ b/test/tests/retractionsTest.js @@ -199,6 +199,64 @@ describe("Retractions", function () { assert.isFalse(Zotero.Retractions.isRetracted(item)); }); + + it("should refresh stored data for a version bump when the list is unchanged", async function () { + var doi = '10.1234/cdefg'; + var hash = Zotero.Utilities.Internal.sha1(doi); + var line = Zotero.Retractions.TYPE_DOI + hash.substr(0, 5) + ' 12345\n'; + var version = Zotero.Retractions._version; + var reason = "Error in Data"; + + server.respond(function (req) { + if (req.method == 'GET' && req.url == baseURL + 'list') { + // The list itself never changes, so a conditional request gets a 304 + if (req.requestHeaders['If-None-Match'] == 'unchanged') { + req.respond(304, {}, ''); + return; + } + req.respond(200, { 'Content-Type': 'text/plain', 'ETag': 'unchanged' }, line); + } + else if (req.method == 'POST' && req.url == baseURL + 'search') { + req.respond( + 200, + { 'Content-Type': 'application/json' }, + JSON.stringify([ + { + doi: hash, + retractionDOI: '10.1234/defgh', + date: '2019-01-02', + reasons: [reason], + urls: [] + } + ]) + ); + } + }); + + try { + await Zotero.Retractions.updateFromServer(); + + let promise = waitForItemEvent('refresh'); + let item = createUnsavedDataObject('item', { itemType: 'journalArticle' }); + item.setField('DOI', doi); + await item.saveTx(); + await promise; + assert.sameMembers((await Zotero.Retractions.getData(item)).reasons, [reason]); + + // An update with the same version gets a 304 and leaves the stored data alone + reason = "Error in Text"; + await Zotero.Retractions.updateFromServer(); + assert.sameMembers((await Zotero.Retractions.getData(item)).reasons, ["Error in Data"]); + + // A version bump skips the conditional request and rewrites it + Zotero.Retractions._version = version + 1; + await Zotero.Retractions.updateFromServer(); + assert.sameMembers((await Zotero.Retractions.getData(item)).reasons, [reason]); + } + finally { + Zotero.Retractions._version = version; + } + }); });