From bde9a74f9db7bb529bed85ed250d7e720c72b6fc Mon Sep 17 00:00:00 2001 From: Dan Stillman <dstillman@zotero.org> Date: Fri, 25 Jun 2021 01:47:10 -0400 Subject: [PATCH] Clear caches properly when deleting duplicate translator Among other things, if the name of a translator was changed, until restart Zotero.Translators.getAllForType() would return the old cache entry pointing to a file that no longer existed. --- .../zotero/xpcom/translation/translators.js | 39 ++++++++-- test/tests/translatorsTest.js | 73 +++++++++++++++++++ 2 files changed, 107 insertions(+), 5 deletions(-) diff --git a/chrome/content/zotero/xpcom/translation/translators.js b/chrome/content/zotero/xpcom/translation/translators.js index 0344042fe7..83c87f6074 100644 --- a/chrome/content/zotero/xpcom/translation/translators.js +++ b/chrome/content/zotero/xpcom/translation/translators.js @@ -167,7 +167,7 @@ Zotero.Translators = new function() { + existingTranslator.fileName + " with same ID as " + translator.fileName); yield OS.File.remove(existingTranslator.path); - delete _translators[translator.translatorID]; + yield removeFromCaches(existingTranslator); } // If cached translator is newer or the same, delete the current one else { @@ -175,6 +175,7 @@ Zotero.Translators = new function() { + " with same ID is already loaded -- deleting " + translator.fileName); yield OS.File.remove(translator.path); + yield removeFromDBCache(translator.fileName); continue; } } @@ -184,11 +185,12 @@ Zotero.Translators = new function() { for (let type in Zotero.Translator.TRANSLATOR_TYPES) { if (translator.translatorType & Zotero.Translator.TRANSLATOR_TYPES[type]) { _cache[type].push(translator); - if ((translator.translatorType & Zotero.Translator.TRANSLATOR_TYPES.web) && translator.targetAll) { - _cache.webWithTargetAll.push(translator); - } } } + if ((translator.translatorType & Zotero.Translator.TRANSLATOR_TYPES.web) + && translator.targetAll) { + _cache.webWithTargetAll.push(translator); + } if (!dbCacheEntry) { yield this.cacheInDB( @@ -244,6 +246,33 @@ Zotero.Translators = new function() { }; + async function removeFromCaches({ translatorID, translatorType, targetAll, fileName }) { + await removeFromDBCache(fileName); + + delete _translators[translatorID]; + + for (let typeName in Zotero.Translator.TRANSLATOR_TYPES) { + if (translatorType & Zotero.Translator.TRANSLATOR_TYPES[typeName]) { + let pos = _cache[typeName].findIndex(x => x.translatorID == translatorID); + if (pos != -1) { + _cache[typeName].splice(pos, 1); + } + } + } + if ((translatorType & Zotero.Translator.TRANSLATOR_TYPES.web) && targetAll) { + let pos = _cache.webWithTargetAll.findIndex(x => x.translatorID == translatorID); + if (pos != -1) { + _cache.webWithTargetAll.splice(pos, 1); + } + } + } + + + async function removeFromDBCache(fileName) { + await Zotero.DB.queryAsync("DELETE FROM translatorCache WHERE fileName=?", fileName); + } + + /** * Loads a translator from JSON, with optional code * @@ -531,7 +560,7 @@ Zotero.Translators = new function() { "REPLACE INTO translatorCache VALUES (?, ?, ?)", [fileName, JSON.stringify(metadataJSON), lastModifiedTime] ); - } + }; this.makeTranslatorProvider = function (methods) { var requiredMethods = [ diff --git a/test/tests/translatorsTest.js b/test/tests/translatorsTest.js index b9b1ffe0d7..ca4db7f85a 100644 --- a/test/tests/translatorsTest.js +++ b/test/tests/translatorsTest.js @@ -1,6 +1,79 @@ "use strict"; describe("Zotero.Translators", function () { + describe("#init()", function () { + async function testUpdateCache({ translatorID, oldLabel, newLabel, oldLastUpdated, newLastUpdated }) { + var oldTranslator = buildDummyTranslator('web', `function doDetect() {}; function doSearch(); {}`, { + label: oldLabel, + translatorID, + translatorType: 8, + lastUpdated: oldLastUpdated + }); + await Zotero.Translators.save(oldTranslator.metadata, oldTranslator.code); + await Zotero.Translators.reinit(); + var matched = (await Zotero.Translators.getAllForType('search')) + .filter(x => x.translatorID == translatorID); + assert.lengthOf(matched, 1); + assert.equal(matched[0].label, oldLabel); + var oldPath = matched[0].path; + assert.isTrue(await OS.File.exists(oldPath)); + + var rows = await Zotero.DB.valueQueryAsync( + "SELECT COUNT(*) FROM translatorCache WHERE fileName=?", + oldTranslator.label + ".js" + ); + assert.equal(rows, 1); + + var newTranslator = buildDummyTranslator('web', `function doDetect() {}; function doSearch(); {}`, { + label: newLabel, + translatorID, + translatorType: 8, + lastUpdated: newLastUpdated + }); + await Zotero.Translators.save(newTranslator.metadata, newTranslator.code); + await Zotero.Translators.reinit(); + + matched = (await Zotero.Translators.getAllForType('search')) + .filter(x => x.translatorID == translatorID); + assert.lengthOf(matched, 1); + assert.equal(matched[0].label, newLabel); + assert.isFalse(await OS.File.exists(oldPath)); + assert.isTrue(await OS.File.exists(matched[0].path)); + + rows = await Zotero.DB.valueQueryAsync( + "SELECT COUNT(*) FROM translatorCache WHERE fileName=?", + oldTranslator.label + ".js" + ); + assert.equal(rows, 0); + + rows = await Zotero.DB.valueQueryAsync( + "SELECT COUNT(*) FROM translatorCache WHERE fileName=?", + newTranslator.label + ".js" + ); + assert.equal(rows, 1); + } + + it("should update cache when deleting old translator with same id", async function () { + await testUpdateCache({ + translatorID: 'f678741b-b82d-48a3-a7c2-26764d08cc34', + oldLabel: 'Old Test Translator', + newLabel: 'New Test Translator', + oldLastUpdated: '2021-06-25 00:00:00', + newLastUpdated: '2021-06-25 00:05:00', + }); + }); + + it("should update cache when deleting second translator with same id and timestamp", async function () { + await testUpdateCache({ + translatorID: '4bff4da8-b3f6-47dc-957a-dcb96dc48d4f', + oldLabel: 'Old Test Translator 2', + newLabel: 'New Test Translator 2', + oldLastUpdated: '2021-06-25 00:00:00', + newLastUpdated: '2021-06-25 00:00:00', + }); + }); + }); + describe("#getWebTranslatorsForLocation()", function () { var genericTranslator, topLevelTranslator, frameTranslator; var noMatchURL = 'http://notowls.com/citation/penguin-migration-patterns';