Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Endpoint] Fix saga to ensure they are started only after store is created #55245

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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];
};