chore: Migrate test definition PK to string nanoid (no-changelog) (#1…

…1742)

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.',

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\`);`,
+		);
+	}
+}

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,
 ];

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);`);
+	}
+}

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,
 ];

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");`,
+		);
+	}
+}

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 };

packages/cli/src/databases/repositories/test-definition.repository.ee.ts

@@ -37,7 +37,7 @@ export class TestDefinitionRepository extends Repository<TestDefinition> {
 		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<TestDefinition> {
 		});
 	}
 
-	async deleteById(id: number, accessibleWorkflowIds: string[]) {
+	async deleteById(id: string, accessibleWorkflowIds: string[]) {
 		return await this.delete({
 			id,
 			workflow: {

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) {

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) {