Endpoint: middleware receive immutable versions of state and actions (#…

…63802) Middleware receive state and actions, but they shouldn't mutate either. With this PR, middleware using the `substateMiddlewareFactory` helper will have this enforced via typescript. * replace `MiddlewareFactory` with `ImmutableMiddlewareFactory` * Added types: `ImmutableMiddleware` and `ImmutableMiddlewareAPI` which are similar to the ones built into redux but which enforce that state and actions aren't mutated (and which allow `Immutable` versions of actions to be dispatched. No changes to runtime code.
elastic · Apr 17, 2020 · c0c21d1 · c0c21d1
1 parent a8c32b7
commit c0c21d1
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 47 deletions.
diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/action.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/action.ts
@@ -10,6 +10,9 @@ import { RoutingAction } from './routing';
 import { PolicyListAction } from './policy_list';
 import { PolicyDetailsAction } from './policy_details';
 
+/**
+ * The entire set of redux actions recognized by our reducer.
+ */
 export type AppAction =
   | HostAction
   | AlertAction

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts
@@ -6,13 +6,15 @@
 
 import { IIndexPattern } from 'src/plugins/data/public';
 import { AlertResultList, AlertDetails } from '../../../../../common/types';
-import { AppAction } from '../action';
-import { MiddlewareFactory, AlertListState } from '../../types';
+import { ImmutableMiddlewareFactory, AlertListState } from '../../types';
 import { isOnAlertPage, apiQueryParams, hasSelectedAlert, uiQueryParams } from './selectors';
 import { cloneHttpFetchQuery } from '../../../../common/clone_http_fetch_query';
 import { EndpointAppConstants } from '../../../../../common/types';
 
-export const alertMiddlewareFactory: MiddlewareFactory<AlertListState> = (coreStart, depsStart) => {
+export const alertMiddlewareFactory: ImmutableMiddlewareFactory<AlertListState> = (
+  coreStart,
+  depsStart
+) => {
   async function fetchIndexPatterns(): Promise<IIndexPattern[]> {
     const { indexPatterns } = depsStart.data;
     const eventsPattern: { indexPattern: string } = await coreStart.http.get(
@@ -29,7 +31,7 @@ export const alertMiddlewareFactory: MiddlewareFactory<AlertListState> = (coreSt
     return [indexPattern];
   }
 
-  return api => next => async (action: AppAction) => {
+  return api => next => async action => {
     next(action);
     const state = api.getState();
     if (action.type === 'userChangedUrl' && isOnAlertPage(state)) {

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/hosts/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/hosts/middleware.ts
@@ -4,13 +4,12 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { MiddlewareFactory } from '../../types';
+import { ImmutableMiddlewareFactory } from '../../types';
 import { pageIndex, pageSize, isOnHostPage, hasSelectedHost, uiQueryParams } from './selectors';
 import { HostListState } from '../../types';
-import { AppAction } from '../action';
 
-export const hostMiddlewareFactory: MiddlewareFactory<HostListState> = coreStart => {
-  return ({ getState, dispatch }) => next => async (action: AppAction) => {
+export const hostMiddlewareFactory: ImmutableMiddlewareFactory<HostListState> = coreStart => {
+  return ({ getState, dispatch }) => next => async action => {
     next(action);
     const state = getState();
     if (

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts
@@ -4,44 +4,25 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import {
-  createStore,
-  compose,
-  applyMiddleware,
-  Store,
-  MiddlewareAPI,
-  Dispatch,
-  Middleware,
-} from 'redux';
+import { createStore, compose, applyMiddleware, Store } from 'redux';
 import { CoreStart } from 'kibana/public';
 import { appReducer } from './reducer';
 import { alertMiddlewareFactory } from './alerts/middleware';
 import { hostMiddlewareFactory } from './hosts';
 import { policyListMiddlewareFactory } from './policy_list';
 import { policyDetailsMiddlewareFactory } from './policy_details';
-import { GlobalState, MiddlewareFactory } from '../types';
-import { AppAction } from './action';
+import { ImmutableMiddlewareFactory, SubstateMiddlewareFactory } from '../types';
 import { EndpointPluginStartDependencies } from '../../../plugin';
 
 const composeWithReduxDevTools = (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__
   ? (window as any).__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ name: 'EndpointApp' })
   : compose;
 
-export type Selector<S, R> = (state: S) => R;
-
-/**
- * Wrap Redux Middleware and adjust 'getState()' to return the namespace from 'GlobalState that applies to the given Middleware concern.
- *
- * @param selector
- * @param middleware
- */
-export const substateMiddlewareFactory = <Substate>(
-  selector: Selector<GlobalState, Substate>,
-  middleware: Middleware<{}, Substate, Dispatch<AppAction>>
-): Middleware<{}, GlobalState, Dispatch<AppAction>> => {
+export const substateMiddlewareFactory: SubstateMiddlewareFactory = (selector, middleware) => {
   return api => {
-    const substateAPI: MiddlewareAPI<Dispatch<AppAction>, Substate> = {
+    const substateAPI = {
       ...api,
+      // Return just the substate instead of global state.
       getState() {
         return selector(api.getState());
       },
@@ -66,7 +47,7 @@ export const appStoreFactory: (middlewareDeps?: {
    * Any additional Redux Middlewares
    * (should only be used for testing - example: to inject the action spy middleware)
    */
-  additionalMiddleware?: Array<ReturnType<MiddlewareFactory>>;
+  additionalMiddleware?: Array<ReturnType<ImmutableMiddlewareFactory>>;
 }) => Store = middlewareDeps => {
   let middleware;
   if (middlewareDeps) {

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/policy_details/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/policy_details/middleware.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { MiddlewareFactory, PolicyDetailsState, UpdatePolicyResponse } from '../../types';
+import { ImmutableMiddlewareFactory, PolicyDetailsState, UpdatePolicyResponse } from '../../types';
 import { policyIdFromParams, isOnPolicyDetailsPage, policyDetails } from './selectors';
 import {
   sendGetDatasource,
@@ -14,7 +14,7 @@ import {
 import { PolicyData } from '../../../../../common/types';
 import { factory as policyConfigFactory } from '../../../../../common/models/policy_config';
 
-export const policyDetailsMiddlewareFactory: MiddlewareFactory<PolicyDetailsState> = coreStart => {
+export const policyDetailsMiddlewareFactory: ImmutableMiddlewareFactory<PolicyDetailsState> = coreStart => {
   const http = coreStart.http;
 
   return ({ getState, dispatch }) => next => async action => {

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/policy_details/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/policy_details/selectors.ts
@@ -23,8 +23,8 @@ export const isOnPolicyDetailsPage = (state: Immutable<PolicyDetailsState>) => {
 };
 
 /** Returns the policyId from the url */
-export const policyIdFromParams: (state: PolicyDetailsState) => string = createSelector(
-  (state: PolicyDetailsState) => state.location,
+export const policyIdFromParams: (state: Immutable<PolicyDetailsState>) => string = createSelector(
+  state => state.location,
   (location: PolicyDetailsState['location']) => {
     if (location) {
       return location.pathname.split('/')[2];

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/middleware.ts
@@ -4,11 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { GetPolicyListResponse, MiddlewareFactory, PolicyListState } from '../../types';
+import { GetPolicyListResponse, ImmutableMiddlewareFactory, PolicyListState } from '../../types';
 import { sendGetEndpointSpecificDatasources } from './services/ingest';
 import { isOnPolicyListPage, urlSearchParams } from './selectors';
 
-export const policyListMiddlewareFactory: MiddlewareFactory<PolicyListState> = coreStart => {
+export const policyListMiddlewareFactory: ImmutableMiddlewareFactory<PolicyListState> = coreStart => {
   const http = coreStart.http;
 
   return ({ getState, dispatch }) => next => async action => {

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/test_utils.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/test_utils.ts
@@ -5,7 +5,7 @@
  */
 
 import { Dispatch } from 'redux';
-import { AppAction, GlobalState, MiddlewareFactory } from '../types';
+import { AppAction, GlobalState, ImmutableMiddlewareFactory } from '../types';
 
 /**
  * Utilities for testing Redux middleware
@@ -35,7 +35,7 @@ export interface MiddlewareActionSpyHelper<S = GlobalState, A extends AppAction
   /**
    * Redux middleware that enables spying on the action that are dispatched through the store
    */
-  actionSpyMiddleware: ReturnType<MiddlewareFactory<S>>;
+  actionSpyMiddleware: ReturnType<ImmutableMiddlewareFactory<S>>;
 }
 
 /**
@@ -109,7 +109,7 @@ export const createSpyMiddleware = <
       return spyDispatch.mock;
     },
 
-    actionSpyMiddleware: api => {
+    actionSpyMiddleware: () => {
       return next => {
         spyDispatch = jest.fn(action => {
           next(action);

diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts
@@ -4,7 +4,13 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { Dispatch, MiddlewareAPI, Action as ReduxAction, AnyAction as ReduxAnyAction } from 'redux';
+import {
+  Dispatch,
+  Action as ReduxAction,
+  AnyAction as ReduxAnyAction,
+  Action,
+  Middleware,
+} from 'redux';
 import { IIndexPattern } from 'src/plugins/data/public';
 import {
   HostMetadata,
@@ -28,12 +34,59 @@ import {
 } from '../../../../ingest_manager/common';
 
 export { AppAction };
-export type MiddlewareFactory<S = GlobalState> = (
+
+/**
+ * like redux's `MiddlewareAPI` but `getState` returns an `Immutable` version of
+ * state and `dispatch` accepts `Immutable` versions of actions.
+ */
+export interface ImmutableMiddlewareAPI<S, A extends Action> {
+  dispatch: Dispatch<A | Immutable<A>>;
+  getState(): Immutable<S>;
+}
+
+/**
+ * Like redux's `Middleware` but without the ability to mutate actions or state.
+ * Differences:
+ *   * `getState` returns an `Immutable` version of state
+ *   * `dispatch` accepts `Immutable` versions of actions
+ *   * `action`s received will be `Immutable`
+ */
+export type ImmutableMiddleware<S, A extends Action> = (
+  api: ImmutableMiddlewareAPI<S, A>
+) => (next: Dispatch<A | Immutable<A>>) => (action: Immutable<A>) => unknown;
+
+/**
+ * Takes application-standard middleware dependencies
+ * and returns a redux middleware.
+ * Middleware will be of the `ImmutableMiddleware` variety. Not able to directly
+ * change actions or state.
+ */
+export type ImmutableMiddlewareFactory<S = GlobalState> = (
   coreStart: CoreStart,
   depsStart: EndpointPluginStartDependencies
-) => (
-  api: MiddlewareAPI<Dispatch<AppAction>, S>
-) => (next: Dispatch<AppAction>) => (action: AppAction) => unknown;
+) => ImmutableMiddleware<S, AppAction>;
+
+/**
+ * Simple type for a redux selector.
+ */
+type Selector<S, R> = (state: S) => R;
+
+/**
+ * Takes a selector and an `ImmutableMiddleware`. The
+ * middleware's version of `getState` will receive
+ * the result of the selector instead of the global state.
+ *
+ * This allows middleware to have knowledge of only a subsection of state.
+ *
+ * `selector` returns an `Immutable` version of the substate.
+ * `middleware` must be an `ImmutableMiddleware`.
+ *
+ * Returns a regular middleware, meant to be used with `applyMiddleware`.
+ */
+export type SubstateMiddlewareFactory = <Substate>(
+  selector: Selector<GlobalState, Immutable<Substate>>,
+  middleware: ImmutableMiddleware<Substate, AppAction>
+) => Middleware<{}, GlobalState, Dispatch<AppAction | Immutable<AppAction>>>;
 
 export interface HostListState {
   hosts: HostMetadata[];