From dfdefc15d31b401092760926756150a839f52487 Mon Sep 17 00:00:00 2001
From: toasted-nutbread <toasted-nutbread@users.noreply.github.com>
Date: Tue, 27 Oct 2020 19:26:30 -0400
Subject: [PATCH] Options fixes (#962)

* Fix default options not having the correct versions

* Move schema validation

* Remove legacy version number

* Add tests for OptionsUtil.getDefault()

* Remove unused getValidValueOrDefault
---
 ext/bg/data/options-schema.json         |  5 ----
 ext/bg/js/options.js                    | 34 +++++++++++++++++++------
 ext/bg/js/settings/backup-controller.js |  1 -
 test/test-options-util.js               | 30 +++++++++++++++++++++-
 4 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/ext/bg/data/options-schema.json b/ext/bg/data/options-schema.json
index c0a39ea2..d344a7b9 100644
--- a/ext/bg/data/options-schema.json
+++ b/ext/bg/data/options-schema.json
@@ -62,7 +62,6 @@
                     "options": {
                         "type": "object",
                         "required": [
-                            "version",
                             "general",
                             "audio",
                             "scanning",
@@ -72,10 +71,6 @@
                             "anki"
                         ],
                         "properties": {
-                            "version": {
-                                "type": "integer",
-                                "minimum": 0
-                            },
                             "general": {
                                 "type": "object",
                                 "required": [
diff --git a/ext/bg/js/options.js b/ext/bg/js/options.js
index 9e89e9bd..668be98e 100644
--- a/ext/bg/js/options.js
+++ b/ext/bg/js/options.js
@@ -82,7 +82,13 @@ class OptionsUtil {
         }
 
         // Generic updates
-        return await this._applyUpdates(options, this._getVersionUpdates());
+        options = await this._applyUpdates(options, this._getVersionUpdates());
+
+        // Validation
+        options = this._schemaValidator.getValidValueOrDefault(this._optionsSchema, options);
+
+        // Result
+        return options;
     }
 
     async load() {
@@ -105,10 +111,10 @@ class OptionsUtil {
 
         if (typeof options !== 'undefined') {
             options = await this.update(options);
+        } else {
+            options = this.getDefault();
         }
 
-        options = this._schemaValidator.getValidValueOrDefault(this._optionsSchema, options);
-
         return options;
     }
 
@@ -126,11 +132,10 @@ class OptionsUtil {
     }
 
     getDefault() {
-        return this._schemaValidator.getValidValueOrDefault(this._optionsSchema);
-    }
-
-    getValidValueOrDefault(options) {
-        return this._schemaValidator.getValidValueOrDefault(this._optionsSchema, options);
+        const optionsVersion = this._getVersionUpdates().length;
+        const options = this._schemaValidator.getValidValueOrDefault(this._optionsSchema);
+        options.version = optionsVersion;
+        return options;
     }
 
     createValidatingProxy(options) {
@@ -468,6 +473,10 @@ class OptionsUtil {
             {
                 async: true,
                 update: this._updateVersion4.bind(this)
+            },
+            {
+                async: false,
+                update: this._updateVersion5.bind(this)
             }
         ];
     }
@@ -586,4 +595,13 @@ class OptionsUtil {
         await this._addFieldTemplatesToOptions(options, '/bg/data/anki-field-templates-upgrade-v4.handlebars');
         return options;
     }
+
+    _updateVersion5(options) {
+        // Version 5 changes:
+        //  Removed legacy version number from profile options.
+        for (const profile of options.profiles) {
+            delete profile.options.version;
+        }
+        return options;
+    }
 }
diff --git a/ext/bg/js/settings/backup-controller.js b/ext/bg/js/settings/backup-controller.js
index 454ede85..d5fa28bc 100644
--- a/ext/bg/js/settings/backup-controller.js
+++ b/ext/bg/js/settings/backup-controller.js
@@ -336,7 +336,6 @@ class BackupController {
 
         // Upgrade options
         optionsFull = await this._optionsUtil.update(optionsFull);
-        optionsFull = this._optionsUtil.getValidValueOrDefault(optionsFull);
 
         // Check for warnings
         const sanitizationWarnings = this._settingsImportSanitizeOptions(optionsFull, true);
diff --git a/test/test-options-util.js b/test/test-options-util.js
index a7dc41d1..00786dd6 100644
--- a/test/test-options-util.js
+++ b/test/test-options-util.js
@@ -63,6 +63,7 @@ function clone(value) {
 
 function createProfileOptionsTestData1() {
     return {
+        version: 14,
         general: {
             enable: true,
             enableClipboardPopups: false,
@@ -495,7 +496,7 @@ function createOptionsUpdatedTestData1() {
             }
         ],
         profileCurrent: 0,
-        version: 4,
+        version: 5,
         global: {
             database: {
                 prefixWildcardsSupported: false
@@ -517,6 +518,32 @@ async function testUpdate(extDir) {
     assert.deepStrictEqual(optionsUpdated, optionsExpected);
 }
 
+async function testDefault(extDir) {
+    const data = [
+        (options) => options,
+        (options) => {
+            delete options.profiles[0].options.audio.autoPlay;
+        },
+        (options) => {
+            options.profiles[0].options.audio.autoPlay = void 0;
+        }
+    ];
+
+    const vm = createVM(extDir);
+    const [OptionsUtil] = vm.get(['OptionsUtil']);
+    const optionsUtil = new OptionsUtil();
+    await optionsUtil.prepare();
+
+    for (const modify of data) {
+        const options = optionsUtil.getDefault();
+
+        const optionsModified = clone(options);
+        modify(optionsModified);
+
+        const optionsUpdated = await optionsUtil.update(clone(optionsModified));
+        assert.deepStrictEqual(clone(optionsUpdated), clone(options));
+    }
+}
 
 async function testFieldTemplatesUpdate(extDir) {
     const vm = createVM(extDir);
@@ -590,6 +617,7 @@ ${update4}
 async function main() {
     const extDir = path.join(__dirname, '..', 'ext');
     await testUpdate(extDir);
+    await testDefault(extDir);
     await testFieldTemplatesUpdate(extDir);
 }