From 1d3cb9f5ac4f03cb867bb0e5c0f113fe7f7ff972 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: Fri, 3 Jan 2025 12:19:09 +0100 Subject: [PATCH] refactor(core): Extract Worfklow import request payload into a DTO (#12441) --- packages/@n8n/api-types/src/dto/index.ts | 2 + .../import-workflow-from-url.dto.test.ts | 63 ++++++++++++++++ .../workflows/import-workflow-from-url.dto.ts | 6 ++ .../__tests__/workflows.controller.test.ts | 72 +++++++++++++++++++ .../cli/src/workflows/workflow.request.ts | 2 - .../cli/src/workflows/workflows.controller.ts | 20 +++--- 6 files changed, 152 insertions(+), 13 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts create mode 100644 packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts create mode 100644 packages/cli/src/workflows/__tests__/workflows.controller.test.ts diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 6dbfe17361..f16758a89c 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -35,3 +35,5 @@ export { CommunityRegisteredRequestDto } from './license/community-registered-re export { VariableListRequestDto } from './variables/variables-list-request.dto'; export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one-request.dto'; export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto'; + +export { ImportWorkflowFromUrlDto } from './workflows/import-workflow-from-url.dto'; diff --git a/packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts b/packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts new file mode 100644 index 0000000000..3c17e873e0 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/workflows/__tests__/import-workflow-from-url.dto.test.ts @@ -0,0 +1,63 @@ +import { ImportWorkflowFromUrlDto } from '../import-workflow-from-url.dto'; + +describe('ImportWorkflowFromUrlDto', () => { + describe('Valid requests', () => { + test('should validate $name', () => { + const result = ImportWorkflowFromUrlDto.safeParse({ + url: 'https://example.com/workflow.json', + }); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'invalid URL (not ending with .json)', + url: 'https://example.com/workflow', + expectedErrorPath: ['url'], + }, + { + name: 'invalid URL (missing protocol)', + url: 'example.com/workflow.json', + expectedErrorPath: ['url'], + }, + { + name: 'invalid URL (not a URL)', + url: 'not-a-url', + expectedErrorPath: ['url'], + }, + { + name: 'missing URL', + url: undefined, + expectedErrorPath: ['url'], + }, + { + name: 'null URL', + url: null, + expectedErrorPath: ['url'], + }, + { + name: 'invalid URL (ends with .json but not a valid URL)', + url: 'not-a-url.json', + expectedErrorPath: ['url'], + }, + { + name: 'valid URL with query parameters', + url: 'https://example.com/workflow.json?param=value', + }, + { + name: 'valid URL with fragments', + url: 'https://example.com/workflow.json#section', + }, + ])('should fail validation for $name', ({ url, expectedErrorPath }) => { + const result = ImportWorkflowFromUrlDto.safeParse({ url }); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts b/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts new file mode 100644 index 0000000000..310e620fde --- /dev/null +++ b/packages/@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +export class ImportWorkflowFromUrlDto extends Z.class({ + url: z.string().url().endsWith('.json'), +}) {} diff --git a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts new file mode 100644 index 0000000000..4168d487bf --- /dev/null +++ b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts @@ -0,0 +1,72 @@ +import type { ImportWorkflowFromUrlDto } from '@n8n/api-types/src/dto/workflows/import-workflow-from-url.dto'; +import axios from 'axios'; +import type { Response } from 'express'; +import { mock } from 'jest-mock-extended'; + +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import type { AuthenticatedRequest } from '@/requests'; + +import { WorkflowsController } from '../workflows.controller'; + +jest.mock('axios'); + +describe('WorkflowsController', () => { + const controller = Object.create(WorkflowsController.prototype); + const axiosMock = axios.get as jest.Mock; + const req = mock(); + const res = mock(); + + describe('getFromUrl', () => { + describe('should return workflow data', () => { + it('when the URL points to a valid JSON file', async () => { + const mockWorkflowData = { + nodes: [], + connections: {}, + }; + + axiosMock.mockResolvedValue({ data: mockWorkflowData }); + + const query: ImportWorkflowFromUrlDto = { url: 'https://example.com/workflow.json' }; + const result = await controller.getFromUrl(req, res, query); + + expect(result).toEqual(mockWorkflowData); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + }); + + describe('should throw a BadRequestError', () => { + const query: ImportWorkflowFromUrlDto = { url: 'https://example.com/invalid.json' }; + + it('when the URL does not point to a valid JSON file', async () => { + axiosMock.mockRejectedValue(new Error('Network Error')); + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + + it('when the data is not a valid n8n workflow JSON', async () => { + const invalidWorkflowData = { + nodes: 'not an array', + connections: 'not an object', + }; + + axiosMock.mockResolvedValue({ data: invalidWorkflowData }); + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + + it('when the data is missing required fields', async () => { + const incompleteWorkflowData = { + nodes: [], + // Missing connections field + }; + + axiosMock.mockResolvedValue({ data: incompleteWorkflowData }); + + await expect(controller.getFromUrl(req, res, query)).rejects.toThrow(BadRequestError); + expect(axiosMock).toHaveBeenCalledWith(query.url); + }); + }); + }); +}); diff --git a/packages/cli/src/workflows/workflow.request.ts b/packages/cli/src/workflows/workflow.request.ts index 4098384abb..47fd2cdb93 100644 --- a/packages/cli/src/workflows/workflow.request.ts +++ b/packages/cli/src/workflows/workflow.request.ts @@ -69,6 +69,4 @@ export declare namespace WorkflowRequest { {}, { destinationProjectId: string } >; - - type FromUrl = AuthenticatedRequest<{}, {}, {}, { url?: string }>; } diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index b12dfdce5a..865b38450e 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -1,3 +1,4 @@ +import { ImportWorkflowFromUrlDto } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In, type FindOptionsRelations } from '@n8n/typeorm'; @@ -18,7 +19,7 @@ import { SharedWorkflowRepository } from '@/databases/repositories/shared-workfl import { TagRepository } from '@/databases/repositories/tag.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import * as Db from '@/db'; -import { Delete, Get, Patch, Post, ProjectScope, Put, RestController } from '@/decorators'; +import { Delete, Get, Patch, Post, ProjectScope, Put, Query, RestController } from '@/decorators'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -29,6 +30,7 @@ import { validateEntity } from '@/generic-helpers'; import type { IWorkflowResponse } from '@/interfaces'; import { License } from '@/license'; import { listQueryMiddleware } from '@/middlewares'; +import { AuthenticatedRequest } from '@/requests'; import * as ResponseHelper from '@/response-helper'; import { NamingService } from '@/services/naming.service'; import { ProjectService } from '@/services/project.service.ee'; @@ -215,18 +217,14 @@ export class WorkflowsController { } @Get('/from-url') - async getFromUrl(req: WorkflowRequest.FromUrl) { - if (req.query.url === undefined) { - throw new BadRequestError('The parameter "url" is missing!'); - } - if (!/^http[s]?:\/\/.*\.json$/i.exec(req.query.url)) { - throw new BadRequestError( - 'The parameter "url" is not valid! It does not seem to be a URL pointing to a n8n workflow JSON file.', - ); - } + async getFromUrl( + _req: AuthenticatedRequest, + _res: express.Response, + @Query query: ImportWorkflowFromUrlDto, + ) { let workflowData: IWorkflowResponse | undefined; try { - const { data } = await axios.get(req.query.url); + const { data } = await axios.get(query.url); workflowData = data; } catch (error) { throw new BadRequestError('The URL does not point to valid JSON file!');