Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Expressions] Fix setup and start contracts #110841

Merged
merged 6 commits into from
Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/plugins/expressions/common/execution/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export class Execution<
const ast = execution.ast || parseExpression(this.expression);

this.state = createExecutionContainer({
...executor.state.get(),
...executor.state,
state: 'not-started',
ast,
});
Expand Down
117 changes: 63 additions & 54 deletions src/plugins/expressions/common/executor/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class TypesRegistry implements IRegistry<ExpressionType> {
}

public get(id: string): ExpressionType | null {
return this.executor.state.selectors.getType(id);
return this.executor.getType(id) ?? null;
}

public toJS(): Record<string, ExpressionType> {
Expand All @@ -71,7 +71,7 @@ export class FunctionsRegistry implements IRegistry<ExpressionFunction> {
}

public get(id: string): ExpressionFunction | null {
return this.executor.state.selectors.getFunction(id);
return this.executor.getFunction(id) ?? null;
}

public toJS(): Record<string, ExpressionFunction> {
Expand All @@ -95,22 +95,44 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
return executor;
}

public readonly state: ExecutorContainer<Context>;
public readonly container: ExecutorContainer<Context>;

/**
* @deprecated
*/
public readonly functions: FunctionsRegistry;
public readonly functions = new FunctionsRegistry(this);

/**
* @deprecated
*/
public readonly types: TypesRegistry;
public readonly types = new TypesRegistry(this);

protected parent?: Executor<Context>;

constructor(state?: ExecutorState<Context>) {
this.state = createExecutorContainer<Context>(state);
this.functions = new FunctionsRegistry(this);
this.types = new TypesRegistry(this);
this.container = createExecutorContainer<Context>(state);
}

public get state(): ExecutorState<Context> {
const parent = this.parent?.state;
const state = this.container.get();

return {
...(parent ?? {}),
...state,
types: {
...(parent?.types ?? {}),
...state.types,
},
functions: {
...(parent?.functions ?? {}),
...state.functions,
},
context: {
...(parent?.context ?? {}),
...state.context,
},
};
}

public registerFunction(
Expand All @@ -119,15 +141,18 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
const fn = new ExpressionFunction(
typeof functionDefinition === 'object' ? functionDefinition : functionDefinition()
);
this.state.transitions.addFunction(fn);
this.container.transitions.addFunction(fn);
}

public getFunction(name: string): ExpressionFunction | undefined {
return this.state.get().functions[name];
return this.container.get().functions[name] ?? this.parent?.getFunction(name);
}

public getFunctions(): Record<string, ExpressionFunction> {
return { ...this.state.get().functions };
return {
...(this.parent?.getFunctions() ?? {}),
...this.container.get().functions,
};
}

public registerType(
Expand All @@ -136,23 +161,30 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
const type = new ExpressionType(
typeof typeDefinition === 'object' ? typeDefinition : typeDefinition()
);
this.state.transitions.addType(type);

this.container.transitions.addType(type);
}

public getType(name: string): ExpressionType | undefined {
return this.state.get().types[name];
return this.container.get().types[name] ?? this.parent?.getType(name);
}

public getTypes(): Record<string, ExpressionType> {
return { ...this.state.get().types };
return {
...(this.parent?.getTypes() ?? {}),
...this.container.get().types,
};
}

public extendContext(extraContext: Record<string, unknown>) {
this.state.transitions.extendContext(extraContext);
this.container.transitions.extendContext(extraContext);
}

public get context(): Record<string, unknown> {
return this.state.selectors.getContext();
return {
...(this.parent?.context ?? {}),
...this.container.selectors.getContext(),
};
}

/**
Expand Down Expand Up @@ -199,18 +231,15 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
) {
for (const link of ast.chain) {
const { function: fnName, arguments: fnArgs } = link;
const fn = getByAlias(this.state.get().functions, fnName);
const fn = getByAlias(this.getFunctions(), fnName);

if (fn) {
// if any of arguments are expressions we should migrate those first
link.arguments = mapValues(fnArgs, (asts, argName) => {
return asts.map((arg) => {
if (arg && typeof arg === 'object') {
return this.walkAst(arg, action);
}
return arg;
});
});
link.arguments = mapValues(fnArgs, (asts) =>
asts.map((arg) =>
arg != null && typeof arg === 'object' ? this.walkAst(arg, action) : arg
)
);

action(fn, link);
}
Expand Down Expand Up @@ -275,39 +304,19 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u

private migrate(ast: SerializableRecord, version: string) {
return this.walkAst(cloneDeep(ast) as ExpressionAstExpression, (fn, link) => {
if (!fn.migrations[version]) return link;
const updatedAst = fn.migrations[version](link) as ExpressionAstFunction;
link.arguments = updatedAst.arguments;
link.type = updatedAst.type;
if (!fn.migrations[version]) {
return;
}

({ arguments: link.arguments, type: link.type } = fn.migrations[version](
link
) as ExpressionAstFunction);
});
}

public fork(): Executor<Context> {
const initialState = this.state.get();
const fork = new Executor<Context>(initialState);

/**
* Synchronize registry state - make any new types, functions and context
* also available in the forked instance of `Executor`.
*/
this.state.state$.subscribe(({ types, functions, context }) => {
const state = fork.state.get();
fork.state.set({
...state,
types: {
...types,
...state.types,
},
functions: {
...functions,
...state.functions,
},
context: {
...context,
...state.context,
},
});
});
const fork = new Executor<Context>();
fork.parent = this;

return fork;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,16 @@ describe('ExpressionsService', () => {
const expressions = new ExpressionsService();

expect(expressions.setup()).toMatchObject({
getFunction: expect.any(Function),
getFunctions: expect.any(Function),
getRenderer: expect.any(Function),
getRenderers: expect.any(Function),
getType: expect.any(Function),
getTypes: expect.any(Function),
registerFunction: expect.any(Function),
registerType: expect.any(Function),
registerRenderer: expect.any(Function),
run: expect.any(Function),
fork: expect.any(Function),
});
});

Expand All @@ -30,7 +35,16 @@ describe('ExpressionsService', () => {
expressions.setup();

expect(expressions.start()).toMatchObject({
getFunction: expect.any(Function),
getFunctions: expect.any(Function),
getRenderer: expect.any(Function),
getRenderers: expect.any(Function),
getType: expect.any(Function),
getTypes: expect.any(Function),
registerFunction: expect.any(Function),
registerType: expect.any(Function),
registerRenderer: expect.any(Function),
execute: expect.any(Function),
run: expect.any(Function),
});
});
Expand All @@ -54,21 +68,21 @@ describe('ExpressionsService', () => {
const service = new ExpressionsService();
const fork = service.fork();

expect(fork.executor.state.get().types).toEqual(service.executor.state.get().types);
expect(fork.getTypes()).toEqual(service.getTypes());
});

test('fork keeps all functions of the origin service', () => {
const service = new ExpressionsService();
const fork = service.fork();

expect(fork.executor.state.get().functions).toEqual(service.executor.state.get().functions);
expect(fork.getFunctions()).toEqual(service.getFunctions());
});

test('fork keeps context of the origin service', () => {
const service = new ExpressionsService();
const fork = service.fork();

expect(fork.executor.state.get().context).toEqual(service.executor.state.get().context);
expect(fork.executor.state.context).toEqual(service.executor.state.context);
});

test('newly registered functions in origin are also available in fork', () => {
Expand All @@ -82,7 +96,7 @@ describe('ExpressionsService', () => {
fn: () => {},
});

expect(fork.executor.state.get().functions).toEqual(service.executor.state.get().functions);
expect(fork.getFunctions()).toEqual(service.getFunctions());
});

test('newly registered functions in fork are NOT available in origin', () => {
Expand All @@ -96,14 +110,15 @@ describe('ExpressionsService', () => {
fn: () => {},
});

expect(Object.values(fork.executor.state.get().functions)).toHaveLength(
Object.values(service.executor.state.get().functions).length + 1
expect(Object.values(fork.getFunctions())).toHaveLength(
Object.values(service.getFunctions()).length + 1
);
});

test('fork can execute an expression with newly registered function', async () => {
const service = new ExpressionsService();
const fork = service.fork();
fork.start();

service.registerFunction({
name: '__test__',
Expand All @@ -118,5 +133,28 @@ describe('ExpressionsService', () => {

expect(result).toBe('123');
});

test('throw on fork if the service is already started', async () => {
const service = new ExpressionsService();
service.start();

expect(() => service.fork()).toThrow();
});
});

describe('.execute()', () => {
test('throw if the service is not started', () => {
const expressions = new ExpressionsService();

expect(() => expressions.execute('foo', null)).toThrow();
});
});

describe('.run()', () => {
test('throw if the service is not started', () => {
const expressions = new ExpressionsService();

expect(() => expressions.run('foo', null)).toThrow();
});
});
});
Loading