From 35807ad7d049e368fb8183905f7d637dbbb74a0c Mon Sep 17 00:00:00 2001 From: Evan Morikawa Date: Tue, 14 Feb 2017 09:03:57 -0800 Subject: [PATCH] fix(db): wait for models to be setup before activating packages Summary: Some plugins (like the Salesforce Plugin) register Models with the database store. Upon plugin load, we check to see if there are newly registered models, if there are, we try and refresh the database. Unfortunately the database refresh was not being `await`ed for. This means the `activate` method of the plugin was called before the models were created creating DB errors. Making `refreshDatabaseSchema` something you can `await` for requied adding an async ready hook on the DatabaseStore itself that will work in all windows, even though setup is only happening in one window. Test Plan: Turn DatabaseStore.DEBUG_TO_LOG = true and run before and after the patch with SFDC enabled. See that after the patch the DB setup happens before activate gets called Reviewers: halla, spang, juan Reviewed By: juan Differential Revision: https://phab.nylas.com/D3906 --- spec/package-manager-spec.coffee | 4 ++-- src/flux/stores/database-store.es6 | 22 +++++++++++++++++++++- src/package-manager.coffee | 22 ++++++++++++---------- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/spec/package-manager-spec.coffee b/spec/package-manager-spec.coffee index c1eaa1c18..183200146 100644 --- a/spec/package-manager-spec.coffee +++ b/spec/package-manager-spec.coffee @@ -487,7 +487,7 @@ describe "PackageManager", -> expect(['theme']).toContain(theme.getType()) for theme in themes it "refreshes the database after activating packages with models", -> - spyOn(DatabaseStore, "refreshDatabaseSchema") + spyOn(DatabaseStore, "refreshDatabaseSchema").andReturn(Promise.resolve()) package2 = NylasEnv.packages.loadPackage('package-with-models') NylasEnv.packages.activatePackages([package2]) expect(DatabaseStore.refreshDatabaseSchema).toHaveBeenCalled() @@ -533,7 +533,7 @@ describe "PackageManager", -> expect(NylasEnv.config.get('core.disabledPackages')).not.toContain packageName it 'refreshes the DB when loading a package with models', -> - spyOn(DatabaseStore, "refreshDatabaseSchema") + spyOn(DatabaseStore, "refreshDatabaseSchema").andReturn(Promise.resolve()) packageName = "package-with-models" NylasEnv.config.pushAtKeyPath('core.disabledPackages', packageName) NylasEnv.packages.observeDisabledPackages() diff --git a/src/flux/stores/database-store.es6 b/src/flux/stores/database-store.es6 index 1c85b5f2a..4093db485 100644 --- a/src/flux/stores/database-store.es6 +++ b/src/flux/stores/database-store.es6 @@ -98,8 +98,26 @@ class DatabaseStore extends NylasStore { setTimeout(() => this._onPhaseChange(), 0); } + async _asyncWaitForReady() { + return new Promise((resolve) => { + const app = remote.getGlobal('application') + const phase = app.databasePhase() + if (phase === DatabasePhase.Setup) { + resolve() + return + } + + const listener = () => { + this._emitter.removeListener('ready', listener); + resolve() + } + this._emitter.on('ready', listener) + }) + } + async _onPhaseChange() { if (NylasEnv.inSpecMode()) { + this._emitter.emit('ready') return; } @@ -122,6 +140,7 @@ class DatabaseStore extends NylasStore { w(); } this._waiting = []; + this._emitter.emit('ready') }); } else if (phase === DatabasePhase.Close) { this._open = false; @@ -137,13 +156,14 @@ class DatabaseStore extends NylasStore { // extremely frequently as new models are added when packages load. refreshDatabaseSchema() { if (!NylasEnv.isWorkWindow()) { - return; + return Promise.resolve(); } const app = remote.getGlobal('application'); const phase = app.databasePhase(); if (phase !== DatabasePhase.Setup) { app.setDatabasePhase(DatabasePhase.Setup); } + return this._asyncWaitForReady() } async _openDatabase() { diff --git a/src/package-manager.coffee b/src/package-manager.coffee index bfefc4f5e..d0a1409d9 100644 --- a/src/package-manager.coffee +++ b/src/package-manager.coffee @@ -571,10 +571,10 @@ class PackageManager for pack in packages @loadPackage(pack.name) - @refreshDatabaseSchema() + setupPromise = @refreshDatabaseSchema() for pack in packages - promise = @activatePackage(pack.name) + promise = @activatePackage(pack.name, setupPromise) promises.push(promise) @observeDisabledPackages() promises @@ -588,19 +588,21 @@ class PackageManager # entry in `packagesWithDatabaseObjects`. refreshDatabaseSchema: -> if @packagesWithDatabaseObjects.length > 0 - DatabaseStore.refreshDatabaseSchema() - @packagesWithDatabaseObjects = [] + return DatabaseStore.refreshDatabaseSchema().then => + @packagesWithDatabaseObjects = [] + return Promise.resolve() # Activate a single package by name - activatePackage: (name) -> + activatePackage: (name, setupPromise = Promise.resolve()) -> if pack = @getActivePackage(name) Q(pack) else if pack = @loadPackage(name) - pack.activate().then => - @activePackages[pack.name] = pack - @emitter.emit 'did-activate-package', pack - @onPluginsChanged() - pack + setupPromise.then => + pack.activate().then => + @activePackages[pack.name] = pack + @emitter.emit 'did-activate-package', pack + @onPluginsChanged() + pack else Q.reject(new Error("Failed to load package '#{name}'"))