From 2c66f5f43edf6ae32d2b2509c4334c85984b259c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 27 Dec 2023 17:10:59 +0100 Subject: [PATCH 1/6] refactor(core): Move `typeorm` operators from `WaitTracker` to `ExecutionRepository` (no-changelog) --- packages/cli/src/WaitTracker.ts | 28 +------------------ .../repositories/execution.repository.ts | 26 +++++++++++++++++ 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index 80eb337eafa59..4bbf3dd026904 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -4,11 +4,6 @@ import { WorkflowOperationError, } from 'n8n-workflow'; import { Container, Service } from 'typedi'; -import type { FindManyOptions, ObjectLiteral } from 'typeorm'; -import { Not, LessThanOrEqual } from 'typeorm'; -import { DateUtils } from 'typeorm/util/DateUtils'; - -import config from '@/config'; import * as ResponseHelper from '@/ResponseHelper'; import type { IExecutionResponse, @@ -18,7 +13,6 @@ import type { import { WorkflowRunner } from '@/WorkflowRunner'; import { recoverExecutionDataFromEventLogMessages } from './eventbus/MessageEventBus/recoverEvents'; import { ExecutionRepository } from '@db/repositories/execution.repository'; -import type { ExecutionEntity } from '@db/entities/ExecutionEntity'; import { OwnershipService } from './services/ownership.service'; import { Logger } from '@/Logger'; @@ -48,28 +42,8 @@ export class WaitTracker { async getWaitingExecutions() { this.logger.debug('Wait tracker querying database for waiting executions'); - // Find all the executions which should be triggered in the next 70 seconds - const findQuery: FindManyOptions = { - select: ['id', 'waitTill'], - where: { - waitTill: LessThanOrEqual(new Date(Date.now() + 70000)), - status: Not('crashed'), - }, - order: { - waitTill: 'ASC', - }, - }; - - const dbType = config.getEnv('database.type'); - if (dbType === 'sqlite') { - // This is needed because of issue in TypeORM <> SQLite: - // https://github.com/typeorm/typeorm/issues/2286 - (findQuery.where! as ObjectLiteral).waitTill = LessThanOrEqual( - DateUtils.mixedDateToUtcDatetimeString(new Date(Date.now() + 70000)), - ); - } - const executions = await this.executionRepository.findMultipleExecutions(findQuery); + const executions = await this.executionRepository.getWaitingExecutions(); if (executions.length === 0) { return; diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 67b3d0de4bf34..6fdc5bfb886a8 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -14,6 +14,7 @@ import type { FindManyOptions, FindOneOptions, FindOptionsWhere, + ObjectLiteral, SelectQueryBuilder, } from 'typeorm'; import { parse, stringify } from 'flatted'; @@ -516,4 +517,29 @@ export class ExecutionRepository extends Repository { async deleteByIds(executionIds: string[]) { return this.delete({ id: In(executionIds) }); } + + async getWaitingExecutions() { + // Find all the executions which should be triggered in the next 70 seconds + const findQuery: FindManyOptions = { + select: ['id', 'waitTill'], + where: { + waitTill: LessThanOrEqual(new Date(Date.now() + 70000)), + status: Not('crashed'), + }, + order: { + waitTill: 'ASC', + }, + }; + + const dbType = config.getEnv('database.type'); + if (dbType === 'sqlite') { + // This is needed because of issue in TypeORM <> SQLite: + // https://github.com/typeorm/typeorm/issues/2286 + (findQuery.where! as ObjectLiteral).waitTill = LessThanOrEqual( + DateUtils.mixedDateToUtcDatetimeString(new Date(Date.now() + 70000)), + ); + } + + return this.findMultipleExecutions(findQuery); + } } From e48e7d8c20be76335c8dfdcf35ca30790380a269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 27 Dec 2023 17:48:58 +0100 Subject: [PATCH 2/6] Add quick test --- .../repositories/execution.repository.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 packages/cli/test/unit/repositories/execution.repository.test.ts diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts new file mode 100644 index 0000000000000..278ced9bb1a16 --- /dev/null +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -0,0 +1,35 @@ +import { mock } from 'jest-mock-extended'; +import Container from 'typedi'; +import { EntityManager, DataSource } from 'typeorm'; +import { mockInstance } from '../../shared/mocking'; +import { ExecutionRepository } from '@/databases/repositories/execution.repository'; + +describe('ExecutionRepository', () => { + const entityManager = mockInstance(EntityManager); + const dataSource = mockInstance(DataSource, { manager: entityManager }); + dataSource.getMetadata.mockReturnValue(mock()); + Object.assign(entityManager, { connection: dataSource }); + + const executionRepository = Container.get(ExecutionRepository); + + describe('getWaitingExecutions()', () => { + test('should', async () => { + jest.spyOn(executionRepository, 'findMultipleExecutions').mockResolvedValueOnce([]); + + await executionRepository.getWaitingExecutions(); + + const expectedQuery = expect.objectContaining({ + order: { waitTill: 'ASC' }, + select: ['id', 'waitTill'], + where: expect.objectContaining({ + waitTill: expect.objectContaining({ + _type: 'lessThanOrEqual', + _value: expect.any(String), + }), + }), + }); + + expect(executionRepository.findMultipleExecutions).toHaveBeenCalledWith(expectedQuery); + }); + }); +}); From 9430dca364b17f69a31bfb7fb6fc5ab706b6db4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 27 Dec 2023 18:05:01 +0100 Subject: [PATCH 3/6] Check on both DB types --- .../cli/test/unit/repositories/execution.repository.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index 278ced9bb1a16..53d70b375aeb3 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -3,6 +3,7 @@ import Container from 'typedi'; import { EntityManager, DataSource } from 'typeorm'; import { mockInstance } from '../../shared/mocking'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import config from '@/config'; describe('ExecutionRepository', () => { const entityManager = mockInstance(EntityManager); @@ -13,7 +14,8 @@ describe('ExecutionRepository', () => { const executionRepository = Container.get(ExecutionRepository); describe('getWaitingExecutions()', () => { - test('should', async () => { + test.each(['sqlite', 'postgres'])('should be called with expected args', async (dbType) => { + jest.spyOn(config, 'getEnv').mockReturnValueOnce(dbType); jest.spyOn(executionRepository, 'findMultipleExecutions').mockResolvedValueOnce([]); await executionRepository.getWaitingExecutions(); From a33242612a0d855ad97ce3efe445da2cc8f8dafe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 27 Dec 2023 18:07:57 +0100 Subject: [PATCH 4/6] Improve msg display --- .../repositories/execution.repository.test.ts | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index 53d70b375aeb3..8ebd74a199bb1 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -14,24 +14,27 @@ describe('ExecutionRepository', () => { const executionRepository = Container.get(ExecutionRepository); describe('getWaitingExecutions()', () => { - test.each(['sqlite', 'postgres'])('should be called with expected args', async (dbType) => { - jest.spyOn(config, 'getEnv').mockReturnValueOnce(dbType); - jest.spyOn(executionRepository, 'findMultipleExecutions').mockResolvedValueOnce([]); + test.each(['sqlite', 'postgres'])( + 'on %s, should be called with expected args', + async (dbType) => { + jest.spyOn(config, 'getEnv').mockReturnValueOnce(dbType); + jest.spyOn(executionRepository, 'findMultipleExecutions').mockResolvedValueOnce([]); - await executionRepository.getWaitingExecutions(); + await executionRepository.getWaitingExecutions(); - const expectedQuery = expect.objectContaining({ - order: { waitTill: 'ASC' }, - select: ['id', 'waitTill'], - where: expect.objectContaining({ - waitTill: expect.objectContaining({ - _type: 'lessThanOrEqual', - _value: expect.any(String), + const expectedQuery = expect.objectContaining({ + order: { waitTill: 'ASC' }, + select: ['id', 'waitTill'], + where: expect.objectContaining({ + waitTill: expect.objectContaining({ + _type: 'lessThanOrEqual', + _value: expect.any(String), + }), }), - }), - }); + }); - expect(executionRepository.findMultipleExecutions).toHaveBeenCalledWith(expectedQuery); - }); + expect(executionRepository.findMultipleExecutions).toHaveBeenCalledWith(expectedQuery); + }, + ); }); }); From 2f92f622870ab60908748fed50a1b815788031cf 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: Thu, 28 Dec 2023 15:31:25 +0100 Subject: [PATCH 5/6] update the tests --- .../repositories/execution.repository.ts | 25 ++++++------ .../repositories/execution.repository.test.ts | 40 ++++++++++++------- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 6fdc5bfb886a8..b8fc7f887e484 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -520,26 +520,25 @@ export class ExecutionRepository extends Repository { async getWaitingExecutions() { // Find all the executions which should be triggered in the next 70 seconds - const findQuery: FindManyOptions = { - select: ['id', 'waitTill'], - where: { - waitTill: LessThanOrEqual(new Date(Date.now() + 70000)), - status: Not('crashed'), - }, - order: { - waitTill: 'ASC', - }, + const waitTill = new Date(Date.now() + 70000); + const where: FindOptionsWhere = { + waitTill: LessThanOrEqual(waitTill), + status: Not('crashed'), }; const dbType = config.getEnv('database.type'); if (dbType === 'sqlite') { // This is needed because of issue in TypeORM <> SQLite: // https://github.com/typeorm/typeorm/issues/2286 - (findQuery.where! as ObjectLiteral).waitTill = LessThanOrEqual( - DateUtils.mixedDateToUtcDatetimeString(new Date(Date.now() + 70000)), - ); + where.waitTill = LessThanOrEqual(DateUtils.mixedDateToUtcDatetimeString(waitTill)); } - return this.findMultipleExecutions(findQuery); + return this.findMultipleExecutions({ + select: ['id', 'waitTill'], + where, + order: { + waitTill: 'ASC', + }, + }); } } diff --git a/packages/cli/test/unit/repositories/execution.repository.test.ts b/packages/cli/test/unit/repositories/execution.repository.test.ts index 8ebd74a199bb1..e101e77557204 100644 --- a/packages/cli/test/unit/repositories/execution.repository.test.ts +++ b/packages/cli/test/unit/repositories/execution.repository.test.ts @@ -1,39 +1,51 @@ import { mock } from 'jest-mock-extended'; import Container from 'typedi'; -import { EntityManager, DataSource } from 'typeorm'; -import { mockInstance } from '../../shared/mocking'; -import { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import type { EntityMetadata } from 'typeorm'; +import { EntityManager, DataSource, Not, LessThanOrEqual } from 'typeorm'; + import config from '@/config'; +import { ExecutionEntity } from '@db/entities/ExecutionEntity'; +import { ExecutionRepository } from '@db/repositories/execution.repository'; + +import { mockInstance } from '../../shared/mocking'; describe('ExecutionRepository', () => { const entityManager = mockInstance(EntityManager); const dataSource = mockInstance(DataSource, { manager: entityManager }); - dataSource.getMetadata.mockReturnValue(mock()); + dataSource.getMetadata.mockReturnValue(mock({ target: ExecutionEntity })); Object.assign(entityManager, { connection: dataSource }); const executionRepository = Container.get(ExecutionRepository); + const mockDate = new Date('2023-12-28 12:34:56.789Z'); + + beforeAll(() => { + jest.clearAllMocks(); + jest.useFakeTimers().setSystemTime(mockDate); + }); + + afterAll(() => jest.useRealTimers()); describe('getWaitingExecutions()', () => { test.each(['sqlite', 'postgres'])( 'on %s, should be called with expected args', async (dbType) => { jest.spyOn(config, 'getEnv').mockReturnValueOnce(dbType); - jest.spyOn(executionRepository, 'findMultipleExecutions').mockResolvedValueOnce([]); + entityManager.find.mockResolvedValueOnce([]); await executionRepository.getWaitingExecutions(); - const expectedQuery = expect.objectContaining({ + expect(entityManager.find).toHaveBeenCalledWith(ExecutionEntity, { order: { waitTill: 'ASC' }, select: ['id', 'waitTill'], - where: expect.objectContaining({ - waitTill: expect.objectContaining({ - _type: 'lessThanOrEqual', - _value: expect.any(String), - }), - }), + where: { + status: Not('crashed'), + waitTill: LessThanOrEqual( + dbType === 'sqlite' + ? '2023-12-28 12:36:06.789' + : new Date('2023-12-28T12:36:06.789Z'), + ), + }, }); - - expect(executionRepository.findMultipleExecutions).toHaveBeenCalledWith(expectedQuery); }, ); }); From 77341e640d51c88954f36617a48bfabb6241e76a 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: Thu, 28 Dec 2023 15:32:36 +0100 Subject: [PATCH 6/6] lintfix --- packages/cli/src/databases/repositories/execution.repository.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index 292558bf7ca5f..89b032ab065ae 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -14,7 +14,6 @@ import type { FindManyOptions, FindOneOptions, FindOptionsWhere, - ObjectLiteral, SelectQueryBuilder, } from 'typeorm'; import { parse, stringify } from 'flatted';