From 1d2666b37cfdb42c7ae7ab18235e071fdd25d2c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 21 Dec 2023 18:22:32 +0100 Subject: [PATCH] refactor(Peekalink Node): Stricter typing for Peekalink api call + Tests (no-changelog) (#8125) This PR is an example for how we can 1. improve typing and remove boilerplate code in may of our nodes 2. use nock to write effective unit tests for nodes that make external calls ## Review / Merge checklist - [x] PR title and summary are descriptive - [x] Add tests --- packages/nodes-base/.eslintrc.js | 6 + .../nodes/Peekalink/GenericFunctions.ts | 41 ----- .../nodes/Peekalink/Peekalink.node.ts | 80 ++++---- .../Peekalink/test/Peekalink.node.test.ts | 171 ++++++++++++++++++ .../nodes-base/test/nodes/ExecuteWorkflow.ts | 8 + packages/nodes-base/test/nodes/Helpers.ts | 11 ++ 6 files changed, 231 insertions(+), 86 deletions(-) delete mode 100644 packages/nodes-base/nodes/Peekalink/GenericFunctions.ts create mode 100644 packages/nodes-base/nodes/Peekalink/test/Peekalink.node.test.ts diff --git a/packages/nodes-base/.eslintrc.js b/packages/nodes-base/.eslintrc.js index 5229bbe9644..5193ea70b71 100644 --- a/packages/nodes-base/.eslintrc.js +++ b/packages/nodes-base/.eslintrc.js @@ -151,5 +151,11 @@ module.exports = { 'n8n-nodes-base/node-param-type-options-password-missing': 'error', }, }, + { + files: ['**/*.test.ts', '**/test/**/*.ts'], + rules: { + 'import/no-extraneous-dependencies': 'off', + }, + }, ], }; diff --git a/packages/nodes-base/nodes/Peekalink/GenericFunctions.ts b/packages/nodes-base/nodes/Peekalink/GenericFunctions.ts deleted file mode 100644 index 222b4bac00c..00000000000 --- a/packages/nodes-base/nodes/Peekalink/GenericFunctions.ts +++ /dev/null @@ -1,41 +0,0 @@ -import type { OptionsWithUri } from 'request'; - -import type { - JsonObject, - IExecuteFunctions, - IHookFunctions, - ILoadOptionsFunctions, - IDataObject, -} from 'n8n-workflow'; -import { NodeApiError } from 'n8n-workflow'; - -export async function peekalinkApiRequest( - this: IHookFunctions | IExecuteFunctions | ILoadOptionsFunctions, - method: string, - resource: string, - - body: any = {}, - qs: IDataObject = {}, - uri?: string, - option: IDataObject = {}, -): Promise { - try { - const credentials = await this.getCredentials('peekalinkApi'); - let options: OptionsWithUri = { - headers: { - 'X-API-Key': credentials.apiKey, - }, - method, - qs, - body, - uri: uri || `https://api.peekalink.io${resource}`, - json: true, - }; - - options = Object.assign({}, options, option); - - return await this.helpers.request(options); - } catch (error) { - throw new NodeApiError(this.getNode(), error as JsonObject); - } -} diff --git a/packages/nodes-base/nodes/Peekalink/Peekalink.node.ts b/packages/nodes-base/nodes/Peekalink/Peekalink.node.ts index 920564fb5a4..638ca9f4093 100644 --- a/packages/nodes-base/nodes/Peekalink/Peekalink.node.ts +++ b/packages/nodes-base/nodes/Peekalink/Peekalink.node.ts @@ -1,14 +1,17 @@ -import type { - IExecuteFunctions, - IDataObject, - INodeExecutionData, - INodeType, - INodeTypeDescription, +import { + Node, + NodeApiError, + type IExecuteFunctions, + type INodeExecutionData, + type INodeTypeDescription, + type JsonObject, } from 'n8n-workflow'; -import { peekalinkApiRequest } from './GenericFunctions'; +export const apiUrl = 'https://api.peekalink.io'; -export class Peekalink implements INodeType { +type Operation = 'preview' | 'isAvailable'; + +export class Peekalink extends Node { description: INodeTypeDescription = { displayName: 'Peekalink', name: 'peekalink', @@ -61,44 +64,31 @@ export class Peekalink implements INodeType { ], }; - async execute(this: IExecuteFunctions): Promise { - const items = this.getInputData(); - const returnData: IDataObject[] = []; - const length = items.length; - let responseData; - const operation = this.getNodeParameter('operation', 0); + async execute(context: IExecuteFunctions): Promise { + const items = context.getInputData(); + const operation = context.getNodeParameter('operation', 0) as Operation; + const credentials = await context.getCredentials('peekalinkApi'); - for (let i = 0; i < length; i++) { - try { - if (operation === 'isAvailable') { - const url = this.getNodeParameter('url', i) as string; - const body: IDataObject = { - link: url, - }; - - responseData = await peekalinkApiRequest.call(this, 'POST', '/is-available/', body); + const returnData = await Promise.all( + items.map(async (_, i) => { + try { + const link = context.getNodeParameter('url', i) as string; + // eslint-disable-next-line @typescript-eslint/no-unsafe-return + return await context.helpers.request({ + method: 'POST', + uri: operation === 'preview' ? apiUrl : `${apiUrl}/is-available/`, + body: { link }, + headers: { 'X-API-Key': credentials.apiKey }, + json: true, + }); + } catch (error) { + if (context.continueOnFail()) { + return { error: error.message }; + } + throw new NodeApiError(context.getNode(), error as JsonObject); } - if (operation === 'preview') { - const url = this.getNodeParameter('url', i) as string; - const body: IDataObject = { - link: url, - }; - - responseData = await peekalinkApiRequest.call(this, 'POST', '/', body); - } - if (Array.isArray(responseData)) { - returnData.push.apply(returnData, responseData as IDataObject[]); - } else { - returnData.push(responseData as IDataObject); - } - } catch (error) { - if (this.continueOnFail()) { - returnData.push({ error: error.message }); - continue; - } - throw error; - } - } - return [this.helpers.returnJsonArray(returnData)]; + }), + ); + return [context.helpers.returnJsonArray(returnData)]; } } diff --git a/packages/nodes-base/nodes/Peekalink/test/Peekalink.node.test.ts b/packages/nodes-base/nodes/Peekalink/test/Peekalink.node.test.ts new file mode 100644 index 00000000000..f1262863ce6 --- /dev/null +++ b/packages/nodes-base/nodes/Peekalink/test/Peekalink.node.test.ts @@ -0,0 +1,171 @@ +import { apiUrl } from '../Peekalink.node'; +import type { WorkflowTestData } from '@test/nodes/types'; +import { executeWorkflow } from '@test/nodes/ExecuteWorkflow'; +import * as Helpers from '@test/nodes/Helpers'; + +describe('Peekalink Node', () => { + const exampleComPreview = { + url: 'https://example.com/', + domain: 'example.com', + lastUpdated: '2022-11-13T22:43:20.986744Z', + nextUpdate: '2022-11-20T22:43:20.982384Z', + contentType: 'html', + mimeType: 'text/html', + size: 648, + redirected: false, + title: 'Example Domain', + description: 'This domain is for use in illustrative examples in documents', + name: 'EXAMPLE.COM', + trackersDetected: false, + }; + + const tests: WorkflowTestData[] = [ + { + description: 'should run isAvailable operation', + input: { + workflowData: { + nodes: [ + { + parameters: {}, + id: '8b7bb389-e4ef-424a-bca1-e7ead60e43eb', + name: 'When clicking "Execute Workflow"', + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [740, 380], + }, + { + parameters: { + operation: 'isAvailable', + url: 'https://example.com/', + }, + id: '7354367e-39a7-4fc1-8cdd-442f0b0c7b62', + name: 'Peekalink', + type: 'n8n-nodes-base.peekalink', + typeVersion: 1, + position: [960, 380], + credentials: { + peekalinkApi: 'token', + }, + }, + ], + connections: { + 'When clicking "Execute Workflow"': { + main: [ + [ + { + node: 'Peekalink', + type: 'main', + index: 0, + }, + ], + ], + }, + }, + }, + }, + output: { + nodeExecutionOrder: ['Start'], + nodeData: { + Peekalink: [ + [ + { + json: { + isAvailable: true, + }, + }, + ], + ], + }, + }, + nock: { + baseUrl: apiUrl, + mocks: [ + { + method: 'post', + path: '/is-available/', + statusCode: 200, + responseBody: { isAvailable: true }, + }, + ], + }, + }, + { + description: 'should run preview operation', + input: { + workflowData: { + nodes: [ + { + parameters: {}, + id: '8b7bb389-e4ef-424a-bca1-e7ead60e43eb', + name: 'When clicking "Execute Workflow"', + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [740, 380], + }, + { + parameters: { + operation: 'preview', + url: 'https://example.com/', + }, + id: '7354367e-39a7-4fc1-8cdd-442f0b0c7b62', + name: 'Peekalink', + type: 'n8n-nodes-base.peekalink', + typeVersion: 1, + position: [960, 380], + credentials: { + peekalinkApi: 'token', + }, + }, + ], + connections: { + 'When clicking "Execute Workflow"': { + main: [ + [ + { + node: 'Peekalink', + type: 'main', + index: 0, + }, + ], + ], + }, + }, + }, + }, + output: { + nodeExecutionOrder: ['Start'], + nodeData: { + Peekalink: [ + [ + { + json: exampleComPreview, + }, + ], + ], + }, + }, + nock: { + baseUrl: apiUrl, + mocks: [ + { + method: 'post', + path: '/', + statusCode: 200, + responseBody: exampleComPreview, + }, + ], + }, + }, + ]; + + const nodeTypes = Helpers.setup(tests); + + test.each(tests)('$description', async (testData) => { + const { result } = await executeWorkflow(testData, nodeTypes); + const resultNodeData = Helpers.getResultNodeData(result, testData); + resultNodeData.forEach(({ nodeName, resultData }) => + expect(resultData).toEqual(testData.output.nodeData[nodeName]), + ); + expect(result.finished).toEqual(true); + }); +}); diff --git a/packages/nodes-base/test/nodes/ExecuteWorkflow.ts b/packages/nodes-base/test/nodes/ExecuteWorkflow.ts index a7814f04cd8..528eca40a90 100644 --- a/packages/nodes-base/test/nodes/ExecuteWorkflow.ts +++ b/packages/nodes-base/test/nodes/ExecuteWorkflow.ts @@ -1,3 +1,4 @@ +import nock from 'nock'; import { WorkflowExecute } from 'n8n-core'; import type { INodeTypes, IRun, IRunExecutionData } from 'n8n-workflow'; import { createDeferredPromise, Workflow } from 'n8n-workflow'; @@ -5,6 +6,13 @@ import * as Helpers from './Helpers'; import type { WorkflowTestData } from './types'; export async function executeWorkflow(testData: WorkflowTestData, nodeTypes: INodeTypes) { + if (testData.nock) { + const { baseUrl, mocks } = testData.nock; + const agent = nock(baseUrl); + mocks.forEach(({ method, path, statusCode, responseBody }) => + agent[method](path).reply(statusCode, responseBody), + ); + } const executionMode = testData.trigger?.mode ?? 'manual'; const workflowInstance = new Workflow({ id: 'test', diff --git a/packages/nodes-base/test/nodes/Helpers.ts b/packages/nodes-base/test/nodes/Helpers.ts index be4e7559ce0..9cf4dbe8e67 100644 --- a/packages/nodes-base/test/nodes/Helpers.ts +++ b/packages/nodes-base/test/nodes/Helpers.ts @@ -1,6 +1,7 @@ import { readFileSync, readdirSync, mkdtempSync } from 'fs'; import path from 'path'; import { tmpdir } from 'os'; +import nock from 'nock'; import { isEmpty } from 'lodash'; import { get } from 'lodash'; import { BinaryDataService, Credentials, constructExecutionMetaData } from 'n8n-core'; @@ -231,6 +232,16 @@ export function setup(testData: WorkflowTestData[] | WorkflowTestData) { testData = [testData]; } + if (testData.some((t) => !!t.nock)) { + beforeAll(() => { + nock.disableNetConnect(); + }); + + afterAll(() => { + nock.restore(); + }); + } + const nodeTypes = new NodeTypes(); const nodes = [...new Set(testData.flatMap((data) => data.input.workflowData.nodes))];