Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Use DTOs for source control push/pull requests #12470

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/@n8n/api-types/src/dto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export { UserUpdateRequestDto } from './user/user-update-request.dto';

export { CommunityRegisteredRequestDto } from './license/community-registered-request.dto';

export { PullWorkFolderRequestDto } from './source-control/pull-work-folder-request.dto';
export { PushWorkFolderRequestDto } from './source-control/push-work-folder-request.dto';

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';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { PullWorkFolderRequestDto } from '../pull-work-folder-request.dto';

describe('PullWorkFolderRequestDto', () => {
describe('Valid requests', () => {
test.each([
{
name: 'with force',
request: { force: true },
},
{
name: 'without force',
request: {},
},
])('should validate $name', ({ request }) => {
const result = PullWorkFolderRequestDto.safeParse(request);
expect(result.success).toBe(true);
});
});

describe('Invalid requests', () => {
test.each([
{
name: 'invalid force type',
request: {
force: 'true', // Should be boolean
},
expectedErrorPath: ['force'],
},
])('should fail validation for $name', ({ request, expectedErrorPath }) => {
const result = PullWorkFolderRequestDto.safeParse(request);
expect(result.success).toBe(false);

if (expectedErrorPath) {
expect(result.error?.issues[0].path).toEqual(expectedErrorPath);
}
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { PushWorkFolderRequestDto } from '../push-work-folder-request.dto';

describe('PushWorkFolderRequestDto', () => {
describe('Valid requests', () => {
test.each([
{
name: 'complete valid push request with all fields',
request: {
force: true,
fileNames: [
{
file: 'file1.json',
id: '1',
name: 'File 1',
type: 'workflow',
status: 'modified',
location: 'local',
conflict: false,
updatedAt: '2023-10-01T12:00:00Z',
pushed: true,
},
],
message: 'Initial commit',
},
},
{
name: 'push request with only required fields',
request: {
fileNames: [
{
file: 'file2.json',
id: '2',
name: 'File 2',
type: 'credential',
status: 'new',
location: 'remote',
conflict: true,
updatedAt: '2023-10-02T12:00:00Z',
},
],
},
},
{
name: 'push request with optional fields omitted',
request: {
fileNames: [
{
file: 'file3.json',
id: '3',
name: 'File 3',
type: 'variables',
status: 'deleted',
location: 'local',
conflict: false,
updatedAt: '2023-10-03T12:00:00Z',
},
],
},
},
netroy marked this conversation as resolved.
Show resolved Hide resolved
])('should validate $name', ({ request }) => {
const result = PushWorkFolderRequestDto.safeParse(request);
expect(result.success).toBe(true);
});
});

describe('Invalid requests', () => {
test.each([
{
name: 'missing required fileNames field',
request: {
force: true,
message: 'Initial commit',
},
expectedErrorPath: ['fileNames'],
},
{
name: 'invalid fileNames type',
request: {
fileNames: 'not-an-array', // Should be an array
},
expectedErrorPath: ['fileNames'],
},
{
name: 'invalid fileNames array element',
request: {
fileNames: [
{
file: 'file4.json',
id: '4',
name: 'File 4',
type: 'invalid-type', // Invalid type
status: 'modified',
location: 'local',
conflict: false,
updatedAt: '2023-10-04T12:00:00Z',
},
],
},
expectedErrorPath: ['fileNames', 0, 'type'],
},
{
name: 'invalid force type',
request: {
force: 'true', // Should be boolean
fileNames: [
{
file: 'file5.json',
id: '5',
name: 'File 5',
type: 'workflow',
status: 'modified',
location: 'local',
conflict: false,
updatedAt: '2023-10-05T12:00:00Z',
},
],
},
expectedErrorPath: ['force'],
},
])('should fail validation for $name', ({ request, expectedErrorPath }) => {
const result = PushWorkFolderRequestDto.safeParse(request);
expect(result.success).toBe(false);

if (expectedErrorPath) {
expect(result.error?.issues[0].path).toEqual(expectedErrorPath);
}
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { z } from 'zod';
import { Z } from 'zod-class';

export class PullWorkFolderRequestDto extends Z.class({
force: z.boolean().optional(),
}) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { z } from 'zod';
import { Z } from 'zod-class';

import { SourceControlledFileSchema } from '../../schemas/source-controlled-file.schema';

export class PushWorkFolderRequestDto extends Z.class({
force: z.boolean().optional(),
commitMessage: z.string().optional(),
fileNames: z.array(SourceControlledFileSchema),
}) {}
6 changes: 6 additions & 0 deletions packages/@n8n/api-types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,9 @@ export type { SendWorkerStatusMessage } from './push/worker';

export type { BannerName } from './schemas/bannerName.schema';
export { passwordSchema } from './schemas/password.schema';
export {
type SourceControlledFile,
SOURCE_CONTROL_FILE_LOCATION,
SOURCE_CONTROL_FILE_STATUS,
SOURCE_CONTROL_FILE_TYPE,
} from './schemas/source-controlled-file.schema';
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { z } from 'zod';

const FileTypeSchema = z.enum(['credential', 'workflow', 'tags', 'variables', 'file']);
export const SOURCE_CONTROL_FILE_TYPE = FileTypeSchema.Values;

const FileStatusSchema = z.enum([
'new',
'modified',
'deleted',
'created',
'renamed',
'conflicted',
'ignored',
'staged',
'unknown',
]);
export const SOURCE_CONTROL_FILE_STATUS = FileStatusSchema.Values;

const FileLocationSchema = z.enum(['local', 'remote']);
export const SOURCE_CONTROL_FILE_LOCATION = FileLocationSchema.Values;

export const SourceControlledFileSchema = z.object({
file: z.string(),
id: z.string(),
name: z.string(),
type: FileTypeSchema,
status: FileStatusSchema,
location: FileLocationSchema,
conflict: z.boolean(),
updatedAt: z.string(),
pushed: z.boolean().optional(),
});

export type SourceControlledFile = z.infer<typeof SourceControlledFileSchema>;
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container } from '@n8n/di';
import mock from 'jest-mock-extended/lib/Mock';
import { Cipher, type InstanceSettings } from 'n8n-core';
Expand All @@ -9,7 +10,6 @@ import { SharedCredentialsRepository } from '@/databases/repositories/shared-cre
import { mockInstance } from '@test/mocking';

import { SourceControlExportService } from '../source-control-export.service.ee';
import type { SourceControlledFile } from '../types/source-controlled-file';

// https://github.com/jestjs/jest/issues/4715
function deepSpyOn<O extends object, M extends keyof O>(object: O, methodName: M) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container } from '@n8n/di';
import { constants as fsConstants, accessSync } from 'fs';
import { InstanceSettings } from 'n8n-core';
Expand All @@ -17,7 +18,6 @@ import {
} from '@/environments.ee/source-control/source-control-helper.ee';
import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee';
import type { SourceControlPreferences } from '@/environments.ee/source-control/types/source-control-preferences';
import type { SourceControlledFile } from '@/environments.ee/source-control/types/source-controlled-file';
import { License } from '@/license';
import { mockInstance } from '@test/mocking';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container, Service } from '@n8n/di';
import { rmSync } from 'fs';
import { Credentials, InstanceSettings, Logger } from 'n8n-core';
Expand Down Expand Up @@ -29,7 +30,6 @@ import type { ExportResult } from './types/export-result';
import type { ExportableCredential } from './types/exportable-credential';
import type { ExportableWorkflow } from './types/exportable-workflow';
import type { ResourceOwner } from './types/resource-owner';
import type { SourceControlledFile } from './types/source-controlled-file';
import { VariablesService } from '../variables/variables.service.ee';

@Service()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container } from '@n8n/di';
import { generateKeyPairSync } from 'crypto';
import { constants as fsConstants, mkdirSync, accessSync } from 'fs';
Expand All @@ -16,7 +17,6 @@ import {
} from './constants';
import type { KeyPair } from './types/key-pair';
import type { KeyPairType } from './types/key-pair-type';
import type { SourceControlledFile } from './types/source-controlled-file';

export function stringContainsExpression(testString: string): boolean {
return /^=.*\{\{.*\}\}/.test(testString);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container, Service } from '@n8n/di';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In } from '@n8n/typeorm';
Expand Down Expand Up @@ -37,7 +38,6 @@ import { getCredentialExportPath, getWorkflowExportPath } from './source-control
import type { ExportableCredential } from './types/exportable-credential';
import type { ResourceOwner } from './types/resource-owner';
import type { SourceControlWorkflowVersionId } from './types/source-control-workflow-version-id';
import type { SourceControlledFile } from './types/source-controlled-file';
import { VariablesService } from '../variables/variables.service.ee';

@Service()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { PullWorkFolderRequestDto, PushWorkFolderRequestDto } from '@n8n/api-types';
import type { SourceControlledFile } from '@n8n/api-types';
import express from 'express';
import type { PullResult } from 'simple-git';

import { Get, Post, Patch, RestController, GlobalScope } from '@/decorators';
import { Get, Post, Patch, RestController, GlobalScope, Body } from '@/decorators';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { EventService } from '@/events/event.service';
import { AuthenticatedRequest } from '@/requests';

import { SOURCE_CONTROL_DEFAULT_BRANCH } from './constants';
import {
Expand All @@ -17,7 +20,6 @@ import type { ImportResult } from './types/import-result';
import { SourceControlRequest } from './types/requests';
import { SourceControlGetStatus } from './types/source-control-get-status';
import type { SourceControlPreferences } from './types/source-control-preferences';
import type { SourceControlledFile } from './types/source-controlled-file';

@RestController('/source-control')
export class SourceControlController {
Expand Down Expand Up @@ -164,19 +166,16 @@ export class SourceControlController {
@Post('/push-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] })
@GlobalScope('sourceControl:push')
async pushWorkfolder(
req: SourceControlRequest.PushWorkFolder,
req: AuthenticatedRequest,
res: express.Response,
@Body payload: PushWorkFolderRequestDto,
): Promise<SourceControlledFile[]> {
if (this.sourceControlPreferencesService.isBranchReadOnly()) {
throw new BadRequestError('Cannot push onto read-only branch.');
}
despairblue marked this conversation as resolved.
Show resolved Hide resolved

try {
await this.sourceControlService.setGitUserDetails(
`${req.user.firstName} ${req.user.lastName}`,
req.user.email,
);
const result = await this.sourceControlService.pushWorkfolder(req.body);
const result = await this.sourceControlService.pushWorkfolder(payload);
res.statusCode = result.statusCode;
return result.statusResult;
} catch (error) {
Expand All @@ -187,15 +186,12 @@ export class SourceControlController {
@Post('/pull-workfolder', { middlewares: [sourceControlLicensedAndEnabledMiddleware] })
@GlobalScope('sourceControl:pull')
async pullWorkfolder(
req: SourceControlRequest.PullWorkFolder,
req: AuthenticatedRequest,
res: express.Response,
@Body payload: PullWorkFolderRequestDto,
): Promise<SourceControlledFile[] | ImportResult | PullResult | undefined> {
try {
const result = await this.sourceControlService.pullWorkfolder({
force: req.body.force,
variables: req.body.variables,
userId: req.user.id,
});
const result = await this.sourceControlService.pullWorkfolder(req.user.id, payload);
res.statusCode = result.statusCode;
return result.statusResult;
} catch (error) {
Expand Down
Loading
Loading