mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-28 23:37:00 +02:00
fix(core): Prevent re-imported scheduled workflow to execute twice (#20438)
This commit is contained in:
parent
fbc7f6039c
commit
8f7f48043b
|
|
@ -7,6 +7,7 @@ import type { Cipher } from 'n8n-core';
|
|||
|
||||
import { ImportService } from '../import.service';
|
||||
import type { CredentialsRepository, TagRepository } from '@n8n/db';
|
||||
import type { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
|
||||
// Mock fs/promises
|
||||
jest.mock('fs/promises');
|
||||
|
|
@ -24,6 +25,10 @@ jest.mock('@n8n/db', () => ({
|
|||
DataSource: mock<DataSource>(),
|
||||
}));
|
||||
|
||||
jest.mock('@/active-workflow-manager', () => ({
|
||||
ActiveWorkflowManager: mock<ActiveWorkflowManager>(),
|
||||
}));
|
||||
|
||||
describe('ImportService', () => {
|
||||
let importService: ImportService;
|
||||
let mockLogger: Logger;
|
||||
|
|
@ -32,6 +37,7 @@ describe('ImportService', () => {
|
|||
let mockTagRepository: TagRepository;
|
||||
let mockEntityManager: EntityManager;
|
||||
let mockCipher: Cipher;
|
||||
let mockActiveWorkflowManager: ActiveWorkflowManager;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
|
|
@ -42,6 +48,7 @@ describe('ImportService', () => {
|
|||
mockTagRepository = mock<TagRepository>();
|
||||
mockEntityManager = mock<EntityManager>();
|
||||
mockCipher = mock<Cipher>();
|
||||
mockActiveWorkflowManager = mock<ActiveWorkflowManager>();
|
||||
|
||||
// Set up cipher mock
|
||||
mockCipher.decrypt = jest.fn((data: string) => data.replace('encrypted:', ''));
|
||||
|
|
@ -86,6 +93,7 @@ describe('ImportService', () => {
|
|||
mockTagRepository,
|
||||
mockDataSource,
|
||||
mockCipher,
|
||||
mockActiveWorkflowManager,
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import { validateDbTypeForImportEntities } from '@/utils/validate-database-type'
|
|||
import { Cipher } from 'n8n-core';
|
||||
import { decompressFolder } from '@/utils/compression.util';
|
||||
import { z } from 'zod';
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
|
||||
@Service()
|
||||
export class ImportService {
|
||||
|
|
@ -50,6 +51,7 @@ export class ImportService {
|
|||
private readonly tagRepository: TagRepository,
|
||||
private readonly dataSource: DataSource,
|
||||
private readonly cipher: Cipher,
|
||||
private readonly activeWorkflowManager: ActiveWorkflowManager,
|
||||
) {}
|
||||
|
||||
async initRecords() {
|
||||
|
|
@ -70,6 +72,11 @@ export class ImportService {
|
|||
const hasInvalidCreds = workflow.nodes.some((node) => !node.credentials?.id);
|
||||
|
||||
if (hasInvalidCreds) await this.replaceInvalidCreds(workflow);
|
||||
|
||||
// Remove workflows from ActiveWorkflowManager BEFORE transaction to prevent orphaned trigger listeners
|
||||
if (workflow.id) {
|
||||
await this.activeWorkflowManager.remove(workflow.id);
|
||||
}
|
||||
}
|
||||
|
||||
const { manager: dbManager } = this.credentialsRepository;
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
import { nanoid } from 'nanoid';
|
||||
|
||||
import '@/zod-alias-support';
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { ImportWorkflowsCommand } from '@/commands/import/workflow';
|
||||
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import { setupTestCommand } from '@test-integration/utils/test-command';
|
||||
|
|
@ -15,6 +16,8 @@ import { setupTestCommand } from '@test-integration/utils/test-command';
|
|||
import { createMember, createOwner } from '../shared/db/users';
|
||||
|
||||
mockInstance(LoadNodesAndCredentials);
|
||||
mockInstance(ActiveWorkflowManager);
|
||||
|
||||
const command = setupTestCommand(ImportWorkflowsCommand);
|
||||
|
||||
beforeEach(async () => {
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import { mock } from 'jest-mock-extended';
|
|||
import type { INode } from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
import type { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { ImportService } from '@/services/import.service';
|
||||
|
||||
import { createMember, createOwner } from './shared/db/users';
|
||||
|
|
@ -28,6 +29,7 @@ describe('ImportService', () => {
|
|||
let tagRepository: TagRepository;
|
||||
let owner: User;
|
||||
let ownerPersonalProject: Project;
|
||||
let mockActiveWorkflowManager: ActiveWorkflowManager;
|
||||
|
||||
beforeAll(async () => {
|
||||
await testDb.init();
|
||||
|
|
@ -39,7 +41,16 @@ describe('ImportService', () => {
|
|||
|
||||
const credentialsRepository = Container.get(CredentialsRepository);
|
||||
|
||||
importService = new ImportService(mock(), credentialsRepository, tagRepository, mock(), mock());
|
||||
mockActiveWorkflowManager = mock<ActiveWorkflowManager>();
|
||||
|
||||
importService = new ImportService(
|
||||
mock(),
|
||||
credentialsRepository,
|
||||
tagRepository,
|
||||
mock(),
|
||||
mock(),
|
||||
mockActiveWorkflowManager,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
|
|
@ -202,4 +213,11 @@ describe('ImportService', () => {
|
|||
|
||||
expect(dbTag.name).toBe(tag.name); // tag created
|
||||
});
|
||||
|
||||
test('should remove workflow from ActiveWorkflowManager when workflow has ID', async () => {
|
||||
const workflowWithId = await createWorkflow({ active: true });
|
||||
await importService.importWorkflows([workflowWithId], ownerPersonalProject.id);
|
||||
|
||||
expect(mockActiveWorkflowManager.remove).toHaveBeenCalledWith(workflowWithId.id);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user