From c0d2bac94d732d4166633adf9c3b8eaf5d8046be Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Tue, 25 Jul 2023 11:56:38 +0200 Subject: [PATCH 1/2] feat(core): Add cache service (#6729) * add cache service * PR adjustments * switch to maxSize for memory cache --- packages/cli/package.json | 4 +- packages/cli/src/config/schema.ts | 37 ++++ packages/cli/src/services/cache.service.ts | 175 ++++++++++++++++++ .../test/unit/services/cache.service.test.ts | 137 ++++++++++++++ pnpm-lock.yaml | 45 +++++ 5 files changed, 397 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/services/cache.service.ts create mode 100644 packages/cli/test/unit/services/cache.service.test.ts diff --git a/packages/cli/package.json b/packages/cli/package.json index c57f8639ba..f0d9f7f97e 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -116,6 +116,8 @@ "body-parser": "^1.20.1", "body-parser-xml": "^2.0.3", "bull": "^4.10.2", + "cache-manager": "^5.2.3", + "cache-manager-ioredis-yet": "^1.2.2", "callsites": "^3.1.0", "change-case": "^4.1.1", "class-transformer": "^0.5.1", @@ -163,8 +165,8 @@ "passport-cookie": "^1.0.9", "passport-jwt": "^4.0.0", "pg": "^8.8.0", - "pkce-challenge": "^3.0.0", "picocolors": "^1.0.0", + "pkce-challenge": "^3.0.0", "posthog-node": "^2.2.2", "prom-client": "^13.1.0", "psl": "^1.8.0", diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index cc2eb24e82..fdb5bf04da 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1101,4 +1101,41 @@ export const schema = { }, }, }, + + cache: { + enabled: { + doc: 'Whether caching is enabled', + format: Boolean, + default: true, + env: 'N8N_CACHE_ENABLED', + }, + backend: { + doc: 'Backend to use for caching', + format: ['memory', 'redis', 'auto'] as const, + default: 'auto', + env: 'N8N_CACHE_BACKEND', + }, + memory: { + maxSize: { + doc: 'Maximum size of memory cache in bytes', + format: Number, + default: 3 * 1024 * 1024, // 3 MB + env: 'N8N_CACHE_MEMORY_MAX_SIZE', + }, + ttl: { + doc: 'Time to live for cached items in memory (in ms)', + format: Number, + default: 3600 * 1000, // 1 hour + env: 'N8N_CACHE_MEMORY_TTL', + }, + }, + redis: { + ttl: { + doc: 'Time to live for cached items in redis (in ms), 0 for no TTL', + format: Number, + default: 0, + env: 'N8N_CACHE_REDIS_TTL', + }, + }, + }, }; diff --git a/packages/cli/src/services/cache.service.ts b/packages/cli/src/services/cache.service.ts new file mode 100644 index 0000000000..64d203106c --- /dev/null +++ b/packages/cli/src/services/cache.service.ts @@ -0,0 +1,175 @@ +import { Service } from 'typedi'; +import config from '@/config'; +import { caching } from 'cache-manager'; +import type { MemoryCache } from 'cache-manager'; +import type { RedisCache } from 'cache-manager-ioredis-yet'; +import type { RedisOptions } from 'ioredis'; +import { getRedisClusterNodes } from '../GenericHelpers'; +import { LoggerProxy, jsonStringify } from 'n8n-workflow'; + +@Service() +export class CacheService { + private cache: RedisCache | MemoryCache | undefined; + + async init() { + if (!config.getEnv('cache.enabled')) { + throw new Error('Cache is disabled'); + } + + const backend = config.getEnv('cache.backend'); + + if ( + backend === 'redis' || + (backend === 'auto' && config.getEnv('executions.mode') === 'queue') + ) { + // eslint-disable-next-line @typescript-eslint/naming-convention + const { redisInsStore } = await import('cache-manager-ioredis-yet'); + + // #region TEMPORARY Redis Client Code + /* + * TODO: remove once redis service is ready + * this code is just temporary + */ + // eslint-disable-next-line @typescript-eslint/naming-convention + const { default: Redis } = await import('ioredis'); + let lastTimer = 0; + let cumulativeTimeout = 0; + const { host, port, username, password, db }: RedisOptions = + config.getEnv('queue.bull.redis'); + const clusterNodes = getRedisClusterNodes(); + const redisConnectionTimeoutLimit = config.getEnv('queue.bull.redis.timeoutThreshold'); + const usesRedisCluster = clusterNodes.length > 0; + LoggerProxy.debug( + usesRedisCluster + ? `(Cache Service) Initialising Redis cluster connection with nodes: ${clusterNodes + .map((e) => `${e.host}:${e.port}`) + .join(',')}` + : `(Cache Service) Initialising Redis client connection with host: ${ + host ?? 'localhost' + } and port: ${port ?? '6379'}`, + ); + const sharedRedisOptions: RedisOptions = { + username, + password, + db, + enableReadyCheck: false, + maxRetriesPerRequest: null, + }; + const redisClient = usesRedisCluster + ? new Redis.Cluster( + clusterNodes.map((node) => ({ host: node.host, port: node.port })), + { + redisOptions: sharedRedisOptions, + }, + ) + : new Redis({ + host, + port, + ...sharedRedisOptions, + retryStrategy: (): number | null => { + const now = Date.now(); + if (now - lastTimer > 30000) { + // Means we had no timeout at all or last timeout was temporary and we recovered + lastTimer = now; + cumulativeTimeout = 0; + } else { + cumulativeTimeout += now - lastTimer; + lastTimer = now; + if (cumulativeTimeout > redisConnectionTimeoutLimit) { + LoggerProxy.error( + `Unable to connect to Redis after ${redisConnectionTimeoutLimit}. Exiting process.`, + ); + process.exit(1); + } + } + return 500; + }, + }); + // #endregion TEMPORARY Redis Client Code + const redisStore = redisInsStore(redisClient, { + ttl: config.getEnv('cache.redis.ttl'), + }); + this.cache = await caching(redisStore); + } else { + // using TextEncoder to get the byte length of the string even if it contains unicode characters + const textEncoder = new TextEncoder(); + this.cache = await caching('memory', { + ttl: config.getEnv('cache.memory.ttl'), + maxSize: config.getEnv('cache.memory.maxSize'), + sizeCalculation: (item) => { + return textEncoder.encode(jsonStringify(item, { replaceCircularRefs: true })).length; + }, + }); + } + } + + async destroy() { + if (this.cache) { + await this.reset(); + this.cache = undefined; + } + } + + async getCache(): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache; + } + + async get(key: string): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.get(key) as T; + } + + async set(key: string, value: T, ttl?: number): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.set(key, value, ttl); + } + + async delete(key: string): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.del(key); + } + + async reset(): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.reset(); + } + + async keys(): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.keys() ?? []; + } + + async setMany(values: Array<[string, T]>, ttl?: number): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.mset(values, ttl); + } + + async getMany(keys: string[]): Promise> { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.mget(...keys) as Promise>; + } + + async deleteMany(keys: string[]): Promise { + if (!this.cache) { + await this.init(); + } + return this.cache?.store.mdel(...keys); + } +} diff --git a/packages/cli/test/unit/services/cache.service.test.ts b/packages/cli/test/unit/services/cache.service.test.ts new file mode 100644 index 0000000000..ed68b239d8 --- /dev/null +++ b/packages/cli/test/unit/services/cache.service.test.ts @@ -0,0 +1,137 @@ +import Container from 'typedi'; +import { CacheService } from '@/services/cache.service'; +import type { MemoryCache } from 'cache-manager'; +// import type { RedisCache } from 'cache-manager-ioredis-yet'; +import config from '@/config'; + +const cacheService = Container.get(CacheService); + +function setDefaultConfig() { + config.set('executions.mode', 'regular'); + config.set('cache.backend', 'auto'); + config.set('cache.memory.maxSize', 1 * 1024 * 1024); +} + +describe('cacheService', () => { + beforeEach(async () => { + setDefaultConfig(); + await Container.get(CacheService).destroy(); + }); + + test('should create a memory cache by default', async () => { + await cacheService.init(); + await expect(cacheService.getCache()).resolves.toBeDefined(); + const candidate = (await cacheService.getCache()) as MemoryCache; + // type guard to check that a MemoryCache is returned and not a RedisCache (which does not have a size property) + expect(candidate.store.size).toBeDefined(); + }); + + test('should cache and retrieve a value', async () => { + await cacheService.init(); + await expect(cacheService.getCache()).resolves.toBeDefined(); + await cacheService.set('testString', 'test'); + await cacheService.set('testNumber', 123); + + await expect(cacheService.get('testString')).resolves.toBe('test'); + expect(typeof (await cacheService.get('testString'))).toBe('string'); + await expect(cacheService.get('testNumber')).resolves.toBe(123); + expect(typeof (await cacheService.get('testNumber'))).toBe('number'); + }); + + test('should honour ttl values', async () => { + // set default TTL to 10ms + config.set('cache.memory.ttl', 10); + + await cacheService.set('testString', 'test'); + await cacheService.set('testNumber', 123, 1000); + + const store = (await cacheService.getCache())?.store; + + expect(store).toBeDefined(); + + await expect(store!.ttl('testString')).resolves.toBeLessThanOrEqual(100); + await expect(store!.ttl('testNumber')).resolves.toBeLessThanOrEqual(1000); + + await expect(cacheService.get('testString')).resolves.toBe('test'); + await expect(cacheService.get('testNumber')).resolves.toBe(123); + + await new Promise((resolve) => setTimeout(resolve, 20)); + + await expect(cacheService.get('testString')).resolves.toBeUndefined(); + await expect(cacheService.get('testNumber')).resolves.toBe(123); + }); + + test('should set and remove values', async () => { + await cacheService.set('testString', 'test'); + await expect(cacheService.get('testString')).resolves.toBe('test'); + await cacheService.delete('testString'); + await expect(cacheService.get('testString')).resolves.toBeUndefined(); + }); + + test('should calculate maxSize', async () => { + config.set('cache.memory.maxSize', 16); + await cacheService.destroy(); + + // 16 bytes because stringify wraps the string in quotes, so 2 bytes for the quotes + await cacheService.set('testString', 'withoutUnicode'); + await expect(cacheService.get('testString')).resolves.toBe('withoutUnicode'); + + await cacheService.destroy(); + + // should not fit! + await cacheService.set('testString', 'withUnicodeԱԲԳ'); + await expect(cacheService.get('testString')).resolves.toBeUndefined(); + }); + + test('should set and get complex objects', async () => { + interface TestObject { + test: string; + test2: number; + test3?: TestObject & { test4: TestObject }; + } + + const testObject: TestObject = { + test: 'test', + test2: 123, + test3: { + test: 'test3', + test2: 123, + test4: { + test: 'test4', + test2: 123, + }, + }, + }; + + await cacheService.set('testObject', testObject); + await expect(cacheService.get('testObject')).resolves.toMatchObject(testObject); + }); + + test('should set and get multiple values', async () => { + config.set('executions.mode', 'regular'); + config.set('cache.backend', 'auto'); + + await cacheService.setMany([ + ['testString', 'test'], + ['testString2', 'test2'], + ]); + await cacheService.setMany([ + ['testNumber', 123], + ['testNumber2', 456], + ]); + await expect( + cacheService.getMany(['testString', 'testString2']), + ).resolves.toStrictEqual(['test', 'test2']); + await expect( + cacheService.getMany(['testNumber', 'testNumber2']), + ).resolves.toStrictEqual([123, 456]); + }); + // This test is skipped because it requires the Redis service + // test('should create a redis cache if asked', async () => { + // config.set('cache.backend', 'redis'); + // await cacheService.init(); + // expect(cacheService.getCacheInstance()).toBeDefined(); + // const candidate = cacheService.getCacheInstance() as RedisCache; + // expect(candidate.store.client).toBeDefined(); + // }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dace0742e4..ca1dd00054 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -244,6 +244,12 @@ importers: bull: specifier: ^4.10.2 version: 4.10.2 + cache-manager: + specifier: ^5.2.3 + version: 5.2.3 + cache-manager-ioredis-yet: + specifier: ^1.2.2 + version: 1.2.2 callsites: specifier: ^3.1.0 version: 3.1.0 @@ -9786,6 +9792,23 @@ packages: unset-value: 1.0.0 dev: true + /cache-manager-ioredis-yet@1.2.2: + resolution: {integrity: sha512-o03N/tQxfFONZ1XLGgIxOFHuQQpjpRdnSAL1THG1YWZIVp1JMUfjU3ElSAjFN1LjbJXa55IpC8waG+VEoLUCUw==} + engines: {node: '>= 16.17.0'} + dependencies: + cache-manager: 5.2.3 + ioredis: 5.3.2 + transitivePeerDependencies: + - supports-color + dev: false + + /cache-manager@5.2.3: + resolution: {integrity: sha512-9OErI8fksFkxAMJ8Mco0aiZSdphyd90HcKiOMJQncSlU1yq/9lHHxrT8PDayxrmr9IIIZPOAEfXuGSD7g29uog==} + dependencies: + lodash.clonedeep: 4.5.0 + lru-cache: 9.1.2 + dev: false + /cachedir@2.3.0: resolution: {integrity: sha512-A+Fezp4zxnit6FanDmv9EqXNAi3vt9DWp51/71UEhXukb7QUuvtv9344h91dyAxuTLoSYJFU299qzR3tzwPAhw==} engines: {node: '>=6'} @@ -14213,6 +14236,23 @@ packages: - supports-color dev: false + /ioredis@5.3.2: + resolution: {integrity: sha512-1DKMMzlIHM02eBBVOFQ1+AolGjs6+xEcM4PDL7NqOS6szq7H9jSaEkIUH6/a5Hl241LzW6JLSiAbNvTQjUupUA==} + engines: {node: '>=12.22.0'} + dependencies: + '@ioredis/commands': 1.2.0 + cluster-key-slot: 1.1.1 + debug: 4.3.4(supports-color@8.1.1) + denque: 2.1.0 + lodash.defaults: 4.2.0 + lodash.isarguments: 3.1.0 + redis-errors: 1.2.0 + redis-parser: 3.0.0 + standard-as-callback: 2.1.0 + transitivePeerDependencies: + - supports-color + dev: false + /ip@1.1.8: resolution: {integrity: sha512-PuExPYUiu6qMBQb4l06ecm6T6ujzhmh+MeJcW9wa89PoAz5pvd4zPgN5WJV104mb6S2T1AwNIAaB70JNrLQWhg==} dev: false @@ -16246,6 +16286,11 @@ packages: engines: {node: '>=12'} dev: false + /lru-cache@9.1.2: + resolution: {integrity: sha512-ERJq3FOzJTxBbFjZ7iDs+NiK4VI9Wz+RdrrAB8dio1oV+YvdPzUEE4QNiT2VD51DkIbCYRUUzCRkssXCHqSnKQ==} + engines: {node: 14 || >=16.14} + dev: false + /lru-memoizer@2.1.4: resolution: {integrity: sha512-IXAq50s4qwrOBrXJklY+KhgZF+5y98PDaNo0gi/v2KQBFLyWr+JyFvijZXkGKjQj/h9c0OwoE+JZbwUXce76hQ==} dependencies: From ed09e9c695109a715a4c8d47338bbb0b794eb009 Mon Sep 17 00:00:00 2001 From: OlegIvaniv Date: Tue, 25 Jul 2023 13:26:11 +0200 Subject: [PATCH 2/2] Revert "test(editor): Add canvas actions E2E tests" (#6736) Revert "test(editor): Add canvas actions E2E tests (#6723)" This reverts commit 052d82b2208c1b2e6f62c6004822c6278c15278b. --- cypress/e2e/12-canvas-actions.cy.ts | 36 ------------------- cypress/pages/workflow.ts | 3 -- cypress/support/commands.ts | 30 +++++----------- cypress/support/index.ts | 2 +- .../Node/NodeCreator/ItemTypes/NodeItem.vue | 4 --- 5 files changed, 10 insertions(+), 65 deletions(-) diff --git a/cypress/e2e/12-canvas-actions.cy.ts b/cypress/e2e/12-canvas-actions.cy.ts index 3f23f90484..d336294f48 100644 --- a/cypress/e2e/12-canvas-actions.cy.ts +++ b/cypress/e2e/12-canvas-actions.cy.ts @@ -78,42 +78,6 @@ describe('Canvas Actions', () => { WorkflowPage.getters.nodeConnections().should('have.length', 1); }); - it('should add a connected node dragging from node creator', () => { - WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); - cy.get('.plus-endpoint').should('be.visible').click(); - WorkflowPage.getters.nodeCreatorSearchBar().should('be.visible'); - WorkflowPage.getters.nodeCreatorSearchBar().type(CODE_NODE_NAME); - cy.drag( - WorkflowPage.getters.nodeCreatorNodeItems().first(), - [100, 100], - { - realMouse: true, - abs: true - } - ); - cy.get('body').type('{esc}'); - WorkflowPage.getters.canvasNodes().should('have.length', 2); - WorkflowPage.getters.nodeConnections().should('have.length', 1); - }); - - it('should open a category when trying to drag and drop it on the canvas', () => { - WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); - cy.get('.plus-endpoint').should('be.visible').click(); - WorkflowPage.getters.nodeCreatorSearchBar().should('be.visible'); - WorkflowPage.getters.nodeCreatorSearchBar().type(CODE_NODE_NAME); - cy.drag( - WorkflowPage.getters.nodeCreatorActionItems().first(), - [100, 100], - { - realMouse: true, - abs: true - } - ); - WorkflowPage.getters.nodeCreatorCategoryItems().its('length').should('be.gt', 0); - WorkflowPage.getters.canvasNodes().should('have.length', 1); - WorkflowPage.getters.nodeConnections().should('have.length', 0); - }); - it('should add disconnected node if nothing is selected', () => { WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); // Deselect nodes diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts index e269ce09fd..d324314dcc 100644 --- a/cypress/pages/workflow.ts +++ b/cypress/pages/workflow.ts @@ -96,9 +96,6 @@ export class WorkflowPage extends BasePage { nodeCredentialsSelect: () => cy.getByTestId('node-credentials-select'), nodeCredentialsEditButton: () => cy.getByTestId('credential-edit-button'), nodeCreatorItems: () => cy.getByTestId('item-iterator-item'), - nodeCreatorNodeItems: () => cy.getByTestId('node-creator-node-item'), - nodeCreatorActionItems: () => cy.getByTestId('node-creator-action-item'), - nodeCreatorCategoryItems: () => cy.getByTestId('node-creator-category-item'), ndvParameters: () => cy.getByTestId('parameter-item'), nodeCredentialsLabel: () => cy.getByTestId('credentials-label'), getConnectionBetweenNodes: (sourceNodeName: string, targetNodeName: string) => diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 812404ff7a..6c7adaea8f 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -103,31 +103,19 @@ Cypress.Commands.add('paste', { prevSubject: true }, (selector, pastePayload) => Cypress.Commands.add('drag', (selector, pos, options) => { const index = options?.index || 0; const [xDiff, yDiff] = pos; - const element = typeof selector === 'string' ? cy.get(selector).eq(index) : selector; + const element = cy.get(selector).eq(index); element.should('exist'); - element.then(([$el]) => { - const originalLocation = $el.getBoundingClientRect(); - const newPosition = { - x: options?.abs ? xDiff : originalLocation.x + xDiff, - y: options?.abs ? yDiff : originalLocation.y + yDiff, - } + const originalLocation = Cypress.$(selector)[index].getBoundingClientRect(); - if(options?.realMouse) { - element.realMouseDown(); - element.realMouseMove(newPosition.x, newPosition.y); - element.realMouseUp(); - } else { - element.trigger('mousedown', {force: true}); - element.trigger('mousemove', { - which: 1, - pageX: newPosition.x, - pageY: newPosition.y, - force: true, - }); - element.trigger('mouseup', {force: true}); - } + element.trigger('mousedown', { force: true }); + element.trigger('mousemove', { + which: 1, + pageX: options?.abs ? xDiff : originalLocation.right + xDiff, + pageY: options?.abs ? yDiff : originalLocation.top + yDiff, + force: true, }); + element.trigger('mouseup', { force: true }); }); Cypress.Commands.add('draganddrop', (draggableSelector, droppableSelector) => { diff --git a/cypress/support/index.ts b/cypress/support/index.ts index f1602b3e06..196a14d9ec 100644 --- a/cypress/support/index.ts +++ b/cypress/support/index.ts @@ -31,7 +31,7 @@ declare global { grantBrowserPermissions(...permissions: string[]): void; readClipboard(): Chainable; paste(pastePayload: string): void; - drag(selector: string | Cypress.Chainable>, target: [number, number], options?: {abs?: boolean, index?: number, realMouse?: boolean}): void; + drag(selector: string, target: [number, number], options?: {abs?: true, index?: number}): void; draganddrop(draggableSelector: string, droppableSelector: string): void; } } diff --git a/packages/editor-ui/src/components/Node/NodeCreator/ItemTypes/NodeItem.vue b/packages/editor-ui/src/components/Node/NodeCreator/ItemTypes/NodeItem.vue index 8554a1a080..dd5a427f17 100644 --- a/packages/editor-ui/src/components/Node/NodeCreator/ItemTypes/NodeItem.vue +++ b/packages/editor-ui/src/components/Node/NodeCreator/ItemTypes/NodeItem.vue @@ -9,7 +9,6 @@ :title="displayName" :show-action-arrow="showActionArrow" :is-trigger="isTrigger" - :data-test-id="dataTestId" >