Browse Source

fix(core): Improved validation of state machine configs

Fixes #2521
Michael Bromley 2 years ago
parent
commit
b44cc886aa

+ 26 - 0
packages/core/src/common/finite-state-machine/validate-transition-definition.spec.ts

@@ -77,4 +77,30 @@ describe('FSM validateTransitionDefinition()', () => {
         expect(result.valid).toBe(false);
         expect(result.error).toBe('The following states are unreachable: Unreachable');
     });
+
+    it('invalid - non-existent transition', () => {
+        const valid: Transitions<'Start' | 'End' | 'Unreachable'> = {
+            Start: { to: ['End'] },
+            End: { to: ['Bad' as any] },
+            Unreachable: { to: [] },
+        };
+
+        const result = validateTransitionDefinition(valid, 'Start');
+
+        expect(result.valid).toBe(false);
+        expect(result.error).toBe('The state "End" has a transition to an unknown state "Bad"');
+    });
+
+    it('invalid - missing initial state', () => {
+        const valid: Transitions<'Start' | 'End' | 'Unreachable'> = {
+            Start: { to: ['End'] },
+            End: { to: ['Start'] },
+            Unreachable: { to: [] },
+        };
+
+        const result = validateTransitionDefinition(valid, 'Created' as any);
+
+        expect(result.valid).toBe(false);
+        expect(result.error).toBe('The initial state "Created" is not defined');
+    });
 });

+ 18 - 2
packages/core/src/common/finite-state-machine/validate-transition-definition.ts

@@ -10,6 +10,12 @@ export function validateTransitionDefinition<T extends string>(
     transitions: Transitions<T>,
     initialState: T,
 ): { valid: boolean; error?: string } {
+    if (!transitions[initialState]) {
+        return {
+            valid: false,
+            error: `The initial state "${initialState}" is not defined`,
+        };
+    }
     const states = Object.keys(transitions) as T[];
     const result: { [State in T]: ValidationResult } = states.reduce((res, state) => {
         return {
@@ -21,7 +27,7 @@ export function validateTransitionDefinition<T extends string>(
     // walk the state graph starting with the initialState and
     // check whether all states are reachable.
     function allStatesReached(): boolean {
-        return Object.values(result).every((r) => (r as ValidationResult).reachable);
+        return Object.values(result).every(r => (r as ValidationResult).reachable);
     }
     function walkGraph(state: T) {
         const candidates = transitions[state].to;
@@ -30,12 +36,22 @@ export function validateTransitionDefinition<T extends string>(
             return true;
         }
         for (const candidate of candidates) {
+            if (result[candidate] === undefined) {
+                throw new Error(`The state "${state}" has a transition to an unknown state "${candidate}"`);
+            }
             if (!result[candidate].reachable) {
                 walkGraph(candidate);
             }
         }
     }
-    walkGraph(initialState);
+    try {
+        walkGraph(initialState);
+    } catch (e: any) {
+        return {
+            valid: false,
+            error: e.message,
+        };
+    }
 
     if (!allStatesReached()) {
         return {

+ 6 - 3
packages/core/src/service/helpers/fulfillment-state-machine/fulfillment-state-machine.ts

@@ -8,7 +8,7 @@ import { StateMachineConfig, Transitions } from '../../../common/finite-state-ma
 import { validateTransitionDefinition } from '../../../common/finite-state-machine/validate-transition-definition';
 import { awaitPromiseOrObservable } from '../../../common/utils';
 import { ConfigService } from '../../../config/config.service';
-import { TransactionalConnection } from '../../../connection/index';
+import { Logger } from '../../../config/logger/vendure-logger';
 import { Fulfillment } from '../../../entity/fulfillment/fulfillment.entity';
 import { Order } from '../../../entity/order/order.entity';
 
@@ -19,7 +19,7 @@ export class FulfillmentStateMachine {
     readonly config: StateMachineConfig<FulfillmentState, FulfillmentTransitionData>;
     private readonly initialState: FulfillmentState = 'Created';
 
-    constructor(private configService: ConfigService, private connection: TransactionalConnection) {
+    constructor(private configService: ConfigService) {
         this.config = this.initConfig();
     }
 
@@ -59,7 +59,10 @@ export class FulfillmentStateMachine {
         );
 
         const validationResult = validateTransitionDefinition(allTransitions, 'Pending');
-
+        if (!validationResult.valid && validationResult.error) {
+            Logger.error(`The fulfillment process has an invalid configuration:`);
+            throw new Error(validationResult.error);
+        }
         return {
             transitions: allTransitions,
             onTransitionStart: async (fromState, toState, data) => {

+ 7 - 6
packages/core/src/service/helpers/order-state-machine/order-state-machine.ts

@@ -8,8 +8,7 @@ import { StateMachineConfig, Transitions } from '../../../common/finite-state-ma
 import { validateTransitionDefinition } from '../../../common/finite-state-machine/validate-transition-definition';
 import { awaitPromiseOrObservable } from '../../../common/utils';
 import { ConfigService } from '../../../config/config.service';
-import { OrderProcess } from '../../../config/order/order-process';
-import { TransactionalConnection } from '../../../connection/transactional-connection';
+import { Logger } from '../../../config/logger/vendure-logger';
 import { Order } from '../../../entity/order/order.entity';
 
 import { OrderState, OrderTransitionData } from './order-state';
@@ -19,7 +18,7 @@ export class OrderStateMachine {
     readonly config: StateMachineConfig<OrderState, OrderTransitionData>;
     private readonly initialState: OrderState = 'Created';
 
-    constructor(private configService: ConfigService, private connection: TransactionalConnection) {
+    constructor(private configService: ConfigService) {
         this.config = this.initConfig();
     }
 
@@ -46,15 +45,17 @@ export class OrderStateMachine {
     private initConfig(): StateMachineConfig<OrderState, OrderTransitionData> {
         const orderProcesses = this.configService.orderOptions.process ?? [];
 
-        const emptyProcess: OrderProcess<any> = { transitions: {} };
         const allTransitions = orderProcesses.reduce(
             (transitions, process) =>
                 mergeTransitionDefinitions(transitions, process.transitions as Transitions<any>),
             {} as Transitions<OrderState>,
         );
 
-        const validationResult = validateTransitionDefinition(allTransitions, 'AddingItems');
-
+        const validationResult = validateTransitionDefinition(allTransitions, this.initialState);
+        if (!validationResult.valid && validationResult.error) {
+            Logger.error(`The order process has an invalid configuration:`);
+            throw new Error(validationResult.error);
+        }
         return {
             transitions: allTransitions,
             onTransitionStart: async (fromState, toState, data) => {

+ 6 - 2
packages/core/src/service/helpers/payment-state-machine/payment-state-machine.ts

@@ -8,6 +8,7 @@ import { StateMachineConfig, Transitions } from '../../../common/finite-state-ma
 import { validateTransitionDefinition } from '../../../common/finite-state-machine/validate-transition-definition';
 import { awaitPromiseOrObservable } from '../../../common/utils';
 import { ConfigService } from '../../../config/config.service';
+import { Logger } from '../../../config/logger/vendure-logger';
 import { Order } from '../../../entity/order/order.entity';
 import { Payment } from '../../../entity/payment/payment.entity';
 
@@ -52,8 +53,11 @@ export class PaymentStateMachine {
             {} as Transitions<PaymentState>,
         );
 
-        validateTransitionDefinition(allTransitions, this.initialState);
-
+        const validationResult = validateTransitionDefinition(allTransitions, this.initialState);
+        if (!validationResult.valid && validationResult.error) {
+            Logger.error(`The payment process has an invalid configuration:`);
+            throw new Error(validationResult.error);
+        }
         return {
             transitions: allTransitions,
             onTransitionStart: async (fromState, toState, data) => {