refactor(core): Use DI in source-control. add more tests (#12554)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2025-01-10 16:10:19 +01:00 committed by GitHub
parent b2cbed9865
commit 25a79ccf40
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 590 additions and 217 deletions

View file

@ -349,15 +349,6 @@ export const schema = {
},
},
sourceControl: {
defaultKeyPairType: {
doc: 'Default SSH key type to use when generating SSH keys',
format: ['rsa', 'ed25519'] as const,
default: 'ed25519',
env: 'N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE',
},
},
workflowHistory: {
enabled: {
doc: 'Whether to save workflow history versions',

View file

@ -1,86 +1,261 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container } from '@n8n/di';
import mock from 'jest-mock-extended/lib/Mock';
import { mock, captor } from 'jest-mock-extended';
import { Cipher, type InstanceSettings } from 'n8n-core';
import { ApplicationError, deepCopy } from 'n8n-workflow';
import fsp from 'node:fs/promises';
import type { CredentialsEntity } from '@/databases/entities/credentials-entity';
import type { SharedCredentials } from '@/databases/entities/shared-credentials';
import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository';
import { mockInstance } from '@test/mocking';
import type { SharedWorkflow } from '@/databases/entities/shared-workflow';
import type { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository';
import type { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository';
import type { TagRepository } from '@/databases/repositories/tag.repository';
import type { WorkflowTagMappingRepository } from '@/databases/repositories/workflow-tag-mapping.repository';
import type { WorkflowRepository } from '@/databases/repositories/workflow.repository';
import type { VariablesService } from '../../variables/variables.service.ee';
import { SourceControlExportService } from '../source-control-export.service.ee';
// https://github.com/jestjs/jest/issues/4715
function deepSpyOn<O extends object, M extends keyof O>(object: O, methodName: M) {
const spy = jest.fn();
const originalMethod = object[methodName];
if (typeof originalMethod !== 'function') {
throw new ApplicationError(`${methodName.toString()} is not a function`, { level: 'warning' });
}
object[methodName] = function (...args: unknown[]) {
const clonedArgs = deepCopy(args);
spy(...clonedArgs);
return originalMethod.apply(this, args);
} as O[M];
return spy;
}
describe('SourceControlExportService', () => {
const cipher = Container.get(Cipher);
const sharedCredentialsRepository = mock<SharedCredentialsRepository>();
const sharedWorkflowRepository = mock<SharedWorkflowRepository>();
const workflowRepository = mock<WorkflowRepository>();
const tagRepository = mock<TagRepository>();
const workflowTagMappingRepository = mock<WorkflowTagMappingRepository>();
const variablesService = mock<VariablesService>();
const service = new SourceControlExportService(
mock(),
mock(),
mock(),
mock<InstanceSettings>({ n8nFolder: '' }),
variablesService,
tagRepository,
sharedCredentialsRepository,
sharedWorkflowRepository,
workflowRepository,
workflowTagMappingRepository,
mock<InstanceSettings>({ n8nFolder: '/mock/n8n' }),
);
describe('exportCredentialsToWorkFolder', () => {
it('should export credentials to work folder', async () => {
/**
* Arrange
*/
// @ts-expect-error Private method
const replaceSpy = deepSpyOn(service, 'replaceCredentialData');
const fsWriteFile = jest.spyOn(fsp, 'writeFile');
mockInstance(SharedCredentialsRepository).findByCredentialIds.mockResolvedValue([
beforeEach(() => jest.clearAllMocks());
describe('exportCredentialsToWorkFolder', () => {
const credentialData = {
authUrl: 'test',
accessTokenUrl: 'test',
clientId: 'test',
clientSecret: 'test',
oauthTokenData: {
access_token: 'test',
token_type: 'test',
expires_in: 123,
refresh_token: 'test',
},
};
const mockCredentials = mock({
id: 'cred1',
name: 'Test Credential',
type: 'oauth2',
data: cipher.encrypt(credentialData),
});
it('should export credentials to work folder', async () => {
sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([
mock<SharedCredentials>({
credentials: mock<CredentialsEntity>({
data: Container.get(Cipher).encrypt(
JSON.stringify({
authUrl: 'test',
accessTokenUrl: 'test',
clientId: 'test',
clientSecret: 'test',
oauthTokenData: {
access_token: 'test',
token_type: 'test',
expires_in: 123,
refresh_token: 'test',
},
}),
),
credentials: mockCredentials,
project: mock({
type: 'personal',
projectRelations: [
{
role: 'project:personalOwner',
user: mock({ email: 'user@example.com' }),
},
],
}),
}),
]);
/**
* Act
*/
await service.exportCredentialsToWorkFolder([mock<SourceControlledFile>()]);
// Act
const result = await service.exportCredentialsToWorkFolder([mock()]);
/**
* Assert
*/
expect(replaceSpy).toHaveBeenCalledWith({
authUrl: 'test',
accessTokenUrl: 'test',
clientId: 'test',
clientSecret: 'test',
// Assert
expect(result.count).toBe(1);
expect(result.files).toHaveLength(1);
const dataCaptor = captor<string>();
expect(fsWriteFile).toHaveBeenCalledWith(
'/mock/n8n/git/credential_stubs/cred1.json',
dataCaptor,
);
expect(JSON.parse(dataCaptor.value)).toEqual({
id: 'cred1',
name: 'Test Credential',
type: 'oauth2',
data: {
authUrl: '',
accessTokenUrl: '',
clientId: '',
clientSecret: '',
},
ownedBy: {
type: 'personal',
personalEmail: 'user@example.com',
},
});
});
it('should handle team project credentials', async () => {
sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([
mock<SharedCredentials>({
credentials: mockCredentials,
project: mock({
type: 'team',
id: 'team1',
name: 'Test Team',
}),
}),
]);
// Act
const result = await service.exportCredentialsToWorkFolder([
mock<SourceControlledFile>({ id: 'cred1' }),
]);
// Assert
expect(result.count).toBe(1);
const dataCaptor = captor<string>();
expect(fsWriteFile).toHaveBeenCalledWith(
'/mock/n8n/git/credential_stubs/cred1.json',
dataCaptor,
);
expect(JSON.parse(dataCaptor.value)).toEqual({
id: 'cred1',
name: 'Test Credential',
type: 'oauth2',
data: {
authUrl: '',
accessTokenUrl: '',
clientId: '',
clientSecret: '',
},
ownedBy: {
type: 'team',
teamId: 'team1',
teamName: 'Test Team',
},
});
});
it('should handle missing credentials', async () => {
// Arrange
sharedCredentialsRepository.findByCredentialIds.mockResolvedValue([]);
// Act
const result = await service.exportCredentialsToWorkFolder([
mock<SourceControlledFile>({ id: 'cred1' }),
]);
// Assert
expect(result.missingIds).toHaveLength(1);
expect(result.missingIds?.[0]).toBe('cred1');
});
});
describe('exportTagsToWorkFolder', () => {
it('should export tags to work folder', async () => {
// Arrange
tagRepository.find.mockResolvedValue([mock()]);
workflowTagMappingRepository.find.mockResolvedValue([mock()]);
// Act
const result = await service.exportTagsToWorkFolder();
// Assert
expect(result.count).toBe(1);
expect(result.files).toHaveLength(1);
});
it('should not export empty tags', async () => {
// Arrange
tagRepository.find.mockResolvedValue([]);
// Act
const result = await service.exportTagsToWorkFolder();
// Assert
expect(result.count).toBe(0);
expect(result.files).toHaveLength(0);
});
});
describe('exportVariablesToWorkFolder', () => {
it('should export variables to work folder', async () => {
// Arrange
variablesService.getAllCached.mockResolvedValue([mock()]);
// Act
const result = await service.exportVariablesToWorkFolder();
// Assert
expect(result.count).toBe(1);
expect(result.files).toHaveLength(1);
});
it('should not export empty variables', async () => {
// Arrange
variablesService.getAllCached.mockResolvedValue([]);
// Act
const result = await service.exportVariablesToWorkFolder();
// Assert
expect(result.count).toBe(0);
expect(result.files).toHaveLength(0);
});
});
describe('exportWorkflowsToWorkFolder', () => {
it('should export workflows to work folder', async () => {
// Arrange
workflowRepository.findByIds.mockResolvedValue([mock()]);
sharedWorkflowRepository.findByWorkflowIds.mockResolvedValue([
mock<SharedWorkflow>({
project: mock({
type: 'personal',
projectRelations: [{ role: 'project:personalOwner', user: mock() }],
}),
workflow: mock(),
}),
]);
// Act
const result = await service.exportWorkflowsToWorkFolder([mock()]);
// Assert
expect(result.count).toBe(1);
expect(result.files).toHaveLength(1);
});
it('should throw an error if workflow has no owner', async () => {
// Arrange
sharedWorkflowRepository.findByWorkflowIds.mockResolvedValue([
mock<SharedWorkflow>({
project: mock({
type: 'personal',
projectRelations: [],
}),
workflow: mock({
display: () => 'TestWorkflow',
}),
}),
]);
// Act & Assert
await expect(service.exportWorkflowsToWorkFolder([mock()])).rejects.toThrow(
'Workflow TestWorkflow has no owner',
);
});
});
});

View file

@ -1,6 +1,7 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container } from '@n8n/di';
import { constants as fsConstants, accessSync } from 'fs';
import { mock } from 'jest-mock-extended';
import { InstanceSettings } from 'n8n-core';
import path from 'path';
@ -16,10 +17,8 @@ import {
getTrackingInformationFromPullResult,
sourceControlFoldersExistCheck,
} 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 { License } from '@/license';
import { mockInstance } from '@test/mocking';
import type { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee';
import type { License } from '@/license';
const pushResult: SourceControlledFile[] = [
{
@ -151,12 +150,13 @@ const pullResult: SourceControlledFile[] = [
},
];
const license = mockInstance(License);
const license = mock<License>();
const sourceControlPreferencesService = mock<SourceControlPreferencesService>();
beforeAll(async () => {
jest.resetAllMocks();
license.isSourceControlLicensed.mockReturnValue(true);
Container.get(SourceControlPreferencesService).getPreferences = () => ({
sourceControlPreferencesService.getPreferences.mockReturnValue({
branchName: 'main',
connected: true,
repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git',
@ -245,17 +245,4 @@ describe('Source Control', () => {
workflowUpdates: 3,
});
});
it('should class validate correct preferences', async () => {
const validPreferences: Partial<SourceControlPreferences> = {
branchName: 'main',
repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git',
branchReadOnly: false,
branchColor: '#5296D6',
};
const validationResult = await Container.get(
SourceControlPreferencesService,
).validateSourceControlPreferences(validPreferences);
expect(validationResult).toBeTruthy();
});
});

View file

@ -0,0 +1,180 @@
import * as fastGlob from 'fast-glob';
import { mock } from 'jest-mock-extended';
import { type InstanceSettings } from 'n8n-core';
import fsp from 'node:fs/promises';
import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
import type { WorkflowRepository } from '@/databases/repositories/workflow.repository';
import { SourceControlImportService } from '../source-control-import.service.ee';
jest.mock('fast-glob');
describe('SourceControlImportService', () => {
const workflowRepository = mock<WorkflowRepository>();
const service = new SourceControlImportService(
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
workflowRepository,
mock(),
mock<InstanceSettings>({ n8nFolder: '/mock/n8n' }),
);
const globMock = fastGlob.default as unknown as jest.Mock<Promise<string[]>, string[]>;
const fsReadFile = jest.spyOn(fsp, 'readFile');
beforeEach(() => jest.clearAllMocks());
describe('getRemoteVersionIdsFromFiles', () => {
const mockWorkflowFile = '/mock/workflow1.json';
it('should parse workflow files correctly', async () => {
globMock.mockResolvedValue([mockWorkflowFile]);
const mockWorkflowData = {
id: 'workflow1',
versionId: 'v1',
name: 'Test Workflow',
};
fsReadFile.mockResolvedValue(JSON.stringify(mockWorkflowData));
const result = await service.getRemoteVersionIdsFromFiles();
expect(fsReadFile).toHaveBeenCalledWith(mockWorkflowFile, { encoding: 'utf8' });
expect(result).toHaveLength(1);
expect(result[0]).toEqual(
expect.objectContaining({
id: 'workflow1',
versionId: 'v1',
name: 'Test Workflow',
}),
);
});
it('should filter out files without valid workflow data', async () => {
globMock.mockResolvedValue(['/mock/invalid.json']);
fsReadFile.mockResolvedValue('{}');
const result = await service.getRemoteVersionIdsFromFiles();
expect(result).toHaveLength(0);
});
});
describe('getRemoteCredentialsFromFiles', () => {
it('should parse credential files correctly', async () => {
globMock.mockResolvedValue(['/mock/credential1.json']);
const mockCredentialData = {
id: 'cred1',
name: 'Test Credential',
type: 'oauth2',
};
fsReadFile.mockResolvedValue(JSON.stringify(mockCredentialData));
const result = await service.getRemoteCredentialsFromFiles();
expect(result).toHaveLength(1);
expect(result[0]).toEqual(
expect.objectContaining({
id: 'cred1',
name: 'Test Credential',
type: 'oauth2',
}),
);
});
it('should filter out files without valid credential data', async () => {
globMock.mockResolvedValue(['/mock/invalid.json']);
fsReadFile.mockResolvedValue('{}');
const result = await service.getRemoteCredentialsFromFiles();
expect(result).toHaveLength(0);
});
});
describe('getRemoteVariablesFromFile', () => {
it('should parse variables file correctly', async () => {
globMock.mockResolvedValue(['/mock/variables.json']);
const mockVariablesData = [
{ key: 'VAR1', value: 'value1' },
{ key: 'VAR2', value: 'value2' },
];
fsReadFile.mockResolvedValue(JSON.stringify(mockVariablesData));
const result = await service.getRemoteVariablesFromFile();
expect(result).toEqual(mockVariablesData);
});
it('should return empty array if no variables file found', async () => {
globMock.mockResolvedValue([]);
const result = await service.getRemoteVariablesFromFile();
expect(result).toHaveLength(0);
});
});
describe('getRemoteTagsAndMappingsFromFile', () => {
it('should parse tags and mappings file correctly', async () => {
globMock.mockResolvedValue(['/mock/tags.json']);
const mockTagsData = {
tags: [{ id: 'tag1', name: 'Tag 1' }],
mappings: [{ workflowId: 'workflow1', tagId: 'tag1' }],
};
fsReadFile.mockResolvedValue(JSON.stringify(mockTagsData));
const result = await service.getRemoteTagsAndMappingsFromFile();
expect(result.tags).toEqual(mockTagsData.tags);
expect(result.mappings).toEqual(mockTagsData.mappings);
});
it('should return empty tags and mappings if no file found', async () => {
globMock.mockResolvedValue([]);
const result = await service.getRemoteTagsAndMappingsFromFile();
expect(result.tags).toHaveLength(0);
expect(result.mappings).toHaveLength(0);
});
});
describe('getLocalVersionIdsFromDb', () => {
const now = new Date();
jest.useFakeTimers({ now });
it('should replace invalid updatedAt with current timestamp', async () => {
const mockWorkflows = [
{
id: 'workflow1',
name: 'Test Workflow',
updatedAt: 'invalid-date',
},
] as unknown as WorkflowEntity[];
workflowRepository.find.mockResolvedValue(mockWorkflows);
const result = await service.getLocalVersionIdsFromDb();
expect(result[0].updatedAt).toBe(now.toISOString());
});
});
});

View file

@ -0,0 +1,27 @@
import { mock } from 'jest-mock-extended';
import type { InstanceSettings } from 'n8n-core';
import { SourceControlPreferencesService } from '../source-control-preferences.service.ee';
import type { SourceControlPreferences } from '../types/source-control-preferences';
describe('SourceControlPreferencesService', () => {
const instanceSettings = mock<InstanceSettings>({ n8nFolder: '' });
const service = new SourceControlPreferencesService(
instanceSettings,
mock(),
mock(),
mock(),
mock(),
);
it('should class validate correct preferences', async () => {
const validPreferences: Partial<SourceControlPreferences> = {
branchName: 'main',
repositoryUrl: 'git@example.com:n8ntest/n8n_testrepo.git',
branchReadOnly: false,
branchColor: '#5296D6',
};
const validationResult = await service.validateSourceControlPreferences(validPreferences);
expect(validationResult).toBeTruthy();
});
});

View file

@ -10,6 +10,8 @@ describe('SourceControlService', () => {
Container.get(InstanceSettings),
mock(),
mock(),
mock(),
mock(),
);
const sourceControlService = new SourceControlService(
mock(),

View file

@ -1,5 +1,5 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container, Service } from '@n8n/di';
import { Service } from '@n8n/di';
import { rmSync } from 'fs';
import { Credentials, InstanceSettings, Logger } from 'n8n-core';
import { ApplicationError, type ICredentialDataDecryptedObject } from 'n8n-workflow';
@ -44,6 +44,10 @@ export class SourceControlExportService {
private readonly logger: Logger,
private readonly variablesService: VariablesService,
private readonly tagRepository: TagRepository,
private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly sharedWorkflowRepository: SharedWorkflowRepository,
private readonly workflowRepository: WorkflowRepository,
private readonly workflowTagMappingRepository: WorkflowTagMappingRepository,
instanceSettings: InstanceSettings,
) {
this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER);
@ -106,17 +110,16 @@ export class SourceControlExportService {
try {
sourceControlFoldersExistCheck([this.workflowExportFolder]);
const workflowIds = candidates.map((e) => e.id);
const sharedWorkflows =
await Container.get(SharedWorkflowRepository).findByWorkflowIds(workflowIds);
const workflows = await Container.get(WorkflowRepository).findByIds(workflowIds);
const sharedWorkflows = await this.sharedWorkflowRepository.findByWorkflowIds(workflowIds);
const workflows = await this.workflowRepository.findByIds(workflowIds);
// determine owner of each workflow to be exported
const owners: Record<string, ResourceOwner> = {};
sharedWorkflows.forEach((e) => {
const project = e.project;
sharedWorkflows.forEach((sharedWorkflow) => {
const project = sharedWorkflow.project;
if (!project) {
throw new ApplicationError(`Workflow ${e.workflow.display()} has no owner`);
throw new ApplicationError(`Workflow ${sharedWorkflow.workflow.display()} has no owner`);
}
if (project.type === 'personal') {
@ -124,14 +127,16 @@ export class SourceControlExportService {
(pr) => pr.role === 'project:personalOwner',
);
if (!ownerRelation) {
throw new ApplicationError(`Workflow ${e.workflow.display()} has no owner`);
throw new ApplicationError(
`Workflow ${sharedWorkflow.workflow.display()} has no owner`,
);
}
owners[e.workflowId] = {
owners[sharedWorkflow.workflowId] = {
type: 'personal',
personalEmail: ownerRelation.user.email,
};
} else if (project.type === 'team') {
owners[e.workflowId] = {
owners[sharedWorkflow.workflowId] = {
type: 'team',
teamId: project.id,
teamName: project.name,
@ -156,6 +161,7 @@ export class SourceControlExportService {
})),
};
} catch (error) {
if (error instanceof ApplicationError) throw error;
throw new ApplicationError('Failed to export workflows to work folder', { cause: error });
}
}
@ -204,7 +210,7 @@ export class SourceControlExportService {
files: [],
};
}
const mappings = await Container.get(WorkflowTagMappingRepository).find();
const mappings = await this.workflowTagMappingRepository.find();
const fileName = path.join(this.gitFolder, SOURCE_CONTROL_TAGS_EXPORT_FILE);
await fsWriteFile(
fileName,
@ -260,9 +266,10 @@ export class SourceControlExportService {
try {
sourceControlFoldersExistCheck([this.credentialExportFolder]);
const credentialIds = candidates.map((e) => e.id);
const credentialsToBeExported = await Container.get(
SharedCredentialsRepository,
).findByCredentialIds(credentialIds, 'credential:owner');
const credentialsToBeExported = await this.sharedCredentialsRepository.findByCredentialIds(
credentialIds,
'credential:owner',
);
let missingIds: string[] = [];
if (credentialsToBeExported.length !== credentialIds.length) {
const foundCredentialIds = credentialsToBeExported.map((e) => e.credentialsId);

View file

@ -1,5 +1,5 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container, Service } from '@n8n/di';
import { Service } from '@n8n/di';
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
import { In } from '@n8n/typeorm';
import glob from 'fast-glob';
@ -53,7 +53,15 @@ export class SourceControlImportService {
private readonly errorReporter: ErrorReporter,
private readonly variablesService: VariablesService,
private readonly activeWorkflowManager: ActiveWorkflowManager,
private readonly credentialsRepository: CredentialsRepository,
private readonly projectRepository: ProjectRepository,
private readonly tagRepository: TagRepository,
private readonly sharedWorkflowRepository: SharedWorkflowRepository,
private readonly sharedCredentialsRepository: SharedCredentialsRepository,
private readonly userRepository: UserRepository,
private readonly variablesRepository: VariablesRepository,
private readonly workflowRepository: WorkflowRepository,
private readonly workflowTagMappingRepository: WorkflowTagMappingRepository,
instanceSettings: InstanceSettings,
) {
this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER);
@ -91,7 +99,7 @@ export class SourceControlImportService {
}
async getLocalVersionIdsFromDb(): Promise<SourceControlWorkflowVersionId[]> {
const localWorkflows = await Container.get(WorkflowRepository).find({
const localWorkflows = await this.workflowRepository.find({
select: ['id', 'name', 'versionId', 'updatedAt'],
});
return localWorkflows.map((local) => {
@ -146,7 +154,7 @@ export class SourceControlImportService {
}
async getLocalCredentialsFromDb(): Promise<Array<ExportableCredential & { filename: string }>> {
const localCredentials = await Container.get(CredentialsRepository).find({
const localCredentials = await this.credentialsRepository.find({
select: ['id', 'name', 'type'],
});
return localCredentials.map((local) => ({
@ -201,24 +209,22 @@ export class SourceControlImportService {
const localTags = await this.tagRepository.find({
select: ['id', 'name'],
});
const localMappings = await Container.get(WorkflowTagMappingRepository).find({
const localMappings = await this.workflowTagMappingRepository.find({
select: ['workflowId', 'tagId'],
});
return { tags: localTags, mappings: localMappings };
}
async importWorkflowFromWorkFolder(candidates: SourceControlledFile[], userId: string) {
const personalProject =
await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId);
const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId);
const workflowManager = this.activeWorkflowManager;
const candidateIds = candidates.map((c) => c.id);
const existingWorkflows = await Container.get(WorkflowRepository).findByIds(candidateIds, {
const existingWorkflows = await this.workflowRepository.findByIds(candidateIds, {
fields: ['id', 'name', 'versionId', 'active'],
});
const allSharedWorkflows = await Container.get(SharedWorkflowRepository).findWithFields(
candidateIds,
{ select: ['workflowId', 'role', 'projectId'] },
);
const allSharedWorkflows = await this.sharedWorkflowRepository.findWithFields(candidateIds, {
select: ['workflowId', 'role', 'projectId'],
});
const importWorkflowsResult = [];
// Due to SQLite concurrency issues, we cannot save all workflows at once
@ -235,9 +241,7 @@ export class SourceControlImportService {
const existingWorkflow = existingWorkflows.find((e) => e.id === importedWorkflow.id);
importedWorkflow.active = existingWorkflow?.active ?? false;
this.logger.debug(`Updating workflow id ${importedWorkflow.id ?? 'new'}`);
const upsertResult = await Container.get(WorkflowRepository).upsert({ ...importedWorkflow }, [
'id',
]);
const upsertResult = await this.workflowRepository.upsert({ ...importedWorkflow }, ['id']);
if (upsertResult?.identifiers?.length !== 1) {
throw new ApplicationError('Failed to upsert workflow', {
extra: { workflowId: importedWorkflow.id ?? 'new' },
@ -253,7 +257,7 @@ export class SourceControlImportService {
? await this.findOrCreateOwnerProject(importedWorkflow.owner)
: null;
await Container.get(SharedWorkflowRepository).upsert(
await this.sharedWorkflowRepository.upsert(
{
workflowId: importedWorkflow.id,
projectId: remoteOwnerProject?.id ?? personalProject.id,
@ -276,7 +280,7 @@ export class SourceControlImportService {
const error = ensureError(e);
this.logger.error(`Failed to activate workflow ${existingWorkflow.id}`, { error });
} finally {
await Container.get(WorkflowRepository).update(
await this.workflowRepository.update(
{ id: existingWorkflow.id },
{ versionId: importedWorkflow.versionId },
);
@ -295,16 +299,15 @@ export class SourceControlImportService {
}
async importCredentialsFromWorkFolder(candidates: SourceControlledFile[], userId: string) {
const personalProject =
await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId);
const personalProject = await this.projectRepository.getPersonalProjectForUserOrFail(userId);
const candidateIds = candidates.map((c) => c.id);
const existingCredentials = await Container.get(CredentialsRepository).find({
const existingCredentials = await this.credentialsRepository.find({
where: {
id: In(candidateIds),
},
select: ['id', 'name', 'type', 'data'],
});
const existingSharedCredentials = await Container.get(SharedCredentialsRepository).find({
const existingSharedCredentials = await this.sharedCredentialsRepository.find({
select: ['credentialsId', 'role'],
where: {
credentialsId: In(candidateIds),
@ -336,7 +339,7 @@ export class SourceControlImportService {
}
this.logger.debug(`Updating credential id ${newCredentialObject.id as string}`);
await Container.get(CredentialsRepository).upsert(newCredentialObject, ['id']);
await this.credentialsRepository.upsert(newCredentialObject, ['id']);
const isOwnedLocally = existingSharedCredentials.some(
(c) => c.credentialsId === credential.id && c.role === 'credential:owner',
@ -352,7 +355,7 @@ export class SourceControlImportService {
newSharedCredential.projectId = remoteOwnerProject?.id ?? personalProject.id;
newSharedCredential.role = 'credential:owner';
await Container.get(SharedCredentialsRepository).upsert({ ...newSharedCredential }, [
await this.sharedCredentialsRepository.upsert({ ...newSharedCredential }, [
'credentialsId',
'projectId',
]);
@ -388,7 +391,7 @@ export class SourceControlImportService {
const existingWorkflowIds = new Set(
(
await Container.get(WorkflowRepository).find({
await this.workflowRepository.find({
select: ['id'],
})
).map((e) => e.id),
@ -417,7 +420,7 @@ export class SourceControlImportService {
await Promise.all(
mappedTags.mappings.map(async (mapping) => {
if (!existingWorkflowIds.has(String(mapping.workflowId))) return;
await Container.get(WorkflowTagMappingRepository).upsert(
await this.workflowTagMappingRepository.upsert(
{ tagId: String(mapping.tagId), workflowId: String(mapping.workflowId) },
{
skipUpdateIfNoValuesChanged: true,
@ -464,12 +467,12 @@ export class SourceControlImportService {
overriddenKeys.splice(overriddenKeys.indexOf(variable.key), 1);
}
try {
await Container.get(VariablesRepository).upsert({ ...variable }, ['id']);
await this.variablesRepository.upsert({ ...variable }, ['id']);
} catch (errorUpsert) {
if (isUniqueConstraintError(errorUpsert as Error)) {
this.logger.debug(`Variable ${variable.key} already exists, updating instead`);
try {
await Container.get(VariablesRepository).update({ key: variable.key }, { ...variable });
await this.variablesRepository.update({ key: variable.key }, { ...variable });
} catch (errorUpdate) {
this.logger.debug(`Failed to update variable ${variable.key}, skipping`);
this.logger.debug((errorUpdate as Error).message);
@ -484,11 +487,11 @@ export class SourceControlImportService {
if (overriddenKeys.length > 0 && valueOverrides) {
for (const key of overriddenKeys) {
result.imported.push(key);
const newVariable = Container.get(VariablesRepository).create({
const newVariable = this.variablesRepository.create({
key,
value: valueOverrides[key],
});
await Container.get(VariablesRepository).save(newVariable, { transaction: false });
await this.variablesRepository.save(newVariable, { transaction: false });
}
}
@ -498,32 +501,30 @@ export class SourceControlImportService {
}
private async findOrCreateOwnerProject(owner: ResourceOwner): Promise<Project | null> {
const projectRepository = Container.get(ProjectRepository);
const userRepository = Container.get(UserRepository);
if (typeof owner === 'string' || owner.type === 'personal') {
const email = typeof owner === 'string' ? owner : owner.personalEmail;
const user = await userRepository.findOne({
const user = await this.userRepository.findOne({
where: { email },
});
if (!user) {
return null;
}
return await projectRepository.getPersonalProjectForUserOrFail(user.id);
return await this.projectRepository.getPersonalProjectForUserOrFail(user.id);
} else if (owner.type === 'team') {
let teamProject = await projectRepository.findOne({
let teamProject = await this.projectRepository.findOne({
where: { id: owner.teamId },
});
if (!teamProject) {
try {
teamProject = await projectRepository.save(
projectRepository.create({
teamProject = await this.projectRepository.save(
this.projectRepository.create({
id: owner.teamId,
name: owner.teamName,
type: 'team',
}),
);
} catch (e) {
teamProject = await projectRepository.findOne({
teamProject = await this.projectRepository.findOne({
where: { id: owner.teamId },
});
if (!teamProject) {

View file

@ -1,4 +1,4 @@
import { Container, Service } from '@n8n/di';
import { Service } from '@n8n/di';
import type { ValidationError } from 'class-validator';
import { validate } from 'class-validator';
import { rm as fsRm } from 'fs/promises';
@ -7,7 +7,6 @@ import { ApplicationError, jsonParse } from 'n8n-workflow';
import { writeFile, chmod, readFile } from 'node:fs/promises';
import path from 'path';
import config from '@/config';
import { SettingsRepository } from '@/databases/repositories/settings.repository';
import {
@ -17,6 +16,7 @@ import {
SOURCE_CONTROL_PREFERENCES_DB_KEY,
} from './constants';
import { generateSshKeyPair, isSourceControlLicensed } from './source-control-helper.ee';
import { SourceControlConfig } from './source-control.config';
import type { KeyPairType } from './types/key-pair-type';
import { SourceControlPreferences } from './types/source-control-preferences';
@ -34,6 +34,8 @@ export class SourceControlPreferencesService {
private readonly instanceSettings: InstanceSettings,
private readonly logger: Logger,
private readonly cipher: Cipher,
private readonly settingsRepository: SettingsRepository,
private readonly sourceControlConfig: SourceControlConfig,
) {
this.sshFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_SSH_FOLDER);
this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER);
@ -64,9 +66,7 @@ export class SourceControlPreferencesService {
}
private async getKeyPairFromDatabase() {
const dbSetting = await Container.get(SettingsRepository).findByKey(
'features.sourceControl.sshKeys',
);
const dbSetting = await this.settingsRepository.findByKey('features.sourceControl.sshKeys');
if (!dbSetting?.value) return null;
@ -120,7 +120,7 @@ export class SourceControlPreferencesService {
async deleteKeyPair() {
try {
await fsRm(this.sshFolder, { recursive: true });
await Container.get(SettingsRepository).delete({ key: 'features.sourceControl.sshKeys' });
await this.settingsRepository.delete({ key: 'features.sourceControl.sshKeys' });
} catch (e) {
const error = e instanceof Error ? e : new Error(`${e}`);
this.logger.error(`Failed to delete SSH key pair: ${error.message}`);
@ -133,14 +133,12 @@ export class SourceControlPreferencesService {
async generateAndSaveKeyPair(keyPairType?: KeyPairType): Promise<SourceControlPreferences> {
if (!keyPairType) {
keyPairType =
this.getPreferences().keyGeneratorType ??
(config.get('sourceControl.defaultKeyPairType') as KeyPairType) ??
'ed25519';
this.getPreferences().keyGeneratorType ?? this.sourceControlConfig.defaultKeyPairType;
}
const keyPair = await generateSshKeyPair(keyPairType);
try {
await Container.get(SettingsRepository).save({
await this.settingsRepository.save({
key: 'features.sourceControl.sshKeys',
value: JSON.stringify({
encryptedPrivateKey: this.cipher.encrypt(keyPair.privateKey),
@ -211,7 +209,7 @@ export class SourceControlPreferencesService {
if (saveToDb) {
const settingsValue = JSON.stringify(this._sourceControlPreferences);
try {
await Container.get(SettingsRepository).save(
await this.settingsRepository.save(
{
key: SOURCE_CONTROL_PREFERENCES_DB_KEY,
value: settingsValue,
@ -229,7 +227,7 @@ export class SourceControlPreferencesService {
async loadFromDbAndApplySourceControlPreferences(): Promise<
SourceControlPreferences | undefined
> {
const loadedPreferences = await Container.get(SettingsRepository).findOne({
const loadedPreferences = await this.settingsRepository.findOne({
where: { key: SOURCE_CONTROL_PREFERENCES_DB_KEY },
});
if (loadedPreferences) {

View file

@ -0,0 +1,8 @@
import { Config, Env } from '@n8n/config';
@Config
export class SourceControlConfig {
/** Default SSH key type to use when generating SSH keys. */
@Env('N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE')
defaultKeyPairType: 'ed25519' | 'rsa' = 'ed25519';
}

View file

@ -1,27 +0,0 @@
import { Container } from '@n8n/di';
import { License } from '@/license';
export function isVariablesEnabled(): boolean {
const license = Container.get(License);
return license.isVariablesEnabled();
}
export function canCreateNewVariable(variableCount: number): boolean {
if (!isVariablesEnabled()) {
return false;
}
const license = Container.get(License);
// This defaults to -1 which is what we want if we've enabled
// variables via the config
const limit = license.getVariablesLimit();
if (limit === -1) {
return true;
}
return limit > variableCount;
}
export function getVariablesLimit(): number {
const license = Container.get(License);
return license.getVariablesLimit();
}

View file

@ -1,4 +1,4 @@
import { Container, Service } from '@n8n/di';
import { Service } from '@n8n/di';
import type { Variables } from '@/databases/entities/variables';
import { VariablesRepository } from '@/databases/repositories/variables.repository';
@ -6,23 +6,21 @@ import { generateNanoId } from '@/databases/utils/generators';
import { VariableCountLimitReachedError } from '@/errors/variable-count-limit-reached.error';
import { VariableValidationError } from '@/errors/variable-validation.error';
import { EventService } from '@/events/event.service';
import { License } from '@/license';
import { CacheService } from '@/services/cache/cache.service';
import { canCreateNewVariable } from './environment-helpers';
@Service()
export class VariablesService {
constructor(
protected cacheService: CacheService,
protected variablesRepository: VariablesRepository,
private readonly cacheService: CacheService,
private readonly variablesRepository: VariablesRepository,
private readonly eventService: EventService,
private readonly license: License,
) {}
async getAllCached(state?: 'empty'): Promise<Variables[]> {
let variables = await this.cacheService.get('variables', {
async refreshFn() {
return await Container.get(VariablesService).findAll();
},
refreshFn: async () => await this.findAll(),
});
if (variables === undefined) {
@ -77,7 +75,7 @@ export class VariablesService {
}
async create(variable: Omit<Variables, 'id'>): Promise<Variables> {
if (!canCreateNewVariable(await this.getCount())) {
if (!this.canCreateNewVariable(await this.getCount())) {
throw new VariableCountLimitReachedError('Variables limit reached');
}
this.validateVariable(variable);
@ -100,4 +98,17 @@ export class VariablesService {
await this.updateCache();
return (await this.getCached(id))!;
}
private canCreateNewVariable(variableCount: number): boolean {
if (!this.license.isVariablesEnabled()) {
return false;
}
// This defaults to -1 which is what we want if we've enabled
// variables via the config
const limit = this.license.getVariablesLimit();
if (limit === -1) {
return true;
}
return limit > variableCount;
}
}

View file

@ -12,7 +12,6 @@ import config from '@/config';
import { inE2ETests, LICENSE_FEATURES, N8N_VERSION } from '@/constants';
import { CredentialTypes } from '@/credential-types';
import { CredentialsOverwrites } from '@/credentials-overwrites';
import { getVariablesLimit } from '@/environments.ee/variables/environment-helpers';
import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee';
import { License } from '@/license';
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
@ -326,7 +325,7 @@ export class FrontendService {
}
if (this.license.isVariablesEnabled()) {
this.settings.variables.limit = getVariablesLimit();
this.settings.variables.limit = this.license.getVariablesLimit();
}
if (this.license.isWorkflowHistoryLicensed() && config.getEnv('workflowHistory.enabled')) {

View file

@ -10,6 +10,7 @@ import fsp from 'node:fs/promises';
import { CredentialsRepository } from '@/databases/repositories/credentials.repository';
import { ProjectRepository } from '@/databases/repositories/project.repository';
import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository';
import { UserRepository } from '@/databases/repositories/user.repository';
import { SourceControlImportService } from '@/environments.ee/source-control/source-control-import.service.ee';
import type { ExportableCredential } from '@/environments.ee/source-control/types/exportable-credential';
@ -21,20 +22,36 @@ import { randomCredentialPayload } from '../shared/random';
import * as testDb from '../shared/test-db';
describe('SourceControlImportService', () => {
let credentialsRepository: CredentialsRepository;
let projectRepository: ProjectRepository;
let sharedCredentialsRepository: SharedCredentialsRepository;
let userRepository: UserRepository;
let service: SourceControlImportService;
const cipher = mockInstance(Cipher);
beforeAll(async () => {
await testDb.init();
credentialsRepository = Container.get(CredentialsRepository);
projectRepository = Container.get(ProjectRepository);
sharedCredentialsRepository = Container.get(SharedCredentialsRepository);
userRepository = Container.get(UserRepository);
service = new SourceControlImportService(
mock(),
mock(),
mock(),
mock(),
credentialsRepository,
projectRepository,
mock(),
mock(),
sharedCredentialsRepository,
userRepository,
mock(),
mock(),
mock(),
mock<InstanceSettings>({ n8nFolder: '/some-path' }),
);
await testDb.init();
});
afterEach(async () => {
@ -75,7 +92,7 @@ describe('SourceControlImportService', () => {
const personalProject = await getPersonalProject(member);
const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
const sharing = await sharedCredentialsRepository.findOneBy({
credentialsId: CREDENTIAL_ID,
projectId: personalProject.id,
role: 'credential:owner',
@ -112,7 +129,7 @@ describe('SourceControlImportService', () => {
const personalProject = await getPersonalProject(importingUser);
const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
const sharing = await sharedCredentialsRepository.findOneBy({
credentialsId: CREDENTIAL_ID,
projectId: personalProject.id,
role: 'credential:owner',
@ -149,7 +166,7 @@ describe('SourceControlImportService', () => {
const personalProject = await getPersonalProject(importingUser);
const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
const sharing = await sharedCredentialsRepository.findOneBy({
credentialsId: CREDENTIAL_ID,
projectId: personalProject.id,
role: 'credential:owner',
@ -190,7 +207,7 @@ describe('SourceControlImportService', () => {
const personalProject = await getPersonalProject(importingUser);
const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
const sharing = await sharedCredentialsRepository.findOneBy({
credentialsId: CREDENTIAL_ID,
projectId: personalProject.id,
role: 'credential:owner',
@ -223,7 +240,7 @@ describe('SourceControlImportService', () => {
cipher.encrypt.mockReturnValue('some-encrypted-data');
{
const project = await Container.get(ProjectRepository).findOne({
const project = await projectRepository.findOne({
where: [
{
id: '1234-asdf',
@ -241,7 +258,7 @@ describe('SourceControlImportService', () => {
importingUser.id,
);
const sharing = await Container.get(SharedCredentialsRepository).findOne({
const sharing = await sharedCredentialsRepository.findOne({
where: {
credentialsId: CREDENTIAL_ID,
role: 'credential:owner',
@ -288,7 +305,7 @@ describe('SourceControlImportService', () => {
importingUser.id,
);
const sharing = await Container.get(SharedCredentialsRepository).findOneBy({
const sharing = await sharedCredentialsRepository.findOneBy({
credentialsId: CREDENTIAL_ID,
projectId: project.id,
role: 'credential:owner',
@ -332,7 +349,7 @@ describe('SourceControlImportService', () => {
);
await expect(
Container.get(SharedCredentialsRepository).findBy({
sharedCredentialsRepository.findBy({
credentialsId: credential.id,
}),
).resolves.toMatchObject([
@ -342,7 +359,7 @@ describe('SourceControlImportService', () => {
},
]);
await expect(
Container.get(CredentialsRepository).findBy({
credentialsRepository.findBy({
id: credential.id,
}),
).resolves.toMatchObject([

View file

@ -1,7 +1,6 @@
import type { SourceControlledFile } from '@n8n/api-types';
import { Container } from '@n8n/di';
import config from '@/config';
import type { User } from '@/databases/entities/user';
import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee';
import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee';
@ -21,11 +20,17 @@ const testServer = utils.setupTestServer({
enabledFeatures: ['feat:sourceControl', 'feat:sharing'],
});
let sourceControlPreferencesService: SourceControlPreferencesService;
beforeAll(async () => {
owner = await createUser({ role: 'global:owner' });
authOwnerAgent = testServer.authAgentFor(owner);
Container.get(SourceControlPreferencesService).isSourceControlConnected = () => true;
sourceControlPreferencesService = Container.get(SourceControlPreferencesService);
await sourceControlPreferencesService.setPreferences({
connected: true,
keyGeneratorType: 'rsa',
});
});
describe('GET /sourceControl/preferences', () => {
@ -65,19 +70,11 @@ describe('GET /sourceControl/preferences', () => {
});
test('refreshing key pairsshould return new rsa key', async () => {
config.set('sourceControl.defaultKeyPairType', 'rsa');
await authOwnerAgent
.post('/source-control/generate-key-pair')
.send()
.expect(200)
.expect((res) => {
expect(
Container.get(SourceControlPreferencesService).getPreferences().keyGeneratorType,
).toBe('rsa');
expect(res.body.data).toHaveProperty('publicKey');
expect(res.body.data).toHaveProperty('keyGeneratorType');
expect(res.body.data.keyGeneratorType).toBe('rsa');
expect(res.body.data.publicKey).toContain('ssh-rsa');
});
const res = await authOwnerAgent.post('/source-control/generate-key-pair').send().expect(200);
expect(res.body.data).toHaveProperty('publicKey');
expect(res.body.data).toHaveProperty('keyGeneratorType');
expect(res.body.data.keyGeneratorType).toBe('rsa');
expect(res.body.data.publicKey).toContain('ssh-rsa');
});
});