From 76262ef8998751a0ebf63f9338f40acf825e0fd0 Mon Sep 17 00:00:00 2001 From: Eugene Date: Thu, 14 Nov 2024 18:47:57 +0100 Subject: [PATCH] chore: Migrate test definition PK to string nanoid (no-changelog) (#11742) --- .../databases/entities/test-definition.ee.ts | 18 ++--------- ...748663-MigrateTestDefinitionKeyToString.ts | 18 +++++++++++ .../src/databases/migrations/mysqldb/index.ts | 2 ++ ...748663-MigrateTestDefinitionKeyToString.ts | 30 +++++++++++++++++++ .../databases/migrations/postgresdb/index.ts | 2 ++ ...748663-MigrateTestDefinitionKeyToString.ts | 25 ++++++++++++++++ .../src/databases/migrations/sqlite/index.ts | 2 ++ .../test-definition.repository.ee.ts | 4 +-- .../evaluation/test-definition.service.ee.ts | 10 +++---- .../test-definitions.controller.ee.ts | 16 ++-------- 10 files changed, 92 insertions(+), 35 deletions(-) create mode 100644 packages/cli/src/databases/migrations/mysqldb/1731582748663-MigrateTestDefinitionKeyToString.ts create mode 100644 packages/cli/src/databases/migrations/postgresdb/1731582748663-MigrateTestDefinitionKeyToString.ts create mode 100644 packages/cli/src/databases/migrations/sqlite/1731582748663-MigrateTestDefinitionKeyToString.ts diff --git a/packages/cli/src/databases/entities/test-definition.ee.ts b/packages/cli/src/databases/entities/test-definition.ee.ts index f8ec8d6cb7..dd39ebef02 100644 --- a/packages/cli/src/databases/entities/test-definition.ee.ts +++ b/packages/cli/src/databases/entities/test-definition.ee.ts @@ -1,18 +1,10 @@ -import { - Column, - Entity, - Generated, - Index, - ManyToOne, - PrimaryColumn, - RelationId, -} from '@n8n/typeorm'; +import { Column, Entity, Index, ManyToOne, RelationId } from '@n8n/typeorm'; import { Length } from 'class-validator'; import { AnnotationTagEntity } from '@/databases/entities/annotation-tag-entity.ee'; import { WorkflowEntity } from '@/databases/entities/workflow-entity'; -import { WithTimestamps } from './abstract-entity'; +import { WithTimestampsAndStringId } from './abstract-entity'; /** * Entity representing a Test Definition @@ -24,11 +16,7 @@ import { WithTimestamps } from './abstract-entity'; @Entity() @Index(['workflow']) @Index(['evaluationWorkflow']) -export class TestDefinition extends WithTimestamps { - @Generated() - @PrimaryColumn() - id: number; - +export class TestDefinition extends WithTimestampsAndStringId { @Column({ length: 255 }) @Length(1, 255, { message: 'Test definition name must be $constraint1 to $constraint2 characters long.', diff --git a/packages/cli/src/databases/migrations/mysqldb/1731582748663-MigrateTestDefinitionKeyToString.ts b/packages/cli/src/databases/migrations/mysqldb/1731582748663-MigrateTestDefinitionKeyToString.ts new file mode 100644 index 0000000000..793aa3a262 --- /dev/null +++ b/packages/cli/src/databases/migrations/mysqldb/1731582748663-MigrateTestDefinitionKeyToString.ts @@ -0,0 +1,18 @@ +import type { MigrationContext, IrreversibleMigration } from '@/databases/types'; + +export class MigrateTestDefinitionKeyToString1731582748663 implements IrreversibleMigration { + async up(context: MigrationContext) { + const { queryRunner, tablePrefix } = context; + + await queryRunner.query( + `ALTER TABLE ${tablePrefix}test_definition CHANGE id tmp_id int NOT NULL AUTO_INCREMENT;`, + ); + await queryRunner.query( + `ALTER TABLE ${tablePrefix}test_definition ADD COLUMN id varchar(36) NOT NULL;`, + ); + await queryRunner.query(`UPDATE ${tablePrefix}test_definition SET id = CONVERT(tmp_id, CHAR);`); + await queryRunner.query( + `CREATE INDEX \`TMP_idx_${tablePrefix}test_definition_id\` ON ${tablePrefix}test_definition (\`id\`);`, + ); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index 5f0b23d601..4b219e9a26 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -43,6 +43,7 @@ import { MigrateIntegerKeysToString1690000000001 } from './1690000000001-Migrate import { SeparateExecutionData1690000000030 } from './1690000000030-SeparateExecutionData'; import { FixExecutionDataType1690000000031 } from './1690000000031-FixExecutionDataType'; import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActivatedAtUserSetting'; +import { MigrateTestDefinitionKeyToString1731582748663 } from './1731582748663-MigrateTestDefinitionKeyToString'; import { CreateLdapEntities1674509946020 } from '../common/1674509946020-CreateLdapEntities'; import { PurgeInvalidWorkflowConnections1675940580449 } from '../common/1675940580449-PurgeInvalidWorkflowConnections'; import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030-RemoveResetPasswordColumns'; @@ -142,4 +143,5 @@ export const mysqlMigrations: Migration[] = [ UpdateProcessedDataValueColumnToText1729607673464, CreateTestDefinitionTable1730386903556, AddDescriptionToTestDefinition1731404028106, + MigrateTestDefinitionKeyToString1731582748663, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/1731582748663-MigrateTestDefinitionKeyToString.ts b/packages/cli/src/databases/migrations/postgresdb/1731582748663-MigrateTestDefinitionKeyToString.ts new file mode 100644 index 0000000000..9afc1c3ae3 --- /dev/null +++ b/packages/cli/src/databases/migrations/postgresdb/1731582748663-MigrateTestDefinitionKeyToString.ts @@ -0,0 +1,30 @@ +import type { MigrationContext, IrreversibleMigration } from '@/databases/types'; + +export class MigrateTestDefinitionKeyToString1731582748663 implements IrreversibleMigration { + async up(context: MigrationContext) { + const { queryRunner, tablePrefix } = context; + + await queryRunner.query( + `ALTER TABLE ${tablePrefix}test_definition RENAME COLUMN id to tmp_id;`, + ); + await queryRunner.query(`ALTER TABLE ${tablePrefix}test_definition ADD COLUMN id varchar(36);`); + await queryRunner.query(`UPDATE ${tablePrefix}test_definition SET id = tmp_id::text;`); + await queryRunner.query( + `ALTER TABLE ${tablePrefix}test_definition ALTER COLUMN id SET NOT NULL;`, + ); + await queryRunner.query( + `ALTER TABLE ${tablePrefix}test_definition ALTER COLUMN tmp_id DROP DEFAULT;`, + ); + await queryRunner.query(`DROP SEQUENCE IF EXISTS ${tablePrefix}test_definition_id_seq;`); + await queryRunner.query( + `CREATE UNIQUE INDEX "pk_${tablePrefix}test_definition_id" ON ${tablePrefix}test_definition ("id");`, + ); + + await queryRunner.query( + `ALTER TABLE ${tablePrefix}test_definition DROP CONSTRAINT IF EXISTS "PK_${tablePrefix}245a0013672c8cdc7727afa9b99";`, + ); + + await queryRunner.query(`ALTER TABLE ${tablePrefix}test_definition DROP COLUMN tmp_id;`); + await queryRunner.query(`ALTER TABLE ${tablePrefix}test_definition ADD PRIMARY KEY (id);`); + } +} diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index 5372256371..689840b937 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -43,6 +43,7 @@ import { AddMissingPrimaryKeyOnExecutionData1690787606731 } from './169078760673 import { MigrateToTimestampTz1694091729095 } from './1694091729095-MigrateToTimestampTz'; import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActivatedAtUserSetting'; import { FixExecutionMetadataSequence1721377157740 } from './1721377157740-FixExecutionMetadataSequence'; +import { MigrateTestDefinitionKeyToString1731582748663 } from './1731582748663-MigrateTestDefinitionKeyToString'; import { CreateLdapEntities1674509946020 } from '../common/1674509946020-CreateLdapEntities'; import { PurgeInvalidWorkflowConnections1675940580449 } from '../common/1675940580449-PurgeInvalidWorkflowConnections'; import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030-RemoveResetPasswordColumns'; @@ -142,4 +143,5 @@ export const postgresMigrations: Migration[] = [ UpdateProcessedDataValueColumnToText1729607673464, CreateTestDefinitionTable1730386903556, AddDescriptionToTestDefinition1731404028106, + MigrateTestDefinitionKeyToString1731582748663, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/1731582748663-MigrateTestDefinitionKeyToString.ts b/packages/cli/src/databases/migrations/sqlite/1731582748663-MigrateTestDefinitionKeyToString.ts new file mode 100644 index 0000000000..4ffa3da983 --- /dev/null +++ b/packages/cli/src/databases/migrations/sqlite/1731582748663-MigrateTestDefinitionKeyToString.ts @@ -0,0 +1,25 @@ +import type { MigrationContext, IrreversibleMigration } from '@/databases/types'; + +export class MigrateTestDefinitionKeyToString1731582748663 implements IrreversibleMigration { + transaction = false as const; + + async up(context: MigrationContext) { + const { queryRunner, tablePrefix } = context; + + await queryRunner.query(` + CREATE TABLE "${tablePrefix}TMP_test_definition" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(255) NOT NULL, "workflowId" varchar(36) NOT NULL, "evaluationWorkflowId" varchar(36), "annotationTagId" varchar(16), "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "description" text, CONSTRAINT "FK_${tablePrefix}test_definition_annotation_tag" FOREIGN KEY ("annotationTagId") REFERENCES "annotation_tag_entity" ("id") ON DELETE SET NULL ON UPDATE NO ACTION, CONSTRAINT "FK_${tablePrefix}test_definition_evaluation_workflow_entity" FOREIGN KEY ("evaluationWorkflowId") REFERENCES "workflow_entity" ("id") ON DELETE SET NULL ON UPDATE NO ACTION, CONSTRAINT "FK_${tablePrefix}test_definition_workflow_entity" FOREIGN KEY ("workflowId") REFERENCES "workflow_entity" ("id") ON DELETE CASCADE ON UPDATE NO ACTION);`); + await queryRunner.query( + `INSERT INTO "${tablePrefix}TMP_test_definition" SELECT * FROM "${tablePrefix}test_definition";`, + ); + await queryRunner.query(`DROP TABLE "${tablePrefix}test_definition";`); + await queryRunner.query( + `ALTER TABLE "${tablePrefix}TMP_test_definition" RENAME TO "${tablePrefix}test_definition";`, + ); + await queryRunner.query( + `CREATE INDEX "idx_${tablePrefix}test_definition_workflow_id" ON "${tablePrefix}test_definition" ("workflowId");`, + ); + await queryRunner.query( + `CREATE INDEX "idx_${tablePrefix}test_definition_evaluation_workflow_id" ON "${tablePrefix}test_definition" ("evaluationWorkflowId");`, + ); + } +} diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index e9b09ff483..5c7c6d9a07 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -40,6 +40,7 @@ import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActiv import { AddApiKeysTable1724951148974 } from './1724951148974-AddApiKeysTable'; import { AddMissingPrimaryKeyOnAnnotationTagMapping1728659839644 } from './1728659839644-AddMissingPrimaryKeyOnAnnotationTagMapping'; import { AddDescriptionToTestDefinition1731404028106 } from './1731404028106-AddDescriptionToTestDefinition'; +import { MigrateTestDefinitionKeyToString1731582748663 } from './1731582748663-MigrateTestDefinitionKeyToString'; import { UniqueWorkflowNames1620821879465 } from '../common/1620821879465-UniqueWorkflowNames'; import { UpdateWorkflowCredentials1630330987096 } from '../common/1630330987096-UpdateWorkflowCredentials'; import { AddNodeIds1658930531669 } from '../common/1658930531669-AddNodeIds'; @@ -136,6 +137,7 @@ const sqliteMigrations: Migration[] = [ UpdateProcessedDataValueColumnToText1729607673464, CreateTestDefinitionTable1730386903556, AddDescriptionToTestDefinition1731404028106, + MigrateTestDefinitionKeyToString1731582748663, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/databases/repositories/test-definition.repository.ee.ts b/packages/cli/src/databases/repositories/test-definition.repository.ee.ts index e9ec3da65d..ecd4bdde34 100644 --- a/packages/cli/src/databases/repositories/test-definition.repository.ee.ts +++ b/packages/cli/src/databases/repositories/test-definition.repository.ee.ts @@ -37,7 +37,7 @@ export class TestDefinitionRepository extends Repository { return { testDefinitions, count }; } - async getOne(id: number, accessibleWorkflowIds: string[]) { + async getOne(id: string, accessibleWorkflowIds: string[]) { return await this.findOne({ where: { id, @@ -49,7 +49,7 @@ export class TestDefinitionRepository extends Repository { }); } - async deleteById(id: number, accessibleWorkflowIds: string[]) { + async deleteById(id: string, accessibleWorkflowIds: string[]) { return await this.delete({ id, workflow: { diff --git a/packages/cli/src/evaluation/test-definition.service.ee.ts b/packages/cli/src/evaluation/test-definition.service.ee.ts index 404167078b..d32a2e153d 100644 --- a/packages/cli/src/evaluation/test-definition.service.ee.ts +++ b/packages/cli/src/evaluation/test-definition.service.ee.ts @@ -30,7 +30,7 @@ export class TestDefinitionService { workflowId?: string; evaluationWorkflowId?: string; annotationTagId?: string; - id?: number; + id?: string; }) { const entity: TestDefinitionLike = {}; @@ -72,13 +72,13 @@ export class TestDefinitionService { workflowId?: string; evaluationWorkflowId?: string; annotationTagId?: string; - id?: number; + id?: string; }) { const entity = this.toEntityLike(attrs); return this.testDefinitionRepository.create(entity); } - async findOne(id: number, accessibleWorkflowIds: string[]) { + async findOne(id: string, accessibleWorkflowIds: string[]) { return await this.testDefinitionRepository.getOne(id, accessibleWorkflowIds); } @@ -88,7 +88,7 @@ export class TestDefinitionService { return await this.testDefinitionRepository.save(test); } - async update(id: number, attrs: TestDefinitionLike) { + async update(id: string, attrs: TestDefinitionLike) { if (attrs.name) { const updatedTest = this.toEntity(attrs); await validateEntity(updatedTest); @@ -115,7 +115,7 @@ export class TestDefinitionService { } } - async delete(id: number, accessibleWorkflowIds: string[]) { + async delete(id: string, accessibleWorkflowIds: string[]) { const deleteResult = await this.testDefinitionRepository.deleteById(id, accessibleWorkflowIds); if (deleteResult.affected === 0) { diff --git a/packages/cli/src/evaluation/test-definitions.controller.ee.ts b/packages/cli/src/evaluation/test-definitions.controller.ee.ts index c73afaeb3d..eaa7745c51 100644 --- a/packages/cli/src/evaluation/test-definitions.controller.ee.ts +++ b/packages/cli/src/evaluation/test-definitions.controller.ee.ts @@ -2,7 +2,6 @@ import express from 'express'; import assert from 'node:assert'; import { Get, Post, Patch, RestController, Delete } from '@/decorators'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { @@ -11,21 +10,12 @@ import { } from '@/evaluation/test-definition.schema'; import { listQueryMiddleware } from '@/middlewares'; import { getSharedWorkflowIds } from '@/public-api/v1/handlers/workflows/workflows.service'; -import { isPositiveInteger } from '@/utils'; import { TestDefinitionService } from './test-definition.service.ee'; import { TestDefinitionsRequest } from './test-definitions.types.ee'; @RestController('/evaluation/test-definitions') export class TestDefinitionsController { - private validateId(id: string) { - if (!isPositiveInteger(id)) { - throw new BadRequestError('Test ID is not a number'); - } - - return Number(id); - } - constructor(private readonly testDefinitionService: TestDefinitionService) {} @Get('/', { middlewares: listQueryMiddleware }) @@ -40,7 +30,7 @@ export class TestDefinitionsController { @Get('/:id') async getOne(req: TestDefinitionsRequest.GetOne) { - const testDefinitionId = this.validateId(req.params.id); + const { id: testDefinitionId } = req.params; const userAccessibleWorkflowIds = await getSharedWorkflowIds(req.user, ['workflow:read']); @@ -82,7 +72,7 @@ export class TestDefinitionsController { @Delete('/:id') async delete(req: TestDefinitionsRequest.Delete) { - const testDefinitionId = this.validateId(req.params.id); + const { id: testDefinitionId } = req.params; const userAccessibleWorkflowIds = await getSharedWorkflowIds(req.user, ['workflow:read']); @@ -96,7 +86,7 @@ export class TestDefinitionsController { @Patch('/:id') async patch(req: TestDefinitionsRequest.Patch, res: express.Response) { - const testDefinitionId = this.validateId(req.params.id); + const { id: testDefinitionId } = req.params; const bodyParseResult = testDefinitionPatchRequestBodySchema.safeParse(req.body); if (!bodyParseResult.success) {