From 729b72715df05684b1402938202685f03cb3e22c Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Thu, 9 Jan 2025 19:26:22 +0100 Subject: [PATCH 01/14] ignore project local neovim configuration --- .gitignore | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 55fdff59..dd4b3cbd 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,9 @@ node_modules *.launch .settings/ *.sublime-workspace +.nvim.lua +.nvimrc +.exrc # IDE - VSCode .vscode/* @@ -58,4 +61,4 @@ Thumbs.db .cache-loader/ .nx/cache -.nx/workspace-data \ No newline at end of file +.nx/workspace-data From 8bcb94d773ca20ceac90eeda6856d1dc350ffa27 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Thu, 9 Jan 2025 19:27:58 +0100 Subject: [PATCH 02/14] Implement blocking hooks --- .../src/lib/blocks/block-executor.ts | 4 + libs/execution/src/lib/execution-context.ts | 25 +++++ libs/execution/src/lib/hooks/hook.ts | 98 +++++++++++++++++++ libs/execution/src/lib/hooks/index.ts | 5 + libs/execution/src/lib/index.ts | 1 + libs/interpreter-lib/src/interpreter.spec.ts | 86 +++++++++++++++- libs/interpreter-lib/src/interpreter.ts | 47 ++++++--- 7 files changed, 252 insertions(+), 14 deletions(-) create mode 100644 libs/execution/src/lib/hooks/hook.ts create mode 100644 libs/execution/src/lib/hooks/index.ts diff --git a/libs/execution/src/lib/blocks/block-executor.ts b/libs/execution/src/lib/blocks/block-executor.ts index c0126c22..0f31da9a 100644 --- a/libs/execution/src/lib/blocks/block-executor.ts +++ b/libs/execution/src/lib/blocks/block-executor.ts @@ -36,11 +36,15 @@ export abstract class AbstractBlockExecutor input: IOTypeImplementation, context: ExecutionContext, ): Promise | null>> { + await context.executeHooks('before', input, context); // FIXME #634: Pass the blocktype to also execute block specific hooks + const executionResult = await this.doExecute(input, context); if (R.isOk(executionResult)) { this.logBlockResult(executionResult.right, context); } + + await context.executeHooks('after', input, context); // FIXME #634: Pass the blocktype to also execute block specific hooks return executionResult; } diff --git a/libs/execution/src/lib/execution-context.ts b/libs/execution/src/lib/execution-context.ts index 8c6c9a1b..c644762a 100644 --- a/libs/execution/src/lib/execution-context.ts +++ b/libs/execution/src/lib/execution-context.ts @@ -4,6 +4,7 @@ // eslint-disable-next-line unicorn/prefer-node-protocol import { strict as assert } from 'assert'; +import { inspect } from 'node:util'; import { type BlockDefinition, @@ -26,13 +27,16 @@ import { } from '@jvalue/jayvee-language-server'; import { assertUnreachable, isReference } from 'langium'; +import { type BlockExecutor } from './blocks'; import { type JayveeConstraintExtension } from './constraints'; import { type DebugGranularity, type DebugTargets, } from './debugging/debug-configuration'; import { type JayveeExecExtension } from './extension'; +import { type Hook, type HookContext } from './hooks'; import { type Logger } from './logging/logger'; +import { type IOTypeImplementation } from './types'; export type StackNode = | BlockDefinition @@ -55,6 +59,7 @@ export class ExecutionContext { debugTargets: DebugTargets; }, public readonly evaluationContext: EvaluationContext, + public readonly hookContext: HookContext, ) { logger.setLoggingContext(pipeline.name); } @@ -133,6 +138,26 @@ export class ExecutionContext { return property; } + public async executeHooks( + position: 'before' | 'after', + input: IOTypeImplementation, + context: ExecutionContext, + ) { + const node = this.getCurrentNode(); + assert( + isBlockDefinition(node), + `Expected node to be \`BlockDefinition\`: ${inspect(node)}`, + ); + + const blocktype = node.type.ref?.name; + assert( + blocktype !== undefined, + `Expected block definition to have a blocktype: ${inspect(node)}`, + ); + + return this.hookContext.executeHooks(position, blocktype, input, context); + } + private getDefaultPropertyValue( propertyName: string, valueType: ValueType, diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts new file mode 100644 index 00000000..2e1f100e --- /dev/null +++ b/libs/execution/src/lib/hooks/hook.ts @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: 2025 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + +import { type ExecutionContext } from '../execution-context'; +import { type IOTypeImplementation } from '../types'; + +/** A hook can be executed `before` or `after` a block*/ +export type HookPosition = 'before' | 'after'; + +const AllBlocks = '*'; + +async function executeTheseHooks( + hooks: HookSpec[], + blocktype: string, + input: IOTypeImplementation, + context: ExecutionContext, +) { + await Promise.all( + hooks.map(async ({ blocking, hook }) => { + if (blocking) { + await hook(blocktype, input, context); + } else { + // eslint-disable-next-line @typescript-eslint/no-empty-function + hook(blocktype, input, context).catch(() => {}); + } + }), + ); +} + +export class HookContext { + private hooks: { + before: Record; + after: Record; + } = { before: {}, after: {} }; + + public addHook(hook: Hook, opts: HookOptions) { + const blocktypes: string[] = + typeof opts.blocktypes === 'string' + ? [opts.blocktypes] + : opts.blocktypes ?? [AllBlocks]; + + blocktypes.forEach((blocktype) => { + if (this.hooks[opts.position][blocktype] === undefined) { + this.hooks[opts.position][blocktype] = []; + } + this.hooks[opts.position][blocktype]?.push({ + blocking: opts.blocking ?? false, + hook, + }); + }); + } + + public async executeHooks( + position: HookPosition, + blocktype: string, + input: IOTypeImplementation, + context: ExecutionContext, + ) { + const general = executeTheseHooks( + this.hooks[position][AllBlocks] ?? [], + blocktype, + input, + context, + ); + const blockSpecific = executeTheseHooks( + this.hooks[position][blocktype] ?? [], + blocktype, + input, + context, + ); + + // eslint-disable-next-line @typescript-eslint/no-empty-function + return Promise.all([general, blockSpecific]).then(() => {}); + } +} + +export interface HookOptions { + /** Whether the hook is executed `before` or `after` a block.*/ + // FIXME: find a better name than `position` + position: HookPosition; + /** Whether the pipeline should await the hooks completion. `false` if omitted.*/ + blocking?: boolean; + /** Optionally specify one or more blocks to limit this hook to. If omitted, the hook will be executed on all blocks*/ + // FIXME #634: Add `BlockExecutor` type + blocktypes?: string | string[]; +} + +export type Hook = ( + blocktype: string, + input: IOTypeImplementation, + context: ExecutionContext, +) => Promise; + +interface HookSpec { + blocking: boolean; + hook: Hook; +} diff --git a/libs/execution/src/lib/hooks/index.ts b/libs/execution/src/lib/hooks/index.ts new file mode 100644 index 00000000..5ad055ec --- /dev/null +++ b/libs/execution/src/lib/hooks/index.ts @@ -0,0 +1,5 @@ +// SPDX-FileCopyrightText: 2025 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + +export * from './hook'; diff --git a/libs/execution/src/lib/index.ts b/libs/execution/src/lib/index.ts index cd267ae5..5c4a03ab 100644 --- a/libs/execution/src/lib/index.ts +++ b/libs/execution/src/lib/index.ts @@ -13,3 +13,4 @@ export * from './types/value-types/visitors'; export * from './execution-context'; export * from './extension'; export * from './logging'; +export * from './hooks'; diff --git a/libs/interpreter-lib/src/interpreter.spec.ts b/libs/interpreter-lib/src/interpreter.spec.ts index 08a534a2..10b882a3 100644 --- a/libs/interpreter-lib/src/interpreter.spec.ts +++ b/libs/interpreter-lib/src/interpreter.spec.ts @@ -2,10 +2,17 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import { type JayveeModel } from '@jvalue/jayvee-language-server'; import { readJvTestAssetHelper } from '@jvalue/jayvee-language-server/test'; import { DefaultJayveeInterpreter } from './interpreter'; -import { ExitCode } from './parsing-util'; +import { ExitCode, extractAstNodeFromString } from './parsing-util'; + +function infiniteLoop() { + setTimeout(function () { + infiniteLoop(); + }, 3000); +} describe('Interpreter', () => { const readJvTestAsset = readJvTestAssetHelper(__dirname, '../../../'); @@ -26,4 +33,81 @@ describe('Interpreter', () => { expect(exitCode).toEqual(ExitCode.SUCCESS); }); }); + + describe('hooks', () => { + it('should execute a general hook on every block', async () => { + const exampleFilePath = 'example/cars.jv'; + const model = readJvTestAsset(exampleFilePath); + + const interpreter = new DefaultJayveeInterpreter({ + pipelineMatcher: () => true, + debug: true, + debugGranularity: 'peek', + debugTarget: 'all', + env: new Map(), + }); + + const program = await interpreter.parseModel( + async (services, loggerFactory) => + await extractAstNodeFromString( + model, + services, + loggerFactory.createLogger(), + ), + ); + expect(program).toBeDefined(); + assert(program !== undefined); + + const spy = vi.fn>().mockResolvedValue(undefined); + + program.addHook( + async () => { + return spy(); + }, + { position: 'before', blocking: true }, + ); + + const exitCode = await interpreter.interpretProgram(program); + expect(exitCode).toEqual(ExitCode.SUCCESS); + + expect(spy).toHaveBeenCalledTimes(6); + }); + + it('should not wait for non-blocking hooks', async () => { + const exampleFilePath = 'example/cars.jv'; + const model = readJvTestAsset(exampleFilePath); + + const interpreter = new DefaultJayveeInterpreter({ + pipelineMatcher: () => true, + debug: true, + debugGranularity: 'peek', + debugTarget: 'all', + env: new Map(), + }); + + const program = await interpreter.parseModel( + async (services, loggerFactory) => + await extractAstNodeFromString( + model, + services, + loggerFactory.createLogger(), + ), + ); + expect(program).toBeDefined(); + assert(program !== undefined); + + program.addHook( + () => { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition, no-constant-condition + while (true) { + infiniteLoop(); + } + }, + { position: 'before', blocking: false }, + ); + + const exitCode = await interpreter.interpretProgram(program); + expect(exitCode).toEqual(ExitCode.SUCCESS); + }); + }); }); diff --git a/libs/interpreter-lib/src/interpreter.ts b/libs/interpreter-lib/src/interpreter.ts index 19140ad2..b9657cd6 100644 --- a/libs/interpreter-lib/src/interpreter.ts +++ b/libs/interpreter-lib/src/interpreter.ts @@ -11,6 +11,9 @@ import { type DebugTargets, DefaultConstraintExtension, ExecutionContext, + type Hook, + HookContext, + type HookOptions, type JayveeConstraintExtension, type JayveeExecExtension, type Logger, @@ -52,6 +55,20 @@ export interface InterpreterOptions { debugTarget: DebugTargets; } +export class JayveeProgram { + private _hooks: HookContext = new HookContext(); + + constructor(public model: JayveeModel) {} + + public addHook(hook: Hook, opts: HookOptions) { + this._hooks.addHook(hook, opts); + } + + public get hooks() { + return this._hooks; + } +} + export interface JayveeInterpreter { /** * Interprets a parsed Jayvee model. @@ -59,7 +76,7 @@ export interface JayveeInterpreter { * @param extractAstNodeFn the Jayvee model. * @returns the exit code indicating whether interpretation was successful or not. */ - interpretModel(model: JayveeModel): Promise; + interpretProgram(program: JayveeProgram): Promise; /** * Interprets a file as a Jayvee model. @@ -80,18 +97,18 @@ export interface JayveeInterpreter { interpretString(modelString: string): Promise; /** - * Parses a model without executing it. + * Parses a program without executing it. * Also sets up the environment so that the model can be properly executed. * * @param extractAstNodeFn method that extracts the AST node - * @returns the parsed Jayvee model, or undefined on failure. + * @returns the parsed Jayvee program, or undefined on failure. */ parseModel( extractAstNodeFn: ( services: JayveeServices, loggerFactory: LoggerFactory, ) => Promise, - ): Promise; + ): Promise; } export class DefaultJayveeInterpreter implements JayveeInterpreter { @@ -116,11 +133,11 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { return this; } - async interpretModel(model: JayveeModel): Promise { + async interpretProgram(program: JayveeProgram): Promise { await this.prepareInterpretation(); - const interpretationExitCode = await this.interpretJayveeModel( - model, + const interpretationExitCode = await this.interpretJayveeProgram( + program, new StdExecExtension(), new DefaultConstraintExtension(), ); @@ -145,7 +162,7 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { return ExitCode.FAILURE; } - return await this.interpretModel(model); + return await this.interpretProgram(model); } async interpretString(modelString: string): Promise { @@ -166,7 +183,7 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { return ExitCode.FAILURE; } - return await this.interpretModel(model); + return await this.interpretProgram(model); } async parseModel( @@ -174,12 +191,12 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { services: JayveeServices, loggerFactory: LoggerFactory, ) => Promise, - ): Promise { + ): Promise { await this.prepareInterpretation(); try { const model = await extractAstNodeFn(this.services, this.loggerFactory); - return model; + return new JayveeProgram(model); } catch (e) { this.loggerFactory .createLogger() @@ -213,11 +230,12 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { } } - private async interpretJayveeModel( - model: JayveeModel, + private async interpretJayveeProgram( + program: JayveeProgram, executionExtension: JayveeExecExtension, constraintExtension: JayveeConstraintExtension, ): Promise { + const model = program.model; const selectedPipelines = model.pipelines.filter((pipeline) => this.options.pipelineMatcher(pipeline), ); @@ -237,6 +255,7 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { pipeline, executionExtension, constraintExtension, + program.hooks, ); }, ); @@ -252,6 +271,7 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { pipeline: PipelineDefinition, executionExtension: JayveeExecExtension, constraintExtension: JayveeConstraintExtension, + hooks: HookContext, ): Promise { const executionContext = new ExecutionContext( pipeline, @@ -270,6 +290,7 @@ export class DefaultJayveeInterpreter implements JayveeInterpreter { this.services.operators.EvaluatorRegistry, this.services.ValueTypeProvider, ), + hooks, ); logPipelineOverview( From 2db9aec4e1f4ce38806755c8fc15f9f98c43bdd3 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Fri, 10 Jan 2025 11:00:44 +0100 Subject: [PATCH 03/14] Add test for non blocking hooks --- libs/execution/src/lib/hooks/hook.ts | 4 ++-- libs/interpreter-lib/src/interpreter.spec.ts | 19 +++++++------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts index 2e1f100e..1cbf9c74 100644 --- a/libs/execution/src/lib/hooks/hook.ts +++ b/libs/execution/src/lib/hooks/hook.ts @@ -45,7 +45,7 @@ export class HookContext { this.hooks[opts.position][blocktype] = []; } this.hooks[opts.position][blocktype]?.push({ - blocking: opts.blocking ?? false, + blocking: opts.blocking ?? true, hook, }); }); @@ -79,7 +79,7 @@ export interface HookOptions { /** Whether the hook is executed `before` or `after` a block.*/ // FIXME: find a better name than `position` position: HookPosition; - /** Whether the pipeline should await the hooks completion. `false` if omitted.*/ + /** Whether the pipeline should await the hooks completion. `true` if omitted.*/ blocking?: boolean; /** Optionally specify one or more blocks to limit this hook to. If omitted, the hook will be executed on all blocks*/ // FIXME #634: Add `BlockExecutor` type diff --git a/libs/interpreter-lib/src/interpreter.spec.ts b/libs/interpreter-lib/src/interpreter.spec.ts index 10b882a3..f4b7da19 100644 --- a/libs/interpreter-lib/src/interpreter.spec.ts +++ b/libs/interpreter-lib/src/interpreter.spec.ts @@ -8,12 +8,6 @@ import { readJvTestAssetHelper } from '@jvalue/jayvee-language-server/test'; import { DefaultJayveeInterpreter } from './interpreter'; import { ExitCode, extractAstNodeFromString } from './parsing-util'; -function infiniteLoop() { - setTimeout(function () { - infiniteLoop(); - }, 3000); -} - describe('Interpreter', () => { const readJvTestAsset = readJvTestAssetHelper(__dirname, '../../../'); @@ -58,7 +52,9 @@ describe('Interpreter', () => { expect(program).toBeDefined(); assert(program !== undefined); - const spy = vi.fn>().mockResolvedValue(undefined); + const spy = vi + .fn>() + .mockResolvedValue(undefined); program.addHook( async () => { @@ -98,16 +94,15 @@ describe('Interpreter', () => { program.addHook( () => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition, no-constant-condition - while (true) { - infiniteLoop(); - } + return new Promise((resolve) => { + setTimeout(resolve, 30000); + }); }, { position: 'before', blocking: false }, ); const exitCode = await interpreter.interpretProgram(program); expect(exitCode).toEqual(ExitCode.SUCCESS); - }); + }, 10000); }); }); From 0965249846bf483210dc848dc5c870d438790103 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Fri, 10 Jan 2025 11:02:21 +0100 Subject: [PATCH 04/14] fix: set default to non-blocking --- libs/execution/src/lib/hooks/hook.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts index 1cbf9c74..2e1f100e 100644 --- a/libs/execution/src/lib/hooks/hook.ts +++ b/libs/execution/src/lib/hooks/hook.ts @@ -45,7 +45,7 @@ export class HookContext { this.hooks[opts.position][blocktype] = []; } this.hooks[opts.position][blocktype]?.push({ - blocking: opts.blocking ?? true, + blocking: opts.blocking ?? false, hook, }); }); @@ -79,7 +79,7 @@ export interface HookOptions { /** Whether the hook is executed `before` or `after` a block.*/ // FIXME: find a better name than `position` position: HookPosition; - /** Whether the pipeline should await the hooks completion. `true` if omitted.*/ + /** Whether the pipeline should await the hooks completion. `false` if omitted.*/ blocking?: boolean; /** Optionally specify one or more blocks to limit this hook to. If omitted, the hook will be executed on all blocks*/ // FIXME #634: Add `BlockExecutor` type From f43cf9155febad3f09c800549cbdb5c5d08dd3b9 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Fri, 10 Jan 2025 13:47:21 +0100 Subject: [PATCH 05/14] fix: postBlock hooks recieve the block's output --- .../src/lib/blocks/block-execution-util.ts | 2 + .../src/lib/blocks/block-executor.ts | 4 - libs/execution/src/lib/execution-context.ts | 13 +- libs/execution/src/lib/hooks/hook-context.ts | 183 ++++++++++++++++++ libs/execution/src/lib/hooks/hook.ts | 105 +++------- libs/execution/src/lib/hooks/index.ts | 1 + libs/interpreter-lib/src/interpreter.spec.ts | 8 +- libs/interpreter-lib/src/interpreter.ts | 16 +- 8 files changed, 236 insertions(+), 96 deletions(-) create mode 100644 libs/execution/src/lib/hooks/hook-context.ts diff --git a/libs/execution/src/lib/blocks/block-execution-util.ts b/libs/execution/src/lib/blocks/block-execution-util.ts index f18a6d1d..5cda5588 100644 --- a/libs/execution/src/lib/blocks/block-execution-util.ts +++ b/libs/execution/src/lib/blocks/block-execution-util.ts @@ -62,11 +62,13 @@ export async function executeBlocks( executionContext.enterNode(block); + await executionContext.executeHooks(inputValue); // FIXME #634: Pass the blocktype to also execute block specific hooks const executionResult = await executeBlock( inputValue, block, executionContext, ); + await executionContext.executeHooks(inputValue, executionResult); // FIXME #634: Pass the blocktype to also execute block specific hooks if (R.isErr(executionResult)) { return executionResult; } diff --git a/libs/execution/src/lib/blocks/block-executor.ts b/libs/execution/src/lib/blocks/block-executor.ts index 0f31da9a..c0126c22 100644 --- a/libs/execution/src/lib/blocks/block-executor.ts +++ b/libs/execution/src/lib/blocks/block-executor.ts @@ -36,15 +36,11 @@ export abstract class AbstractBlockExecutor input: IOTypeImplementation, context: ExecutionContext, ): Promise | null>> { - await context.executeHooks('before', input, context); // FIXME #634: Pass the blocktype to also execute block specific hooks - const executionResult = await this.doExecute(input, context); if (R.isOk(executionResult)) { this.logBlockResult(executionResult.right, context); } - - await context.executeHooks('after', input, context); // FIXME #634: Pass the blocktype to also execute block specific hooks return executionResult; } diff --git a/libs/execution/src/lib/execution-context.ts b/libs/execution/src/lib/execution-context.ts index c644762a..cfa179e1 100644 --- a/libs/execution/src/lib/execution-context.ts +++ b/libs/execution/src/lib/execution-context.ts @@ -27,14 +27,14 @@ import { } from '@jvalue/jayvee-language-server'; import { assertUnreachable, isReference } from 'langium'; -import { type BlockExecutor } from './blocks'; +import { type Result } from './blocks'; import { type JayveeConstraintExtension } from './constraints'; import { type DebugGranularity, type DebugTargets, } from './debugging/debug-configuration'; import { type JayveeExecExtension } from './extension'; -import { type Hook, type HookContext } from './hooks'; +import { type HookContext, type HookPosition } from './hooks'; import { type Logger } from './logging/logger'; import { type IOTypeImplementation } from './types'; @@ -138,10 +138,9 @@ export class ExecutionContext { return property; } - public async executeHooks( - position: 'before' | 'after', - input: IOTypeImplementation, - context: ExecutionContext, + public executeHooks( + input: IOTypeImplementation | null, + output?: Result, ) { const node = this.getCurrentNode(); assert( @@ -155,7 +154,7 @@ export class ExecutionContext { `Expected block definition to have a blocktype: ${inspect(node)}`, ); - return this.hookContext.executeHooks(position, blocktype, input, context); + return this.hookContext.executeHooks(blocktype, input, this, output); } private getDefaultPropertyValue( diff --git a/libs/execution/src/lib/hooks/hook-context.ts b/libs/execution/src/lib/hooks/hook-context.ts new file mode 100644 index 00000000..ba3efa9f --- /dev/null +++ b/libs/execution/src/lib/hooks/hook-context.ts @@ -0,0 +1,183 @@ +// eslint-disable-next-line unicorn/prefer-node-protocol +import assert from 'assert'; + +import { type Result } from '../blocks'; +import { type ExecutionContext } from '../execution-context'; +import { type IOTypeImplementation } from '../types'; + +import { + type HookOptions, + type HookPosition, + type PostBlockHook, + type PreBlockHook, + isPreBlockHook, +} from './hook'; + +const AllBlocks = '*'; + +interface HookSpec { + blocking: boolean; + hook: H; +} + +async function executeTheseHooks( + hooks: HookSpec[], + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, +): Promise; +async function executeTheseHooks( + hooks: HookSpec[], + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, + output: Result, +): Promise; +async function executeTheseHooks( + hooks: HookSpec[] | HookSpec[], + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, + output?: Result, +) { + const position = output === undefined ? 'preBlock' : 'postBlock'; + return ( + Promise.all( + hooks.map(async ({ blocking, hook }) => { + if (blocking) { + if (isPreBlockHook(hook, position)) { + await hook(blocktype, input, context); + } else { + assert(output !== undefined, 'Guaranteed to be a postBlock hook'); + await hook(blocktype, input, output, context); + } + } else { + if (isPreBlockHook(hook, position)) { + hook(blocktype, input, context) + // eslint-disable-next-line @typescript-eslint/no-empty-function + .catch(() => {}); + } else { + assert(output !== undefined, 'Guaranteed to be a postBlock hook'); + hook(blocktype, input, output, context) + // eslint-disable-next-line @typescript-eslint/no-empty-function + .catch(() => {}); + } + } + }), + ) + // eslint-disable-next-line @typescript-eslint/no-empty-function + .then(() => {}) + ); +} + +export class HookContext { + private hooks: { + pre: Record[]>; + post: Record[]>; + } = { pre: {}, post: {} }; + + public addHook( + position: 'preBlock', + hook: PreBlockHook, + opts: HookOptions, + ): void; + public addHook( + position: 'postBlock', + hook: PostBlockHook, + opts: HookOptions, + ): void; + public addHook( + position: HookPosition, + hook: PreBlockHook | PostBlockHook, + opts: HookOptions, + ): void; + public addHook( + position: HookPosition, + hook: PreBlockHook | PostBlockHook, + opts: HookOptions, + ) { + const blocktypes: string[] = + typeof opts.blocktypes === 'string' + ? [opts.blocktypes] + : opts.blocktypes ?? [AllBlocks]; + + blocktypes.forEach((blocktype) => { + if (isPreBlockHook(hook, position)) { + if (this.hooks.pre[blocktype] === undefined) { + this.hooks.pre[blocktype] = []; + } + this.hooks.pre[blocktype].push({ + blocking: opts.blocking ?? false, + hook, + }); + } else { + if (this.hooks.post[blocktype] === undefined) { + this.hooks.post[blocktype] = []; + } + this.hooks.post[blocktype].push({ + blocking: opts.blocking ?? false, + hook, + }); + } + }); + } + + public async executeHooks( + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, + ): Promise; + public async executeHooks( + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, + output: Result, + ): Promise; + public async executeHooks( + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, + output?: Result, + ): Promise; + public async executeHooks( + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, + output?: Result, + ) { + if (output === undefined) { + const general = executeTheseHooks( + this.hooks.pre[AllBlocks] ?? [], + blocktype, + input, + context, + ); + const blockSpecific = executeTheseHooks( + this.hooks.pre[blocktype] ?? [], + blocktype, + input, + context, + ); + + // eslint-disable-next-line @typescript-eslint/no-empty-function + return Promise.all([general, blockSpecific]).then(() => {}); + } + const general = executeTheseHooks( + this.hooks.post[AllBlocks] ?? [], + blocktype, + input, + context, + output, + ); + const blockSpecific = executeTheseHooks( + this.hooks.post[blocktype] ?? [], + blocktype, + input, + context, + output, + ); + + // eslint-disable-next-line @typescript-eslint/no-empty-function + return Promise.all([general, blockSpecific]).then(() => {}); + } +} diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts index 2e1f100e..2c2aa82b 100644 --- a/libs/execution/src/lib/hooks/hook.ts +++ b/libs/execution/src/lib/hooks/hook.ts @@ -2,97 +2,46 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import { type Result } from '../blocks'; import { type ExecutionContext } from '../execution-context'; import { type IOTypeImplementation } from '../types'; -/** A hook can be executed `before` or `after` a block*/ -export type HookPosition = 'before' | 'after'; - -const AllBlocks = '*'; - -async function executeTheseHooks( - hooks: HookSpec[], - blocktype: string, - input: IOTypeImplementation, - context: ExecutionContext, -) { - await Promise.all( - hooks.map(async ({ blocking, hook }) => { - if (blocking) { - await hook(blocktype, input, context); - } else { - // eslint-disable-next-line @typescript-eslint/no-empty-function - hook(blocktype, input, context).catch(() => {}); - } - }), - ); -} - -export class HookContext { - private hooks: { - before: Record; - after: Record; - } = { before: {}, after: {} }; - - public addHook(hook: Hook, opts: HookOptions) { - const blocktypes: string[] = - typeof opts.blocktypes === 'string' - ? [opts.blocktypes] - : opts.blocktypes ?? [AllBlocks]; - - blocktypes.forEach((blocktype) => { - if (this.hooks[opts.position][blocktype] === undefined) { - this.hooks[opts.position][blocktype] = []; - } - this.hooks[opts.position][blocktype]?.push({ - blocking: opts.blocking ?? false, - hook, - }); - }); - } - - public async executeHooks( - position: HookPosition, - blocktype: string, - input: IOTypeImplementation, - context: ExecutionContext, - ) { - const general = executeTheseHooks( - this.hooks[position][AllBlocks] ?? [], - blocktype, - input, - context, - ); - const blockSpecific = executeTheseHooks( - this.hooks[position][blocktype] ?? [], - blocktype, - input, - context, - ); - - // eslint-disable-next-line @typescript-eslint/no-empty-function - return Promise.all([general, blockSpecific]).then(() => {}); - } -} +/** When to execute the hook.*/ +export type HookPosition = 'preBlock' | 'postBlock'; export interface HookOptions { - /** Whether the hook is executed `before` or `after` a block.*/ - // FIXME: find a better name than `position` - position: HookPosition; /** Whether the pipeline should await the hooks completion. `false` if omitted.*/ blocking?: boolean; /** Optionally specify one or more blocks to limit this hook to. If omitted, the hook will be executed on all blocks*/ - // FIXME #634: Add `BlockExecutor` type + // FIXME #634: Add `BlockExecutor[]` variant blocktypes?: string | string[]; } -export type Hook = ( +/** This function will be executed before a block.*/ +export type PreBlockHook = ( + blocktype: string, + input: IOTypeImplementation | null, + context: ExecutionContext, +) => Promise; + +export function isPreBlockHook( + hook: PreBlockHook | PostBlockHook, + position: HookPosition, +): hook is PreBlockHook { + return position === 'preBlock'; +} + +/** This function will be executed before a block.*/ +export type PostBlockHook = ( blocktype: string, - input: IOTypeImplementation, + input: IOTypeImplementation | null, + output: Result, context: ExecutionContext, ) => Promise; -interface HookSpec { - blocking: boolean; - hook: Hook; +export function isPostBlockHook( + hook: PreBlockHook | PostBlockHook, + position: HookPosition, +): hook is PreBlockHook { + return position === 'postBlock'; } diff --git a/libs/execution/src/lib/hooks/index.ts b/libs/execution/src/lib/hooks/index.ts index 5ad055ec..20bb5603 100644 --- a/libs/execution/src/lib/hooks/index.ts +++ b/libs/execution/src/lib/hooks/index.ts @@ -3,3 +3,4 @@ // SPDX-License-Identifier: AGPL-3.0-only export * from './hook'; +export * from './hook-context'; diff --git a/libs/interpreter-lib/src/interpreter.spec.ts b/libs/interpreter-lib/src/interpreter.spec.ts index f4b7da19..eca3f064 100644 --- a/libs/interpreter-lib/src/interpreter.spec.ts +++ b/libs/interpreter-lib/src/interpreter.spec.ts @@ -57,10 +57,11 @@ describe('Interpreter', () => { .mockResolvedValue(undefined); program.addHook( + 'preBlock', async () => { return spy(); }, - { position: 'before', blocking: true }, + { blocking: true }, ); const exitCode = await interpreter.interpretProgram(program); @@ -93,12 +94,13 @@ describe('Interpreter', () => { assert(program !== undefined); program.addHook( - () => { + 'postBlock', + (): Promise => { return new Promise((resolve) => { setTimeout(resolve, 30000); }); }, - { position: 'before', blocking: false }, + { blocking: false }, ); const exitCode = await interpreter.interpretProgram(program); diff --git a/libs/interpreter-lib/src/interpreter.ts b/libs/interpreter-lib/src/interpreter.ts index b9657cd6..ab26c890 100644 --- a/libs/interpreter-lib/src/interpreter.ts +++ b/libs/interpreter-lib/src/interpreter.ts @@ -11,12 +11,14 @@ import { type DebugTargets, DefaultConstraintExtension, ExecutionContext, - type Hook, HookContext, type HookOptions, + type HookPosition, type JayveeConstraintExtension, type JayveeExecExtension, type Logger, + type PostBlockHook, + type PreBlockHook, executeBlocks, isErr, logExecutionDuration, @@ -56,12 +58,18 @@ export interface InterpreterOptions { } export class JayveeProgram { - private _hooks: HookContext = new HookContext(); + private _hooks = new HookContext(); constructor(public model: JayveeModel) {} - public addHook(hook: Hook, opts: HookOptions) { - this._hooks.addHook(hook, opts); + // FIXME #634: Pass the blocktype to also execute block specific hooks + /** Add a hook to all blocks in the pipeline.*/ + public addHook( + position: HookPosition, + hook: PreBlockHook | PostBlockHook, + opts: HookOptions, + ) { + this._hooks.addHook(position, hook, opts); } public get hooks() { From c6b208969658cfed6ffb9781ad8d450eb89a4d9a Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Fri, 10 Jan 2025 14:17:20 +0100 Subject: [PATCH 06/14] Make `opts` an optional parameter --- libs/interpreter-lib/src/interpreter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/interpreter-lib/src/interpreter.ts b/libs/interpreter-lib/src/interpreter.ts index ab26c890..fbc01511 100644 --- a/libs/interpreter-lib/src/interpreter.ts +++ b/libs/interpreter-lib/src/interpreter.ts @@ -67,9 +67,9 @@ export class JayveeProgram { public addHook( position: HookPosition, hook: PreBlockHook | PostBlockHook, - opts: HookOptions, + opts?: HookOptions, ) { - this._hooks.addHook(position, hook, opts); + this._hooks.addHook(position, hook, opts ?? {}); } public get hooks() { From c3609eaa970d2096aa63578ccef83a36c3799177 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Fri, 10 Jan 2025 14:32:10 +0100 Subject: [PATCH 07/14] Fix pipeline --- libs/execution/src/lib/execution-context.ts | 2 +- libs/execution/src/lib/hooks/hook-context.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/libs/execution/src/lib/execution-context.ts b/libs/execution/src/lib/execution-context.ts index cfa179e1..879658ba 100644 --- a/libs/execution/src/lib/execution-context.ts +++ b/libs/execution/src/lib/execution-context.ts @@ -34,7 +34,7 @@ import { type DebugTargets, } from './debugging/debug-configuration'; import { type JayveeExecExtension } from './extension'; -import { type HookContext, type HookPosition } from './hooks'; +import { type HookContext } from './hooks'; import { type Logger } from './logging/logger'; import { type IOTypeImplementation } from './types'; diff --git a/libs/execution/src/lib/hooks/hook-context.ts b/libs/execution/src/lib/hooks/hook-context.ts index ba3efa9f..5d7f28be 100644 --- a/libs/execution/src/lib/hooks/hook-context.ts +++ b/libs/execution/src/lib/hooks/hook-context.ts @@ -1,3 +1,7 @@ +// SPDX-FileCopyrightText: 2025 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + // eslint-disable-next-line unicorn/prefer-node-protocol import assert from 'assert'; From cfbbe2b5be1cd82a391c5df08bf336308c62d1ef Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 10:06:40 +0100 Subject: [PATCH 08/14] Fix typo --- libs/execution/src/lib/hooks/hook.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts index 2c2aa82b..500ade49 100644 --- a/libs/execution/src/lib/hooks/hook.ts +++ b/libs/execution/src/lib/hooks/hook.ts @@ -42,6 +42,6 @@ export type PostBlockHook = ( export function isPostBlockHook( hook: PreBlockHook | PostBlockHook, position: HookPosition, -): hook is PreBlockHook { +): hook is PostBlockHook { return position === 'postBlock'; } From 454147c2eedd211c17fc1252b5d02a5ce1bfaf44 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 10:51:54 +0100 Subject: [PATCH 09/14] Add logging --- libs/execution/src/lib/hooks/hook-context.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libs/execution/src/lib/hooks/hook-context.ts b/libs/execution/src/lib/hooks/hook-context.ts index 5d7f28be..1272754a 100644 --- a/libs/execution/src/lib/hooks/hook-context.ts +++ b/libs/execution/src/lib/hooks/hook-context.ts @@ -150,12 +150,16 @@ export class HookContext { output?: Result, ) { if (output === undefined) { + context.logger.logInfo(`Executing general pre-block-hooks`); const general = executeTheseHooks( this.hooks.pre[AllBlocks] ?? [], blocktype, input, context, ); + context.logger.logInfo( + `Executing pre-block-hooks for blocktype ${blocktype}`, + ); const blockSpecific = executeTheseHooks( this.hooks.pre[blocktype] ?? [], blocktype, @@ -166,6 +170,7 @@ export class HookContext { // eslint-disable-next-line @typescript-eslint/no-empty-function return Promise.all([general, blockSpecific]).then(() => {}); } + context.logger.logInfo(`Executing general post-block-hooks`); const general = executeTheseHooks( this.hooks.post[AllBlocks] ?? [], blocktype, @@ -173,6 +178,9 @@ export class HookContext { context, output, ); + context.logger.logInfo( + `Executing post-block-hooks for blocktype ${blocktype}`, + ); const blockSpecific = executeTheseHooks( this.hooks.post[blocktype] ?? [], blocktype, From f5831c15889692dd7ce5a0181075a1cf484868ae Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 12:42:49 +0100 Subject: [PATCH 10/14] Improve typing for public 'addHook' --- libs/interpreter-lib/src/interpreter.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libs/interpreter-lib/src/interpreter.ts b/libs/interpreter-lib/src/interpreter.ts index fbc01511..a5979e57 100644 --- a/libs/interpreter-lib/src/interpreter.ts +++ b/libs/interpreter-lib/src/interpreter.ts @@ -62,8 +62,17 @@ export class JayveeProgram { constructor(public model: JayveeModel) {} - // FIXME #634: Pass the blocktype to also execute block specific hooks - /** Add a hook to all blocks in the pipeline.*/ + /** Add a hook to one or more blocks in the pipeline.*/ + public addHook( + position: 'preBlock', + hook: PreBlockHook, + opts?: HookOptions, + ): void; + public addHook( + position: 'postBlock', + hook: PostBlockHook, + opts?: HookOptions, + ): void; public addHook( position: HookPosition, hook: PreBlockHook | PostBlockHook, From c124cadb357fb26013135ec38dbe68ddc4478195 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 12:41:47 +0100 Subject: [PATCH 11/14] Add tests for block specific hooks --- .../src/lib/blocks/block-execution-util.ts | 6 +- libs/execution/src/lib/hooks/hook.ts | 1 - libs/interpreter-lib/src/interpreter.spec.ts | 57 +++++++++++++++++++ .../valid-builtin-and-composite-blocks.jv | 52 +++++++++++++++++ 4 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv diff --git a/libs/execution/src/lib/blocks/block-execution-util.ts b/libs/execution/src/lib/blocks/block-execution-util.ts index 5cda5588..1d1f234b 100644 --- a/libs/execution/src/lib/blocks/block-execution-util.ts +++ b/libs/execution/src/lib/blocks/block-execution-util.ts @@ -62,13 +62,15 @@ export async function executeBlocks( executionContext.enterNode(block); - await executionContext.executeHooks(inputValue); // FIXME #634: Pass the blocktype to also execute block specific hooks + await executionContext.executeHooks(inputValue); + const executionResult = await executeBlock( inputValue, block, executionContext, ); - await executionContext.executeHooks(inputValue, executionResult); // FIXME #634: Pass the blocktype to also execute block specific hooks + await executionContext.executeHooks(inputValue, executionResult); + if (R.isErr(executionResult)) { return executionResult; } diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts index 500ade49..894f9c80 100644 --- a/libs/execution/src/lib/hooks/hook.ts +++ b/libs/execution/src/lib/hooks/hook.ts @@ -13,7 +13,6 @@ export interface HookOptions { /** Whether the pipeline should await the hooks completion. `false` if omitted.*/ blocking?: boolean; /** Optionally specify one or more blocks to limit this hook to. If omitted, the hook will be executed on all blocks*/ - // FIXME #634: Add `BlockExecutor[]` variant blocktypes?: string | string[]; } diff --git a/libs/interpreter-lib/src/interpreter.spec.ts b/libs/interpreter-lib/src/interpreter.spec.ts index eca3f064..6534c7e8 100644 --- a/libs/interpreter-lib/src/interpreter.spec.ts +++ b/libs/interpreter-lib/src/interpreter.spec.ts @@ -70,6 +70,63 @@ describe('Interpreter', () => { expect(spy).toHaveBeenCalledTimes(6); }); + it('should execute a block specific hook only on that blocktype', async () => { + const exampleFilePath = + 'libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv'; + const model = readJvTestAsset(exampleFilePath); + + const interpreter = new DefaultJayveeInterpreter({ + pipelineMatcher: () => true, + debug: true, + debugGranularity: 'peek', + debugTarget: 'all', + env: new Map(), + }); + + const program = await interpreter.parseModel( + async (services, loggerFactory) => + await extractAstNodeFromString( + model, + services, + loggerFactory.createLogger(), + ), + ); + expect(program).toBeDefined(); + assert(program !== undefined); + + const sqlite_spy = vi + .fn>() + .mockResolvedValue(undefined); + + program.addHook( + 'preBlock', + async (blocktype) => { + return sqlite_spy(blocktype); + }, + { blocking: true, blocktypes: 'SQLiteLoader' }, + ); + + const interpreter_spy = vi + .fn>() + .mockResolvedValue(undefined); + + program.addHook( + 'postBlock', + async (blocktype) => { + return interpreter_spy(blocktype); + }, + { blocking: true, blocktypes: 'CSVFileInterpreter' }, + ); + + const exitCode = await interpreter.interpretProgram(program); + expect(exitCode).toEqual(ExitCode.SUCCESS); + + expect(sqlite_spy).toHaveBeenCalledTimes(1); + expect(sqlite_spy).toHaveBeenCalledWith('SQLiteLoader'); + expect(interpreter_spy).toHaveBeenCalledTimes(1); + expect(interpreter_spy).toHaveBeenCalledWith('CSVFileInterpreter'); + }); + it('should not wait for non-blocking hooks', async () => { const exampleFilePath = 'example/cars.jv'; const model = readJvTestAsset(exampleFilePath); diff --git a/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv b/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv new file mode 100644 index 00000000..b1cc12c9 --- /dev/null +++ b/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: 2025 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + +pipeline CarsPipeline { + + CarsExtractor + -> CarsInterpreter + -> NameHeaderWriter + -> CarsTableInterpreter + -> CarsLoader; + + + block CarsExtractor oftype HttpExtractor { + url: "https://gist.githubusercontent.com/noamross/e5d3e859aa0c794be10b/raw/b999fb4425b54c63cab088c0ce2c0d6ce961a563/cars.csv"; + } + + block CarsInterpreter oftype CSVFileInterpreter { + enclosing: '"'; + } + + block NameHeaderWriter oftype CellWriter { + at: cell A1; + + write: [ + "name" + ]; + } + + block CarsTableInterpreter oftype TableInterpreter { + header: true; + columns: [ + "name" oftype text, + "mpg" oftype decimal, + "cyl" oftype integer, + "disp" oftype decimal, + "hp" oftype integer, + "drat" oftype decimal, + "wt" oftype decimal, + "qsec" oftype decimal, + "vs" oftype integer, + "am" oftype integer, + "gear" oftype integer, + "carb" oftype integer + ]; + } + + block CarsLoader oftype SQLiteLoader { + table: "Cars"; + file: "./cars.sqlite"; + } +} From 9578a69738eaea4abf591003a3978b3828235591 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 17:31:44 +0100 Subject: [PATCH 12/14] Add test to verify hooks are called with the correct parameters --- libs/interpreter-lib/src/interpreter.spec.ts | 100 ++++++++++++++++++ .../valid-builtin-and-composite-blocks.jv | 29 +++-- 2 files changed, 118 insertions(+), 11 deletions(-) diff --git a/libs/interpreter-lib/src/interpreter.spec.ts b/libs/interpreter-lib/src/interpreter.spec.ts index 6534c7e8..028ea2e8 100644 --- a/libs/interpreter-lib/src/interpreter.spec.ts +++ b/libs/interpreter-lib/src/interpreter.spec.ts @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import { Table, isOk } from '@jvalue/jayvee-execution'; import { type JayveeModel } from '@jvalue/jayvee-language-server'; import { readJvTestAssetHelper } from '@jvalue/jayvee-language-server/test'; @@ -127,6 +128,105 @@ describe('Interpreter', () => { expect(interpreter_spy).toHaveBeenCalledWith('CSVFileInterpreter'); }); + it('should be called with the correct parameters', async () => { + const exampleFilePath = + 'libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv'; + const model = readJvTestAsset(exampleFilePath); + + const interpreter = new DefaultJayveeInterpreter({ + pipelineMatcher: () => true, + debug: true, + debugGranularity: 'peek', + debugTarget: 'all', + env: new Map(), + }); + + const program = await interpreter.parseModel( + async (services, loggerFactory) => + await extractAstNodeFromString( + model, + services, + loggerFactory.createLogger(), + ), + ); + expect(program).toBeDefined(); + assert(program !== undefined); + + const parameter_spy = vi + .fn>() + .mockResolvedValue(undefined); + + const EXPECTED_NAMES = [ + 'Mazda RX4', + 'Mazda RX4 Wag', + 'Datsun 710', + 'Hornet 4 Drive', + 'Hornet Sportabout', + 'Valiant', + 'Duster 360', + 'Merc 240D', + 'Merc 230', + 'Merc 280', + 'Merc 280C', + 'Merc 450SE', + 'Merc 450SL', + 'Merc 450SLC', + 'Cadillac Fleetwood', + 'Lincoln Continental', + 'Chrysler Imperial', + 'Fiat 128', + 'Honda Civic', + 'Toyota Corolla', + 'Toyota Corona', + 'Dodge Challenger', + 'AMC Javelin', + 'Camaro Z28', + 'Pontiac Firebird', + 'Fiat X1-9', + 'Porsche 914-2', + 'Lotus Europa', + 'Ford Pantera L', + 'Ferrari Dino', + 'Maserati Bora', + 'Volvo 142E', + ]; + + program.addHook( + 'postBlock', + async (blocktype, input, output) => { + expect(blocktype).toBe('TableTransformer'); + + expect(input).not.toBeNull(); + assert(input != null); + assert(input instanceof Table); + + expect(input.getNumberOfColumns()).toBe(1); + expect(input.getColumn('name')?.values).toStrictEqual(EXPECTED_NAMES); + + expect(isOk(output)).toBe(true); + assert(isOk(output)); + const out = output.right; + expect(out).not.toBeNull(); + assert(out != null); + assert(out instanceof Table); + + expect(out.getNumberOfColumns()).toBe(2); + expect(out.getColumn('name')?.values).toStrictEqual(EXPECTED_NAMES); + expect(out.getColumn('nameCopy')?.values).toStrictEqual( + EXPECTED_NAMES, + ); + + return parameter_spy(); + }, + { blocking: true, blocktypes: 'TableTransformer' }, + ); + + const exitCode = await interpreter.interpretProgram(program); + expect(exitCode).toEqual(ExitCode.SUCCESS); + + expect(parameter_spy).toHaveBeenCalledTimes(1); + }); + it('should not wait for non-blocking hooks', async () => { const exampleFilePath = 'example/cars.jv'; const model = readJvTestAsset(exampleFilePath); diff --git a/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv b/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv index b1cc12c9..3595e7b4 100644 --- a/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv +++ b/libs/interpreter-lib/test/assets/hooks/valid-builtin-and-composite-blocks.jv @@ -8,6 +8,7 @@ pipeline CarsPipeline { -> CarsInterpreter -> NameHeaderWriter -> CarsTableInterpreter + -> CarsTableTransformer -> CarsLoader; @@ -31,20 +32,26 @@ pipeline CarsPipeline { header: true; columns: [ "name" oftype text, - "mpg" oftype decimal, - "cyl" oftype integer, - "disp" oftype decimal, - "hp" oftype integer, - "drat" oftype decimal, - "wt" oftype decimal, - "qsec" oftype decimal, - "vs" oftype integer, - "am" oftype integer, - "gear" oftype integer, - "carb" oftype integer ]; } + transform copy { + from s oftype text; + to t oftype text; + + t: s; + } + + block CarsTableTransformer oftype TableTransformer { + inputColumns: [ + "name", + ]; + + outputColumn: "nameCopy"; + + uses: copy; + } + block CarsLoader oftype SQLiteLoader { table: "Cars"; file: "./cars.sqlite"; From 36a392ff81374e01a81ac6d2d2c64e35ee542388 Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 17:35:47 +0100 Subject: [PATCH 13/14] Simplify typing by splitting overloaded functions --- libs/execution/src/lib/execution-context.ts | 11 +- libs/execution/src/lib/hooks/hook-context.ts | 131 ++++++++----------- 2 files changed, 61 insertions(+), 81 deletions(-) diff --git a/libs/execution/src/lib/execution-context.ts b/libs/execution/src/lib/execution-context.ts index 879658ba..5463ea62 100644 --- a/libs/execution/src/lib/execution-context.ts +++ b/libs/execution/src/lib/execution-context.ts @@ -154,7 +154,16 @@ export class ExecutionContext { `Expected block definition to have a blocktype: ${inspect(node)}`, ); - return this.hookContext.executeHooks(blocktype, input, this, output); + if (output === undefined) { + return this.hookContext.executePreBlockHooks(blocktype, input, this); + } + + return this.hookContext.executePostBlockHooks( + blocktype, + input, + this, + output, + ); } private getDefaultPropertyValue( diff --git a/libs/execution/src/lib/hooks/hook-context.ts b/libs/execution/src/lib/hooks/hook-context.ts index 1272754a..53b1960b 100644 --- a/libs/execution/src/lib/hooks/hook-context.ts +++ b/libs/execution/src/lib/hooks/hook-context.ts @@ -2,9 +2,6 @@ // // SPDX-License-Identifier: AGPL-3.0-only -// eslint-disable-next-line unicorn/prefer-node-protocol -import assert from 'assert'; - import { type Result } from '../blocks'; import { type ExecutionContext } from '../execution-context'; import { type IOTypeImplementation } from '../types'; @@ -24,53 +21,41 @@ interface HookSpec { hook: H; } -async function executeTheseHooks( +// eslint-disable-next-line @typescript-eslint/no-empty-function +function noop() {} + +async function executePreBlockHooks( hooks: HookSpec[], blocktype: string, input: IOTypeImplementation | null, context: ExecutionContext, -): Promise; -async function executeTheseHooks( +) { + await Promise.all( + hooks.map(async ({ blocking, hook }) => { + if (blocking) { + await hook(blocktype, input, context); + } else { + hook(blocktype, input, context).catch(noop); + } + }), + ); +} + +async function executePostBlockHooks( hooks: HookSpec[], blocktype: string, input: IOTypeImplementation | null, context: ExecutionContext, output: Result, -): Promise; -async function executeTheseHooks( - hooks: HookSpec[] | HookSpec[], - blocktype: string, - input: IOTypeImplementation | null, - context: ExecutionContext, - output?: Result, ) { - const position = output === undefined ? 'preBlock' : 'postBlock'; - return ( - Promise.all( - hooks.map(async ({ blocking, hook }) => { - if (blocking) { - if (isPreBlockHook(hook, position)) { - await hook(blocktype, input, context); - } else { - assert(output !== undefined, 'Guaranteed to be a postBlock hook'); - await hook(blocktype, input, output, context); - } - } else { - if (isPreBlockHook(hook, position)) { - hook(blocktype, input, context) - // eslint-disable-next-line @typescript-eslint/no-empty-function - .catch(() => {}); - } else { - assert(output !== undefined, 'Guaranteed to be a postBlock hook'); - hook(blocktype, input, output, context) - // eslint-disable-next-line @typescript-eslint/no-empty-function - .catch(() => {}); - } - } - }), - ) - // eslint-disable-next-line @typescript-eslint/no-empty-function - .then(() => {}) + await Promise.all( + hooks.map(async ({ blocking, hook }) => { + if (blocking) { + await hook(blocktype, input, output, context); + } else { + hook(blocktype, input, output, context).catch(noop); + } + }), ); } @@ -126,52 +111,39 @@ export class HookContext { }); } - public async executeHooks( + public async executePreBlockHooks( blocktype: string, input: IOTypeImplementation | null, context: ExecutionContext, - ): Promise; - public async executeHooks( + ) { + context.logger.logInfo(`Executing general pre-block-hooks`); + const general = executePreBlockHooks( + this.hooks.pre[AllBlocks] ?? [], + blocktype, + input, + context, + ); + context.logger.logInfo( + `Executing pre-block-hooks for blocktype ${blocktype}`, + ); + const blockSpecific = executePreBlockHooks( + this.hooks.pre[blocktype] ?? [], + blocktype, + input, + context, + ); + + await Promise.all([general, blockSpecific]); + } + + public async executePostBlockHooks( blocktype: string, input: IOTypeImplementation | null, context: ExecutionContext, output: Result, - ): Promise; - public async executeHooks( - blocktype: string, - input: IOTypeImplementation | null, - context: ExecutionContext, - output?: Result, - ): Promise; - public async executeHooks( - blocktype: string, - input: IOTypeImplementation | null, - context: ExecutionContext, - output?: Result, ) { - if (output === undefined) { - context.logger.logInfo(`Executing general pre-block-hooks`); - const general = executeTheseHooks( - this.hooks.pre[AllBlocks] ?? [], - blocktype, - input, - context, - ); - context.logger.logInfo( - `Executing pre-block-hooks for blocktype ${blocktype}`, - ); - const blockSpecific = executeTheseHooks( - this.hooks.pre[blocktype] ?? [], - blocktype, - input, - context, - ); - - // eslint-disable-next-line @typescript-eslint/no-empty-function - return Promise.all([general, blockSpecific]).then(() => {}); - } context.logger.logInfo(`Executing general post-block-hooks`); - const general = executeTheseHooks( + const general = executePostBlockHooks( this.hooks.post[AllBlocks] ?? [], blocktype, input, @@ -181,7 +153,7 @@ export class HookContext { context.logger.logInfo( `Executing post-block-hooks for blocktype ${blocktype}`, ); - const blockSpecific = executeTheseHooks( + const blockSpecific = executePostBlockHooks( this.hooks.post[blocktype] ?? [], blocktype, input, @@ -189,7 +161,6 @@ export class HookContext { output, ); - // eslint-disable-next-line @typescript-eslint/no-empty-function - return Promise.all([general, blockSpecific]).then(() => {}); + await Promise.all([general, blockSpecific]); } } From 81b41beddb8621a8bd5cfaad212c745a13db060f Mon Sep 17 00:00:00 2001 From: Jonas Zeltner Date: Tue, 14 Jan 2025 17:43:25 +0100 Subject: [PATCH 14/14] Simplify `HookOptions.blocktypes` type --- libs/execution/src/lib/hooks/hook-context.ts | 9 ++------- libs/execution/src/lib/hooks/hook.ts | 2 +- libs/interpreter-lib/src/interpreter.spec.ts | 6 +++--- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/libs/execution/src/lib/hooks/hook-context.ts b/libs/execution/src/lib/hooks/hook-context.ts index 53b1960b..adfe9c26 100644 --- a/libs/execution/src/lib/hooks/hook-context.ts +++ b/libs/execution/src/lib/hooks/hook-context.ts @@ -85,12 +85,7 @@ export class HookContext { hook: PreBlockHook | PostBlockHook, opts: HookOptions, ) { - const blocktypes: string[] = - typeof opts.blocktypes === 'string' - ? [opts.blocktypes] - : opts.blocktypes ?? [AllBlocks]; - - blocktypes.forEach((blocktype) => { + for (const blocktype of opts.blocktypes ?? [AllBlocks]) { if (isPreBlockHook(hook, position)) { if (this.hooks.pre[blocktype] === undefined) { this.hooks.pre[blocktype] = []; @@ -108,7 +103,7 @@ export class HookContext { hook, }); } - }); + } } public async executePreBlockHooks( diff --git a/libs/execution/src/lib/hooks/hook.ts b/libs/execution/src/lib/hooks/hook.ts index 894f9c80..64803658 100644 --- a/libs/execution/src/lib/hooks/hook.ts +++ b/libs/execution/src/lib/hooks/hook.ts @@ -13,7 +13,7 @@ export interface HookOptions { /** Whether the pipeline should await the hooks completion. `false` if omitted.*/ blocking?: boolean; /** Optionally specify one or more blocks to limit this hook to. If omitted, the hook will be executed on all blocks*/ - blocktypes?: string | string[]; + blocktypes?: string[]; } /** This function will be executed before a block.*/ diff --git a/libs/interpreter-lib/src/interpreter.spec.ts b/libs/interpreter-lib/src/interpreter.spec.ts index 028ea2e8..401e30f4 100644 --- a/libs/interpreter-lib/src/interpreter.spec.ts +++ b/libs/interpreter-lib/src/interpreter.spec.ts @@ -104,7 +104,7 @@ describe('Interpreter', () => { async (blocktype) => { return sqlite_spy(blocktype); }, - { blocking: true, blocktypes: 'SQLiteLoader' }, + { blocking: true, blocktypes: ['SQLiteLoader'] }, ); const interpreter_spy = vi @@ -116,7 +116,7 @@ describe('Interpreter', () => { async (blocktype) => { return interpreter_spy(blocktype); }, - { blocking: true, blocktypes: 'CSVFileInterpreter' }, + { blocking: true, blocktypes: ['CSVFileInterpreter'] }, ); const exitCode = await interpreter.interpretProgram(program); @@ -218,7 +218,7 @@ describe('Interpreter', () => { return parameter_spy(); }, - { blocking: true, blocktypes: 'TableTransformer' }, + { blocking: true, blocktypes: ['TableTransformer'] }, ); const exitCode = await interpreter.interpretProgram(program);