diff --git a/x-pack/plugins/ml/public/factories/__tests__/state_factory.js b/x-pack/plugins/ml/public/factories/__tests__/state_factory.js index 77b4ad21000f..d8b177301e5c 100644 --- a/x-pack/plugins/ml/public/factories/__tests__/state_factory.js +++ b/x-pack/plugins/ml/public/factories/__tests__/state_factory.js @@ -13,9 +13,11 @@ import { stateFactoryProvider } from '../state_factory'; describe('ML - mlStateFactory', () => { let stateFactory; + let AppState; beforeEach(ngMock.module('kibana')); beforeEach(ngMock.inject(($injector) => { + AppState = $injector.get('AppState'); const Private = $injector.get('Private'); stateFactory = Private(stateFactoryProvider); })); @@ -53,4 +55,49 @@ describe('ML - mlStateFactory', () => { state.set('testValue', 10); state.changed(); }); + + it(`Malformed AppState, state falls back to undefined, doesn't throw an error`, () => { + const state = stateFactory('testName'); + + // First update the state without interfering with AppState + state.set('testValue', 1); + expect(state.get('testValue')).to.be(1); + + // Manipulate AppState first, then set and get the custom state. + const appState = new AppState(); + appState.fetch(); + appState.testName = undefined; + appState.save(); + state.set('testValue', 2); + expect(state.get('testValue')).to.be(2); + + // Now set the custom state, then manipulate AppState, then get the custom state. + // Because AppState was malformed between set and get, the custom state will fallback + // to the default state, in this case undefined + state.set('testValue', 3); + appState.fetch(); + appState.testName = undefined; + appState.save(); + expect(state.get('testValue')).to.be(undefined); + }); + + it(`Malformed AppState, state falls back to default, doesn't throw an error`, () => { + const state = stateFactory('testName', { + testValue: 1 + }); + + // First update the state without interfering with AppState + state.set('testValue', 2); + expect(state.get('testValue')).to.be(2); + + // Now set the custom state, then manipulate AppState, then get the custom state. + // Because AppState was malformed between set and get, the custom state will fallback + // to the default state, in this case 1 + state.set('testValue', 3); + const appState = new AppState(); + appState.fetch(); + appState.testName = undefined; + appState.save(); + expect(state.get('testValue')).to.be(1); + }); }); diff --git a/x-pack/plugins/ml/public/factories/state_factory.js b/x-pack/plugins/ml/public/factories/state_factory.js index 6248bd92ce9f..f9db571e70f2 100644 --- a/x-pack/plugins/ml/public/factories/state_factory.js +++ b/x-pack/plugins/ml/public/factories/state_factory.js @@ -19,11 +19,7 @@ import { listenerFactoryProvider } from './listener_factory'; // Have a look at the unit tests which demonstrate basic usage. export function stateFactoryProvider(AppState) { - return function (stateName, defaultState) { - if (typeof stateName !== 'string') { - throw 'stateName needs to be of type `string`'; - } - + function initializeAppState(stateName, defaultState) { const appState = new AppState(); appState.fetch(); @@ -49,6 +45,16 @@ export function stateFactoryProvider(AppState) { } } + return appState; + } + + return function (stateName, defaultState) { + if (typeof stateName !== 'string') { + throw 'stateName needs to be of type `string`'; + } + + let appState = initializeAppState(stateName, defaultState); + // () two times here, because the Provider first returns // the Factory, which then returns the actual listener const listener = listenerFactoryProvider()(); @@ -60,11 +66,13 @@ export function stateFactoryProvider(AppState) { // on the state. const state = { get(name) { - return appState[stateName] && appState[stateName][name]; + updateAppState(); + return appState[stateName][name]; }, // only if value doesn't match the existing one, the state gets updated // and saved. set(name, value) { + updateAppState(); if (!_.isEqual(appState[stateName][name], value)) { appState[stateName][name] = value; appState.save(); @@ -73,6 +81,7 @@ export function stateFactoryProvider(AppState) { return state; }, reset() { + updateAppState(); if (!_.isEqual(appState[stateName], defaultState)) { appState[stateName] = _.cloneDeep(defaultState); appState.save(); @@ -92,6 +101,16 @@ export function stateFactoryProvider(AppState) { } }; + // gets the current state of AppState, if for whatever reason this custom + // state isn't part of AppState anymore, reinitialize it + function updateAppState() { + appState.fetch(); + if (typeof appState[stateName] === 'undefined') { + appState = initializeAppState(stateName, defaultState); + changed = true; + } + } + return state; }; }