refactor: Improve performance of source control methods (#25298)

This commit is contained in:
Ali Elkhateeb 2026-02-05 16:39:30 +02:00 committed by GitHub
parent 2d02bb4e63
commit fb2c5946c3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 672 additions and 475 deletions

View File

@ -1,5 +1,5 @@
import { GlobalConfig } from '@n8n/config';
import type { SelectQueryBuilder } from '@n8n/typeorm';
import { In, type SelectQueryBuilder } from '@n8n/typeorm';
import { mock } from 'jest-mock-extended';
import { WorkflowEntity } from '../../entities';
@ -511,4 +511,24 @@ describe('WorkflowRepository', () => {
expect(queryBuilder.leftJoin).toHaveBeenCalledWith('workflow.activeVersion', 'activeVersion');
});
});
describe('findByIds', () => {
it('should return an empty array and not call the database when no workflow ids are provided', async () => {
const findSpy = jest.spyOn(workflowRepository, 'find');
const workflowIds: string[] = [];
const result = await workflowRepository.findByIds(workflowIds);
expect(result).toEqual([]);
expect(findSpy).not.toHaveBeenCalled();
});
it('should call the database when workflow ids are provided', async () => {
const findSpy = jest.spyOn(workflowRepository, 'find').mockResolvedValue([]);
const workflowIds = ['workflow1'];
const result = await workflowRepository.findByIds(workflowIds);
expect(result).toEqual([]);
expect(findSpy).toHaveBeenCalledTimes(1);
expect(findSpy).toHaveBeenCalledWith({ where: { id: In(workflowIds) } });
});
});
});

View File

@ -118,6 +118,10 @@ export class WorkflowRepository extends Repository<WorkflowEntity> {
}
async findByIds(workflowIds: string[], { fields }: { fields?: string[] } = {}) {
if (workflowIds.length === 0) {
return [];
}
const options: FindManyOptions<WorkflowEntity> = {
where: { id: In(workflowIds) },
};

View File

@ -2,7 +2,7 @@ import { defineConfig, globalIgnores } from 'eslint/config';
import { nodeConfig } from '@n8n/eslint-config/node';
export default defineConfig(
globalIgnores(['scripts/**/*.mjs', 'jest.config*.js', 'test/*-testcontainers.js']),
globalIgnores(['scripts/**/*.mjs', 'jest.config*.js', 'test/*-testcontainers.js', 'coverage/**']),
nodeConfig,
{
rules: {

View File

@ -18,11 +18,13 @@ import {
getTrackingInformationFromPullResult,
isWorkflowModified,
sourceControlFoldersExistCheck,
areSameCredentials,
} from '../source-control-helper.ee';
import type { SourceControlPreferencesService } from '@/modules/source-control.ee/source-control-preferences.service.ee';
import type { License } from '@/license';
import type { SourceControlWorkflowVersionId } from '../types/source-control-workflow-version-id';
import type { StatusExportableCredential } from '../types/exportable-credential';
function createWorkflowVersion(
overrides: Partial<SourceControlWorkflowVersionId> = {},
@ -491,3 +493,75 @@ describe('readFoldersFromSourceControlFile', () => {
});
});
});
describe('areSameCredentials', () => {
const mockCredential = (
overrides: Partial<StatusExportableCredential> = {},
): StatusExportableCredential => {
return {
filename: 'cred1.json',
id: 'cred1',
name: 'My Credential',
type: 'credential',
ownedBy: {
type: 'team',
projectId: 'project1',
projectName: 'Project 1',
teamId: 'team1',
teamName: 'Team 1',
},
data: {},
isGlobal: false,
...overrides,
};
};
it('should return true when credentials are the same', () => {
const creds1 = mockCredential();
const creds2 = mockCredential();
expect(areSameCredentials(creds1, creds2)).toBe(true);
});
it('should return true when only data is different', () => {
const creds1 = mockCredential({ data: { accessToken: 'access token' } });
const creds2 = mockCredential({ data: { accessToken: 'different access token' } });
expect(areSameCredentials(creds1, creds2)).toBe(true);
});
it('should return false when names are different', () => {
const creds1 = mockCredential();
const creds2 = mockCredential({ name: 'Different Name' });
expect(areSameCredentials(creds1, creds2)).toBe(false);
});
it('should return false when types are different', () => {
const creds1 = mockCredential();
const creds2 = mockCredential({ type: 'different type' });
expect(areSameCredentials(creds1, creds2)).toBe(false);
});
it('should return false when ownedBy are different', () => {
const creds1 = mockCredential();
const creds2 = mockCredential({
ownedBy: {
type: 'personal',
personalEmail: 'test2@example.com',
projectId: 'project2',
projectName: 'Project 2',
},
});
expect(areSameCredentials(creds1, creds2)).toBe(false);
});
it('should return false when isGlobal are different', () => {
const creds1 = mockCredential();
const creds2 = mockCredential({ isGlobal: true });
expect(areSameCredentials(creds1, creds2)).toBe(false);
});
});

View File

@ -832,21 +832,57 @@ describe('getStatus', () => {
});
describe('workflows', () => {
describe('owner changes', () => {
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
const user = mock<User>({ role: GLOBAL_ADMIN_ROLE });
const createWorkflow = (
overrides: Partial<SourceControlWorkflowVersionId> = {},
): SourceControlWorkflowVersionId => ({
id: 'wf1',
name: 'Test Workflow',
versionId: 'version1',
filename: 'workflows/wf1.json',
parentFolderId: 'folder1',
updatedAt: '2023-07-10T10:10:59.000Z',
...overrides,
const createWorkflow = (
overrides: Partial<SourceControlWorkflowVersionId> = {},
): SourceControlWorkflowVersionId => ({
id: 'wf1',
name: 'Test Workflow',
versionId: 'version1',
filename: 'workflows/wf1.json',
parentFolderId: 'folder1',
updatedAt: '2023-07-10T10:10:59.000Z',
...overrides,
});
describe('missing workflows (verbose)', () => {
it('should include workflows missing in local', async () => {
const remote = createWorkflow({ id: 'wf-remote', filename: 'workflows/wf-remote.json' });
sourceControlImportService.getRemoteVersionIdsFromFiles.mockResolvedValue([remote]);
sourceControlImportService.getLocalVersionIdsFromDb.mockResolvedValue([]);
const result = await sourceControlStatusService.getStatus(user, {
direction: 'pull',
verbose: true,
preferLocalVersion: false,
});
if (Array.isArray(result)) fail('Expected result to be an object.');
expect(result.wfMissingInLocal).toMatchObject([remote]);
expect(result.sourceControlledFiles).toHaveLength(1);
});
it('should include workflows missing in remote', async () => {
const local = createWorkflow({ id: 'wf-local', filename: 'workflows/wf-local.json' });
sourceControlImportService.getRemoteVersionIdsFromFiles.mockResolvedValue([]);
sourceControlImportService.getLocalVersionIdsFromDb.mockResolvedValue([local]);
const result = await sourceControlStatusService.getStatus(user, {
direction: 'push',
verbose: true,
preferLocalVersion: false,
});
if (Array.isArray(result)) fail('Expected result to be an object.');
expect(result.wfMissingInRemote).toMatchObject([local]);
expect(result.sourceControlledFiles).toHaveLength(1);
});
});
describe('owner changes', () => {
describe('team project ownership changes (detected)', () => {
it('should detect when team project changes', async () => {
const local = createWorkflow({

View File

@ -17,3 +17,4 @@ export const SOURCE_CONTROL_README = `
`;
export const SOURCE_CONTROL_DEFAULT_NAME = 'n8n user';
export const SOURCE_CONTROL_DEFAULT_EMAIL = 'n8n@example.com';
export const SOURCE_CONTROL_WRITE_FILE_BATCH_SIZE = 20;

View File

@ -27,6 +27,7 @@ import {
SOURCE_CONTROL_PROJECT_EXPORT_FOLDER,
SOURCE_CONTROL_TAGS_EXPORT_FILE,
SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER,
SOURCE_CONTROL_WRITE_FILE_BATCH_SIZE,
} from './constants';
import {
getCredentialExportPath,
@ -48,6 +49,7 @@ import type { ExportableWorkflow } from './types/exportable-workflow';
import type { RemoteResourceOwner } from './types/resource-owner';
import type { SourceControlContext } from './types/source-control-context';
import { ExportableVariable } from './types/exportable-variable';
import chunk from 'lodash/chunk';
@Service()
export class SourceControlExportService {
@ -110,25 +112,28 @@ export class SourceControlExportService {
workflowsToBeExported: IWorkflowDb[],
owners: Record<string, RemoteResourceOwner>,
) {
await Promise.all(
workflowsToBeExported.map(async (e) => {
const fileName = this.getWorkflowPath(e.id);
const sanitizedWorkflow: ExportableWorkflow = {
id: e.id,
name: e.name,
nodes: e.nodes,
connections: e.connections,
settings: e.settings,
triggerCount: e.triggerCount,
versionId: e.versionId,
owner: owners[e.id],
parentFolderId: e.parentFolder?.id ?? null,
isArchived: e.isArchived,
};
this.logger.debug(`Writing workflow ${e.id} to ${fileName}`);
return await fsWriteFile(fileName, JSON.stringify(sanitizedWorkflow, null, 2));
}),
);
const workflowChunks = chunk(workflowsToBeExported, SOURCE_CONTROL_WRITE_FILE_BATCH_SIZE);
for (const workflowChunk of workflowChunks) {
await Promise.all(
workflowChunk.map(async (workflow) => {
const fileName = this.getWorkflowPath(workflow.id);
const sanitizedWorkflow: ExportableWorkflow = {
id: workflow.id,
name: workflow.name,
nodes: workflow.nodes,
connections: workflow.connections,
settings: workflow.settings,
triggerCount: workflow.triggerCount,
versionId: workflow.versionId,
owner: owners[workflow.id],
parentFolderId: workflow.parentFolder?.id ?? null,
isArchived: workflow.isArchived,
};
this.logger.debug(`Writing workflow ${workflow.id} to ${fileName}`);
return await fsWriteFile(fileName, JSON.stringify(sanitizedWorkflow, null, 2));
}),
);
}
}
async exportWorkflowsToWorkFolder(candidates: SourceControlledFile[]): Promise<ExportResult> {

View File

@ -22,6 +22,7 @@ import type { KeyPair } from './types/key-pair';
import type { KeyPairType } from './types/key-pair-type';
import type { SourceControlWorkflowVersionId } from './types/source-control-workflow-version-id';
import type { StatusResourceOwner } from './types/resource-owner';
import type { StatusExportableCredential } from './types/exportable-credential';
export function stringContainsExpression(testString: string): boolean {
return /^=.*\{\{.*\}\}/.test(testString);
@ -275,3 +276,15 @@ export function isWorkflowModified(
return hasVersionIdChanged || hasParentFolderIdChanged || ownerChanged;
}
export function areSameCredentials(
credA: StatusExportableCredential,
credB: StatusExportableCredential,
): boolean {
return (
credA.name === credB.name &&
credA.type === credB.type &&
!hasOwnerChanged(credA.ownedBy, credB.ownedBy) &&
Boolean(credA.isGlobal) === Boolean(credB.isGlobal)
);
}