Dictionary importer refactoring (#1867)

* Remove map of schemas

* Don't reuse dictionary importer instances

* Refactor

* Update how progress callback is used

6 files changed, 38 insertions, 51 deletions

diff --git a/dev/translator-vm.js b/dev/translator-vm.js
index d616afc5..c826abec 100644
--- a/dev/translator-vm.js
+++ b/dev/translator-vm.js
@@ -76,15 +76,14 @@ class TranslatorVM extends DatabaseVM {
 
         // Setup database
         const dictionaryImporterMediaLoader = new DatabaseVMDictionaryImporterMediaLoader();
-        const dictionaryImporter = new DictionaryImporter(dictionaryImporterMediaLoader);
+        const dictionaryImporter = new DictionaryImporter(dictionaryImporterMediaLoader, null);
         const dictionaryDatabase = new DictionaryDatabase();
         await dictionaryDatabase.prepare();
 
         const {errors} = await dictionaryImporter.importDictionary(
             dictionaryDatabase,
             testDictionaryContent,
-            {prefixWildcardsSupported: true},
-            () => {}
+            {prefixWildcardsSupported: true}
         );
 
         assert.deepStrictEqual(errors.length, 0);
diff --git a/ext/js/language/dictionary-importer-threaded.js b/ext/js/language/dictionary-importer-threaded.js
index a251906b..c9b6fb24 100644
--- a/ext/js/language/dictionary-importer-threaded.js
+++ b/ext/js/language/dictionary-importer-threaded.js
@@ -20,7 +20,11 @@
  */
 
 class DictionaryImporterThreaded {
-    importDictionary(archiveContent, details, onProgress) {
+    constructor(onProgress) {
+        this._onProgress = onProgress;
+    }
+
+    importDictionary(archiveContent, details) {
         return new Promise((resolve, reject) => {
             const dictionaryImporterMediaLoader = new DictionaryImporterMediaLoader();
             const worker = new Worker('/js/language/dictionary-importer-worker-main.js', {});
@@ -30,13 +34,13 @@ class DictionaryImporterThreaded {
                     case 'complete':
                         worker.removeEventListener('message', onMessage);
                         worker.terminate();
-                        this._onComplete(params, resolve, reject);
+                        this._onMessageComplete(params, resolve, reject);
                         break;
                     case 'progress':
-                        this._onProgress(params, onProgress);
+                        this._onMessageProgress(params);
                         break;
                     case 'getImageResolution':
-                        this._onGetImageResolution(params, worker, dictionaryImporterMediaLoader);
+                        this._onMessageGetImageResolution(params, worker, dictionaryImporterMediaLoader);
                         break;
                 }
             };
@@ -50,7 +54,7 @@ class DictionaryImporterThreaded {
 
     // Private
 
-    _onComplete(params, resolve, reject) {
+    _onMessageComplete(params, resolve, reject) {
         const {error} = params;
         if (typeof error !== 'undefined') {
             reject(deserializeError(error));
@@ -59,19 +63,13 @@ class DictionaryImporterThreaded {
         }
     }
 
-    _formatResult(data) {
-        const {result, errors} = data;
-        const errors2 = errors.map((error) => deserializeError(error));
-        return {result, errors: errors2};
-    }
-
-    _onProgress(params, onProgress) {
-        if (typeof onProgress !== 'function') { return; }
+    _onMessageProgress(params) {
+        if (typeof this._onProgress !== 'function') { return; }
         const {args} = params;
-        onProgress(...args);
+        this._onProgress(...args);
     }
 
-    async _onGetImageResolution(params, worker, dictionaryImporterMediaLoader) {
+    async _onMessageGetImageResolution(params, worker, dictionaryImporterMediaLoader) {
         const {id, mediaType, content} = params;
         let response;
         try {
@@ -82,4 +80,10 @@ class DictionaryImporterThreaded {
         }
         worker.postMessage({action: 'getImageResolution.response', params: response});
     }
+
+    _formatResult(data) {
+        const {result, errors} = data;
+        const errors2 = errors.map((error) => deserializeError(error));
+        return {result, errors: errors2};
+    }
 }
diff --git a/ext/js/language/dictionary-importer-worker.js b/ext/js/language/dictionary-importer-worker.js
index f44a10f9..014b6542 100644
--- a/ext/js/language/dictionary-importer-worker.js
+++ b/ext/js/language/dictionary-importer-worker.js
@@ -64,8 +64,8 @@ class DictionaryImporterWorker {
     async _importDictionary(archiveContent, importDetails, onProgress) {
         const dictionaryDatabase = await this._getPreparedDictionaryDatabase();
         try {
-            const dictionaryImporter = new DictionaryImporter(this._mediaLoader);
-            const {result, errors} = await dictionaryImporter.importDictionary(dictionaryDatabase, archiveContent, importDetails, onProgress);
+            const dictionaryImporter = new DictionaryImporter(this._mediaLoader, onProgress);
+            const {result, errors} = await dictionaryImporter.importDictionary(dictionaryDatabase, archiveContent, importDetails);
             return {
                 result,
                 errors: errors.map((error) => serializeError(error))
diff --git a/ext/js/language/dictionary-importer.js b/ext/js/language/dictionary-importer.js
index 962758c3..f3e86654 100644
--- a/ext/js/language/dictionary-importer.js
+++ b/ext/js/language/dictionary-importer.js
@@ -22,12 +22,12 @@
  */
 
 class DictionaryImporter {
-    constructor(mediaLoader) {
+    constructor(mediaLoader, onProgress) {
         this._mediaLoader = mediaLoader;
-        this._schemas = new Map();
+        this._onProgress = onProgress;
     }
 
-    async importDictionary(dictionaryDatabase, archiveContent, details, onProgress) {
+    async importDictionary(dictionaryDatabase, archiveContent, details) {
         if (!dictionaryDatabase) {
             throw new Error('Invalid database');
         }
@@ -35,7 +35,7 @@ class DictionaryImporter {
             throw new Error('Database is not ready');
         }
 
-        const hasOnProgress = (typeof onProgress === 'function');
+        const hasOnProgress = (typeof this._onProgress === 'function');
 
         // Read archive
         const archive = await JSZip.loadAsync(archiveContent);
@@ -143,7 +143,7 @@ class DictionaryImporter {
 
                 loadedCount += count;
                 if (hasOnProgress) {
-                    onProgress(total, loadedCount);
+                    this._onProgress(total, loadedCount);
                 }
             }
         };
@@ -178,17 +178,6 @@ class DictionaryImporter {
     }
 
     async _getSchema(fileName) {
-        let schemaPromise = this._schemas.get(fileName);
-        if (typeof schemaPromise !== 'undefined') {
-            return schemaPromise;
-        }
-
-        schemaPromise = this._createSchema(fileName);
-        this._schemas.set(fileName, schemaPromise);
-        return schemaPromise;
-    }
-
-    async _createSchema(fileName) {
         const schema = await this._fetchJsonAsset(fileName);
         return new JsonSchema(schema);
     }
diff --git a/ext/js/pages/settings/dictionary-import-controller.js b/ext/js/pages/settings/dictionary-import-controller.js
index 5e51a48a..b18eeb6b 100644
--- a/ext/js/pages/settings/dictionary-import-controller.js
+++ b/ext/js/pages/settings/dictionary-import-controller.js
@@ -181,9 +181,9 @@ class DictionaryImportController {
     }
 
     async _importDictionary(file, importDetails, onProgress) {
-        const dictionaryImporter = new DictionaryImporterThreaded();
+        const dictionaryImporter = new DictionaryImporterThreaded(onProgress);
         const archiveContent = await this._readFile(file);
-        const {result, errors} = await dictionaryImporter.importDictionary(archiveContent, importDetails, onProgress);
+        const {result, errors} = await dictionaryImporter.importDictionary(archiveContent, importDetails);
         yomichan.api.triggerDatabaseUpdated('dictionary', 'import');
         const errors2 = await this._addDictionarySettings(result.sequenced, result.title);
 
diff --git a/test/test-database.js b/test/test-database.js
index ac7e825b..21c653be 100644
--- a/test/test-database.js
+++ b/test/test-database.js
@@ -41,9 +41,9 @@ function createTestDictionaryArchive(dictionary, dictionaryName) {
 }
 
 
-function createDictionaryImporter() {
+function createDictionaryImporter(onProgress) {
     const dictionaryImporterMediaLoader = new DatabaseVMDictionaryImporterMediaLoader();
-    return new DictionaryImporter(dictionaryImporterMediaLoader);
+    return new DictionaryImporter(dictionaryImporterMediaLoader, onProgress);
 }
 
 
@@ -131,7 +131,6 @@ async function testDatabase1() {
     ];
 
     // Setup database
-    const dictionaryImporter = createDictionaryImporter();
     const dictionaryDatabase = new DictionaryDatabase();
     await dictionaryDatabase.prepare();
 
@@ -146,13 +145,11 @@ async function testDatabase1() {
 
         // Import data
         let progressEvent = false;
+        const dictionaryImporter = createDictionaryImporter(() => { progressEvent = true; });
         const {result, errors} = await dictionaryImporter.importDictionary(
             dictionaryDatabase,
             testDictionarySource,
-            {prefixWildcardsSupported: true},
-            () => {
-                progressEvent = true;
-            }
+            {prefixWildcardsSupported: true}
         );
         vm.assert.deepStrictEqual(errors, []);
         vm.assert.deepStrictEqual(result, expectedSummary);
@@ -781,7 +778,6 @@ async function testDatabase2() {
     ]);
 
     // Setup database
-    const dictionaryImporter = createDictionaryImporter();
     const dictionaryDatabase = new DictionaryDatabase();
 
     // Error: not prepared
@@ -796,17 +792,17 @@ async function testDatabase2() {
     await assert.rejects(async () => await dictionaryDatabase.findTagForTitle('tag', title));
     await assert.rejects(async () => await dictionaryDatabase.getDictionaryInfo());
     await assert.rejects(async () => await dictionaryDatabase.getDictionaryCounts(titles, true));
-    await assert.rejects(async () => await dictionaryImporter.importDictionary(dictionaryDatabase, testDictionarySource, {}, () => {}));
+    await assert.rejects(async () => await createDictionaryImporter().importDictionary(dictionaryDatabase, testDictionarySource, {}));
 
     await dictionaryDatabase.prepare();
 
     // Error: already prepared
     await assert.rejects(async () => await dictionaryDatabase.prepare());
 
-    await dictionaryImporter.importDictionary(dictionaryDatabase, testDictionarySource, {}, () => {});
+    await createDictionaryImporter().importDictionary(dictionaryDatabase, testDictionarySource, {});
 
     // Error: dictionary already imported
-    await assert.rejects(async () => await dictionaryImporter.importDictionary(dictionaryDatabase, testDictionarySource, {}, () => {}));
+    await assert.rejects(async () => await createDictionaryImporter().importDictionary(dictionaryDatabase, testDictionarySource, {}));
 
     await dictionaryDatabase.close();
 }
@@ -823,7 +819,6 @@ async function testDatabase3() {
     ];
 
     // Setup database
-    const dictionaryImporter = createDictionaryImporter();
     const dictionaryDatabase = new DictionaryDatabase();
     await dictionaryDatabase.prepare();
 
@@ -833,7 +828,7 @@ async function testDatabase3() {
 
         let error = null;
         try {
-            await dictionaryImporter.importDictionary(dictionaryDatabase, testDictionarySource, {}, () => {});
+            await createDictionaryImporter().importDictionary(dictionaryDatabase, testDictionarySource, {});
         } catch (e) {
             error = e;
         }