From 2142e83c022d6ad722e6a70cfed6e9a4aeb43696 Mon Sep 17 00:00:00 2001 From: Oscar Lorentzon Date: Sat, 24 Apr 2021 15:36:15 +0200 Subject: [PATCH] refactor: validate state transition in context --- src/state/StateContext.ts | 26 +++++++++++------- src/state/StateService.ts | 1 - src/state/StateTransitionMatrix.ts | 25 ++++++++++-------- src/viewer/Viewer.ts | 26 +++++++----------- test/viewer/Viewer.test.ts | 42 +++--------------------------- 5 files changed, 43 insertions(+), 77 deletions(-) diff --git a/src/state/StateContext.ts b/src/state/StateContext.ts index 9bc22f34..7075d2ae 100644 --- a/src/state/StateContext.ts +++ b/src/state/StateContext.ts @@ -16,15 +16,17 @@ export class StateContext implements IStateContext { constructor(transitionMode?: TransitionMode) { this._transitions = new StateTransitionMatrix(); - this._state = this._transitions.initialize({ - alpha: 1, - camera: new Camera(), - currentIndex: -1, - reference: { alt: 0, lat: 0, lng: 0 }, - trajectory: [], - transitionMode: transitionMode == null ? TransitionMode.Default : transitionMode, - zoom: 0, - }); + this._state = this._transitions.generate( + State.Traversing, + { + alpha: 1, + camera: new Camera(), + currentIndex: -1, + reference: { alt: 0, lat: 0, lng: 0 }, + trajectory: [], + transitionMode: transitionMode == null ? TransitionMode.Default : transitionMode, + zoom: 0, + }); } public get state(): State { @@ -208,6 +210,12 @@ export class StateContext implements IStateContext { } private _transition(to: State): void { + if (!this._transitions.validate(this._state, to)) { + const from = this._transitions.getState(this._state); + console.warn( + `Transition not valid (${State[from]} - ${State[to]})`); + return; + } const state = this._transitions.transition(this._state, to); this._state = state; } diff --git a/src/state/StateService.ts b/src/state/StateService.ts index 86de5d00..55b41599 100644 --- a/src/state/StateService.ts +++ b/src/state/StateService.ts @@ -556,7 +556,6 @@ export class StateService { .next( (context: IStateContext): IStateContext => { action(context); - return context; }); } diff --git a/src/state/StateTransitionMatrix.ts b/src/state/StateTransitionMatrix.ts index 39aab79d..0691a97d 100644 --- a/src/state/StateTransitionMatrix.ts +++ b/src/state/StateTransitionMatrix.ts @@ -6,7 +6,7 @@ import { StateBase } from "./state/StateBase"; import { TraversingState } from "./state/TraversingState"; import { WaitingState } from "./state/WaitingState"; -type StateCreators = Map StateBase>; +type StateCreators = Map StateBase>; export class StateTransitionMatrix { private readonly _creators: StateCreators; @@ -27,7 +27,7 @@ export class StateTransitionMatrix { this._transitions = new Map(); const transitions = this._transitions; - transitions.set(earth, [traverse, wait, waitInteractively]); + transitions.set(earth, [traverse]); transitions.set(traverse, [earth, wait, waitInteractively]); transitions.set(wait, [traverse, waitInteractively]); transitions.set(waitInteractively, [traverse, wait]); @@ -46,21 +46,24 @@ export class StateTransitionMatrix { throw new Error("Invalid state instance"); } - public initialize(state: IStateBase): StateBase { - return new TraversingState(state); + public generate(state: State, options: IStateBase): StateBase { + const stateImplementation = this._creators.get(State[state]); + return new stateImplementation(options); } public transition(state: StateBase, to: State): StateBase { + if (!this.validate(state, to)) { + throw new Error("Invalid transition"); + } + return this.generate(to, state); + } + + public validate(state: StateBase, to: State): boolean { const source = State[this.getState(state)]; const target = State[to]; const transitions = this._transitions; - if (!transitions.has(source) || - !transitions.get(source).includes(target)) { - throw new Error("Invalid transition"); - } - - const stateImplementation = this._creators.get(target); - return new stateImplementation(state); + return transitions.has(source) && + transitions.get(source).includes(target); } } diff --git a/src/viewer/Viewer.ts b/src/viewer/Viewer.ts index 044779c3..c8db7e53 100644 --- a/src/viewer/Viewer.ts +++ b/src/viewer/Viewer.ts @@ -1217,23 +1217,15 @@ export class Viewer extends EventEmitter implements IViewer { * ``` */ public setCameraControls(controls: CameraControls): void { - this._navigator.stateService.state$ - .pipe(first()) - .subscribe( - (s): void => { - if (s === State.Earth && - controls === CameraControls.Street) { - this._navigator.stateService.traverse(); - } else if ( - s === State.Traversing && - controls === CameraControls.Earth) { - this._navigator.stateService.earth(); - } else { - const to = CameraControls[controls]; - console.warn( - `Unsupported camera control transition (${to})`); - } - }) + if (controls === CameraControls.Street) { + this._navigator.stateService.traverse(); + } else if (controls === CameraControls.Earth) { + this._navigator.stateService.earth(); + } else { + const to = CameraControls[controls]; + console.warn( + `Unsupported camera control transition (${to})`); + } } /** diff --git a/test/viewer/Viewer.test.ts b/test/viewer/Viewer.test.ts index 33ece33f..09670baa 100644 --- a/test/viewer/Viewer.test.ts +++ b/test/viewer/Viewer.test.ts @@ -162,66 +162,30 @@ describe("Viewer.setCameraControls", () => { spyOn(console, "warn").and.stub(); }) - it("should set different state", () => { + it("should invoke state transition", () => { const mocks = createMocks(); const viewer = new Viewer({ apiClient: "", container: "" }); const earthSpy = (mocks.navigator.stateService.earth); const traverseSpy = (mocks.navigator.stateService.traverse); - const state$ = >mocks.navigator.stateService.state$; - viewer.setCameraControls(CameraControls.Earth); - state$.next(State.Traversing); expect(earthSpy.calls.count()).toBe(1); expect(traverseSpy.calls.count()).toBe(0); viewer.setCameraControls(CameraControls.Street); - state$.next(State.Earth); expect(earthSpy.calls.count()).toBe(1); expect(traverseSpy.calls.count()).toBe(1); }); - it("should not set same state", () => { + it("should not invoke state transition", () => { const mocks = createMocks(); const viewer = new Viewer({ apiClient: "", container: "" }); const earthSpy = (mocks.navigator.stateService.earth); const traverseSpy = (mocks.navigator.stateService.traverse); - const state$ = >mocks.navigator.stateService.state$; - - viewer.setCameraControls(CameraControls.Earth); - state$.next(State.Earth); - expect(earthSpy.calls.count()).toBe(0); - expect(traverseSpy.calls.count()).toBe(0); - - viewer.setCameraControls(CameraControls.Street); - state$.next(State.Traversing); - expect(earthSpy.calls.count()).toBe(0); - expect(traverseSpy.calls.count()).toBe(0); - }); - - it("should not change state when waiting", () => { - const mocks = createMocks(); - const viewer = new Viewer({ apiClient: "", container: "" }); - - const earthSpy = (mocks.navigator.stateService.earth); - const traverseSpy = (mocks.navigator.stateService.traverse); - - const state$ = >mocks.navigator.stateService.state$; - - viewer.setCameraControls(CameraControls.Earth); - state$.next(State.Waiting); - viewer.setCameraControls(CameraControls.Street); - state$.next(State.Waiting); - expect(earthSpy.calls.count()).toBe(0); - expect(traverseSpy.calls.count()).toBe(0); - - viewer.setCameraControls(CameraControls.Earth); - state$.next(State.WaitingInteractively); - viewer.setCameraControls(CameraControls.Street); - state$.next(State.WaitingInteractively); + viewer.setCameraControls(-1); expect(earthSpy.calls.count()).toBe(0); expect(traverseSpy.calls.count()).toBe(0); });