Skip to content

Commit

Permalink
fix(core): Improved validation of state machine configs
Browse files Browse the repository at this point in the history
Fixes #2521
  • Loading branch information
michaelbromley committed Nov 14, 2023
1 parent 32f592d commit b44cc88
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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();
}

Expand Down Expand Up @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();
}

Expand All @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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) => {
Expand Down

0 comments on commit b44cc88

Please sign in to comment.