diff --git a/packages/cli/src/GenericHelpers.ts b/packages/cli/src/GenericHelpers.ts index 054422bdeb..4d62c05d62 100644 --- a/packages/cli/src/GenericHelpers.ts +++ b/packages/cli/src/GenericHelpers.ts @@ -8,17 +8,14 @@ import type { } from 'n8n-workflow'; import { validate } from 'class-validator'; import { Container } from 'typedi'; -import { Like } from 'typeorm'; import config from '@/config'; -import type { ExecutionPayload, ICredentialsDb, IWorkflowDb } from '@/Interfaces'; +import type { ExecutionPayload, IWorkflowDb } from '@/Interfaces'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; import type { TagEntity } from '@db/entities/TagEntity'; import type { User } from '@db/entities/User'; import type { UserUpdatePayload } from '@/requests'; -import { CredentialsRepository } from '@db/repositories/credentials.repository'; import { ExecutionRepository } from '@db/repositories/execution.repository'; -import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { BadRequestError } from './errors/response-errors/bad-request.error'; /** @@ -43,58 +40,6 @@ export function getSessionId(req: express.Request): string | undefined { return req.headers.sessionid as string | undefined; } -/** - * Generate a unique name for a workflow or credentials entity. - * - * - If the name does not yet exist, it returns the requested name. - * - If the name already exists once, it returns the requested name suffixed with 2. - * - If the name already exists more than once with suffixes, it looks for the max suffix - * and returns the requested name with max suffix + 1. - */ - -export async function generateUniqueName( - requestedName: string, - entityType: 'workflow' | 'credentials', -) { - const findConditions = { - select: ['name' as const], - where: { - name: Like(`${requestedName}%`), - }, - }; - - const found: Array = - entityType === 'workflow' - ? await Container.get(WorkflowRepository).find(findConditions) - : await Container.get(CredentialsRepository).find(findConditions); - - // name is unique - if (found.length === 0) { - return requestedName; - } - - const maxSuffix = found.reduce((acc, { name }) => { - const parts = name.split(`${requestedName} `); - - if (parts.length > 2) return acc; - - const suffix = Number(parts[1]); - - if (!isNaN(suffix) && Math.ceil(suffix) > acc) { - acc = Math.ceil(suffix); - } - - return acc; - }, 0); - - // name is duplicate but no numeric suffixes exist yet - if (maxSuffix === 0) { - return `${requestedName} 2`; - } - - return `${requestedName} ${maxSuffix + 1}`; -} - export async function validateEntity( entity: WorkflowEntity | CredentialsEntity | TagEntity | User | UserUpdatePayload, ): Promise { diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index ebe006987f..9223f7a0f3 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -2,7 +2,6 @@ import express from 'express'; import type { INodeCredentialTestResult } from 'n8n-workflow'; import { deepCopy } from 'n8n-workflow'; -import * as GenericHelpers from '@/GenericHelpers'; import * as ResponseHelper from '@/ResponseHelper'; import config from '@/config'; import { EECredentialsController } from './credentials.controller.ee'; @@ -16,6 +15,7 @@ import { listQueryMiddleware } from '@/middlewares'; import { Logger } from '@/Logger'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; +import { NamingService } from '@/services/naming.service'; export const credentialsController = express.Router(); credentialsController.use('/', EECredentialsController); @@ -38,14 +38,11 @@ credentialsController.get( */ credentialsController.get( '/new', - ResponseHelper.send(async (req: CredentialRequest.NewName): Promise<{ name: string }> => { - const { name: newName } = req.query; + ResponseHelper.send(async (req: CredentialRequest.NewName) => { + const requestedName = req.query.name ?? config.getEnv('credentials.defaultName'); return { - name: await GenericHelpers.generateUniqueName( - newName ?? config.getEnv('credentials.defaultName'), - 'credentials', - ), + name: await Container.get(NamingService).getUniqueCredentialName(requestedName), }; }), ); diff --git a/packages/cli/src/services/naming.service.ts b/packages/cli/src/services/naming.service.ts new file mode 100644 index 0000000000..85539bdc32 --- /dev/null +++ b/packages/cli/src/services/naming.service.ts @@ -0,0 +1,47 @@ +import { Service } from 'typedi'; +import { Like } from 'typeorm'; +import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; + +@Service() +export class NamingService { + constructor( + private readonly workflowRepository: WorkflowRepository, + private readonly credentialsRepository: CredentialsRepository, + ) {} + + async getUniqueWorkflowName(requestedName: string) { + return this.getUniqueName(requestedName, 'workflow'); + } + + async getUniqueCredentialName(requestedName: string) { + return this.getUniqueName(requestedName, 'credential'); + } + + private async getUniqueName(requestedName: string, entity: 'workflow' | 'credential') { + const repository = entity === 'workflow' ? this.workflowRepository : this.credentialsRepository; + + const found: Array<{ name: string }> = await repository.find({ + select: ['name'], + where: { name: Like(`${requestedName}%`) }, + }); + + if (found.length === 0) return requestedName; + + if (found.length === 1) return [requestedName, 2].join(' '); + + const maxSuffix = found.reduce((max, { name }) => { + const [_, strSuffix] = name.split(`${requestedName} `); + + const numSuffix = parseInt(strSuffix); + + if (isNaN(numSuffix)) return max; + + if (numSuffix > max) max = numSuffix; + + return max; + }, 2); + + return [requestedName, maxSuffix + 1].join(' '); + } +} diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 8b36e44f0a..f41e1f2900 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -30,6 +30,7 @@ import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.reposi import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; +import { NamingService } from '@/services/naming.service'; export const workflowsController = express.Router(); workflowsController.use('/', EEWorkflowController); @@ -139,12 +140,9 @@ workflowsController.get( workflowsController.get( '/new', ResponseHelper.send(async (req: WorkflowRequest.NewName) => { - const requestedName = - req.query.name && req.query.name !== '' - ? req.query.name - : config.getEnv('workflows.defaultName'); + const requestedName = req.query.name ?? config.getEnv('workflows.defaultName'); - const name = await GenericHelpers.generateUniqueName(requestedName, 'workflow'); + const name = await Container.get(NamingService).getUniqueWorkflowName(requestedName); const onboardingFlowEnabled = !config.getEnv('workflows.onboardingFlowDisabled') && diff --git a/packages/cli/test/unit/services/naming.service.test.ts b/packages/cli/test/unit/services/naming.service.test.ts new file mode 100644 index 0000000000..df2ff1e9b3 --- /dev/null +++ b/packages/cli/test/unit/services/naming.service.test.ts @@ -0,0 +1,85 @@ +import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; +import { mockInstance } from '../../shared/mocking'; +import { NamingService } from '@/services/naming.service'; +import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; +import type { CredentialsEntity } from '@/databases/entities/CredentialsEntity'; + +describe('NamingService', () => { + const workflowRepository = mockInstance(WorkflowRepository); + const credentialsRepository = mockInstance(CredentialsRepository); + + const namingService = new NamingService(workflowRepository, credentialsRepository); + + beforeEach(() => { + jest.restoreAllMocks(); + }); + + describe('getUniqueWorkflowName()', () => { + test('should return requested name if already unique', async () => { + workflowRepository.find.mockResolvedValue([]); + + const name = await namingService.getUniqueWorkflowName('foo'); + + expect(name).toEqual('foo'); + }); + + test('should return requested name suffixed if already existing once', async () => { + workflowRepository.find.mockResolvedValue([{ name: 'foo' }] as WorkflowEntity[]); + + const name = await namingService.getUniqueWorkflowName('foo'); + + expect(name).toEqual('foo 2'); + }); + + test('should return requested name with incremented suffix if already suffixed', async () => { + const existingNames = [{ name: 'foo' }, { name: 'foo 2' }] as WorkflowEntity[]; + + workflowRepository.find.mockResolvedValue(existingNames); + + const name = await namingService.getUniqueWorkflowName('foo'); + + expect(name).toEqual('foo 3'); + + existingNames.push({ name: 'foo 3' } as WorkflowEntity); + + const _name = await namingService.getUniqueWorkflowName('foo'); + + expect(_name).toEqual('foo 4'); + }); + }); + + describe('getUniqueCredentialName()', () => { + test('should return requested name if already unique', async () => { + credentialsRepository.find.mockResolvedValue([]); + + const name = await namingService.getUniqueCredentialName('foo'); + + expect(name).toEqual('foo'); + }); + + test('should return requested name suffixed if already existing once', async () => { + credentialsRepository.find.mockResolvedValue([{ name: 'foo' }] as CredentialsEntity[]); + + const name = await namingService.getUniqueCredentialName('foo'); + + expect(name).toEqual('foo 2'); + }); + + test('should return requested name with incremented suffix if already suffixed', async () => { + const existingNames = [{ name: 'foo' }, { name: 'foo 2' }] as CredentialsEntity[]; + + credentialsRepository.find.mockResolvedValue(existingNames); + + const name = await namingService.getUniqueCredentialName('foo'); + + expect(name).toEqual('foo 3'); + + existingNames.push({ name: 'foo 3' } as CredentialsEntity); + + const _name = await namingService.getUniqueCredentialName('foo'); + + expect(_name).toEqual('foo 4'); + }); + }); +});