From 9c7d559cd5874238aae7bb79a905a97c946b1aff Mon Sep 17 00:00:00 2001 From: Evan Morikawa Date: Wed, 1 Feb 2017 18:04:58 -0800 Subject: [PATCH] Revert "feat(config): move config to database" This reverts commit a60816c9037293e7c32c29b15701336d7ec87d24. --- spec/stores/account-store-spec.coffee | 5 +- src/browser/application.es6 | 22 +- src/browser/config-persistence-manager.es6 | 225 ++++++++++++++++----- src/config.coffee | 13 +- src/database-helpers.es6 | 34 ---- src/flux/stores/account-store.es6 | 6 +- src/flux/stores/database-store.es6 | 68 +++++-- 7 files changed, 251 insertions(+), 122 deletions(-) delete mode 100644 src/database-helpers.es6 diff --git a/spec/stores/account-store-spec.coffee b/spec/stores/account-store-spec.coffee index 2cdf92d335..3e030a4bea 100644 --- a/spec/stores/account-store-spec.coffee +++ b/spec/stores/account-store-spec.coffee @@ -110,9 +110,10 @@ xdescribe "AccountStore", -> account = (new Account).fromJSON(@json) expect(@instance._accounts.length).toBe 1 expect(@instance._accounts[0]).toEqual account - expect(NylasEnv.config.set.calls.length).toBe 2 + expect(NylasEnv.config.set.calls.length).toBe 3 + expect(NylasEnv.config.set.calls[0].args).toEqual(['nylas.accountTokens', null]) # Version must be updated last since it will trigger other windows to load nylas.accounts - expect(NylasEnv.config.set.calls[1].args).toEqual(['nylas.accountsVersion', 1]) + expect(NylasEnv.config.set.calls[2].args).toEqual(['nylas.accountsVersion', 1]) it "triggers", -> expect(@instance.trigger).toHaveBeenCalled() diff --git a/src/browser/application.es6 b/src/browser/application.es6 index af757a5869..11d2bce2ce 100644 --- a/src/browser/application.es6 +++ b/src/browser/application.es6 @@ -8,23 +8,23 @@ import path from 'path'; import proc from 'child_process' import {EventEmitter} from 'events'; +import SystemTrayManager from './system-tray-manager'; import WindowManager from './window-manager'; import FileListCache from './file-list-cache'; import ApplicationMenu from './application-menu'; import AutoUpdateManager from './auto-update-manager'; -import SystemTrayManager from './system-tray-manager'; import PerformanceMonitor from './performance-monitor' -import DefaultClientHelper from '../default-client-helper'; import NylasProtocolHandler from './nylas-protocol-handler'; import PackageMigrationManager from './package-migration-manager'; import ConfigPersistenceManager from './config-persistence-manager'; +import DefaultClientHelper from '../default-client-helper'; let clipboard = null; // The application's singleton class. // export default class Application extends EventEmitter { - async start(options) { + start(options) { const {resourcePath, configDirPath, version, devMode, specMode, safeMode} = options; // Normalize to make sure drive letter case is consistent on Windows @@ -38,13 +38,12 @@ export default class Application extends EventEmitter { this.fileListCache = new FileListCache(); this.nylasProtocolHandler = new NylasProtocolHandler(this.resourcePath, this.safeMode); - this.configPersistenceManager = new ConfigPersistenceManager({ - configDirPath, resourcePath, specMode}); - await this.configPersistenceManager.setup() + this.temporaryMigrateConfig(); const Config = require('../config'); const config = new Config(); this.config = config; + this.configPersistenceManager = new ConfigPersistenceManager({configDirPath, resourcePath}); config.load(); this.packageMigrationManager = new PackageMigrationManager({config, configDirPath, version}) @@ -103,6 +102,17 @@ export default class Application extends EventEmitter { return this.quitting; } + temporaryMigrateConfig() { + const oldConfigFilePath = fs.resolve(this.configDirPath, 'config.cson'); + const newConfigFilePath = path.join(this.configDirPath, 'config.json'); + if (oldConfigFilePath) { + const CSON = require('season'); + const userConfig = CSON.readFileSync(oldConfigFilePath); + fs.writeFileSync(newConfigFilePath, JSON.stringify(userConfig, null, 2)); + fs.unlinkSync(oldConfigFilePath); + } + } + // Opens a new window based on the options provided. handleLaunchOptions(options) { const {specMode, pathsToOpen, urlsToOpen} = options; diff --git a/src/browser/config-persistence-manager.es6 b/src/browser/config-persistence-manager.es6 index 7ff6470315..9c4c370328 100644 --- a/src/browser/config-persistence-manager.es6 +++ b/src/browser/config-persistence-manager.es6 @@ -1,82 +1,209 @@ import path from 'path'; +import pathWatcher from 'pathwatcher'; import fs from 'fs-plus'; -import {setupDatabase, databasePath} from '../database-helpers' +import {BrowserWindow, dialog, app} from 'electron'; +import {atomicWriteFileSync} from '../fs-utils' + let _ = require('underscore'); _ = _.extend(_, require('../config-utils')); +const RETRY_SAVES = 3 + + export default class ConfigPersistenceManager { - constructor({configDirPath, resourcePath, specMode} = {}) { - this.database = null; - this.specMode = specMode - this.resourcePath = resourcePath; + constructor({configDirPath, resourcePath} = {}) { this.configDirPath = configDirPath; + this.resourcePath = resourcePath; + + this.userWantsToPreserveErrors = false + this.saveRetries = 0 + this.configFilePath = path.join(this.configDirPath, 'config.json') + this.settings = {}; + + this.initializeConfigDirectory(); + this.load(); + this.observe(); + } + + initializeConfigDirectory() { + if (!fs.existsSync(this.configDirPath)) { + fs.makeTreeSync(this.configDirPath); + const templateConfigDirPath = path.join(this.resourcePath, 'dot-nylas'); + fs.copySync(templateConfigDirPath, this.configDirPath); + } + + if (!fs.existsSync(this.configFilePath)) { + this.writeTemplateConfigFile(); + } } - async setup() { - await this._initializeDatabase(); - this._initializeConfigDirectory(); - this._migrateOldConfigs(); + writeTemplateConfigFile() { + const templateConfigPath = path.join(this.resourcePath, 'dot-nylas', 'config.json'); + const templateConfig = fs.readFileSync(templateConfigPath); + fs.writeFileSync(this.configFilePath, templateConfig); } - getRawValues() { - if (!this._selectStatement) { - const q = `SELECT * FROM \`config\` WHERE id = '*'`; - this._selectStatement = this.database.prepare(q) + load() { + this.userWantsToPreserveErrors = false; + + try { + const json = JSON.parse(fs.readFileSync(this.configFilePath)) || {}; + this.settings = json['*']; + this.emitChangeEvent(); + } catch (error) { + global.errorLogger.reportError(error, {event: 'Failed to load config.json'}) + const message = `Failed to load "${path.basename(this.configFilePath)}"`; + let detail = (error.location) ? error.stack : error.message; + + if (error instanceof SyntaxError) { + detail += `\n\nThe file ${this.configFilePath} has incorrect JSON formatting or is empty. Fix the formatting to resolve this error, or reset your settings to continue using N1.` + } else { + detail += `\n\nWe were unable to read the file ${this.configFilePath}. Make sure you have permissions to access this file, and check that the file is not open or being edited and try again.` + } + + const clickedIndex = dialog.showMessageBox({ + type: 'error', + message, + detail, + buttons: ['Quit', 'Try Again', 'Reset Configuration'], + }); + + if (clickedIndex === 0) { + this.userWantsToPreserveErrors = true; + app.quit(); + } else if (clickedIndex === 1) { + this.load(); + } else { + if (fs.existsSync(this.configFilePath)) { + fs.unlinkSync(this.configFilePath); + } + this.writeTemplateConfigFile(); + this.load(); + } } - const row = this._selectStatement.get(); - return JSON.parse(row.data) } - resetConfig(newConfig) { - this._replace(newConfig); + loadSoon = () => { + this._loadDebounced = this._loadDebounced || _.debounce(this.load, 100); + this._loadDebounced(); } - setRawValue(keyPath, value) { - const configData = this.getRawValues(); - if (!keyPath) { - throw new Error("Must specify a keyPath to set the config") + observe() { + // watch the config file for edits. This observer needs to be + // replaced if the config file is deleted. + let watcher = null; + const watchCurrentConfigFile = () => { + try { + if (watcher) { + watcher.close(); + } + watcher = pathWatcher.watch(this.configFilePath, (e) => { + if (e === 'change') { + this.loadSoon(); + } + }); + } catch (error) { + this.observeErrorOccurred(error); + } } + watchCurrentConfigFile(); + + // watch the config directory (non-recursive) to catch the config file + // being deleted and replaced or atomically edited. + try { + let lastctime = null; + pathWatcher.watch(this.configDirPath, () => { + fs.stat(this.configFilePath, (err, stats) => { + if (err) { return; } - // This edits in place - _.setValueForKeyPath(configData, keyPath, value); + const ctime = stats.ctime.getTime(); + if (ctime !== lastctime) { + if (Math.abs(ctime - this.lastSaveTimestamp) > 2000) { + this.loadSoon(); + } + watchCurrentConfigFile(); + lastctime = ctime; + } + }); + }) + } catch (error) { + this.observeErrorOccurred(error); + } + } - this._replace(configData); - return configData + observeErrorOccurred = (error) => { + global.errorLogger.reportError(error) + dialog.showMessageBox({ + type: 'error', + message: 'Configuration Error', + detail: ` + Unable to watch path: ${path.basename(this.configFilePath)}. Make sure you have permissions to + ${this.configFilePath}. On linux there are currently problems with watch + sizes. + `, + buttons: ['Okay'], + }) } - _migrateOldConfigs() { + save = () => { + if (this.userWantsToPreserveErrors) { + return; + } + const allSettings = {'*': this.settings}; + const allSettingsJSON = JSON.stringify(allSettings, null, 2); + this.lastSaveTimestamp = Date.now(); + try { - const oldConfig = path.join(this.configDirPath, 'config.json'); - if (fs.existsSync(oldConfig)) { - const configData = JSON.parse(fs.readFileSync(oldConfig))['*']; - this._replace(configData) - fs.unlinkSync(oldConfig) + atomicWriteFileSync(this.configFilePath, allSettingsJSON) + this.saveRetries = 0 + } catch (error) { + if (this.saveRetries >= RETRY_SAVES) { + global.errorLogger.reportError(error, {event: 'Failed to save config.json'}) + const clickedIndex = dialog.showMessageBox({ + type: 'error', + message: `Failed to save "${path.basename(this.configFilePath)}"`, + detail: `\n\nWe were unable to save the file ${this.configFilePath}. Make sure you have permissions to access this file, and check that the file is not open or being edited and try again.`, + buttons: ['Okay', 'Try again'], + }) + this.saveRetries = 0 + if (clickedIndex === 1) { + this.saveSoon() + } + } else { + this.saveRetries++ + this.saveSoon() } - } catch (err) { - global.errorLogger.reportError(err) } } - _replace(configData) { - if (!this._replaceStatement) { - const q = `REPLACE INTO \`config\` (id, data) VALUES (?,?)`; - this._replaceStatement = this.database.prepare(q) - } - this._replaceStatement.run(['*', JSON.stringify(configData)]) + saveSoon = () => { + this._saveThrottled = this._saveThrottled || _.throttle(this.save, 100); + this._saveThrottled(); } - async _initializeDatabase() { - const dbPath = databasePath(this.configDirPath, this.specMode); - this.database = await setupDatabase(dbPath) - const setupQuery = `CREATE TABLE IF NOT EXISTS \`config\` (id TEXT PRIMARY KEY, data BLOB)`; - this.database.prepare(setupQuery).run() + getRawValuesString = () => { + return JSON.stringify(this.settings); } - _initializeConfigDirectory() { - if (!fs.existsSync(this.configDirPath)) { - fs.makeTreeSync(this.configDirPath); - const templateConfigDirPath = path.join(this.resourcePath, 'dot-nylas'); - fs.copySync(templateConfigDirPath, this.configDirPath); + setRawValue = (keyPath, value, sourceWebcontentsId) => { + if (keyPath) { + _.setValueForKeyPath(this.settings, keyPath, value); + } else { + this.settings = value; } + + this.emitChangeEvent({sourceWebcontentsId}); + this.saveSoon(); + return null; + } + + emitChangeEvent = ({sourceWebcontentsId} = {}) => { + global.application.config.updateSettings(this.settings); + + BrowserWindow.getAllWindows().forEach((win) => { + if ((win.webContents) && (win.webContents.getId() !== sourceWebcontentsId)) { + win.webContents.send('on-config-reloaded', this.settings); + } + }); } } diff --git a/src/config.coffee b/src/config.coffee index aaabd95d30..91ab7946ac 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -321,9 +321,6 @@ class Config # Created during initialization, available as `NylasEnv.config` constructor: -> - # `app` exists in the remote browser process. We do this to use a - # single DB connection for the Config - @configPersistenceManager = app.configPersistenceManager @emitter = new Emitter @schema = type: 'object' @@ -620,18 +617,20 @@ class Config @transact => settings = @getRawValues() settings = @makeValueConformToSchema(null, settings, suppressException: true) - @configPersistenceManager.resetConfig(settings) - @load() + @setRawValue(null, settings) return emitChangeEvent: -> @emitter.emit 'did-change' unless @transactDepth > 0 getRawValues: -> - return @configPersistenceManager.getRawValues() + try + return JSON.parse(app.configPersistenceManager.getRawValuesString()) + catch + return {} setRawValue: (keyPath, value) -> - @configPersistenceManager.setRawValue(keyPath, value, webContentsId) + app.configPersistenceManager.setRawValue(keyPath, value, webContentsId) @load() # Base schema enforcers. These will coerce raw input into the specified type, diff --git a/src/database-helpers.es6 b/src/database-helpers.es6 deleted file mode 100644 index 82bf33a864..0000000000 --- a/src/database-helpers.es6 +++ /dev/null @@ -1,34 +0,0 @@ -import path from 'path'; -import Sqlite3 from 'better-sqlite3'; - -export function setupDatabase(dbPath) { - return new Promise((resolve, reject) => { - const db = new Sqlite3(dbPath, {}); - db.on('close', reject) - db.on('open', () => { - // https://www.sqlite.org/wal.html - // WAL provides more concurrency as readers do not block writers and a writer - // does not block readers. Reading and writing can proceed concurrently. - db.pragma(`journal_mode = WAL`); - - // Note: These are properties of the connection, so they must be set regardless - // of whether the database setup queries are run. - - // https://www.sqlite.org/intern-v-extern-blob.html - // A database page size of 8192 or 16384 gives the best performance for large BLOB I/O. - db.pragma(`main.page_size = 8192`); - db.pragma(`main.cache_size = 20000`); - db.pragma(`main.synchronous = NORMAL`); - - resolve(db); - }); - }) -} - -export function databasePath(configDirPath, specMode = false) { - let dbPath = path.join(configDirPath, 'edgehill.db'); - if (specMode) { - dbPath = path.join(configDirPath, 'edgehill.test.db'); - } - return dbPath -} diff --git a/src/flux/stores/account-store.es6 b/src/flux/stores/account-store.es6 index 5ce25253df..da2966c4ff 100644 --- a/src/flux/stores/account-store.es6 +++ b/src/flux/stores/account-store.es6 @@ -11,6 +11,7 @@ import DatabaseStore from './database-store' const configAccountsKey = "nylas.accounts" const configVersionKey = "nylas.accountsVersion" +const configTokensKey = "nylas.accountTokens" /* @@ -135,9 +136,8 @@ class AccountStore extends NylasStore { _save = () => { this._version += 1 - const configAccounts = this._accounts.map(a => a.toJSON()) - configAccounts.forEach(a => delete a.sync_error) - NylasEnv.config.set(configAccountsKey, configAccounts) + NylasEnv.config.set(configTokensKey, null) + NylasEnv.config.set(configAccountsKey, this._accounts) NylasEnv.config.set(configVersionKey, this._version) this._trigger() } diff --git a/src/flux/stores/database-store.es6 b/src/flux/stores/database-store.es6 index d68759c6ba..33d20ad816 100644 --- a/src/flux/stores/database-store.es6 +++ b/src/flux/stores/database-store.es6 @@ -1,6 +1,7 @@ /* eslint global-require: 0 */ import path from 'path'; import fs from 'fs'; +import Sqlite3 from 'better-sqlite3'; import childProcess from 'child_process'; import PromiseQueue from 'promise-queue'; import {remote, ipcRenderer} from 'electron'; @@ -13,7 +14,6 @@ import Actions from '../actions' import DatabaseChangeRecord from './database-change-record'; import DatabaseTransaction from './database-transaction'; import DatabaseSetupQueryBuilder from './database-setup-query-builder'; -import {setupDatabase, databasePath} from '../../database-helpers' const DatabaseVersion = "23"; const DatabasePhase = { @@ -88,7 +88,11 @@ class DatabaseStore extends NylasStore { this.setupEmitter(); this._emitter.setMaxListeners(100); - this._databasePath = databasePath(NylasEnv.getConfigDirPath(), NylasEnv.inSpecMode()) + if (NylasEnv.inSpecMode()) { + this._databasePath = path.join(NylasEnv.getConfigDirPath(), 'edgehill.test.db'); + } else { + this._databasePath = path.join(NylasEnv.getConfigDirPath(), 'edgehill.db'); + } this._databaseMutationHooks = []; @@ -98,7 +102,7 @@ class DatabaseStore extends NylasStore { setTimeout(() => this._onPhaseChange(), 0); } - async _onPhaseChange() { + _onPhaseChange() { if (NylasEnv.inSpecMode()) { return; } @@ -107,21 +111,23 @@ class DatabaseStore extends NylasStore { const phase = app.databasePhase() if (phase === DatabasePhase.Setup && NylasEnv.isWorkWindow()) { - await this._openDatabase() - this._checkDatabaseVersion({allowUnset: true}, () => { - this._runDatabaseSetup(() => { - app.setDatabasePhase(DatabasePhase.Ready); - setTimeout(() => this._runDatabaseAnalyze(), 60 * 1000); + this._openDatabase(() => { + this._checkDatabaseVersion({allowUnset: true}, () => { + this._runDatabaseSetup(() => { + app.setDatabasePhase(DatabasePhase.Ready); + setTimeout(() => this._runDatabaseAnalyze(), 60 * 1000); + }); }); }); } else if (phase === DatabasePhase.Ready) { - await this._openDatabase() - this._checkDatabaseVersion({}, () => { - this._open = true; - for (const w of this._waiting) { - w(); - } - this._waiting = []; + this._openDatabase(() => { + this._checkDatabaseVersion({}, () => { + this._open = true; + for (const w of this._waiting) { + w(); + } + this._waiting = []; + }); }); } else if (phase === DatabasePhase.Close) { this._open = false; @@ -146,17 +152,37 @@ class DatabaseStore extends NylasStore { } } - async _openDatabase() { - if (this._db) return - try { - this._db = await setupDatabase(this._databasePath) - } catch (err) { + _openDatabase(ready) { + if (this._db) { + ready(); + return; + } + + this._db = new Sqlite3(this._databasePath, {}); + this._db.on('close', (err) => { NylasEnv.showErrorDialog({ title: `Unable to open SQLite database at ${this._databasePath}`, message: err.toString(), }); this._handleSetupError(err); - } + }) + this._db.on('open', () => { + // https://www.sqlite.org/wal.html + // WAL provides more concurrency as readers do not block writers and a writer + // does not block readers. Reading and writing can proceed concurrently. + this._db.pragma(`journal_mode = WAL`); + + // Note: These are properties of the connection, so they must be set regardless + // of whether the database setup queries are run. + + // https://www.sqlite.org/intern-v-extern-blob.html + // A database page size of 8192 or 16384 gives the best performance for large BLOB I/O. + this._db.pragma(`main.page_size = 8192`); + this._db.pragma(`main.cache_size = 20000`); + this._db.pragma(`main.synchronous = NORMAL`); + + ready(); + }); } _checkDatabaseVersion({allowUnset} = {}, ready) {