Repository: n8n-io/n8n
File: packages/cli/test/integration/database/repositories/workflow-dependency.repository.test.ts (TypeScript)

Code before:
import { testDb, createWorkflow } from '@n8n/backend-test-utils';
import { WorkflowDependencyRepository, WorkflowDependencies } from '@n8n/db';
import { Container } from '@n8n/di';

describe('WorkflowDependencyRepository', () => {
	let workflowDependencyRepository: WorkflowDependencyRepository;

	beforeAll(async () => {
		await testDb.init();
		workflowDependencyRepository = Container.get(WorkflowDependencyRepository);
	});

	beforeEach(async () => {
		// Truncate in correct order to respect foreign key constraints
		await testDb.truncate(['WorkflowDependency', 'SharedWorkflow', 'WorkflowEntity']);
	});

	afterAll(async () => {
		await testDb.terminate();
	});

	describe('updateDependenciesForWorkflow()', () => {
		it('should insert new dependencies for a workflow with no existing dependencies', async () => {
			//
			// ARRANGE
			//
			const workflow = await createWorkflow({ versionId: 'v1' });
			const dependencies = new WorkflowDependencies(workflow.id, 1, 1);
			dependencies.add({
				dependencyType: 'credential',
				dependencyKey: 'cred-123',
				dependencyInfo: { name: 'Test Credential' },
			});
			dependencies.add({
				dependencyType: 'nodeType',
				dependencyKey: 'n8n-nodes-base.httpRequest',
				dependencyInfo: null,
			});

			//
			// ACT
			//
			const result = await workflowDependencyRepository.updateDependenciesForWorkflow(
				workflow.id,
				dependencies,
			);

			//
			// ASSERT
			//
			expect(result).toBe(true);
			const savedDependencies = await workflowDependencyRepository.find({
				where: { workflowId: workflow.id },
				order: { dependencyType: 'ASC' },
			});
			expect(savedDependencies).toHaveLength(2);
			expect(savedDependencies[0]).toMatchObject({
				workflowId: workflow.id,
				workflowVersionId: 1,
				dependencyType: 'credential',
				dependencyKey: 'cred-123',
				dependencyInfo: { name: 'Test Credential' },
				indexVersionId: 1,
			});
			expect(savedDependencies[1]).toMatchObject({
				workflowId: workflow.id,
				workflowVersionId: 1,
				dependencyType: 'nodeType',
				dependencyKey: 'n8n-nodes-base.httpRequest',
				dependencyInfo: null,
				indexVersionId: 1,
			});
		});

		it('should replace existing dependencies with newer version', async () => {
			//
			// ARRANGE
			//
			const workflow = await createWorkflow({ versionId: 'v1' });

			// Insert initial dependencies with version 1
			const initialDeps = new WorkflowDependencies(workflow.id, 1, 1);
			initialDeps.add({
				dependencyType: 'credential',
				dependencyKey: 'cred-old',
				dependencyInfo: null,
			});
			await workflowDependencyRepository.updateDependenciesForWorkflow(workflow.id, initialDeps);

			// Create new dependencies with version 2
			const updatedDeps = new WorkflowDependencies(workflow.id, 2, 1);
			updatedDeps.add({
				dependencyType: 'credential',
				dependencyKey: 'cred-new',
				dependencyInfo: { updated: true },
			});
			updatedDeps.add({
				dependencyType: 'webhookPath',
				dependencyKey: '/webhook/test',
				dependencyInfo: null,
			});

			//
			// ACT
			//
			const result = await workflowDependencyRepository.updateDependenciesForWorkflow(
				workflow.id,
				updatedDeps,
			);

			//
			// ASSERT
			//
			expect(result).toBe(true);
			const savedDependencies = await workflowDependencyRepository.find({
				where: { workflowId: workflow.id },
				order: { dependencyType: 'ASC' },
			});
			expect(savedDependencies).toHaveLength(2);
			expect(savedDependencies[0].dependencyKey).toBe('cred-new');
			expect(savedDependencies[0].workflowVersionId).toBe(2);
			expect(savedDependencies[1].dependencyType).toBe('webhookPath');
			expect(savedDependencies[1].workflowVersionId).toBe(2);
		});

		it('should not update when incoming version is older than existing version', async () => {
			//
			// ARRANGE
			//
			const workflow = await createWorkflow({ versionId: 'v2' });

			// Insert dependencies with version 2
			const newerDeps = new WorkflowDependencies(workflow.id, 2, 1);
			newerDeps.add({
				dependencyType: 'credential',
				dependencyKey: 'cred-new',
				dependencyInfo: null,
			});
			await workflowDependencyRepository.updateDependenciesForWorkflow(workflow.id, newerDeps);

			// Try to update with older version 1
			const olderDeps = new WorkflowDependencies(workflow.id, 1, 1);
			olderDeps.add({
				dependencyType: 'credential',
				dependencyKey: 'cred-old',
				dependencyInfo: null,
			});

			//
			// ACT
			//
			const result = await workflowDependencyRepository.updateDependenciesForWorkflow(
				workflow.id,
				olderDeps,
			);

			//
			// ASSERT
			//
			expect(result).toBe(false);
			const savedDependencies = await workflowDependencyRepository.find({
				where: { workflowId: workflow.id },
			});
			expect(savedDependencies).toHaveLength(1);
			expect(savedDependencies[0].dependencyKey).toBe('cred-new');
			expect(savedDependencies[0].workflowVersionId).toBe(2);
		});
	});

	describe('removeDependenciesForWorkflow()', () => {
		it('should remove all dependencies for a workflow', async () => {
			//
			// ARRANGE
			//
			const workflow = await createWorkflow({ versionId: 'v1' });
			const dependencies = new WorkflowDependencies(workflow.id, 1, 1);
			dependencies.add({
				dependencyType: 'credential',
				dependencyKey: 'cred-1',
				dependencyInfo: null,
			});
			dependencies.add({
				dependencyType: 'nodeType',
				dependencyKey: 'node-1',
				dependencyInfo: null,
			});
			await workflowDependencyRepository.updateDependenciesForWorkflow(workflow.id, dependencies);

			//
			// ACT
			//
			const result = await workflowDependencyRepository.removeDependenciesForWorkflow(workflow.id);

			//
			// ASSERT
			//
			expect(result).toBe(true);
			const remainingDeps = await workflowDependencyRepository.find({
				where: { workflowId: workflow.id },
			});
			expect(remainingDeps).toHaveLength(0);
		});

		it('should return false when no dependencies exist to remove', async () => {
			//
			// ARRANGE
			//
			const workflow = await createWorkflow({ versionId: 'v1' });

			//
			// ACT
			//
			const result = await workflowDependencyRepository.removeDependenciesForWorkflow(workflow.id);

			//
			// ASSERT
			//
			expect(result).toBe(false);
		});
	});
});


Review comment (bug):
Could we also add a test that checks that the race condition mitigation works? I.e. test that invokes `updateDependenciesForWorkflow` twice in a row with different versionIds and ensure the latter one ends up winning?

Code after:
				expect(result).toBe(true);
				const remainingDeps = await workflowDependencyRepository.find({
					where: { workflowId: workflow.id },
				});
				expect(remainingDeps).toHaveLength(0);
			});

			it('should return false when no dependencies exist to remove', async () => {
				//
				// ARRANGE
				//
				const workflow = await createWorkflow({ versionId: 'v1' });

				//
				// ACT
				//
				const result = await workflowDependencyRepository.removeDependenciesForWorkflow(
					workflow.id,
				);

				//
				// ASSERT
				//
				expect(result).toBe(false);
			});
		});
	});
}