refactor(Redis Trigger Node): Refactor, fix duplicate triggers, and add unit tests (#9850)

packages/nodes-base/nodes/Redis/RedisTrigger.node.ts

@@ -1,6 +1,5 @@
 import type {
 	ITriggerFunctions,
-	IDataObject,
 	INodeType,
 	INodeTypeDescription,
 	ITriggerResponse,
@@ -9,6 +8,11 @@ import { NodeOperationError } from 'n8n-workflow';
 
 import { redisConnectionTest, setupRedisClient } from './utils';
 
+interface Options {
+	jsonParseBody: boolean;
+	onlyMessage: boolean;
+}
+
 export class RedisTrigger implements INodeType {
 	description: INodeTypeDescription = {
 		displayName: 'Redis Trigger',
@@ -73,45 +77,41 @@ export class RedisTrigger implements INodeType {
 		const credentials = await this.getCredentials('redis');
 
 		const channels = (this.getNodeParameter('channels') as string).split(',');
-		const options = this.getNodeParameter('options') as IDataObject;
+		const options = this.getNodeParameter('options') as Options;
 
 		if (!channels) {
 			throw new NodeOperationError(this.getNode(), 'Channels are mandatory!');
 		}
 
 		const client = setupRedisClient(credentials);
+		await client.connect();
+		await client.ping();
 
-		const manualTriggerFunction = async () => {
-			await client.connect();
-			await client.ping();
-
-			try {
-				for (const channel of channels) {
-					await client.pSubscribe(channel, (message) => {
-						if (options.jsonParseBody) {
-							try {
-								message = JSON.parse(message);
-							} catch (error) {}
-						}
-
-						if (options.onlyMessage) {
-							this.emit([this.helpers.returnJsonArray({ message })]);
-							return;
-						}
-
-						this.emit([this.helpers.returnJsonArray({ channel, message })]);
-					});
-				}
-			} catch (error) {
-				throw new NodeOperationError(this.getNode(), error);
+		const onMessage = (message: string, channel: string) => {
+			if (options.jsonParseBody) {
+				try {
+					message = JSON.parse(message);
+				} catch (error) {}
 			}
+
+			const data = options.onlyMessage ? { message } : { channel, message };
+			this.emit([this.helpers.returnJsonArray(data)]);
 		};
 
+		const manualTriggerFunction = async () =>
+			await new Promise<void>(async (resolve) => {
+				await client.pSubscribe(channels, (message, channel) => {
+					onMessage(message, channel);
+					resolve();
+				});
+			});
+
 		if (this.getMode() === 'trigger') {
-			void manualTriggerFunction();
+			await client.pSubscribe(channels, onMessage);
 		}
 
 		async function closeFunction() {
+			await client.pUnsubscribe();
 			await client.quit();
 		}
 

packages/nodes-base/nodes/Redis/test/RedisTrigger.node.test.ts

@@ -0,0 +1,119 @@
+import { returnJsonArray } from 'n8n-core';
+import { captor, mock } from 'jest-mock-extended';
+import type { ICredentialDataDecryptedObject, ITriggerFunctions } from 'n8n-workflow';
+
+import { RedisTrigger } from '../RedisTrigger.node';
+import { type RedisClientType, setupRedisClient } from '../utils';
+
+jest.mock('../utils', () => {
+	const mockRedisClient = mock<RedisClientType>();
+	return {
+		setupRedisClient: jest.fn().mockReturnValue(mockRedisClient),
+	};
+});
+
+describe('Redis Trigger Node', () => {
+	const channel = 'testing';
+	const credentials = mock<ICredentialDataDecryptedObject>();
+	const triggerFunctions = mock<ITriggerFunctions>({
+		helpers: { returnJsonArray },
+	});
+
+	beforeEach(() => {
+		jest.clearAllMocks();
+
+		triggerFunctions.getCredentials.calledWith('redis').mockResolvedValue(credentials);
+		triggerFunctions.getNodeParameter.calledWith('channels').mockReturnValue(channel);
+	});
+
+	it('should emit in manual mode', async () => {
+		triggerFunctions.getMode.mockReturnValue('manual');
+		triggerFunctions.getNodeParameter.calledWith('options').mockReturnValue({});
+
+		const response = await new RedisTrigger().trigger.call(triggerFunctions);
+		expect(response.manualTriggerFunction).toBeDefined();
+		expect(response.closeFunction).toBeDefined();
+
+		expect(triggerFunctions.getCredentials).toHaveBeenCalledTimes(1);
+		expect(triggerFunctions.getNodeParameter).toHaveBeenCalledTimes(2);
+
+		const mockRedisClient = setupRedisClient(mock());
+		expect(mockRedisClient.connect).toHaveBeenCalledTimes(1);
+		expect(mockRedisClient.ping).toHaveBeenCalledTimes(1);
+
+		// manually trigger the node, like Workflow.runNode does
+		const triggerPromise = response.manualTriggerFunction!();
+
+		const onMessageCaptor = captor<(message: string, channel: string) => unknown>();
+		expect(mockRedisClient.pSubscribe).toHaveBeenCalledWith([channel], onMessageCaptor);
+		expect(triggerFunctions.emit).not.toHaveBeenCalled();
+
+		// simulate a message
+		const onMessage = onMessageCaptor.value;
+		onMessage('{"testing": true}', channel);
+		expect(triggerFunctions.emit).toHaveBeenCalledWith([
+			[{ json: { message: '{"testing": true}', channel } }],
+		]);
+
+		// wait for the promise to resolve
+		await new Promise((resolve) => setImmediate(resolve));
+		await expect(triggerPromise).resolves.toEqual(undefined);
+
+		expect(mockRedisClient.quit).not.toHaveBeenCalled();
+		await response.closeFunction!();
+		expect(mockRedisClient.pUnsubscribe).toHaveBeenCalledTimes(1);
+		expect(mockRedisClient.quit).toHaveBeenCalledTimes(1);
+	});
+
+	it('should emit in trigger mode', async () => {
+		triggerFunctions.getMode.mockReturnValue('trigger');
+		triggerFunctions.getNodeParameter.calledWith('options').mockReturnValue({});
+
+		const response = await new RedisTrigger().trigger.call(triggerFunctions);
+		expect(response.manualTriggerFunction).toBeDefined();
+		expect(response.closeFunction).toBeDefined();
+
+		expect(triggerFunctions.getCredentials).toHaveBeenCalledTimes(1);
+		expect(triggerFunctions.getNodeParameter).toHaveBeenCalledTimes(2);
+
+		const mockRedisClient = setupRedisClient(mock());
+		expect(mockRedisClient.connect).toHaveBeenCalledTimes(1);
+		expect(mockRedisClient.ping).toHaveBeenCalledTimes(1);
+
+		const onMessageCaptor = captor<(message: string, channel: string) => unknown>();
+		expect(mockRedisClient.pSubscribe).toHaveBeenCalledWith([channel], onMessageCaptor);
+		expect(triggerFunctions.emit).not.toHaveBeenCalled();
+
+		// simulate a message
+		const onMessage = onMessageCaptor.value;
+		onMessage('{"testing": true}', channel);
+		expect(triggerFunctions.emit).toHaveBeenCalledWith([
+			[{ json: { message: '{"testing": true}', channel } }],
+		]);
+
+		expect(mockRedisClient.quit).not.toHaveBeenCalled();
+		await response.closeFunction!();
+		expect(mockRedisClient.pUnsubscribe).toHaveBeenCalledTimes(1);
+		expect(mockRedisClient.quit).toHaveBeenCalledTimes(1);
+	});
+
+	it('should parse JSON messages when configured', async () => {
+		triggerFunctions.getMode.mockReturnValue('trigger');
+		triggerFunctions.getNodeParameter.calledWith('options').mockReturnValue({
+			jsonParseBody: true,
+		});
+
+		await new RedisTrigger().trigger.call(triggerFunctions);
+
+		const mockRedisClient = setupRedisClient(mock());
+		const onMessageCaptor = captor<(message: string, channel: string) => unknown>();
+		expect(mockRedisClient.pSubscribe).toHaveBeenCalledWith([channel], onMessageCaptor);
+
+		// simulate a message
+		const onMessage = onMessageCaptor.value;
+		onMessage('{"testing": true}', channel);
+		expect(triggerFunctions.emit).toHaveBeenCalledWith([
+			[{ json: { message: { testing: true }, channel } }],
+		]);
+	});
+});