fix: Lazy load nodes for credentials testing (#4760)

packages/cli/src/CredentialTypes.ts

@@ -18,6 +18,10 @@ class CredentialTypesClass implements ICredentialTypes {
 		return this.getCredential(credentialType).type;
 	}
 
+	getNodeTypesToTestWith(type: string): string[] {
+		return this.knownCredentials[type]?.nodesToTestWith ?? [];
+	}
+
 	private getCredential(type: string): LoadedClass<ICredentialType> {
 		const loadedCredentials = this.loadedCredentials;
 		if (type in loadedCredentials) {

packages/cli/src/CredentialsHelper.ts

@@ -39,6 +39,7 @@ import {
 	IHttpRequestHelper,
 	INodeTypeData,
 	INodeTypes,
+	ICredentialTypes,
 } from 'n8n-workflow';
 
 import * as Db from '@/Db';
@@ -47,25 +48,48 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'
 import { User } from '@db/entities/User';
 import { CredentialsEntity } from '@db/entities/CredentialsEntity';
 import { NodeTypes } from '@/NodeTypes';
-import { CredentialsOverwrites } from '@/CredentialsOverwrites';
 import { CredentialTypes } from '@/CredentialTypes';
+import { CredentialsOverwrites } from '@/CredentialsOverwrites';
 import { whereClause } from './UserManagement/UserManagementHelper';
+import { RESPONSE_ERROR_MESSAGES } from './constants';
 
-const mockNodesData: INodeTypeData = {};
-const mockNodeTypes: INodeTypes = {
-	getAll(): Array<INodeType | IVersionedNodeType> {
-		return Object.values(mockNodesData).map((data) => data.type);
+const mockNode = {
+	name: '',
+	typeVersion: 1,
+	type: 'mock',
+	position: [0, 0],
+	parameters: {} as INodeParameters,
+} as INode;
+
+const mockNodesData: INodeTypeData = {
+	mock: {
+		sourcePath: '',
+		type: {
+			description: { properties: [] as INodeProperties[] },
+		} as INodeType,
 	},
-	getByNameAndVersion(nodeType: string, version?: number): INodeType | undefined {
-		if (mockNodesData[nodeType] === undefined) {
-			return undefined;
+};
+
+const mockNodeTypes: INodeTypes = {
+	getByName(nodeType: string): INodeType | IVersionedNodeType {
+		return mockNodesData[nodeType]?.type;
+	},
+	getByNameAndVersion(nodeType: string, version?: number): INodeType {
+		if (!mockNodesData[nodeType]) {
+			throw new Error(`${RESPONSE_ERROR_MESSAGES.NO_NODE}: ${nodeType}`);
 		}
 		return NodeHelpers.getVersionedNodeType(mockNodesData[nodeType].type, version);
 	},
 };
 
 export class CredentialsHelper extends ICredentialsHelper {
-	private credentialTypes = CredentialTypes();
+	constructor(
+		encryptionKey: string,
+		private credentialTypes: ICredentialTypes = CredentialTypes(),
+		private nodeTypes: INodeTypes = NodeTypes(),
+	) {
+		super(encryptionKey);
+	}
 
 	/**
 	 * Add the required authentication information to the request
@@ -387,16 +411,8 @@ export class CredentialsHelper extends ICredentialsHelper {
 				throw e;
 			}
 		} else {
-			const node = {
-				name: '',
-				typeVersion: 1,
-				type: 'mock',
-				position: [0, 0],
-				parameters: {} as INodeParameters,
-			} as INode;
-
 			const workflow = new Workflow({
-				nodes: [node],
+				nodes: [mockNode],
 				connections: {},
 				active: false,
 				nodeTypes: mockNodeTypes,
@@ -404,7 +420,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 
 			// Resolve expressions if any are set
 			decryptedData = workflow.expression.getComplexParameterValue(
-				node,
+				mockNode,
 				decryptedData as INodeParameters,
 				mode,
 				defaultTimezone,
@@ -457,28 +473,27 @@ export class CredentialsHelper extends ICredentialsHelper {
 		await Db.collections.Credentials.update(findQuery, newCredentialsData);
 	}
 
-	getCredentialTestFunction(
+	private getCredentialTestFunction(
 		credentialType: string,
-		nodeToTestWith?: string,
 	): ICredentialTestFunction | ICredentialTestRequestData | undefined {
-		const nodeTypes = NodeTypes();
-		const allNodes = nodeTypes.getAll();
+		// Check if test is defined on credentials
+		const type = this.credentialTypes.getByName(credentialType);
+		if (type.test) {
+			return {
+				testRequest: type.test,
+			};
+		}
 
-		// Check all the nodes one by one if they have a test function defined
-		for (let i = 0; i < allNodes.length; i++) {
-			const node = allNodes[i];
-
-			if (nodeToTestWith && node.description.name !== nodeToTestWith) {
-				// eslint-disable-next-line no-continue
-				continue;
-			}
+		const nodeTypesToTestWith = this.credentialTypes.getNodeTypesToTestWith(credentialType);
+		for (const nodeName of nodeTypesToTestWith) {
+			const node = this.nodeTypes.getByName(nodeName);
 
 			// Always set to an array even if node is not versioned to not having
 			// to duplicate the logic
 			const allNodeTypes: INodeType[] = [];
 			if (node instanceof VersionedNodeType) {
 				// Node is versioned
-				allNodeTypes.push(...Object.values((node as IVersionedNodeType).nodeVersions));
+				allNodeTypes.push(...Object.values(node.nodeVersions));
 			} else {
 				// Node is not versioned
 				allNodeTypes.push(node as INodeType);
@@ -487,49 +502,35 @@ export class CredentialsHelper extends ICredentialsHelper {
 			// Check each of the node versions for credential tests
 			for (const nodeType of allNodeTypes) {
 				// Check each of teh credentials
-				for (const credential of nodeType.description.credentials ?? []) {
-					if (credential.name === credentialType && !!credential.testedBy) {
-						if (typeof credential.testedBy === 'string') {
-							if (Object.prototype.hasOwnProperty.call(node, 'nodeVersions')) {
+				for (const { name, testedBy } of nodeType.description.credentials ?? []) {
+					if (name === credentialType && !!testedBy) {
+						if (typeof testedBy === 'string') {
+							if (node instanceof VersionedNodeType) {
 								// The node is versioned. So check all versions for test function
 								// starting with the latest
-								const versions = Object.keys((node as IVersionedNodeType).nodeVersions)
-									.sort()
-									.reverse();
+								const versions = Object.keys(node.nodeVersions).sort().reverse();
 								for (const version of versions) {
-									const versionedNode = (node as IVersionedNodeType).nodeVersions[
-										parseInt(version, 10)
-									];
-									if (
-										versionedNode.methods?.credentialTest &&
-										versionedNode.methods?.credentialTest[credential.testedBy]
-									) {
-										return versionedNode.methods?.credentialTest[credential.testedBy];
+									const versionedNode = node.nodeVersions[parseInt(version, 10)];
+									const credentialTest = versionedNode.methods?.credentialTest;
+									if (credentialTest && testedBy in credentialTest) {
+										return credentialTest[testedBy];
 									}
 								}
 							}
 							// Test is defined as string which links to a function
-							return (node as unknown as INodeType).methods?.credentialTest![credential.testedBy];
+							return (node as unknown as INodeType).methods?.credentialTest![testedBy];
 						}
 
 						// Test is defined as JSON with a definition for the request to make
 						return {
 							nodeType,
-							testRequest: credential.testedBy,
+							testRequest: testedBy,
 						};
 					}
 				}
 			}
 		}
 
-		// Check if test is defined on credentials
-		const type = this.credentialTypes.getByName(credentialType);
-		if (type.test) {
-			return {
-				testRequest: type.test,
-			};
-		}
-
 		return undefined;
 	}
 
@@ -537,9 +538,8 @@ export class CredentialsHelper extends ICredentialsHelper {
 		user: User,
 		credentialType: string,
 		credentialsDecrypted: ICredentialsDecrypted,
-		nodeToTestWith?: string,
 	): Promise<INodeCredentialTestResult> {
-		const credentialTestFunction = this.getCredentialTestFunction(credentialType, nodeToTestWith);
+		const credentialTestFunction = this.getCredentialTestFunction(credentialType);
 		if (credentialTestFunction === undefined) {
 			return Promise.resolve({
 				status: 'Error',
@@ -570,8 +570,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 		if (credentialTestFunction.nodeType) {
 			nodeType = credentialTestFunction.nodeType;
 		} else {
-			const nodeTypes = NodeTypes();
-			nodeType = nodeTypes.getByNameAndVersion('n8n-nodes-base.noOp');
+			nodeType = this.nodeTypes.getByNameAndVersion('n8n-nodes-base.noOp');
 		}
 
 		const node: INode = {

packages/cli/src/Interfaces.ts

@@ -15,7 +15,7 @@ import type {
 	ITelemetrySettings,
 	ITelemetryTrackProperties,
 	IWorkflowBase as IWorkflowBaseWorkflow,
-	LoadingDetails,
+	CredentialLoadingDetails,
 	Workflow,
 	WorkflowActivateMode,
 	WorkflowExecuteMode,
@@ -59,7 +59,7 @@ export interface ICustomRequest extends Request {
 }
 
 export interface ICredentialsTypeData {
-	[key: string]: LoadingDetails;
+	[key: string]: CredentialLoadingDetails;
 }
 
 export interface ICredentialsOverwrite {

packages/cli/src/LoadNodesAndCredentials.ts

@@ -366,8 +366,12 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 			}
 
 			for (const type in known.credentials) {
-				const { className, sourcePath } = known.credentials[type];
-				this.known.credentials[type] = { className, sourcePath: path.join(dir, sourcePath) };
+				const { className, sourcePath, nodesToTestWith } = known.credentials[type];
+				this.known.credentials[type] = {
+					className,
+					sourcePath: path.join(dir, sourcePath),
+					nodesToTestWith: nodesToTestWith?.map((nodeName) => `${packageName}.${nodeName}`),
+				};
 			}
 		}
 

packages/cli/src/NodeTypes.ts

@@ -21,10 +21,6 @@ class NodeTypesClass implements INodeTypes {
 		}
 	}
 
-	getAll(): Array<INodeType | IVersionedNodeType> {
-		return Object.values(this.loadedNodes).map(({ type }) => type);
-	}
-
 	/**
 	 * Variant of `getByNameAndVersion` that includes the node's source path, used to locate a node's translations.
 	 */
@@ -43,6 +39,10 @@ class NodeTypesClass implements INodeTypes {
 		return { description: { ...description }, sourcePath: nodeType.sourcePath };
 	}
 
+	getByName(nodeType: string): INodeType | IVersionedNodeType {
+		return this.getNode(nodeType).type;
+	}
+
 	getByNameAndVersion(nodeType: string, version?: number): INodeType {
 		return NodeHelpers.getVersionedNodeType(this.getNode(nodeType).type, version);
 	}

packages/cli/src/credentials/credentials.controller.ee.ts

@@ -106,7 +106,7 @@ EECredentialsController.get(
 EECredentialsController.post(
 	'/test',
 	ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => {
-		const { credentials, nodeToTestWith } = req.body;
+		const { credentials } = req.body;
 
 		const encryptionKey = await EECredentials.getEncryptionKey();
 
@@ -122,7 +122,7 @@ EECredentialsController.post(
 			Object.assign(credentials, { data: decryptedData });
 		}
 
-		return EECredentials.test(req.user, encryptionKey, credentials, nodeToTestWith);
+		return EECredentials.test(req.user, encryptionKey, credentials);
 	}),
 );
 

packages/cli/src/credentials/credentials.controller.ts

@@ -113,10 +113,10 @@ credentialsController.get(
 credentialsController.post(
 	'/test',
 	ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => {
-		const { credentials, nodeToTestWith } = req.body;
+		const { credentials } = req.body;
 
 		const encryptionKey = await CredentialsService.getEncryptionKey();
-		return CredentialsService.test(req.user, encryptionKey, credentials, nodeToTestWith);
+		return CredentialsService.test(req.user, encryptionKey, credentials);
 	}),
 );
 

packages/cli/src/credentials/credentials.service.ts

@@ -282,10 +282,9 @@ export class CredentialsService {
 		user: User,
 		encryptionKey: string,
 		credentials: ICredentialsDecrypted,
-		nodeToTestWith: string | undefined,
 	): Promise<INodeCredentialTestResult> {
 		const helper = new CredentialsHelper(encryptionKey);
 
-		return helper.testCredentials(user, credentials.type, credentials, nodeToTestWith);
+		return helper.testCredentials(user, credentials.type, credentials);
 	}
 }