Skip to content

Commit

Permalink
[Endpoint] Fix saga to start only after store is created and stopped …
Browse files Browse the repository at this point in the history
…on app unmount (#55245)

- added `stop()`/`start()` methods to the Saga Middleware creator factory
- adjust tests based on changes
- changed application `renderApp` to stop sagas when react app is unmounted
  • Loading branch information
paul-tavares committed Jan 21, 2020
1 parent 85edc66 commit 5a5bade
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ import { appStoreFactory } from './store';
export function renderApp(coreStart: CoreStart, { appBasePath, element }: AppMountParameters) {
coreStart.http.get('/api/endpoint/hello-world');

const store = appStoreFactory(coreStart);
const [store, stopSagas] = appStoreFactory(coreStart);

ReactDOM.render(<AppRoot basename={appBasePath} store={store} />, element);

return () => {
ReactDOM.unmountComponentAtNode(element);
stopSagas();
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,21 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { createSagaMiddleware, SagaContext } from './index';
import { applyMiddleware, createStore, Reducer } from 'redux';

import { createSagaMiddleware, SagaContext, SagaMiddleware } from './index';
import { applyMiddleware, createStore, Reducer, Store } from 'redux';

describe('saga', () => {
const INCREMENT_COUNTER = 'INCREMENT';
const DELAYED_INCREMENT_COUNTER = 'DELAYED INCREMENT COUNTER';
const STOP_SAGA_PROCESSING = 'BREAK ASYNC ITERATOR';

const sleep = (ms = 1000) => new Promise(resolve => setTimeout(resolve, ms));
const sleep = (ms = 10) => new Promise(resolve => setTimeout(resolve, ms));
let store: Store;
let reducerA: Reducer;
let sideAffect: (a: unknown, s: unknown) => void;
let sagaExe: (sagaContext: SagaContext) => Promise<void>;
let sagaExeReduxMiddleware: SagaMiddleware;

beforeEach(() => {
reducerA = jest.fn((prevState = { count: 0 }, { type }) => {
Expand Down Expand Up @@ -47,53 +50,63 @@ describe('saga', () => {
}
}
});

sagaExeReduxMiddleware = createSagaMiddleware(sagaExe);
store = createStore(reducerA, applyMiddleware(sagaExeReduxMiddleware));
});

test('it returns Redux Middleware from createSagaMiddleware()', () => {
const sagaMiddleware = createSagaMiddleware(async () => {});
expect(sagaMiddleware).toBeInstanceOf(Function);
afterEach(() => {
sagaExeReduxMiddleware.stop();
});

test('it does nothing if saga is not started', () => {
const store = createStore(reducerA, applyMiddleware(createSagaMiddleware(sagaExe)));
expect(store.getState().count).toEqual(0);
expect(reducerA).toHaveBeenCalled();
expect(sagaExe).toHaveBeenCalled();
expect(sideAffect).not.toHaveBeenCalled();
expect(store.getState()).toEqual({ count: 0 });
expect(sagaExe).not.toHaveBeenCalled();
});
test('it updates store once running', async () => {
const sagaMiddleware = createSagaMiddleware(sagaExe);
const store = createStore(reducerA, applyMiddleware(sagaMiddleware));

test('it can dispatch store actions once running', async () => {
sagaExeReduxMiddleware.start();
expect(store.getState()).toEqual({ count: 0 });
expect(sagaExe).toHaveBeenCalled();

store.dispatch({ type: DELAYED_INCREMENT_COUNTER });
expect(store.getState()).toEqual({ count: 0 });

await sleep(100);
await sleep();

expect(sideAffect).toHaveBeenCalled();
expect(store.getState()).toEqual({ count: 1 });
});
test('it stops processing if break out of loop', async () => {
const sagaMiddleware = createSagaMiddleware(sagaExe);
const store = createStore(reducerA, applyMiddleware(sagaMiddleware));

test('it stops processing if break out of loop', async () => {
sagaExeReduxMiddleware.start();
store.dispatch({ type: DELAYED_INCREMENT_COUNTER });
await sleep(100);
await sleep();

expect(store.getState()).toEqual({ count: 1 });
expect(sideAffect).toHaveBeenCalledTimes(2);

store.dispatch({ type: STOP_SAGA_PROCESSING });
await sleep(100);
await sleep();

store.dispatch({ type: DELAYED_INCREMENT_COUNTER });
await sleep();

expect(store.getState()).toEqual({ count: 1 });
expect(sideAffect).toHaveBeenCalledTimes(2);
});

test('it stops saga middleware when stop() is called', async () => {
sagaExeReduxMiddleware.start();
store.dispatch({ type: DELAYED_INCREMENT_COUNTER });
await sleep();

expect(store.getState()).toEqual({ count: 1 });
expect(sideAffect).toHaveBeenCalledTimes(2);

sagaExeReduxMiddleware.stop();

store.dispatch({ type: DELAYED_INCREMENT_COUNTER });
await sleep(100);
await sleep();

expect(store.getState()).toEqual({ count: 1 });
expect(sideAffect).toHaveBeenCalledTimes(2);
Expand Down
40 changes: 35 additions & 5 deletions x-pack/plugins/endpoint/public/applications/endpoint/lib/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,28 @@ export interface SagaContext<TAction extends AnyAction = AnyAction> {
dispatch: Dispatch<TAction>;
}

export interface SagaMiddleware extends Middleware {
/**
* Start the saga. Should be called after the `store` has been created
*/
start: () => void;

/**
* Stop the saga by exiting the internal generator `for await...of` loop.
*/
stop: () => void;
}

const noop = () => {};
const STOP = Symbol('STOP');

/**
* Creates Saga Middleware for use with Redux.
*
* @param {Saga} saga The `saga` should initialize a long-running `for await...of` loop against
* the return value of the `actionsAndState()` method provided by the `SagaContext`.
*
* @return {Middleware}
* @return {SagaMiddleware}
*
* @example
*
Expand All @@ -64,22 +77,31 @@ const noop = () => {};
* //....
* const store = createStore(reducers, [ endpointsSagaMiddleware ]);
*/
export function createSagaMiddleware(saga: Saga): Middleware {
export function createSagaMiddleware(saga: Saga): SagaMiddleware {
const iteratorInstances = new Set<IteratorInstance>();
let runSaga: () => void = noop;
let stopSaga: () => void = noop;
let runningPromise: Promise<symbol>;

async function* getActionsAndStateIterator(): StoreActionsAndState {
const instance: IteratorInstance = { queue: [], nextResolve: null };
iteratorInstances.add(instance);

try {
while (true) {
yield await nextActionAndState();
const actionAndState = await Promise.race([nextActionAndState(), runningPromise]);

if (actionAndState === STOP) {
break;
}

yield actionAndState as QueuedAction;
}
} finally {
// If the consumer stops consuming this (e.g. `break` or `return` is called in the `for await`
// then this `finally` block will run and unregister this instance and reset `runSaga`
iteratorInstances.delete(instance);
runSaga = noop;
runSaga = stopSaga = noop;
}

function nextActionAndState() {
Expand Down Expand Up @@ -109,7 +131,6 @@ export function createSagaMiddleware(saga: Saga): Middleware {
actionsAndState: getActionsAndStateIterator,
dispatch,
});
runSaga();
}
return (next: Dispatch<AnyAction>) => (action: AnyAction) => {
// Call the next dispatch method in the middleware chain.
Expand All @@ -125,5 +146,14 @@ export function createSagaMiddleware(saga: Saga): Middleware {
};
}

middleware.start = () => {
runningPromise = new Promise(resolve => (stopSaga = () => resolve(STOP)));
runSaga();
};

middleware.stop = () => {
stopSaga();
};

return middleware;
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('endpoint list saga', () => {
let fakeHttpServices: jest.Mocked<HttpSetup>;
let store: Store<EndpointListState>;
let dispatch: Dispatch<EndpointListAction>;
let stopSagas: () => void;

// TODO: consolidate the below ++ helpers in `index.test.ts` into a `test_helpers.ts`??
const generateEndpoint = (): EndpointData => {
Expand Down Expand Up @@ -89,13 +90,19 @@ describe('endpoint list saga', () => {
beforeEach(() => {
fakeCoreStart = coreMock.createStart({ basePath: '/mock' });
fakeHttpServices = fakeCoreStart.http as jest.Mocked<HttpSetup>;
store = createStore(
endpointListReducer,
applyMiddleware(createSagaMiddleware(endpointListSagaFactory()))
);

const sagaMiddleware = createSagaMiddleware(endpointListSagaFactory());
store = createStore(endpointListReducer, applyMiddleware(sagaMiddleware));

sagaMiddleware.start();
stopSagas = sagaMiddleware.stop;
dispatch = store.dispatch;
});

afterEach(() => {
stopSagas();
});

test('it handles `userEnteredEndpointListPage`', async () => {
const apiResponse = getEndpointListApiResponse();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { createStore, compose, applyMiddleware } from 'redux';
import { createStore, compose, applyMiddleware, Store } from 'redux';
import { CoreStart } from 'kibana/public';
import { appSagaFactory } from './saga';
import { appReducer } from './reducer';
Expand All @@ -15,10 +15,13 @@ const composeWithReduxDevTools = (window as any).__REDUX_DEVTOOLS_EXTENSION_COMP
? (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ name: 'EndpointApp' })
: compose;

export const appStoreFactory = (coreStart: CoreStart) => {
export const appStoreFactory = (coreStart: CoreStart): [Store, () => void] => {
const sagaReduxMiddleware = appSagaFactory(coreStart);
const store = createStore(
appReducer,
composeWithReduxDevTools(applyMiddleware(appSagaFactory(coreStart)))
composeWithReduxDevTools(applyMiddleware(sagaReduxMiddleware))
);
return store;

sagaReduxMiddleware.start();
return [store, sagaReduxMiddleware.stop];
};

0 comments on commit 5a5bade

Please sign in to comment.