From b30c37b9ce378f714197a5993839c2bcc6287d0a 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: Fri, 28 Apr 2023 10:23:57 +0200 Subject: [PATCH 1/2] create a unified way to check if an object is empty --- packages/core/src/NodeExecuteFunctions.ts | 7 +-- packages/workflow/src/index.ts | 10 ++- packages/workflow/src/utils.ts | 14 +++++ packages/workflow/test/utils.test.ts | 75 ++++++++++++++++++++++- 4 files changed, 99 insertions(+), 7 deletions(-) diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index ec78d8a1dc653..5d86a914a0d08 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -64,6 +64,7 @@ import type { } from 'n8n-workflow'; import { createDeferredPromise, + isObjectEmpty, NodeApiError, NodeHelpers, NodeOperationError, @@ -727,10 +728,6 @@ export async function proxyRequestToAxios( } } -function isIterator(obj: unknown): boolean { - return obj instanceof Object && Symbol.iterator in obj; -} - function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequestConfig { // Destructure properties with the same name first. const { headers, method, timeout, auth, proxy, url } = n8nRequest; @@ -794,7 +791,7 @@ function convertN8nRequestToAxios(n8nRequest: IHttpRequestOptions): AxiosRequest // if there is a body and it's empty (does not have properties), // make sure not to send anything in it as some services fail when // sending GET request with empty body. - if (isIterator(body) || Object.keys(body).length > 0) { + if (typeof body === 'object' && !isObjectEmpty(body)) { axiosRequest.data = body; } } diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index 63e71425fa225..eee0e5f3431a0 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -23,7 +23,15 @@ export * from './WorkflowErrors'; export * from './WorkflowHooks'; export * from './VersionedNodeType'; export { LoggerProxy, NodeHelpers, ObservableObject, TelemetryHelpers }; -export { deepCopy, jsonParse, jsonStringify, sleep, fileTypeFromMimeType, assert } from './utils'; +export { + isObjectEmpty, + deepCopy, + jsonParse, + jsonStringify, + sleep, + fileTypeFromMimeType, + assert, +} from './utils'; export { isINodeProperties, isINodePropertyOptions, diff --git a/packages/workflow/src/utils.ts b/packages/workflow/src/utils.ts index 71d4a50028a35..4db97aac100dc 100644 --- a/packages/workflow/src/utils.ts +++ b/packages/workflow/src/utils.ts @@ -1,5 +1,19 @@ import type { BinaryFileType } from './Interfaces'; +const readStreamClasses = new Set(['ReadStream', 'Readable', 'ReadableStream']); + +export const isObjectEmpty = (obj: object | null | undefined): boolean => { + if (obj === undefined || obj === null) return true; + if (typeof obj === 'object') { + if (Array.isArray(obj)) return obj.length === 0; + if (obj instanceof Set || obj instanceof Map) return obj.size === 0; + if (ArrayBuffer.isView(obj) || obj instanceof ArrayBuffer) return obj.byteLength === 0; + if (Symbol.iterator in obj || readStreamClasses.has(obj.constructor.name)) return false; + return Object.keys(obj).length === 0; + } + return true; +}; + export type Primitives = string | number | boolean | bigint | symbol | null | undefined; /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-argument */ diff --git a/packages/workflow/test/utils.test.ts b/packages/workflow/test/utils.test.ts index 268f270e02624..e26d964976620 100644 --- a/packages/workflow/test/utils.test.ts +++ b/packages/workflow/test/utils.test.ts @@ -1,4 +1,77 @@ -import { jsonParse, jsonStringify, deepCopy } from '@/utils'; +import { jsonParse, jsonStringify, deepCopy, isObjectEmpty } from '@/utils'; + +describe('isObjectEmpty', () => { + it('should handle null and undefined', () => { + expect(isObjectEmpty(null)).toEqual(true); + expect(isObjectEmpty(undefined)).toEqual(true); + }); + + it('should handle arrays', () => { + expect(isObjectEmpty([])).toEqual(true); + expect(isObjectEmpty([1, 2, 3])).toEqual(false); + }); + + it('should handle Set and Map', () => { + expect(isObjectEmpty(new Set())).toEqual(true); + expect(isObjectEmpty(new Set([1, 2, 3]))).toEqual(false); + + expect(isObjectEmpty(new Map())).toEqual(true); + expect( + isObjectEmpty( + new Map([ + ['a', 1], + ['b', 2], + ]), + ), + ).toEqual(false); + }); + + it('should handle Buffer, ArrayBuffer, and Uint8Array', () => { + expect(isObjectEmpty(Buffer.from(''))).toEqual(true); + expect(isObjectEmpty(Buffer.from('abcd'))).toEqual(false); + + expect(isObjectEmpty(Uint8Array.from([]))).toEqual(true); + expect(isObjectEmpty(Uint8Array.from([1, 2, 3]))).toEqual(false); + + expect(isObjectEmpty(new ArrayBuffer(0))).toEqual(true); + expect(isObjectEmpty(new ArrayBuffer(1))).toEqual(false); + }); + + it('should handle plain objects', () => { + expect(isObjectEmpty({})).toEqual(true); + expect(isObjectEmpty({ a: 1, b: 2 })).toEqual(false); + }); + + it('should handle instantiated classes', () => { + expect(isObjectEmpty(new (class Test {})())).toEqual(true); + expect( + isObjectEmpty( + new (class Test { + prop = 123; + })(), + ), + ).toEqual(false); + }); + + it('should not call Object.keys unless a plain object', () => { + const keySpy = jest.spyOn(Object, 'keys'); + const { calls } = keySpy.mock; + + const assertCalls = (count: number) => { + if (calls.length !== count) throw new Error(`Object.keys was called ${calls.length} times`); + }; + + assertCalls(0); + isObjectEmpty(null); + assertCalls(0); + isObjectEmpty([1, 2, 3]); + assertCalls(0); + isObjectEmpty(Buffer.from('123')); + assertCalls(0); + isObjectEmpty({}); + assertCalls(1); + }); +}); describe('jsonParse', () => { it('parses JSON', () => { From 7061534760d8262ad276c11ecbf01de929b87d38 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: Fri, 28 Apr 2023 11:29:32 +0200 Subject: [PATCH 2/2] avoid running `Object.keys` on Buffer objects, to avoid unnecessary memory usage --- packages/nodes-base/credentials/Aws.credentials.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/nodes-base/credentials/Aws.credentials.ts b/packages/nodes-base/credentials/Aws.credentials.ts index 336ee68df3859..35be657f66b60 100644 --- a/packages/nodes-base/credentials/Aws.credentials.ts +++ b/packages/nodes-base/credentials/Aws.credentials.ts @@ -9,6 +9,7 @@ import type { IHttpRequestOptions, INodeProperties, } from 'n8n-workflow'; +import { isObjectEmpty } from 'n8n-workflow'; import type { OptionsWithUri } from 'request'; export const regions = [ @@ -353,7 +354,7 @@ export class Aws implements ICredentialType { }); } - if (body && Object.keys(body).length === 0) { + if (body && typeof body === 'object' && !isObjectEmpty(body)) { body = ''; }