Update dictionary settings structure (#1587)

* Update dictionary settings structure to use an array instead of an object

* Update ensureDictionarySettings implementation

* Remove some usage of ObjectPropertyAccessor

5 files changed, 107 insertions, 61 deletions

diff --git a/ext/js/pages/settings/backup-controller.js b/ext/js/pages/settings/backup-controller.js
index 9b80f0b4..c961d40e 100644
--- a/ext/js/pages/settings/backup-controller.js
+++ b/ext/js/pages/settings/backup-controller.js
@@ -368,7 +368,7 @@ class BackupController {
         }
 
         // Update dictionaries
-        await DictionaryController.ensureDictionarySettings(this._settingsController, void 0, optionsFull, true, false);
+        await DictionaryController.ensureDictionarySettings(this._settingsController, void 0, optionsFull, false, false);
 
         // Assign options
         await this._settingsImportSetOptionsFull(optionsFull);
@@ -404,7 +404,7 @@ class BackupController {
         const optionsFull = this._optionsUtil.getDefault();
 
         // Update dictionaries
-        await DictionaryController.ensureDictionarySettings(this._settingsController, void 0, optionsFull, true, false);
+        await DictionaryController.ensureDictionarySettings(this._settingsController, void 0, optionsFull, false, false);
 
         // Assign options
         try {
diff --git a/ext/js/pages/settings/collapsible-dictionary-controller.js b/ext/js/pages/settings/collapsible-dictionary-controller.js
index 37793c6f..6eb3a9b4 100644
--- a/ext/js/pages/settings/collapsible-dictionary-controller.js
+++ b/ext/js/pages/settings/collapsible-dictionary-controller.js
@@ -15,10 +15,6 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-/* global
- * ObjectPropertyAccessor
- */
-
 class CollapsibleDictionaryController {
     constructor(settingsController) {
         this._settingsController = settingsController;
@@ -65,12 +61,14 @@ class CollapsibleDictionaryController {
 
         this._setupAllSelect(fragment, options);
 
-        for (const dictionary of Object.keys(options.dictionaries)) {
-            const dictionaryInfo = this._dictionaryInfoMap.get(dictionary);
+        const {dictionaries} = options;
+        for (let i = 0, ii = dictionaries.length; i < ii; ++i) {
+            const {name} = dictionaries[i];
+            const dictionaryInfo = this._dictionaryInfoMap.get(name);
             if (typeof dictionaryInfo === 'undefined') { continue; }
 
-            const select = this._addSelect(fragment, dictionary, `rev.${dictionaryInfo.revision}`);
-            select.dataset.setting = ObjectPropertyAccessor.getPathString(['dictionaries', dictionary, 'definitionsCollapsible']);
+            const select = this._addSelect(fragment, name, `rev.${dictionaryInfo.revision}`);
+            select.dataset.setting = `dictionaries[${i}].definitionsCollapsible`;
             this._eventListeners.addEventListener(select, 'settingChanged', this._onDefinitionsCollapsibleChange.bind(this), false);
 
             this._selects.push(select);
@@ -125,7 +123,7 @@ class CollapsibleDictionaryController {
     _updateAllSelect(options) {
         let value = null;
         let varies = false;
-        for (const {definitionsCollapsible} of Object.values(options.dictionaries)) {
+        for (const {definitionsCollapsible} of options.dictionaries) {
             if (value === null) {
                 value = definitionsCollapsible;
             } else if (value !== definitionsCollapsible) {
@@ -140,8 +138,9 @@ class CollapsibleDictionaryController {
     async _setDefinitionsCollapsibleAll(value) {
         const options = await this._settingsController.getOptions();
         const targets = [];
-        for (const dictionary of Object.keys(options.dictionaries)) {
-            const path = ObjectPropertyAccessor.getPathString(['dictionaries', dictionary, 'definitionsCollapsible']);
+        const {dictionaries} = options;
+        for (let i = 0, ii = dictionaries.length; i < ii; ++i) {
+            const path = `dictionaries[${i}].definitionsCollapsible`;
             targets.push({action: 'set', path, value});
         }
         await this._settingsController.modifyProfileSettings(targets);
diff --git a/ext/js/pages/settings/dictionary-controller.js b/ext/js/pages/settings/dictionary-controller.js
index 7862b207..23a47f9a 100644
--- a/ext/js/pages/settings/dictionary-controller.js
+++ b/ext/js/pages/settings/dictionary-controller.js
@@ -17,13 +17,13 @@
 
 /* global
  * DictionaryDatabase
- * ObjectPropertyAccessor
  */
 
 class DictionaryEntry {
-    constructor(dictionaryController, node, dictionaryInfo) {
+    constructor(dictionaryController, node, index, dictionaryInfo) {
         this._dictionaryController = dictionaryController;
         this._node = node;
+        this._index = index;
         this._dictionaryInfo = dictionaryInfo;
         this._eventListeners = new EventListenerCollection();
         this._detailsContainer = null;
@@ -41,6 +41,7 @@ class DictionaryEntry {
 
     prepare() {
         const node = this._node;
+        const index = this._index;
         const {title, revision, prefixWildcardsSupported, version} = this._dictionaryInfo;
 
         this._detailsContainer = node.querySelector('.dictionary-details');
@@ -72,14 +73,14 @@ class DictionaryEntry {
             detailsToggleLink.hidden = !hasDetails;
         }
         if (enabledCheckbox !== null) {
-            enabledCheckbox.dataset.setting = ObjectPropertyAccessor.getPathString(['dictionaries', title, 'enabled']);
+            enabledCheckbox.dataset.setting = `dictionaries[${index}].enabled`;
             this._eventListeners.addEventListener(enabledCheckbox, 'settingChanged', this._onEnabledChanged.bind(this), false);
         }
         if (priorityInput !== null) {
-            priorityInput.dataset.setting = ObjectPropertyAccessor.getPathString(['dictionaries', title, 'priority']);
+            priorityInput.dataset.setting = `dictionaries[${index}].priority`;
         }
         if (allowSecondarySearchesCheckbox !== null) {
-            allowSecondarySearchesCheckbox.dataset.setting = ObjectPropertyAccessor.getPathString(['dictionaries', title, 'allowSecondarySearches']);
+            allowSecondarySearchesCheckbox.dataset.setting = `dictionaries[${index}].allowSecondarySearches`;
         }
         if (deleteButton !== null) {
             this._eventListeners.addEventListener(deleteButton, 'click', this._onDeleteButtonClicked.bind(this), false);
@@ -248,8 +249,9 @@ class DictionaryController {
         this._updateDictionariesEnabledWarnings(options);
     }
 
-    static createDefaultDictionarySettings(enabled) {
+    static createDefaultDictionarySettings(name, enabled) {
         return {
+            name,
             priority: 0,
             enabled,
             allowSecondarySearches: false,
@@ -257,7 +259,7 @@ class DictionaryController {
         };
     }
 
-    static async ensureDictionarySettings(settingsController, dictionaries, optionsFull, modifyOptionsFull, newDictionariesEnabled) {
+    static async ensureDictionarySettings(settingsController, dictionaries, optionsFull, modifyGlobalSettings, newDictionariesEnabled) {
         if (typeof dictionaries === 'undefined') {
             dictionaries = await settingsController.getDictionaryInfo();
         }
@@ -265,24 +267,43 @@ class DictionaryController {
             optionsFull = await settingsController.getOptionsFull();
         }
 
-        const targets = [];
-        const {profiles} = optionsFull;
+        const installedDictionaries = new Set();
         for (const {title} of dictionaries) {
-            for (let i = 0, ii = profiles.length; i < ii; ++i) {
-                const {options: {dictionaries: dictionaryOptions}} = profiles[i];
-                if (Object.prototype.hasOwnProperty.call(dictionaryOptions, title)) { continue; }
+            installedDictionaries.add(title);
+        }
 
-                const value = DictionaryController.createDefaultDictionarySettings(newDictionariesEnabled);
-                if (modifyOptionsFull) {
-                    dictionaryOptions[title] = value;
+        const targets = [];
+        const {profiles} = optionsFull;
+        for (let i = 0, ii = profiles.length; i < ii; ++i) {
+            let modified = false;
+            const missingDictionaries = new Set([...installedDictionaries]);
+            const dictionaryOptionsArray = profiles[i].options.dictionaries;
+            for (let j = dictionaryOptionsArray.length - 1; j >= 0; --j) {
+                const {name} = dictionaryOptionsArray[j];
+                if (installedDictionaries.has(name)) {
+                    missingDictionaries.delete(name);
                 } else {
-                    const path = ObjectPropertyAccessor.getPathString(['profiles', i, 'options', 'dictionaries', title]);
-                    targets.push({action: 'set', path, value});
+                    dictionaryOptionsArray.splice(j, 1);
+                    modified = true;
                 }
             }
+
+            for (const name of missingDictionaries) {
+                const value = DictionaryController.createDefaultDictionarySettings(name, newDictionariesEnabled);
+                dictionaryOptionsArray.push(value);
+                modified = true;
+            }
+
+            if (modified) {
+                targets.push({
+                    action: 'set',
+                    path: `profiles[${i}].options.dictionaries`,
+                    value: dictionaryOptionsArray
+                });
+            }
         }
 
-        if (!modifyOptionsFull && targets.length > 0) {
+        if (modifyGlobalSettings && targets.length > 0) {
             await settingsController.modifyGlobalSettings(targets);
         }
     }
@@ -291,6 +312,9 @@ class DictionaryController {
 
     _onOptionsChanged({options}) {
         this._updateDictionariesEnabledWarnings(options);
+        if (this._dictionaries !== null) {
+            this._updateEntries();
+        }
     }
 
     async _onDatabaseUpdated() {
@@ -298,10 +322,14 @@ class DictionaryController {
         this._databaseStateToken = token;
         this._dictionaries = null;
         const dictionaries = await this._settingsController.getDictionaryInfo();
-        const options = await this._settingsController.getOptions();
         if (this._databaseStateToken !== token) { return; }
         this._dictionaries = dictionaries;
 
+        await this._updateEntries();
+    }
+
+    async _updateEntries() {
+        const dictionaries = this._dictionaries;
         this._updateMainDictionarySelectOptions(dictionaries);
 
         for (const entry of this._dictionaryEntries) {
@@ -318,23 +346,38 @@ class DictionaryController {
             node.hidden = hasDictionary;
         }
 
+        await DictionaryController.ensureDictionarySettings(this._settingsController, dictionaries, void 0, true, false);
+
+        const options = await this._settingsController.getOptions();
         this._updateDictionariesEnabledWarnings(options);
 
-        await DictionaryController.ensureDictionarySettings(this._settingsController, dictionaries, void 0, false, false);
-        for (const dictionary of dictionaries) {
-            this._createDictionaryEntry(dictionary);
+        const dictionaryInfoMap = new Map();
+        for (const dictionary of this._dictionaries) {
+            dictionaryInfoMap.set(dictionary.title, dictionary);
+        }
+
+        const dictionaryOptionsArray = options.dictionaries;
+        for (let i = 0, ii = dictionaryOptionsArray.length; i < ii; ++i) {
+            const {name} = dictionaryOptionsArray[i];
+            const dictionaryInfo = dictionaryInfoMap.get(name);
+            if (typeof dictionaryInfo === 'undefined') { continue; }
+            this._createDictionaryEntry(i, dictionaryInfo);
         }
     }
 
     _updateDictionariesEnabledWarnings(options) {
         let enabledCount = 0;
         if (this._dictionaries !== null) {
+            const enabledDictionaries = new Set();
+            for (const {name, enabled} of options.dictionaries) {
+                if (enabled) {
+                    enabledDictionaries.add(name);
+                }
+            }
+
             for (const {title} of this._dictionaries) {
-                if (Object.prototype.hasOwnProperty.call(options.dictionaries, title)) {
-                    const {enabled} = options.dictionaries[title];
-                    if (enabled) {
-                        ++enabledCount;
-                    }
+                if (enabledDictionaries.has(title)) {
+                    ++enabledCount;
                 }
             }
         }
@@ -459,11 +502,11 @@ class DictionaryController {
         parent.removeChild(node);
     }
 
-    _createDictionaryEntry(dictionary) {
+    _createDictionaryEntry(index, dictionaryInfo) {
         const node = this.instantiateTemplate('dictionary');
         this._dictionaryEntryContainer.appendChild(node);
 
-        const entry = new DictionaryEntry(this, node, dictionary);
+        const entry = new DictionaryEntry(this, node, index, dictionaryInfo);
         this._dictionaryEntries.push(entry);
         entry.prepare();
     }
@@ -553,9 +596,16 @@ class DictionaryController {
         const targets = [];
         for (let i = 0, ii = profiles.length; i < ii; ++i) {
             const {options: {dictionaries}} = profiles[i];
-            if (Object.prototype.hasOwnProperty.call(dictionaries, dictionaryTitle)) {
-                const path = ObjectPropertyAccessor.getPathString(['profiles', i, 'options', 'dictionaries', dictionaryTitle]);
-                targets.push({action: 'delete', path});
+            for (let j = 0, jj = dictionaries.length; j < jj; ++j) {
+                if (dictionaries[j].name !== dictionaryTitle) { continue; }
+                const path = `profiles[${i}].options.dictionaries`;
+                targets.push({
+                    action: 'splice',
+                    path,
+                    start: j,
+                    deleteCount: 1,
+                    items: []
+                });
             }
         }
         await this._settingsController.modifyGlobalSettings(targets);
diff --git a/ext/js/pages/settings/dictionary-import-controller.js b/ext/js/pages/settings/dictionary-import-controller.js
index afa45899..c7b72110 100644
--- a/ext/js/pages/settings/dictionary-import-controller.js
+++ b/ext/js/pages/settings/dictionary-import-controller.js
@@ -19,7 +19,6 @@
  * DictionaryController
  * DictionaryDatabase
  * DictionaryImporter
- * ObjectPropertyAccessor
  */
 
 class DictionaryImportController {
@@ -213,12 +212,12 @@ class DictionaryImportController {
         const profileCount = optionsFull.profiles.length;
         for (let i = 0; i < profileCount; ++i) {
             const {options} = optionsFull.profiles[i];
-            const value = DictionaryController.createDefaultDictionarySettings(true);
-            const path1 = ObjectPropertyAccessor.getPathString(['profiles', i, 'options', 'dictionaries', title]);
-            targets.push({action: 'set', path: path1, value});
+            const value = DictionaryController.createDefaultDictionarySettings(title, true);
+            const path1 = `profiles[${i}].options.dictionaries`;
+            targets.push({action: 'push', path: path1, items: [value]});
 
             if (sequenced && options.general.mainDictionary === '') {
-                const path2 = ObjectPropertyAccessor.getPathString(['profiles', i, 'options', 'general', 'mainDictionary']);
+                const path2 = `profiles[${i}].options.general.mainDictionary`;
                 targets.push({action: 'set', path: path2, value: title});
             }
         }
@@ -230,9 +229,9 @@ class DictionaryImportController {
         const targets = [];
         const profileCount = optionsFull.profiles.length;
         for (let i = 0; i < profileCount; ++i) {
-            const path1 = ObjectPropertyAccessor.getPathString(['profiles', i, 'options', 'dictionaries']);
-            targets.push({action: 'set', path: path1, value: {}});
-            const path2 = ObjectPropertyAccessor.getPathString(['profiles', i, 'options', 'general', 'mainDictionary']);
+            const path1 = `profiles[${i}].options.dictionaries`;
+            targets.push({action: 'set', path: path1, value: []});
+            const path2 = `profiles[${i}].options.general.mainDictionary`;
             targets.push({action: 'set', path: path2, value: ''});
         }
         return await this._modifyGlobalSettings(targets);
diff --git a/ext/js/pages/settings/secondary-search-dictionary-controller.js b/ext/js/pages/settings/secondary-search-dictionary-controller.js
index 13e1dcf5..8ffd9a9b 100644
--- a/ext/js/pages/settings/secondary-search-dictionary-controller.js
+++ b/ext/js/pages/settings/secondary-search-dictionary-controller.js
@@ -15,10 +15,6 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-/* global
- * ObjectPropertyAccessor
- */
-
 class SecondarySearchDictionaryController {
     constructor(settingsController) {
         this._settingsController = settingsController;
@@ -60,21 +56,23 @@ class SecondarySearchDictionaryController {
 
         const fragment = document.createDocumentFragment();
 
-        for (const dictionary of Object.keys(options.dictionaries)) {
-            const dictionaryInfo = this._dictionaryInfoMap.get(dictionary);
+        const {dictionaries} = options;
+        for (let i = 0, ii = dictionaries.length; i < ii; ++i) {
+            const {name} = dictionaries[i];
+            const dictionaryInfo = this._dictionaryInfoMap.get(name);
             if (typeof dictionaryInfo === 'undefined') { continue; }
 
             const node = this._settingsController.instantiateTemplate('secondary-search-dictionary');
             fragment.appendChild(node);
 
             const nameNode = node.querySelector('.dictionary-title');
-            nameNode.textContent = dictionary;
+            nameNode.textContent = name;
 
             const versionNode = node.querySelector('.dictionary-version');
             versionNode.textContent = `rev.${dictionaryInfo.revision}`;
 
             const toggle = node.querySelector('.dictionary-allow-secondary-searches');
-            toggle.dataset.setting = ObjectPropertyAccessor.getPathString(['dictionaries', dictionary, 'allowSecondarySearches']);
+            toggle.dataset.setting = `dictionaries[${i}].allowSecondarySearches`;
             this._eventListeners.addEventListener(toggle, 'settingChanged', this._onEnabledChanged.bind(this, node), false);
         }