From 06c4c6ee0e8dfe73e29edfe2279f35254f7ec26a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 12 Dec 2023 11:14:04 +0100 Subject: [PATCH 1/3] refactor(core): Don't use DB transactions on ExecutionRepository.createNewExecution (no-changelog) --- packages/cli/src/ActiveExecutions.ts | 4 +-- .../repositories/execution.repository.ts | 35 +++++++++++++------ .../cli/test/unit/ActiveExecutions.test.ts | 4 +-- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/ActiveExecutions.ts b/packages/cli/src/ActiveExecutions.ts index ceb74d628ca27..cd8ce8ecdbe70 100644 --- a/packages/cli/src/ActiveExecutions.ts +++ b/packages/cli/src/ActiveExecutions.ts @@ -60,9 +60,7 @@ export class ActiveExecutions { fullExecutionData.workflowId = workflowId; } - const executionResult = - await Container.get(ExecutionRepository).createNewExecution(fullExecutionData); - executionId = executionResult.id; + executionId = await Container.get(ExecutionRepository).createNewExecution(fullExecutionData); if (executionId === undefined) { throw new ApplicationError('There was an issue assigning an execution id to the execution'); } diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index f8e1350588fa9..a54877567c5ca 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -25,7 +25,7 @@ import type { import config from '@/config'; import type { IGetExecutionsQueryFilter } from '@/executions/executions.service'; import { isAdvancedExecutionFiltersEnabled } from '@/executions/executionHelpers'; -import type { ExecutionData } from '../entities/ExecutionData'; +import { ExecutionData } from '../entities/ExecutionData'; import { ExecutionEntity } from '../entities/ExecutionEntity'; import { ExecutionMetadata } from '../entities/ExecutionMetadata'; import { ExecutionDataRepository } from './executionData.repository'; @@ -213,17 +213,30 @@ export class ExecutionRepository extends Repository { return rest; } - async createNewExecution(execution: ExecutionPayload) { + async createNewExecution(execution: ExecutionPayload): Promise { const { data, workflowData, ...rest } = execution; - - const newExecution = await this.save(rest); - await this.executionDataRepository.save({ - execution: newExecution, - workflowData, - data: stringify(data), - }); - - return newExecution; + const { identifiers: inserted } = await this.manager + .createQueryBuilder() + .insert() + .into(ExecutionEntity) + .values([rest]) + .execute(); + + const { id: executionId } = inserted[0] as { id: string }; + const { connections, nodes, name } = workflowData ?? {}; + await this.manager + .createQueryBuilder() + .insert() + .into(ExecutionData) + .values([ + { + executionId, + workflowData: { connections, nodes, name }, + data: stringify(data), + }, + ]) + .execute(); + return executionId; } async markAsCrashed(executionIds: string[]) { diff --git a/packages/cli/test/unit/ActiveExecutions.test.ts b/packages/cli/test/unit/ActiveExecutions.test.ts index 689bbbefabf4b..6ac1438b4bb0d 100644 --- a/packages/cli/test/unit/ActiveExecutions.test.ts +++ b/packages/cli/test/unit/ActiveExecutions.test.ts @@ -12,9 +12,7 @@ const FAKE_EXECUTION_ID = '15'; const FAKE_SECOND_EXECUTION_ID = '20'; const updateExistingExecution = jest.fn(); -const createNewExecution = jest.fn(async () => { - return { id: FAKE_EXECUTION_ID }; -}); +const createNewExecution = jest.fn(async () => FAKE_EXECUTION_ID); Container.set(ExecutionRepository, { updateExistingExecution, From b9311947218c5a64af1004c670e187e2131796a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 12 Dec 2023 12:24:47 +0100 Subject: [PATCH 2/3] add some basic integration tests for ExecutionRepository.createNewExecution --- .../repositories/execution.repository.ts | 2 +- .../repositories/execution.repository.test.ts | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 packages/cli/test/integration/database/repositories/execution.repository.test.ts diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index a54877567c5ca..90c449d507666 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -236,7 +236,7 @@ export class ExecutionRepository extends Repository { }, ]) .execute(); - return executionId; + return String(executionId); } async markAsCrashed(executionIds: string[]) { diff --git a/packages/cli/test/integration/database/repositories/execution.repository.test.ts b/packages/cli/test/integration/database/repositories/execution.repository.test.ts new file mode 100644 index 0000000000000..d16645367f66b --- /dev/null +++ b/packages/cli/test/integration/database/repositories/execution.repository.test.ts @@ -0,0 +1,53 @@ +import Container from 'typedi'; +import { ExecutionRepository } from '@db/repositories/execution.repository'; +import { ExecutionDataRepository } from '@db/repositories/executionData.repository'; +import * as testDb from '../../shared/testDb'; +import { createWorkflow } from '../../shared/db/workflows'; + +describe('ExecutionRepository', () => { + beforeAll(async () => { + await testDb.init(); + }); + + beforeEach(async () => { + await testDb.truncate(['Workflow', 'Execution']); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + describe('createNewExecution', () => { + it('should save execution data', async () => { + const executionRepo = Container.get(ExecutionRepository); + const workflow = await createWorkflow(); + const executionId = await executionRepo.createNewExecution({ + workflowId: workflow.id, + data: { + resultData: {}, + }, + workflowData: workflow, + mode: 'manual', + startedAt: new Date(), + status: 'new', + finished: false, + }); + + expect(executionId).toBeDefined(); + + const executionEntity = await executionRepo.findOneBy({ id: executionId }); + expect(executionEntity?.id).toEqual(executionId); + expect(executionEntity?.workflowId).toEqual(workflow.id); + expect(executionEntity?.status).toEqual('new'); + + const executionDataRepo = Container.get(ExecutionDataRepository); + const executionData = await executionDataRepo.findOneBy({ executionId }); + expect(executionData?.workflowData).toEqual({ + connections: workflow.connections, + nodes: workflow.nodes, + name: workflow.name, + }); + expect(executionData?.data).toEqual('[{"resultData":"1"},{}]'); + }); + }); +}); From 6296685b99eedeac3ce619aa8d22cd466f35c449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 12 Dec 2023 13:44:21 +0100 Subject: [PATCH 3/3] just use this.insert, it does not use transactions either --- .../repositories/execution.repository.ts | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 90c449d507666..7598ff788f922 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -25,7 +25,7 @@ import type { import config from '@/config'; import type { IGetExecutionsQueryFilter } from '@/executions/executions.service'; import { isAdvancedExecutionFiltersEnabled } from '@/executions/executionHelpers'; -import { ExecutionData } from '../entities/ExecutionData'; +import type { ExecutionData } from '../entities/ExecutionData'; import { ExecutionEntity } from '../entities/ExecutionEntity'; import { ExecutionMetadata } from '../entities/ExecutionMetadata'; import { ExecutionDataRepository } from './executionData.repository'; @@ -215,27 +215,14 @@ export class ExecutionRepository extends Repository { async createNewExecution(execution: ExecutionPayload): Promise { const { data, workflowData, ...rest } = execution; - const { identifiers: inserted } = await this.manager - .createQueryBuilder() - .insert() - .into(ExecutionEntity) - .values([rest]) - .execute(); - + const { identifiers: inserted } = await this.insert(rest); const { id: executionId } = inserted[0] as { id: string }; const { connections, nodes, name } = workflowData ?? {}; - await this.manager - .createQueryBuilder() - .insert() - .into(ExecutionData) - .values([ - { - executionId, - workflowData: { connections, nodes, name }, - data: stringify(data), - }, - ]) - .execute(); + await this.executionDataRepository.insert({ + executionId, + workflowData: { connections, nodes, name }, + data: stringify(data), + }); return String(executionId); }