fix(core): Fix 431 for large dynamic node parameters (#9384)

ivov · web-flow · commit d21ad15c1f12 · 2024-05-21T19:11:02.000+02:00
diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts
@@ -395,7 +395,11 @@ describe('NDV', () => {
 	});
 
 	it('should not retrieve remote options when a parameter value changes', () => {
-		cy.intercept('/rest/dynamic-node-parameters/options?**', cy.spy().as('fetchParameterOptions'));
+		cy.intercept(
+			'POST',
+			'/rest/dynamic-node-parameters/options',
+			cy.spy().as('fetchParameterOptions'),
+		);
 		workflowPage.actions.addInitialNodeToCanvas('E2e Test', { action: 'Remote Options' });
 		// Type something into the field
 		ndv.actions.typeIntoParameterInput('otherField', 'test');
diff --git a/packages/cli/src/controllers/dynamicNodeParameters.controller.ts b/packages/cli/src/controllers/dynamicNodeParameters.controller.ts
@@ -1,54 +1,27 @@
-import type { RequestHandler } from 'express';
-import { NextFunction, Response } from 'express';
-import type {
-	INodeListSearchResult,
-	INodePropertyOptions,
-	ResourceMapperFields,
-} from 'n8n-workflow';
+import type { INodePropertyOptions } from 'n8n-workflow';
 import { jsonParse } from 'n8n-workflow';
 
-import { Get, Middleware, RestController } from '@/decorators';
+import { Post, RestController } from '@/decorators';
 import { getBase } from '@/WorkflowExecuteAdditionalData';
 import { DynamicNodeParametersService } from '@/services/dynamicNodeParameters.service';
 import { DynamicNodeParametersRequest } from '@/requests';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 
-const assertMethodName: RequestHandler = (req, res, next) => {
-	const { methodName } = req.query as DynamicNodeParametersRequest.BaseRequest['query'];
-	if (!methodName) {
-		throw new BadRequestError('Parameter methodName is required.');
-	}
-	next();
-};
-
 @RestController('/dynamic-node-parameters')
 export class DynamicNodeParametersController {
 	constructor(private readonly service: DynamicNodeParametersService) {}
 
-	@Middleware()
-	parseQueryParams(req: DynamicNodeParametersRequest.BaseRequest, _: Response, next: NextFunction) {
-		const { credentials, currentNodeParameters, nodeTypeAndVersion } = req.query;
-		if (!nodeTypeAndVersion) {
-			throw new BadRequestError('Parameter nodeTypeAndVersion is required.');
-		}
-		if (!currentNodeParameters) {
-			throw new BadRequestError('Parameter currentNodeParameters is required.');
-		}
-
-		req.params = {
-			nodeTypeAndVersion: jsonParse(nodeTypeAndVersion),
-			currentNodeParameters: jsonParse(currentNodeParameters),
-			credentials: credentials ? jsonParse(credentials) : undefined,
-		};
-
-		next();
-	}
-
-	/** Returns parameter values which normally get loaded from an external API or get generated dynamically */
-	@Get('/options')
+	@Post('/options')
 	async getOptions(req: DynamicNodeParametersRequest.Options): Promise<INodePropertyOptions[]> {
-		const { path, methodName, loadOptions } = req.query;
-		const { credentials, currentNodeParameters, nodeTypeAndVersion } = req.params;
+		const {
+			credentials,
+			currentNodeParameters,
+			nodeTypeAndVersion,
+			path,
+			methodName,
+			loadOptions,
+		} = req.body;
+
 		const additionalData = await getBase(req.user.id, currentNodeParameters);
 
 		if (methodName) {
@@ -75,13 +48,22 @@ export class DynamicNodeParametersController {
 		return [];
 	}
 
-	@Get('/resource-locator-results', { middlewares: [assertMethodName] })
-	async getResourceLocatorResults(
-		req: DynamicNodeParametersRequest.ResourceLocatorResults,
-	): Promise<INodeListSearchResult | undefined> {
-		const { path, methodName, filter, paginationToken } = req.query;
-		const { credentials, currentNodeParameters, nodeTypeAndVersion } = req.params;
+	@Post('/resource-locator-results')
+	async getResourceLocatorResults(req: DynamicNodeParametersRequest.ResourceLocatorResults) {
+		const {
+			path,
+			methodName,
+			filter,
+			paginationToken,
+			credentials,
+			currentNodeParameters,
+			nodeTypeAndVersion,
+		} = req.body;
+
+		if (!methodName) throw new BadRequestError('Missing `methodName` in request body');
+
 		const additionalData = await getBase(req.user.id, currentNodeParameters);
+
 		return await this.service.getResourceLocatorResults(
 			methodName,
 			path,
@@ -94,13 +76,14 @@ export class DynamicNodeParametersController {
 		);
 	}
 
-	@Get('/resource-mapper-fields', { middlewares: [assertMethodName] })
-	async getResourceMappingFields(
-		req: DynamicNodeParametersRequest.ResourceMapperFields,
-	): Promise<ResourceMapperFields | undefined> {
-		const { path, methodName } = req.query;
-		const { credentials, currentNodeParameters, nodeTypeAndVersion } = req.params;
+	@Post('/resource-mapper-fields')
+	async getResourceMappingFields(req: DynamicNodeParametersRequest.ResourceMapperFields) {
+		const { path, methodName, credentials, currentNodeParameters, nodeTypeAndVersion } = req.body;
+
+		if (!methodName) throw new BadRequestError('Missing `methodName` in request body');
+
 		const additionalData = await getBase(req.user.id, currentNodeParameters);
+
 		return await this.service.getResourceMappingFields(
 			methodName,
 			path,
diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts
@@ -393,21 +393,17 @@ export declare namespace OAuthRequest {
 //      /dynamic-node-parameters
 // ----------------------------------
 export declare namespace DynamicNodeParametersRequest {
-	type BaseRequest<QueryParams = {}> = AuthenticatedRequest<
-		{
-			nodeTypeAndVersion: INodeTypeNameVersion;
-			currentNodeParameters: INodeParameters;
-			credentials?: INodeCredentials;
-		},
+	type BaseRequest<RequestBody = {}> = AuthenticatedRequest<
 		{},
 		{},
 		{
 			path: string;
-			nodeTypeAndVersion: string;
-			currentNodeParameters: string;
+			nodeTypeAndVersion: INodeTypeNameVersion;
+			currentNodeParameters: INodeParameters;
 			methodName?: string;
-			credentials?: string;
-		} & QueryParams
+			credentials?: INodeCredentials;
+		} & RequestBody,
+		{}
 	>;
 
 	/** GET /dynamic-node-parameters/options */
diff --git a/packages/cli/test/integration/controllers/dynamic-node-parameters.controller.test.ts b/packages/cli/test/integration/controllers/dynamic-node-parameters.controller.test.ts
@@ -0,0 +1,80 @@
+import type { SuperTest, Test } from 'supertest';
+import { createOwner } from '../shared/db/users';
+import { setupTestServer } from '../shared/utils';
+import * as AdditionalData from '@/WorkflowExecuteAdditionalData';
+import type {
+	INodeListSearchResult,
+	IWorkflowExecuteAdditionalData,
+	ResourceMapperFields,
+} from 'n8n-workflow';
+import { mock } from 'jest-mock-extended';
+import { DynamicNodeParametersService } from '@/services/dynamicNodeParameters.service';
+
+describe('DynamicNodeParametersController', () => {
+	const testServer = setupTestServer({ endpointGroups: ['dynamic-node-parameters'] });
+	let ownerAgent: SuperTest<Test>;
+
+	beforeAll(async () => {
+		const owner = await createOwner();
+		ownerAgent = testServer.authAgentFor(owner);
+	});
+
+	const commonRequestParams = {
+		credentials: {},
+		currentNodeParameters: {},
+		nodeTypeAndVersion: {},
+		path: 'path',
+		methodName: 'methodName',
+	};
+
+	describe('POST /dynamic-node-parameters/options', () => {
+		jest.spyOn(AdditionalData, 'getBase').mockResolvedValue(mock<IWorkflowExecuteAdditionalData>());
+
+		it('should take params via body', async () => {
+			jest
+				.spyOn(DynamicNodeParametersService.prototype, 'getOptionsViaMethodName')
+				.mockResolvedValue([]);
+
+			await ownerAgent
+				.post('/dynamic-node-parameters/options')
+				.send({
+					...commonRequestParams,
+					loadOptions: 'loadOptions',
+				})
+				.expect(200);
+		});
+	});
+
+	describe('POST /dynamic-node-parameters/resource-locator-results', () => {
+		it('should take params via body', async () => {
+			jest
+				.spyOn(DynamicNodeParametersService.prototype, 'getResourceLocatorResults')
+				.mockResolvedValue(mock<INodeListSearchResult>());
+
+			await ownerAgent
+				.post('/dynamic-node-parameters/resource-locator-results')
+				.send({
+					...commonRequestParams,
+					filter: 'filter',
+					paginationToken: 'paginationToken',
+				})
+				.expect(200);
+		});
+	});
+
+	describe('POST /dynamic-node-parameters/resource-mapper-fields', () => {
+		it('should take params via body', async () => {
+			jest
+				.spyOn(DynamicNodeParametersService.prototype, 'getResourceMappingFields')
+				.mockResolvedValue(mock<ResourceMapperFields>());
+
+			await ownerAgent
+				.post('/dynamic-node-parameters/resource-mapper-fields')
+				.send({
+					...commonRequestParams,
+					loadOptions: 'loadOptions',
+				})
+				.expect(200);
+		});
+	});
+});
diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts
@@ -35,7 +35,8 @@ type EndpointGroup =
 	| 'invitations'
 	| 'debug'
 	| 'project'
-	| 'role';
+	| 'role'
+	| 'dynamic-node-parameters';
 
 export interface SetupProps {
 	endpointGroups?: EndpointGroup[];
diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts
@@ -267,6 +267,13 @@ export const setupTestServer = ({
 						const { RoleController } = await import('@/controllers/role.controller');
 						registerController(app, RoleController);
 						break;
+
+					case 'dynamic-node-parameters':
+						const { DynamicNodeParametersController } = await import(
+							'@/controllers/dynamicNodeParameters.controller'
+						);
+						registerController(app, DynamicNodeParametersController);
+						break;
 				}
 			}
 		}
diff --git a/packages/editor-ui/src/api/nodeTypes.ts b/packages/editor-ui/src/api/nodeTypes.ts
@@ -31,7 +31,7 @@ export async function getNodeParameterOptions(
 	context: IRestApiContext,
 	sendData: DynamicNodeParameters.OptionsRequest,
 ): Promise<INodePropertyOptions[]> {
-	return await makeRestApiRequest(context, 'GET', '/dynamic-node-parameters/options', sendData);
+	return await makeRestApiRequest(context, 'POST', '/dynamic-node-parameters/options', sendData);
 }
 
 export async function getResourceLocatorResults(
@@ -40,7 +40,7 @@ export async function getResourceLocatorResults(
 ): Promise<INodeListSearchResult> {
 	return await makeRestApiRequest(
 		context,
-		'GET',
+		'POST',
 		'/dynamic-node-parameters/resource-locator-results',
 		sendData,
 	);
@@ -52,7 +52,7 @@ export async function getResourceMapperFields(
 ): Promise<ResourceMapperFields> {
 	return await makeRestApiRequest(
 		context,
-		'GET',
+		'POST',
 		'/dynamic-node-parameters/resource-mapper-fields',
 		sendData,
 	);

Original file line number	Diff line number	Diff line change
`@@ -267,6 +267,13 @@ export const setupTestServer = ({`
`267`	`267`	`const { RoleController } = await import('@/controllers/role.controller');`
`268`	`268`	`registerController(app, RoleController);`
`269`	`269`	`break;`
	`270`	`+`
	`271`	`+ case 'dynamic-node-parameters':`
	`272`	`+ const { DynamicNodeParametersController } = await import(`
	`273`	`+ '@/controllers/dynamicNodeParameters.controller'`
	`274`	`+ );`
	`275`	`+ registerController(app, DynamicNodeParametersController);`
	`276`	`+ break;`
`270`	`277`	`}`
`271`	`278`	`}`
`272`	`279`	`}`