From 724e08562f1fc4ab58bfbc0212f297b87dd95465 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, 20 Dec 2024 18:41:05 +0100 Subject: [PATCH] refactor(core): Deduplicate isObjectLiteral, add docs and tests (#12332) --- .../common/1659888469333-AddJsonKeyPinData.ts | 2 +- packages/cli/src/logging/logger.service.ts | 3 +- .../list-query/dtos/base.filter.dto.ts | 3 +- .../services/credentials-tester.service.ts | 3 +- packages/cli/src/utils.ts | 4 --- .../src/workflow-execute-additional-data.ts | 4 +-- packages/core/src/SerializedBuffer.ts | 6 ++-- packages/core/src/__tests__/utils.test.ts | 35 +++++++++++++++++++ packages/core/src/index.ts | 1 + packages/core/src/utils.ts | 18 ++++++++++ 10 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 packages/core/src/__tests__/utils.test.ts create mode 100644 packages/core/src/utils.ts diff --git a/packages/cli/src/databases/migrations/common/1659888469333-AddJsonKeyPinData.ts b/packages/cli/src/databases/migrations/common/1659888469333-AddJsonKeyPinData.ts index 84d3040a10d7b..75ce65ad6bffe 100644 --- a/packages/cli/src/databases/migrations/common/1659888469333-AddJsonKeyPinData.ts +++ b/packages/cli/src/databases/migrations/common/1659888469333-AddJsonKeyPinData.ts @@ -1,7 +1,7 @@ +import { isObjectLiteral } from 'n8n-core'; import type { IDataObject, INodeExecutionData } from 'n8n-workflow'; import type { MigrationContext, IrreversibleMigration } from '@/databases/types'; -import { isObjectLiteral } from '@/utils'; type OldPinnedData = { [nodeName: string]: IDataObject[] }; type NewPinnedData = { [nodeName: string]: INodeExecutionData[] }; diff --git a/packages/cli/src/logging/logger.service.ts b/packages/cli/src/logging/logger.service.ts index 46471c06112f5..46441e5a339ca 100644 --- a/packages/cli/src/logging/logger.service.ts +++ b/packages/cli/src/logging/logger.service.ts @@ -2,7 +2,7 @@ import type { LogScope } from '@n8n/config'; import { GlobalConfig } from '@n8n/config'; import callsites from 'callsites'; import type { TransformableInfo } from 'logform'; -import { InstanceSettings } from 'n8n-core'; +import { InstanceSettings, isObjectLiteral } from 'n8n-core'; import { LoggerProxy, LOG_LEVELS } from 'n8n-workflow'; import path, { basename } from 'node:path'; import pc from 'picocolors'; @@ -10,7 +10,6 @@ import { Service } from 'typedi'; import winston from 'winston'; import { inDevelopment, inProduction } from '@/constants'; -import { isObjectLiteral } from '@/utils'; import { noOp } from './constants'; import type { LogLocationMetadata, LogLevel, LogMetadata } from './types'; diff --git a/packages/cli/src/middlewares/list-query/dtos/base.filter.dto.ts b/packages/cli/src/middlewares/list-query/dtos/base.filter.dto.ts index 678cb86981ccb..9fa5c216774b3 100644 --- a/packages/cli/src/middlewares/list-query/dtos/base.filter.dto.ts +++ b/packages/cli/src/middlewares/list-query/dtos/base.filter.dto.ts @@ -1,9 +1,8 @@ import { plainToInstance, instanceToPlain } from 'class-transformer'; import { validate } from 'class-validator'; +import { isObjectLiteral } from 'n8n-core'; import { ApplicationError, jsonParse } from 'n8n-workflow'; -import { isObjectLiteral } from '@/utils'; - export class BaseFilter { protected static async toFilter(rawFilter: string, Filter: typeof BaseFilter) { const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); diff --git a/packages/cli/src/services/credentials-tester.service.ts b/packages/cli/src/services/credentials-tester.service.ts index 4a999d6541899..6ae7201ac0558 100644 --- a/packages/cli/src/services/credentials-tester.service.ts +++ b/packages/cli/src/services/credentials-tester.service.ts @@ -4,7 +4,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-return */ /* eslint-disable @typescript-eslint/no-unsafe-call */ import get from 'lodash/get'; -import { ErrorReporter, NodeExecuteFunctions, RoutingNode } from 'n8n-core'; +import { ErrorReporter, NodeExecuteFunctions, RoutingNode, isObjectLiteral } from 'n8n-core'; import type { ICredentialsDecrypted, ICredentialTestFunction, @@ -34,7 +34,6 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da import { RESPONSE_ERROR_MESSAGES } from '../constants'; import { CredentialsHelper } from '../credentials-helper'; -import { isObjectLiteral } from '../utils'; const { OAUTH2_CREDENTIAL_TEST_SUCCEEDED, OAUTH2_CREDENTIAL_TEST_FAILED } = RESPONSE_ERROR_MESSAGES; diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index 700f74f9d068a..d701707a11506 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -58,10 +58,6 @@ export function isStringArray(value: unknown): value is string[] { export const isIntegerString = (value: string) => /^\d+$/.test(value); -export function isObjectLiteral(item: unknown): item is { [key: string]: unknown } { - return typeof item === 'object' && item !== null && !Array.isArray(item); -} - export function removeTrailingSlash(path: string) { return path.endsWith('/') ? path.slice(0, -1) : path; } diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index a97bb3d3faf8f..d020b4d868016 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -5,7 +5,7 @@ import type { PushType } from '@n8n/api-types'; import { GlobalConfig } from '@n8n/config'; import { stringify } from 'flatted'; -import { ErrorReporter, WorkflowExecute } from 'n8n-core'; +import { ErrorReporter, WorkflowExecute, isObjectLiteral } from 'n8n-core'; import { ApplicationError, NodeOperationError, Workflow, WorkflowHooks } from 'n8n-workflow'; import type { IDataObject, @@ -45,7 +45,7 @@ import type { IWorkflowErrorData, UpdateExecutionPayload } from '@/interfaces'; import { NodeTypes } from '@/node-types'; import { Push } from '@/push'; import { WorkflowStatisticsService } from '@/services/workflow-statistics.service'; -import { findSubworkflowStart, isObjectLiteral, isWorkflowIdValid } from '@/utils'; +import { findSubworkflowStart, isWorkflowIdValid } from '@/utils'; import * as WorkflowHelpers from '@/workflow-helpers'; import { WorkflowRepository } from './databases/repositories/workflow.repository'; diff --git a/packages/core/src/SerializedBuffer.ts b/packages/core/src/SerializedBuffer.ts index 48395049b9f02..7a96884729bcf 100644 --- a/packages/core/src/SerializedBuffer.ts +++ b/packages/core/src/SerializedBuffer.ts @@ -1,3 +1,5 @@ +import { isObjectLiteral } from './utils'; + /** A nodejs Buffer gone through JSON.stringify */ export type SerializedBuffer = { type: 'Buffer'; @@ -9,10 +11,6 @@ export function toBuffer(serializedBuffer: SerializedBuffer): Buffer { return Buffer.from(serializedBuffer.data); } -function isObjectLiteral(item: unknown): item is { [key: string]: unknown } { - return typeof item === 'object' && item !== null && !Array.isArray(item); -} - export function isSerializedBuffer(candidate: unknown): candidate is SerializedBuffer { return ( isObjectLiteral(candidate) && diff --git a/packages/core/src/__tests__/utils.test.ts b/packages/core/src/__tests__/utils.test.ts new file mode 100644 index 0000000000000..a8532ed5898ed --- /dev/null +++ b/packages/core/src/__tests__/utils.test.ts @@ -0,0 +1,35 @@ +import { isObjectLiteral } from '@/utils'; + +describe('isObjectLiteral', () => { + test.each([ + ['empty object literal', {}, true], + ['object with properties', { foo: 'bar', num: 123 }, true], + ['nested object literal', { nested: { foo: 'bar' } }, true], + ['object with symbol key', { [Symbol.for('foo')]: 'bar' }, true], + ['null', null, false], + ['empty array', [], false], + ['array with values', [1, 2, 3], false], + ['number', 42, false], + ['string', 'string', false], + ['boolean', true, false], + ['undefined', undefined, false], + ['Date object', new Date(), false], + ['RegExp object', new RegExp(''), false], + ['Map object', new Map(), false], + ['Set object', new Set(), false], + ['arrow function', () => {}, false], + ['regular function', function () {}, false], + ['class instance', new (class TestClass {})(), false], + ['object with custom prototype', Object.create({ customMethod: () => {} }), true], + ['Object.create(null)', Object.create(null), false], + ['Buffer', Buffer.from('test'), false], + ['Serialized Buffer', Buffer.from('test').toJSON(), true], + ['Promise', new Promise(() => {}), false], + ])('should return %s for %s', (_, input, expected) => { + expect(isObjectLiteral(input)).toBe(expected); + }); + + it('should return false for Error objects', () => { + expect(isObjectLiteral(new Error())).toBe(false); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1fc9d77399159..bec9767ffa923 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -25,3 +25,4 @@ export * from './node-execution-context'; export * from './PartialExecutionUtils'; export { ErrorReporter } from './error-reporter'; export * from './SerializedBuffer'; +export { isObjectLiteral } from './utils'; diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts new file mode 100644 index 0000000000000..c0d29c83dc1ba --- /dev/null +++ b/packages/core/src/utils.ts @@ -0,0 +1,18 @@ +type ObjectLiteral = { [key: string | symbol]: unknown }; + +/** + * Checks if the provided value is a plain object literal (not null, not an array, not a class instance, and not a primitive). + * This function serves as a type guard. + * + * @param candidate - The value to check + * @returns {boolean} True if the value is an object literal, false otherwise + */ +export function isObjectLiteral(candidate: unknown): candidate is ObjectLiteral { + return ( + typeof candidate === 'object' && + candidate !== null && + !Array.isArray(candidate) && + // eslint-disable-next-line @typescript-eslint/ban-types + (Object.getPrototypeOf(candidate) as Object)?.constructor?.name === 'Object' + ); +}