From c54d8bf4ad751964658832c51386e870ea734685 Mon Sep 17 00:00:00 2001 From: kgkma Date: Wed, 7 Feb 2018 19:08:28 +1100 Subject: [PATCH 1/2] feat(StoreDevtools): Add support for ActionSanitizer and StateSanitizer --- modules/store-devtools/spec/extension.spec.ts | 15 +- modules/store-devtools/spec/store.spec.ts | 132 ++++++++++++++++++ modules/store-devtools/src/config.ts | 7 +- modules/store-devtools/src/devtools.ts | 37 ++++- modules/store-devtools/src/extension.ts | 6 +- modules/store-devtools/src/instrument.ts | 12 +- modules/store-devtools/src/reducer.ts | 58 ++++++-- 7 files changed, 229 insertions(+), 38 deletions(-) diff --git a/modules/store-devtools/spec/extension.spec.ts b/modules/store-devtools/spec/extension.spec.ts index 972951edba..01dd935eae 100644 --- a/modules/store-devtools/spec/extension.spec.ts +++ b/modules/store-devtools/spec/extension.spec.ts @@ -2,13 +2,12 @@ import { Action } from '@ngrx/store'; import { of } from 'rxjs/observable/of'; import { LiftedState } from '../'; -import { DevtoolsExtension, ReduxDevtoolsExtension } from '../src/extension'; import { - createConfig, - noActionSanitizer, - noMonitor, - noStateSanitizer, -} from '../src/instrument'; + DevtoolsExtension, + ReduxDevtoolsExtension, + ReduxDevtoolsExtensionConnection, +} from '../src/extension'; +import { createConfig, noMonitor } from '../src/instrument'; describe('DevtoolsExtension', () => { let reduxDevtoolsExtension: ReduxDevtoolsExtension; @@ -32,8 +31,8 @@ describe('DevtoolsExtension', () => { const defaultOptions = { maxAge: false, monitor: noMonitor, - actionSanitizer: noActionSanitizer, - stateSanitizer: noStateSanitizer, + actionSanitizer: undefined, + stateSanitizer: undefined, name: 'NgRx Store DevTools', serialize: false, logOnly: false, diff --git a/modules/store-devtools/spec/store.spec.ts b/modules/store-devtools/spec/store.spec.ts index 57271373a7..ed391d743f 100644 --- a/modules/store-devtools/spec/store.spec.ts +++ b/modules/store-devtools/spec/store.spec.ts @@ -610,4 +610,136 @@ describe('Store Devtools', () => { expect(fixture.getLiftedState()).toEqual(exportedState); }); }); + + describe('Action and State Sanitizer', () => { + let fixture: Fixture; + + const SANITIZED_TOKEN = 'SANITIZED_ACTION'; + const SANITIZED_COUNTER = 42; + const testActionSanitizer = (action: Action, id: number) => { + return { type: SANITIZED_TOKEN }; + }; + const incrementActionSanitizer = (action: Action, id: number) => { + return { type: 'INCREMENT' }; + }; + const testStateSanitizer = (state: any, index: number) => { + return { state: SANITIZED_COUNTER }; + }; + + afterEach(() => { + fixture.cleanup(); + }); + + it('should function normally with no sanitizers', () => { + fixture = createStore(counter); + + fixture.store.dispatch({ type: 'INCREMENT' }); + + const liftedState = fixture.getLiftedState(); + const currentLiftedState = + liftedState.computedStates[liftedState.currentStateIndex]; + expect(Object.keys(liftedState.actionsById).length).toBe( + Object.keys(liftedState.sanitizedActionsById).length + ); + expect(liftedState.actionsById).toEqual(liftedState.sanitizedActionsById); + expect(currentLiftedState.state).toEqual({ state: 1 }); + expect(currentLiftedState.sanitizedState).toBeUndefined(); + }); + + it('should run the action sanitizer on actions', () => { + fixture = createStore(counter, { + actionSanitizer: testActionSanitizer, + }); + + fixture.store.dispatch({ type: 'INCREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + + const liftedState = fixture.getLiftedState(); + const sanitizedAction = + liftedState.sanitizedActionsById[liftedState.nextActionId - 1]; + const sanitizedAction2 = + liftedState.sanitizedActionsById[liftedState.nextActionId - 2]; + const action = liftedState.actionsById[liftedState.nextActionId - 1]; + const action2 = liftedState.actionsById[liftedState.nextActionId - 2]; + + expect(liftedState.actionsById).not.toEqual( + liftedState.sanitizedActionsById + ); + expect(sanitizedAction.action).toEqual({ type: SANITIZED_TOKEN }); + expect(sanitizedAction2.action).toEqual({ type: SANITIZED_TOKEN }); + expect(action.action).toEqual({ type: 'DECREMENT' }); + expect(action2.action).toEqual({ type: 'INCREMENT' }); + }); + + it('should run the state sanitizer on store state', () => { + fixture = createStore(counter, { + stateSanitizer: testStateSanitizer, + }); + + let liftedState = fixture.getLiftedState(); + let currentLiftedState = + liftedState.computedStates[liftedState.currentStateIndex]; + expect(fixture.getState()).toBe(0); + expect(currentLiftedState.state).toEqual({ state: 0 }); + expect(currentLiftedState.sanitizedState).toBeDefined(); + expect(currentLiftedState.sanitizedState).toEqual({ + state: SANITIZED_COUNTER, + }); + + fixture.store.dispatch({ type: 'INCREMENT' }); + + liftedState = fixture.getLiftedState(); + currentLiftedState = + liftedState.computedStates[liftedState.currentStateIndex]; + expect(fixture.getState()).toBe(1); + expect(currentLiftedState.state).toEqual({ state: 1 }); + expect(currentLiftedState.sanitizedState).toEqual({ + state: SANITIZED_COUNTER, + }); + }); + + it('should run transparently to produce a new lifted store state', () => { + const devtoolsOptions: Partial = { + actionSanitizer: testActionSanitizer, + stateSanitizer: testStateSanitizer, + }; + fixture = createStore(counter, devtoolsOptions); + + fixture.store.dispatch({ type: 'INCREMENT' }); + + const liftedState = fixture.getLiftedState(); + const sanitizedLiftedState = fixture.devtools.getSanitizedState( + liftedState, + devtoolsOptions + ); + const originalAction = + liftedState.actionsById[liftedState.nextActionId - 1]; + const originalState = + liftedState.computedStates[liftedState.currentStateIndex]; + const sanitizedAction = + sanitizedLiftedState.actionsById[liftedState.nextActionId - 1]; + const sanitizedState = + sanitizedLiftedState.computedStates[liftedState.currentStateIndex]; + + expect(originalAction.action).toEqual({ type: 'INCREMENT' }); + expect(originalState.state).toEqual({ state: 1 }); + expect(sanitizedAction.action).toEqual({ type: SANITIZED_TOKEN }); + expect(sanitizedState.state).toEqual({ state: SANITIZED_COUNTER }); + }); + + it('sanitized actions should not affect the store state', () => { + fixture = createStore(counter, { + actionSanitizer: incrementActionSanitizer, + }); + + fixture.store.dispatch({ type: 'DECREMENT' }); + fixture.store.dispatch({ type: 'DECREMENT' }); + + const liftedState = fixture.getLiftedState(); + expect(fixture.getState()).toBe(-2); + expect( + liftedState.computedStates[liftedState.currentStateIndex].state + ).toEqual({ state: -2 }); + }); + }); }); diff --git a/modules/store-devtools/src/config.ts b/modules/store-devtools/src/config.ts index 47ba8d1334..71fcccb4a8 100644 --- a/modules/store-devtools/src/config.ts +++ b/modules/store-devtools/src/config.ts @@ -1,11 +1,14 @@ import { ActionReducer, Action } from '@ngrx/store'; import { InjectionToken, Type } from '@angular/core'; +export type ActionSanitizer = (action: Action, id: number) => Action; +export type StateSanitizer = (state: any, index: number) => any; + export class StoreDevtoolsConfig { maxAge: number | false; monitor: ActionReducer; - actionSanitizer?: (action: A, id: number) => A; - stateSanitizer?: (state: S, index: number) => S; + actionSanitizer?: ActionSanitizer; + stateSanitizer?: StateSanitizer; name?: string; serialize?: boolean; logOnly?: boolean; diff --git a/modules/store-devtools/src/devtools.ts b/modules/store-devtools/src/devtools.ts index b964f35fda..43c120c7e8 100644 --- a/modules/store-devtools/src/devtools.ts +++ b/modules/store-devtools/src/devtools.ts @@ -21,7 +21,12 @@ import { queue } from 'rxjs/scheduler/queue'; import { DevtoolsExtension } from './extension'; import { liftAction, unliftAction, unliftState, applyOperators } from './utils'; -import { liftReducerWith, liftInitialState, LiftedState } from './reducer'; +import { + liftReducerWith, + liftInitialState, + LiftedState, + ComputedState, +} from './reducer'; import * as Actions from './actions'; import { StoreDevtoolsConfig, STORE_DEVTOOLS_CONFIG } from './config'; @@ -68,11 +73,15 @@ export class StoreDevtools implements Observer { [ scan, ({ state: liftedState }: any, [action, reducer]: any) => { - const state = reducer(liftedState, action); + const reducedLiftedState = reducer(liftedState, action); - extension.notify(action, state); + // Extension should be sent the sanitized lifted state + extension.notify( + action, + this.getSanitizedState(reducedLiftedState, config) + ); - return { state, action }; + return { state: reducedLiftedState, action }; }, { state: liftedInitialState, action: null }, ], @@ -97,6 +106,26 @@ export class StoreDevtools implements Observer { this.state = state$; } + /** + * Restructures the lifted state passed in to prepare for sending to the + * Redux Devtools Extension + */ + getSanitizedState(state: LiftedState, config: Partial) { + const sanitizedComputedStates = config.stateSanitizer + ? state.computedStates.map((entry: ComputedState) => ({ + state: entry.sanitizedState, + error: entry.error, + })) + : state.computedStates; + + // Replace action and state logs with their sanitized versions + return { + ...state, + actionsById: state.sanitizedActionsById, + computedStates: sanitizedComputedStates, + }; + } + dispatch(action: Action) { this.dispatcher.next(action); } diff --git a/modules/store-devtools/src/extension.ts b/modules/store-devtools/src/extension.ts index 7394673768..7d904e1b74 100644 --- a/modules/store-devtools/src/extension.ts +++ b/modules/store-devtools/src/extension.ts @@ -11,7 +11,7 @@ import { takeUntil } from 'rxjs/operator/takeUntil'; import { STORE_DEVTOOLS_CONFIG, StoreDevtoolsConfig } from './config'; import { LiftedState } from './reducer'; import { PerformAction } from './actions'; -import { applyOperators } from './utils'; +import { applyOperators, unliftState } from './utils'; export const ExtensionActionTypes = { START: 'START', @@ -36,6 +36,7 @@ export interface ReduxDevtoolsExtensionConfig { name: string | undefined; instanceId: string; maxAge?: number; + actionSanitizer?: (action: Action, id: number) => Action; } export interface ReduxDevtoolsExtension { @@ -86,7 +87,7 @@ export class DevtoolsExtension { // d) any action that is not a PerformAction to err on the side of // caution. if (action instanceof PerformAction) { - const currentState = state.computedStates[state.currentStateIndex].state; + const currentState = unliftState(state); this.extensionConnection.send(action.action, currentState); } else { // Requires full state update; @@ -104,6 +105,7 @@ export class DevtoolsExtension { instanceId: this.instanceId, name: this.config.name, features: this.config.features, + actionSanitizer: this.config.actionSanitizer, }; if (this.config.maxAge !== false /* support === 0 */) { extensionOptions.maxAge = this.config.maxAge; diff --git a/modules/store-devtools/src/instrument.ts b/modules/store-devtools/src/instrument.ts index 0c31d15c95..4125bb0309 100644 --- a/modules/store-devtools/src/instrument.ts +++ b/modules/store-devtools/src/instrument.ts @@ -64,14 +64,6 @@ export function noMonitor(): null { return null; } -export function noActionSanitizer(): null { - return null; -} - -export function noStateSanitizer(): null { - return null; -} - export const DEFAULT_NAME = 'NgRx Store DevTools'; export function createConfig( @@ -80,8 +72,8 @@ export function createConfig( const DEFAULT_OPTIONS: StoreDevtoolsConfig = { maxAge: false, monitor: noMonitor, - actionSanitizer: noActionSanitizer, - stateSanitizer: noStateSanitizer, + actionSanitizer: undefined, + stateSanitizer: undefined, name: DEFAULT_NAME, serialize: false, logOnly: false, diff --git a/modules/store-devtools/src/reducer.ts b/modules/store-devtools/src/reducer.ts index 9319bc5be5..ee57707199 100644 --- a/modules/store-devtools/src/reducer.ts +++ b/modules/store-devtools/src/reducer.ts @@ -8,7 +8,7 @@ import { } from '@ngrx/store'; import { difference, liftAction } from './utils'; import * as Actions from './actions'; -import { StoreDevtoolsConfig } from './config'; +import { StoreDevtoolsConfig, StateSanitizer } from './config'; import { PerformAction } from './actions'; export type InitAction = { @@ -24,15 +24,22 @@ export type Actions = Actions.All | CoreActions; export const INIT_ACTION = { type: INIT }; +export interface ComputedState { + state: any; + sanitizedState?: any; + error: any; +} + export interface LiftedState { monitorState: any; nextActionId: number; actionsById: { [id: number]: { action: Action } }; + sanitizedActionsById: { [id: number]: { action: Action } }; stagedActionIds: number[]; skippedActionIds: number[]; committedState: any; currentStateIndex: number; - computedStates: { state: any; error: any }[]; + computedStates: ComputedState[]; } /** @@ -41,7 +48,7 @@ export interface LiftedState { function computeNextEntry( reducer: ActionReducer, action: Action, - state: LiftedState, + state: any, error: any ) { if (error) { @@ -70,13 +77,14 @@ function computeNextEntry( * Runs the reducer on invalidated actions to get a fresh computation log. */ function recomputeStates( - computedStates: { state: any; error: any }[], + computedStates: ComputedState[], minInvalidatedStateIndex: number, reducer: ActionReducer, committedState: any, actionsById: { [id: number]: { action: Action } }, stagedActionIds: number[], - skippedActionIds: number[] + skippedActionIds: number[], + stateSanitizer?: StateSanitizer ) { // Optimization: exit early and return the same reference // if we know nothing could have changed. @@ -97,11 +105,19 @@ function recomputeStates( const previousError = previousEntry ? previousEntry.error : undefined; const shouldSkip = skippedActionIds.indexOf(actionId) > -1; - const entry = shouldSkip + const entry: ComputedState = shouldSkip ? previousEntry : computeNextEntry(reducer, action, previousState, previousError); - nextComputedStates.push(entry); + if (stateSanitizer) { + const sanitizedEntry = { + ...entry, + sanitizedState: stateSanitizer(entry.state, actionId), + }; + nextComputedStates.push(sanitizedEntry); + } else { + nextComputedStates.push(entry); + } } return nextComputedStates; @@ -115,6 +131,7 @@ export function liftInitialState( monitorState: monitorReducer(undefined, {}), nextActionId: 1, actionsById: { 0: liftAction(INIT_ACTION) }, + sanitizedActionsById: { 0: liftAction(INIT_ACTION) }, stagedActionIds: [0], skippedActionIds: [], committedState: initialCommittedState, @@ -141,6 +158,7 @@ export function liftReducerWith( let { monitorState, actionsById, + sanitizedActionsById, nextActionId, stagedActionIds, skippedActionIds, @@ -153,6 +171,7 @@ export function liftReducerWith( if (!liftedState) { // Prevent mutating initialLiftedState actionsById = Object.create(actionsById); + sanitizedActionsById = Object.create(sanitizedActionsById); } function commitExcessActions(n: number) { @@ -168,6 +187,7 @@ export function liftReducerWith( break; } else { delete actionsById[idsToDelete[i]]; + delete sanitizedActionsById[idsToDelete[i]]; } } @@ -181,7 +201,7 @@ export function liftReducerWith( currentStateIndex > excess ? currentStateIndex - excess : 0; } - // By default, agressively recompute every state whatever happens. + // By default, aggressively recompute every state whatever happens. // This has O(n) performance, so we'll override this to a sensible // value whenever we feel like we don't have to recompute the states. let minInvalidatedStateIndex = 0; @@ -190,6 +210,7 @@ export function liftReducerWith( case Actions.RESET: { // Get back to the state the store was created with. actionsById = { 0: liftAction(INIT_ACTION) }; + sanitizedActionsById = { 0: liftAction(INIT_ACTION) }; nextActionId = 1; stagedActionIds = [0]; skippedActionIds = []; @@ -202,6 +223,7 @@ export function liftReducerWith( // Consider the last committed state the new starting point. // Squash any staged actions into a single committed state. actionsById = { 0: liftAction(INIT_ACTION) }; + sanitizedActionsById = { 0: liftAction(INIT_ACTION) }; nextActionId = 1; stagedActionIds = [0]; skippedActionIds = []; @@ -214,6 +236,7 @@ export function liftReducerWith( // Forget about any staged actions. // Start again from the last committed state. actionsById = { 0: liftAction(INIT_ACTION) }; + sanitizedActionsById = { 0: liftAction(INIT_ACTION) }; nextActionId = 1; stagedActionIds = [0]; skippedActionIds = []; @@ -290,6 +313,10 @@ export function liftReducerWith( // Mutation! This is the hottest path, and we optimize on purpose. // It is safe because we set a new key in a cache dictionary. actionsById[actionId] = liftedAction; + sanitizedActionsById[actionId] = options.actionSanitizer + ? liftAction(options.actionSanitizer(liftedAction.action, actionId)) + : liftedAction; + stagedActionIds = [...stagedActionIds, actionId]; // Optimization: we know that only the new action needs computing. minInvalidatedStateIndex = stagedActionIds.length - 1; @@ -300,6 +327,7 @@ export function liftReducerWith( ({ monitorState, actionsById, + sanitizedActionsById, nextActionId, stagedActionIds, skippedActionIds, @@ -322,7 +350,8 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds + skippedActionIds, + options.stateSanitizer ); commitExcessActions(stagedActionIds.length - options.maxAge); @@ -350,7 +379,8 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds + skippedActionIds, + options.stateSanitizer ); commitExcessActions(stagedActionIds.length - options.maxAge); @@ -366,6 +396,7 @@ export function liftReducerWith( // Add a new action to only recompute state const actionId = nextActionId++; actionsById[actionId] = new PerformAction(liftedAction); + sanitizedActionsById[actionId] = new PerformAction(liftedAction); stagedActionIds = [...stagedActionIds, actionId]; minInvalidatedStateIndex = stagedActionIds.length - 1; @@ -378,7 +409,8 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds + skippedActionIds, + options.stateSanitizer ); currentStateIndex = minInvalidatedStateIndex; @@ -408,13 +440,15 @@ export function liftReducerWith( committedState, actionsById, stagedActionIds, - skippedActionIds + skippedActionIds, + options.stateSanitizer ); monitorState = monitorReducer(monitorState, liftedAction); return { monitorState, actionsById, + sanitizedActionsById, nextActionId, stagedActionIds, skippedActionIds, From c6d61101f5d6e6855c5299b088f8d8f57c986952 Mon Sep 17 00:00:00 2001 From: kgkma Date: Mon, 12 Feb 2018 11:30:04 +1100 Subject: [PATCH 2/2] feat(StoreDevtools): Pass stateSanitizer as argument instead --- modules/store-devtools/spec/extension.spec.ts | 6 +----- modules/store-devtools/spec/store.spec.ts | 2 +- modules/store-devtools/src/devtools.ts | 12 ++++++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/modules/store-devtools/spec/extension.spec.ts b/modules/store-devtools/spec/extension.spec.ts index 01dd935eae..0fc724a3fb 100644 --- a/modules/store-devtools/spec/extension.spec.ts +++ b/modules/store-devtools/spec/extension.spec.ts @@ -2,11 +2,7 @@ import { Action } from '@ngrx/store'; import { of } from 'rxjs/observable/of'; import { LiftedState } from '../'; -import { - DevtoolsExtension, - ReduxDevtoolsExtension, - ReduxDevtoolsExtensionConnection, -} from '../src/extension'; +import { DevtoolsExtension, ReduxDevtoolsExtension } from '../src/extension'; import { createConfig, noMonitor } from '../src/instrument'; describe('DevtoolsExtension', () => { diff --git a/modules/store-devtools/spec/store.spec.ts b/modules/store-devtools/spec/store.spec.ts index ed391d743f..681b319a27 100644 --- a/modules/store-devtools/spec/store.spec.ts +++ b/modules/store-devtools/spec/store.spec.ts @@ -710,7 +710,7 @@ describe('Store Devtools', () => { const liftedState = fixture.getLiftedState(); const sanitizedLiftedState = fixture.devtools.getSanitizedState( liftedState, - devtoolsOptions + devtoolsOptions.stateSanitizer ); const originalAction = liftedState.actionsById[liftedState.nextActionId - 1]; diff --git a/modules/store-devtools/src/devtools.ts b/modules/store-devtools/src/devtools.ts index 43c120c7e8..11332f2e01 100644 --- a/modules/store-devtools/src/devtools.ts +++ b/modules/store-devtools/src/devtools.ts @@ -28,7 +28,11 @@ import { ComputedState, } from './reducer'; import * as Actions from './actions'; -import { StoreDevtoolsConfig, STORE_DEVTOOLS_CONFIG } from './config'; +import { + StoreDevtoolsConfig, + STORE_DEVTOOLS_CONFIG, + StateSanitizer, +} from './config'; @Injectable() export class DevtoolsDispatcher extends ActionsSubject {} @@ -78,7 +82,7 @@ export class StoreDevtools implements Observer { // Extension should be sent the sanitized lifted state extension.notify( action, - this.getSanitizedState(reducedLiftedState, config) + this.getSanitizedState(reducedLiftedState, config.stateSanitizer) ); return { state: reducedLiftedState, action }; @@ -110,8 +114,8 @@ export class StoreDevtools implements Observer { * Restructures the lifted state passed in to prepare for sending to the * Redux Devtools Extension */ - getSanitizedState(state: LiftedState, config: Partial) { - const sanitizedComputedStates = config.stateSanitizer + getSanitizedState(state: LiftedState, stateSanitizer?: StateSanitizer) { + const sanitizedComputedStates = stateSanitizer ? state.computedStates.map((entry: ComputedState) => ({ state: entry.sanitizedState, error: entry.error,