OptionsUtil refactor / options default values (#807)

* Replace _readFile with _fetchAsset for consistency with Backend

* Fix error messages

* Make OptionsUtil non-static

* Update how default options are assigned

* Add createValidatingProxy

* Add validate, update _onApiSetAllSettings

* Remove unused api.optionsSchemaGet

* Remove Backend._optionsSchema

* Update OptionsUtil to create its own JsonSchemaValidator

* Rename Backend._optionsSchemaValidator

* Make getDefault non-async

ext/bg/js/backend.js

@@ -41,8 +41,7 @@ class Backend {
         this._mecab = new Mecab();
         this._clipboardMonitor = new ClipboardMonitor({getClipboard: this._onApiClipboardGet.bind(this)});
         this._options = null;
-        this._optionsSchema = null;
+        this._profileConditionsSchemaValidator = new JsonSchemaValidator();
-        this._optionsSchemaValidator = new JsonSchemaValidator();
         this._profileConditionsSchemaCache = [];
         this._profileConditionsUtil = new ProfileConditions();
         this._defaultAnkiFieldTemplates = null;
@@ -55,6 +54,7 @@ class Backend {
             requestBuilder: this._requestBuilder,
             useCache: false
         });
+        this._optionsUtil = new OptionsUtil();
 
         this._clipboardPasteTarget = null;
         this._clipboardPasteTargetInitialized = false;
@@ -78,7 +78,6 @@ class Backend {
 
         this._messageHandlers = new Map([
             ['requestBackendReadySignal',    {async: false, contentScript: true,  handler: this._onApiRequestBackendReadySignal.bind(this)}],
-            ['optionsSchemaGet',             {async: false, contentScript: true,  handler: this._onApiOptionsSchemaGet.bind(this)}],
             ['optionsGet',                   {async: false, contentScript: true,  handler: this._onApiOptionsGet.bind(this)}],
             ['optionsGetFull',               {async: false, contentScript: true,  handler: this._onApiOptionsGetFull.bind(this)}],
             ['optionsSave',                  {async: true,  contentScript: true,  handler: this._onApiOptionsSave.bind(this)}],
@@ -188,10 +187,9 @@ class Backend {
             }
             await this._translator.prepare();
 
-            this._optionsSchema = await this._fetchAsset('/bg/data/options-schema.json', true);
+            await this._optionsUtil.prepare();
             this._defaultAnkiFieldTemplates = (await this._fetchAsset('/bg/data/default-anki-field-templates.handlebars')).trim();
-            this._options = await OptionsUtil.load();
+            this._options = await this._optionsUtil.load();
-            this._options = this._optionsSchemaValidator.getValidValueOrDefault(this._optionsSchema, this._options);
 
             this._applyOptions('background');
 
@@ -222,7 +220,7 @@ class Backend {
 
     getFullOptions(useSchema=false) {
         const options = this._options;
-        return useSchema ? this._optionsSchemaValidator.createProxy(options, this._optionsSchema) : options;
+        return useSchema ? this._optionsUtil.createValidatingProxy(options) : options;
     }
 
     getOptions(optionsContext, useSchema=false) {
@@ -370,10 +368,6 @@ class Backend {
         }
     }
 
-    _onApiOptionsSchemaGet() {
-        return this._optionsSchema;
-    }
-
     _onApiOptionsGet({optionsContext}) {
         return this.getOptions(optionsContext);
     }
@@ -385,7 +379,7 @@ class Backend {
     async _onApiOptionsSave({source}) {
         this._clearProfileConditionsSchemaCache();
         const options = this.getFullOptions();
-        await OptionsUtil.save(options);
+        await this._optionsUtil.save(options);
         this._applyOptions(source);
     }
 
@@ -787,7 +781,8 @@ class Backend {
     }
 
     async _onApiSetAllSettings({value, source}) {
-        this._options = this._optionsSchemaValidator.getValidValueOrDefault(this._optionsSchema, value);
+        this._optionsUtil.validate(value);
+        this._options = clone(value);
         await this._onApiOptionsSave({source});
     }
 
@@ -1014,7 +1009,7 @@ class Backend {
                 this._profileConditionsSchemaCache.push(schema);
             }
 
-            if (conditionGroups.length > 0 && this._optionsSchemaValidator.isValid(optionsContext, schema)) {
+            if (conditionGroups.length > 0 && this._profileConditionsSchemaValidator.isValid(optionsContext, schema)) {
                 return profile;
             }
             ++index;
@@ -1025,7 +1020,7 @@ class Backend {
 
     _clearProfileConditionsSchemaCache() {
         this._profileConditionsSchemaCache = [];
-        this._optionsSchemaValidator.clearCache();
+        this._profileConditionsSchemaValidator.clearCache();
     }
 
     _checkLastError() {

ext/bg/js/json-schema.js

@@ -149,7 +149,7 @@ class JsonSchemaValidator {
     getValidValueOrDefault(schema, value) {
         let type = this._getValueType(value);
         const schemaType = schema.type;
-        if (!this._isValueTypeAny(value, type, schemaType)) {
+        if (typeof value === 'undefined' || !this._isValueTypeAny(value, type, schemaType)) {
             let assignDefault = true;
 
             const schemaDefault = schema.default;

ext/bg/js/options.js

@@ -15,8 +15,21 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
+/* global
+ * JsonSchemaValidator
+ */
+
 class OptionsUtil {
-    static async update(options) {
+    constructor() {
+        this._schemaValidator = new JsonSchemaValidator();
+        this._optionsSchema = null;
+    }
+
+    async prepare() {
+        this._optionsSchema = await this._fetchAsset('/bg/data/options-schema.json', true);
+    }
+
+    async update(options) {
         // Invalid options
         if (!isObject(options)) {
             options = {};
@@ -72,14 +85,14 @@ class OptionsUtil {
         return await this._applyUpdates(options, this._getVersionUpdates());
     }
 
-    static async load() {
+    async load() {
-        let options = null;
+        let options;
         try {
             const optionsStr = await new Promise((resolve, reject) => {
                 chrome.storage.local.get(['options'], (store) => {
                     const error = chrome.runtime.lastError;
                     if (error) {
-                        reject(new Error(error));
+                        reject(new Error(error.message));
                     } else {
                         resolve(store.options);
                     }
@@ -90,15 +103,21 @@ class OptionsUtil {
             // NOP
         }
 
-        return await this.update(options);
+        if (typeof options !== 'undefined') {
+            options = await this.update(options);
+        }
+
+        options = this._schemaValidator.getValidValueOrDefault(this._optionsSchema, options);
+
+        return options;
     }
 
-    static save(options) {
+    save(options) {
         return new Promise((resolve, reject) => {
             chrome.storage.local.set({options: JSON.stringify(options)}, () => {
                 const error = chrome.runtime.lastError;
                 if (error) {
-                    reject(new Error(error));
+                    reject(new Error(error.message));
                 } else {
                     resolve();
                 }
@@ -106,13 +125,21 @@ class OptionsUtil {
         });
     }
 
-    static async getDefault() {
+    getDefault() {
-        return await this.update({});
+        return this._schemaValidator.getValidValueOrDefault(this._optionsSchema);
+    }
+
+    createValidatingProxy(options) {
+        return this._schemaValidator.createProxy(options, this._optionsSchema);
+    }
+
+    validate(options) {
+        return this._schemaValidator.validate(options, this._optionsSchema);
     }
 
     // Legacy profile updating
 
-    static _legacyProfileUpdateGetUpdates() {
+    _legacyProfileUpdateGetUpdates() {
         return [
             null,
             null,
@@ -203,7 +230,7 @@ class OptionsUtil {
         ];
     }
 
-    static _legacyProfileUpdateGetDefaults() {
+    _legacyProfileUpdateGetDefaults() {
         return {
             general: {
                 enable: true,
@@ -302,7 +329,7 @@ class OptionsUtil {
         };
     }
 
-    static _legacyProfileUpdateAssignDefaults(options) {
+    _legacyProfileUpdateAssignDefaults(options) {
         const defaults = this._legacyProfileUpdateGetDefaults();
 
         const combine = (target, source) => {
@@ -323,7 +350,7 @@ class OptionsUtil {
         return options;
     }
 
-    static _legacyProfileUpdateUpdateVersion(options) {
+    _legacyProfileUpdateUpdateVersion(options) {
         const updates = this._legacyProfileUpdateGetUpdates();
         this._legacyProfileUpdateAssignDefaults(options);
 
@@ -345,20 +372,20 @@ class OptionsUtil {
 
     // Private
 
-    static async _addFieldTemplatesToOptions(options, additionSourceUrl) {
+    async _addFieldTemplatesToOptions(options, additionSourceUrl) {
         let addition = null;
         for (const {options: profileOptions} of options.profiles) {
             const fieldTemplates = profileOptions.anki.fieldTemplates;
             if (fieldTemplates !== null) {
                 if (addition === null) {
-                    addition = await this._readFile(additionSourceUrl);
+                    addition = await this._fetchAsset(additionSourceUrl);
                 }
                 profileOptions.anki.fieldTemplates = this._addFieldTemplatesBeforeEnd(fieldTemplates, addition);
             }
         }
     }
 
-    static async _addFieldTemplatesBeforeEnd(fieldTemplates, addition) {
+    async _addFieldTemplatesBeforeEnd(fieldTemplates, addition) {
         const pattern = /[ \t]*\{\{~?>\s*\(\s*lookup\s*\.\s*"marker"\s*\)\s*~?\}\}/;
         const newline = '\n';
         let replaced = false;
@@ -373,7 +400,7 @@ class OptionsUtil {
         return fieldTemplates;
     }
 
-    static async _readFile(url) {
+    async _fetchAsset(url, json=false) {
         url = chrome.runtime.getURL(url);
         const response = await fetch(url, {
             method: 'GET',
@@ -383,10 +410,13 @@ class OptionsUtil {
             redirect: 'follow',
             referrerPolicy: 'no-referrer'
         });
-        return await response.text();
+        if (!response.ok) {
+            throw new Error(`Failed to fetch ${url}: ${response.status}`);
+        }
+        return await (json ? response.json() : response.text());
     }
 
-    static _getStringHashCode(string) {
+    _getStringHashCode(string) {
         let hashCode = 0;
 
         if (typeof string !== 'string') { return hashCode; }
@@ -399,7 +429,7 @@ class OptionsUtil {
         return hashCode;
     }
 
-    static async _applyUpdates(options, updates) {
+    async _applyUpdates(options, updates) {
         const targetVersion = updates.length;
         let currentVersion = options.version;
 
@@ -417,7 +447,7 @@ class OptionsUtil {
         return options;
     }
 
-    static _getVersionUpdates() {
+    _getVersionUpdates() {
         return [
             {
                 async: false,
@@ -438,7 +468,7 @@ class OptionsUtil {
         ];
     }
 
-    static _updateVersion1(options) {
+    _updateVersion1(options) {
         // Version 1 changes:
         //  Added options.global.database.prefixWildcardsSupported = false.
         options.global = {
@@ -449,7 +479,7 @@ class OptionsUtil {
         return options;
     }
 
-    static _updateVersion2(options) {
+    _updateVersion2(options) {
         // Version 2 changes:
         //  Legacy profile update process moved into this upgrade function.
         for (const profile of options.profiles) {
@@ -461,14 +491,14 @@ class OptionsUtil {
         return options;
     }
 
-    static async _updateVersion3(options) {
+    async _updateVersion3(options) {
         // Version 3 changes:
         //  Pitch accent Anki field templates added.
         await this._addFieldTemplatesToOptions(options, '/bg/data/anki-field-templates-upgrade-v2.handlebars');
         return options;
     }
 
-    static async _updateVersion4(options) {
+    async _updateVersion4(options) {
         // Version 4 changes:
         //  Options conditions converted to string representations.
         //  Added usePopupWindow.

ext/bg/js/settings/backup-controller.js

@@ -26,9 +26,12 @@ class BackupController {
         this._settingsExportToken = null;
         this._settingsExportRevoke = null;
         this._currentVersion = 0;
+        this._optionsUtil = new OptionsUtil();
     }
 
-    prepare() {
+    async prepare() {
+        await this._optionsUtil.prepare();
+
         document.querySelector('#settings-export').addEventListener('click', this._onSettingsExportClick.bind(this), false);
         document.querySelector('#settings-import').addEventListener('click', this._onSettingsImportClick.bind(this), false);
         document.querySelector('#settings-import-file').addEventListener('change', this._onSettingsImportFileChange.bind(this), false);
@@ -140,7 +143,11 @@ class BackupController {
     // Importing
 
     async _settingsImportSetOptionsFull(optionsFull) {
-        await this._settingsController.setAllSettings(optionsFull);
+        try {
+            await this._settingsController.setAllSettings(optionsFull);
+        } catch (e) {
+            yomichan.logError(e);
+        }
     }
 
     _showSettingsImportError(error) {
@@ -322,7 +329,7 @@ class BackupController {
         }
 
         // Upgrade options
-        optionsFull = await OptionsUtil.update(optionsFull);
+        optionsFull = await this._optionsUtil.update(optionsFull);
 
         // Check for warnings
         const sanitizationWarnings = this._settingsImportSanitizeOptions(optionsFull, true);
@@ -368,7 +375,7 @@ class BackupController {
         $('#settings-reset-modal').modal('hide');
 
         // Get default options
-        const optionsFull = await OptionsUtil.getDefault();
+        const optionsFull = this._optionsUtil.getDefault();
 
         // Assign options
         await this._settingsImportSetOptionsFull(optionsFull);

ext/mixed/js/api.js

@@ -49,10 +49,6 @@ const api = (() => {
 
         // Invoke functions
 
-        optionsSchemaGet() {
-            return this._invoke('optionsSchemaGet');
-        }
-
         optionsGet(optionsContext) {
             return this._invoke('optionsGet', {optionsContext});
         }