From b94031bfa68b1244ae3f5a2a1c2a134fac9c978e Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Thu, 20 Mar 2025 10:27:21 +0100 Subject: [PATCH 01/19] Refactor and handle binary data --- .../get-input-connection-data.test.ts | 134 ++++++++++++++++++ .../utils/get-input-connection-data.ts | 81 +++++++---- 2 files changed, 187 insertions(+), 28 deletions(-) diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/get-input-connection-data.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/get-input-connection-data.test.ts index 4e634a196e7..bad2cb90a52 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/get-input-connection-data.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/get-input-connection-data.test.ts @@ -10,10 +10,12 @@ import type { Workflow, INodeType, INodeTypes, + IExecuteFunctions, } from 'n8n-workflow'; import { NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import { ExecuteContext } from '../../execute-context'; +import { makeHandleToolInvocation } from '../get-input-connection-data'; describe('getInputConnectionData', () => { const agentNode = mock({ @@ -364,3 +366,135 @@ describe('getInputConnectionData', () => { }); }); }); + +describe('makeHandleToolInvocation', () => { + const connectedNode = mock({ + name: 'Test Tool Node', + type: 'test.tool', + }); + const execute = jest.fn(); + const connectedNodeType = mock({ + execute, + }); + const contextFactory = jest.fn(); + const toolArgs = { key: 'value' }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should return stringified results when execution is successful', async () => { + const mockContext = mock(); + contextFactory.mockReturnValue(mockContext); + + const mockResult = [[{ json: { result: 'success' } }]]; + execute.mockResolvedValueOnce(mockResult); + + const handleToolInvocation = makeHandleToolInvocation( + contextFactory, + connectedNode, + connectedNodeType, + ); + const result = await handleToolInvocation(toolArgs); + + expect(result).toBe(JSON.stringify([{ result: 'success' }])); + expect(mockContext.addOutputData).toHaveBeenCalledWith(NodeConnectionType.AiTool, 0, [ + [{ json: { response: [{ result: 'success' }] } }], + ]); + }); + + it('should handle binary data and return a warning message', async () => { + const mockContext = mock(); + contextFactory.mockReturnValue(mockContext); + + const mockResult = [[{ json: {}, binary: { file: 'data' } }]]; + execute.mockResolvedValueOnce(mockResult); + + const handleToolInvocation = makeHandleToolInvocation( + contextFactory, + connectedNode, + connectedNodeType, + ); + const result = await handleToolInvocation(toolArgs); + + expect(result).toBe( + '"Error: The Tool attempted to return binary data, which is not supported in Agents"', + ); + expect(mockContext.addOutputData).toHaveBeenCalledWith(NodeConnectionType.AiTool, 0, [ + [ + { + json: { + response: + 'Error: The Tool attempted to return binary data, which is not supported in Agents', + }, + }, + ], + ]); + }); + + it('should continue if json and binary data exist', async () => { + const mockContext = mock(); + contextFactory.mockReturnValue(mockContext); + + const mockResult = [[{ json: { a: 3 }, binary: { file: 'data' } }]]; + execute.mockResolvedValueOnce(mockResult); + + const handleToolInvocation = makeHandleToolInvocation( + contextFactory, + connectedNode, + connectedNodeType, + ); + const result = await handleToolInvocation(toolArgs); + + expect(result).toBe('[{"a":3}]'); + expect(mockContext.addOutputData).toHaveBeenCalledWith(NodeConnectionType.AiTool, 0, [ + [ + { + json: { + response: [{ a: 3 }], + }, + }, + ], + ]); + }); + + it('should handle execution errors and return an error message', async () => { + const mockContext = mock(); + contextFactory.mockReturnValue(mockContext); + + const error = new Error('Execution failed'); + execute.mockRejectedValueOnce(error); + + const handleToolInvocation = makeHandleToolInvocation( + contextFactory, + connectedNode, + connectedNodeType, + ); + const result = await handleToolInvocation(toolArgs); + + expect(result).toBe('Error during node execution: Execution failed'); + expect(mockContext.addOutputData).toHaveBeenCalledWith( + NodeConnectionType.AiTool, + 0, + expect.any(NodeOperationError), + ); + }); + + it('should increment the toolRunIndex for each invocation', async () => { + const mockContext = mock(); + contextFactory.mockReturnValue(mockContext); + + const handleToolInvocation = makeHandleToolInvocation( + contextFactory, + connectedNode, + connectedNodeType, + ); + + await handleToolInvocation(toolArgs); + await handleToolInvocation(toolArgs); + await handleToolInvocation(toolArgs); + + expect(contextFactory).toHaveBeenCalledWith(0); + expect(contextFactory).toHaveBeenCalledWith(1); + expect(contextFactory).toHaveBeenCalledWith(2); + }); +}); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts b/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts index d599d728410..c6512d5e23f 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts @@ -11,6 +11,10 @@ import type { WorkflowExecuteMode, SupplyData, AINodeConnectionType, + IDataObject, + ISupplyDataFunctions, + INodeType, + INode, } from 'n8n-workflow'; import { NodeConnectionType, @@ -24,6 +28,50 @@ import type { ExecuteContext, WebhookContext } from '../../node-execution-contex // eslint-disable-next-line import/no-cycle import { SupplyDataContext } from '../../node-execution-context/supply-data-context'; +export function makeHandleToolInvocation( + contextFactory: (runIndex: number) => ISupplyDataFunctions, + connectedNode: INode, + connectedNodeType: INodeType, +) { + let toolRunIndex = 0; + return async (toolArgs: IDataObject) => { + const runIndex = toolRunIndex++; + const context = contextFactory(runIndex); + context.addInputData(NodeConnectionType.AiTool, [[{ json: toolArgs }]]); + + try { + // Execute the sub-node with the proxied context + const result = await connectedNodeType.execute?.call(context as unknown as IExecuteFunctions); + + // Process and map the results + const mappedResults = result?.[0]?.flatMap((item) => item.json); + let response: string | typeof mappedResults = mappedResults; + + // Warn if any (unusable) binary data was returned + if (result?.some((x) => x.some((y) => y.binary))) { + if (!mappedResults || mappedResults.flatMap((x) => Object.keys(x ?? {})).length === 0) { + response = + 'Error: The Tool attempted to return binary data, which is not supported in Agents'; + } else { + console.warn( + `Response from Tool '${connectedNode.name}' included binary data, which is not supported in Agents. The binary data was omitted from the response.`, + ); + } + } + + // Add output data to the context + context.addOutputData(NodeConnectionType.AiTool, runIndex, [[{ json: { response } }]]); + + // Return the stringified results + return JSON.stringify(response); + } catch (error) { + const nodeError = new NodeOperationError(connectedNode, error as Error); + context.addOutputData(NodeConnectionType.AiTool, runIndex, nodeError); + return 'Error during node execution: ' + (nodeError.description ?? nodeError.message); + } + }; +} + export async function getInputConnectionData( this: ExecuteContext | WebhookContext | SupplyDataContext, workflow: Workflow, @@ -97,37 +145,14 @@ export async function getInputConnectionData( * This keeps track of how many times this specific AI tool node has been invoked. * It is incremented on every invocation of the tool to keep the output of each invocation separate from each other. */ - let toolRunIndex = 0; const supplyData = createNodeAsTool({ node: connectedNode, nodeType: connectedNodeType, - handleToolInvocation: async (toolArgs) => { - const runIndex = toolRunIndex++; - const context = contextFactory(runIndex, {}); - context.addInputData(NodeConnectionType.AiTool, [[{ json: toolArgs }]]); - - try { - // Execute the sub-node with the proxied context - const result = await connectedNodeType.execute?.call( - context as unknown as IExecuteFunctions, - ); - - // Process and map the results - const mappedResults = result?.[0]?.flatMap((item) => item.json); - - // Add output data to the context - context.addOutputData(NodeConnectionType.AiTool, runIndex, [ - [{ json: { response: mappedResults } }], - ]); - - // Return the stringified results - return JSON.stringify(mappedResults); - } catch (error) { - const nodeError = new NodeOperationError(connectedNode, error as Error); - context.addOutputData(NodeConnectionType.AiTool, runIndex, nodeError); - return 'Error during node execution: ' + nodeError.description; - } - }, + handleToolInvocation: makeHandleToolInvocation( + (i) => contextFactory(i, {}), + connectedNode, + connectedNodeType, + ), }); nodes.push(supplyData); } else { From 286138069c18128a3274c26c2c17b2338ad83dd6 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Thu, 20 Mar 2025 14:27:08 +0100 Subject: [PATCH 02/19] update names --- .../utils/get-input-connection-data.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts b/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts index c6512d5e23f..6f13eddfba9 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts @@ -30,8 +30,8 @@ import { SupplyDataContext } from '../../node-execution-context/supply-data-cont export function makeHandleToolInvocation( contextFactory: (runIndex: number) => ISupplyDataFunctions, - connectedNode: INode, - connectedNodeType: INodeType, + node: INode, + nodeType: INodeType, ) { let toolRunIndex = 0; return async (toolArgs: IDataObject) => { @@ -41,7 +41,7 @@ export function makeHandleToolInvocation( try { // Execute the sub-node with the proxied context - const result = await connectedNodeType.execute?.call(context as unknown as IExecuteFunctions); + const result = await nodeType.execute?.call(context as unknown as IExecuteFunctions); // Process and map the results const mappedResults = result?.[0]?.flatMap((item) => item.json); @@ -54,7 +54,7 @@ export function makeHandleToolInvocation( 'Error: The Tool attempted to return binary data, which is not supported in Agents'; } else { console.warn( - `Response from Tool '${connectedNode.name}' included binary data, which is not supported in Agents. The binary data was omitted from the response.`, + `Response from Tool '${node.name}' included binary data, which is not supported in Agents. The binary data was omitted from the response.`, ); } } @@ -65,7 +65,7 @@ export function makeHandleToolInvocation( // Return the stringified results return JSON.stringify(response); } catch (error) { - const nodeError = new NodeOperationError(connectedNode, error as Error); + const nodeError = new NodeOperationError(node, error as Error); context.addOutputData(NodeConnectionType.AiTool, runIndex, nodeError); return 'Error during node execution: ' + (nodeError.description ?? nodeError.message); } From e06c552a6a0471ec60862247f6a597b8ab5f9cd3 Mon Sep 17 00:00:00 2001 From: oleg Date: Thu, 20 Mar 2025 14:44:21 +0100 Subject: [PATCH 03/19] feat(Simple Vector Store Node): Implement store cleaning based on age/used memory (#13986) --- .../VectorStoreInMemory.node.ts | 8 +- .../VectorStoreInMemoryInsert.node.ts | 4 +- .../VectorStoreInMemoryLoad.node.ts | 4 +- .../shared/MemoryManager/MemoryCalculator.ts | 89 +++++ .../MemoryManager/MemoryVectorStoreManager.ts | 311 ++++++++++++++++++ .../MemoryManager/StoreCleanupService.ts | 157 +++++++++ .../shared/MemoryManager/config.ts | 51 +++ .../test/MemoryCalculator.test.ts | 202 ++++++++++++ .../test/MemoryVectorStoreManager.test.ts | 249 ++++++++++++++ .../test/StoreCleanupService.test.ts | 289 ++++++++++++++++ .../shared/MemoryManager/test/config.test.ts | 74 +++++ .../shared/MemoryManager/types.ts | 70 ++++ .../shared/MemoryVectorStoreManager.test.ts | 44 --- .../shared/MemoryVectorStoreManager.ts | 52 --- 14 files changed, 1500 insertions(+), 104 deletions(-) create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/MemoryCalculator.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/MemoryVectorStoreManager.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/StoreCleanupService.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/config.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryCalculator.test.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryVectorStoreManager.test.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/StoreCleanupService.test.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/config.test.ts create mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/types.ts delete mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.test.ts delete mode 100644 packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.ts diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts index 1ba321ebce9..2f949db872c 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts @@ -2,12 +2,12 @@ import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; import type { INodeProperties } from 'n8n-workflow'; import { createVectorStoreNode } from '../shared/createVectorStoreNode/createVectorStoreNode'; -import { MemoryVectorStoreManager } from '../shared/MemoryVectorStoreManager'; +import { MemoryVectorStoreManager } from '../shared/MemoryManager/MemoryVectorStoreManager'; const insertFields: INodeProperties[] = [ { displayName: - 'The embedded data are stored in the server memory, so they will be lost when the server is restarted. Additionally, if the amount of data is too large, it may cause the server to crash due to insufficient memory.', + 'For experimental use only: Data is stored in memory and will be lost if n8n restarts. Data may also be cleared if available memory gets low. More info', name: 'notice', type: 'notice', default: '', @@ -48,7 +48,7 @@ export class VectorStoreInMemory extends createVectorStoreNode 0) { + // For each key, estimate the key name plus a typical value + // plus some overhead for object structure + totalMetadataSize += metadataKeys * AVG_METADATA_SIZE_BYTES; + } + } + } + + // Fixed size components (embedding vectors and overhead) + // Each embedding is a fixed-size array of floating point numbers + const embeddingSize = documents.length * EMBEDDING_SIZE_BYTES; + + // Object overhead, each vector is stored with additional JS object structure + const overhead = documents.length * VECTOR_OVERHEAD_BYTES; + + // Calculate total batch size with a safety factor to avoid underestimation + const calculatedSize = totalContentSize + totalMetadataSize + embeddingSize + overhead; + + return Math.ceil(calculatedSize); + } + + /** + * Calculate the size of a vector store by examining its contents + */ + calculateVectorStoreSize(vectorStore: MemoryVectorStore): number { + if (!vectorStore.memoryVectors || vectorStore.memoryVectors.length === 0) { + return 0; + } + + let storeSize = 0; + + // Calculate size of each vector + for (const vector of vectorStore.memoryVectors) { + // Size of embedding (float64 array) + storeSize += vector.embedding.length * FLOAT_SIZE_BYTES; + + // Size of content string (2 bytes per character in JS) + storeSize += vector.content ? vector.content.length * CHAR_SIZE_BYTES : 0; + + // Estimate metadata size + if (vector.metadata) { + // Use a more accurate calculation for metadata + const metadataStr = JSON.stringify(vector.metadata); + storeSize += metadataStr.length * CHAR_SIZE_BYTES; + } + + // Add overhead for object structure + storeSize += VECTOR_OVERHEAD_BYTES; + } + + return Math.ceil(storeSize); + } +} diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/MemoryVectorStoreManager.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/MemoryVectorStoreManager.ts new file mode 100644 index 00000000000..7fad05bce7a --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/MemoryVectorStoreManager.ts @@ -0,0 +1,311 @@ +import type { Document } from '@langchain/core/documents'; +import type { Embeddings } from '@langchain/core/embeddings'; +import { MemoryVectorStore } from 'langchain/vectorstores/memory'; +import type { Logger } from 'n8n-workflow'; + +import { getConfig, mbToBytes, hoursToMs } from './config'; +import { MemoryCalculator } from './MemoryCalculator'; +import { StoreCleanupService } from './StoreCleanupService'; +import type { VectorStoreMetadata, VectorStoreStats } from './types'; + +/** + * Manages in-memory vector stores with memory limits and auto-cleanup + */ +export class MemoryVectorStoreManager { + private static instance: MemoryVectorStoreManager | null = null; + + // Storage + protected vectorStoreBuffer: Map; + + protected storeMetadata: Map; + + protected memoryUsageBytes: number = 0; + + // Dependencies + protected memoryCalculator: MemoryCalculator; + + protected cleanupService: StoreCleanupService; + + protected static logger: Logger; + + // Config values + protected maxMemorySizeBytes: number; + + protected inactiveTtlMs: number; + + // Inactive TTL cleanup timer + protected ttlCleanupIntervalId: NodeJS.Timeout | null = null; + + protected constructor( + protected embeddings: Embeddings, + protected logger: Logger, + ) { + // Initialize storage + this.vectorStoreBuffer = new Map(); + this.storeMetadata = new Map(); + this.logger = logger; + + const config = getConfig(); + this.maxMemorySizeBytes = mbToBytes(config.maxMemoryMB); + this.inactiveTtlMs = hoursToMs(config.ttlHours); + + // Initialize services + this.memoryCalculator = new MemoryCalculator(); + this.cleanupService = new StoreCleanupService( + this.maxMemorySizeBytes, + this.inactiveTtlMs, + this.vectorStoreBuffer, + this.storeMetadata, + this.handleCleanup.bind(this), + ); + + this.setupTtlCleanup(); + } + + /** + * Get singleton instance + */ + static getInstance(embeddings: Embeddings, logger: Logger): MemoryVectorStoreManager { + if (!MemoryVectorStoreManager.instance) { + MemoryVectorStoreManager.instance = new MemoryVectorStoreManager(embeddings, logger); + } else { + // We need to update the embeddings in the existing instance. + // This is important as embeddings instance is wrapped in a logWrapper, + // which relies on supplyDataFunctions context which changes on each workflow run + MemoryVectorStoreManager.instance.embeddings = embeddings; + MemoryVectorStoreManager.instance.vectorStoreBuffer.forEach((vectorStoreInstance) => { + vectorStoreInstance.embeddings = embeddings; + }); + } + + return MemoryVectorStoreManager.instance; + } + + /** + * Set up timer for TTL-based cleanup + */ + private setupTtlCleanup(): void { + // Skip setup if TTL is disabled + if (this.inactiveTtlMs <= 0) { + return; + } + + // Cleanup check interval (run every hour) + const CLEANUP_INTERVAL_MS = 60 * 60 * 1000; + + // Clear any existing interval + if (this.ttlCleanupIntervalId) { + clearInterval(this.ttlCleanupIntervalId); + } + + // Setup new interval for TTL cleanup + this.ttlCleanupIntervalId = setInterval(() => { + this.cleanupService.cleanupInactiveStores(); + }, CLEANUP_INTERVAL_MS); + } + + /** + * Handle cleanup events from the cleanup service + */ + private handleCleanup(removedKeys: string[], freedBytes: number, reason: 'ttl' | 'memory'): void { + // Update total memory usage + this.memoryUsageBytes -= freedBytes; + + // Log cleanup event + if (reason === 'ttl') { + const ttlHours = Math.round(this.inactiveTtlMs / (60 * 60 * 1000)); + this.logger.info( + `TTL cleanup: removed ${removedKeys.length} inactive vector stores (${ttlHours}h TTL) to free ${Math.round(freedBytes / (1024 * 1024))}MB of memory`, + ); + } else { + this.logger.info( + `Memory cleanup: removed ${removedKeys.length} oldest vector stores to free ${Math.round(freedBytes / (1024 * 1024))}MB of memory`, + ); + } + } + + /** + * Get or create a vector store by key + */ + async getVectorStore(memoryKey: string): Promise { + let vectorStoreInstance = this.vectorStoreBuffer.get(memoryKey); + + if (!vectorStoreInstance) { + vectorStoreInstance = await MemoryVectorStore.fromExistingIndex(this.embeddings); + this.vectorStoreBuffer.set(memoryKey, vectorStoreInstance); + + this.storeMetadata.set(memoryKey, { + size: 0, + createdAt: new Date(), + lastAccessed: new Date(), + }); + } else { + const metadata = this.storeMetadata.get(memoryKey); + if (metadata) { + metadata.lastAccessed = new Date(); + } + } + + return vectorStoreInstance; + } + + /** + * Reset a store's metadata when it's cleared + */ + protected clearStoreMetadata(memoryKey: string): void { + const metadata = this.storeMetadata.get(memoryKey); + if (metadata) { + this.memoryUsageBytes -= metadata.size; + metadata.size = 0; + metadata.lastAccessed = new Date(); + } + } + + /** + * Get memory usage in bytes + */ + getMemoryUsage(): number { + return this.memoryUsageBytes; + } + + /** + * Get memory usage as a formatted string (MB) + */ + getMemoryUsageFormatted(): string { + return `${Math.round(this.memoryUsageBytes / (1024 * 1024))}MB`; + } + + /** + * Recalculate memory usage from actual vector store contents + * This ensures tracking accuracy for large stores + */ + recalculateMemoryUsage(): void { + this.memoryUsageBytes = 0; + + // Recalculate for each store + for (const [key, vectorStore] of this.vectorStoreBuffer.entries()) { + const storeSize = this.memoryCalculator.calculateVectorStoreSize(vectorStore); + + // Update metadata + const metadata = this.storeMetadata.get(key); + if (metadata) { + metadata.size = storeSize; + this.memoryUsageBytes += storeSize; + } + } + + this.logger.debug(`Recalculated vector store memory: ${this.getMemoryUsageFormatted()}`); + } + + /** + * Add documents to a vector store + */ + async addDocuments( + memoryKey: string, + documents: Document[], + clearStore?: boolean, + ): Promise { + if (clearStore) { + this.clearStoreMetadata(memoryKey); + this.vectorStoreBuffer.delete(memoryKey); + } + + // Fast batch estimation instead of per-document calculation + const estimatedAddedSize = this.memoryCalculator.estimateBatchSize(documents); + + // Clean up old stores if necessary + this.cleanupService.cleanupOldestStores(estimatedAddedSize); + + const vectorStoreInstance = await this.getVectorStore(memoryKey); + + // Get vector count before adding documents + const vectorCountBefore = vectorStoreInstance.memoryVectors?.length || 0; + + await vectorStoreInstance.addDocuments(documents); + + // Update store metadata and memory tracking + const metadata = this.storeMetadata.get(memoryKey); + if (metadata) { + metadata.size += estimatedAddedSize; + metadata.lastAccessed = new Date(); + this.memoryUsageBytes += estimatedAddedSize; + } + + // Get updated vector count + const vectorCount = vectorStoreInstance.memoryVectors?.length || 0; + + // Periodically recalculate actual memory usage to avoid drift + if ( + (vectorCount > 0 && vectorCount % 100 === 0) || + documents.length > 20 || + (vectorCountBefore === 0 && vectorCount > 0) + ) { + this.recalculateMemoryUsage(); + } + + // Logging memory usage + const maxMemoryMB = + this.maxMemorySizeBytes > 0 + ? (this.maxMemorySizeBytes / (1024 * 1024)).toFixed(0) + : 'unlimited'; + + this.logger.debug( + `Vector store memory: ${this.getMemoryUsageFormatted()}/${maxMemoryMB}MB (${vectorCount} vectors in ${this.vectorStoreBuffer.size} stores)`, + ); + } + + /** + * Get statistics about the vector store memory usage + */ + getStats(): VectorStoreStats { + const now = Date.now(); + let inactiveStoreCount = 0; + + // Always recalculate when getting stats to ensure accuracy + this.recalculateMemoryUsage(); + + const stats: VectorStoreStats = { + totalSizeBytes: this.memoryUsageBytes, + totalSizeMB: Math.round((this.memoryUsageBytes / (1024 * 1024)) * 100) / 100, + percentOfLimit: + this.maxMemorySizeBytes > 0 + ? Math.round((this.memoryUsageBytes / this.maxMemorySizeBytes) * 100) + : 0, + maxMemoryMB: this.maxMemorySizeBytes > 0 ? this.maxMemorySizeBytes / (1024 * 1024) : -1, // -1 indicates unlimited + storeCount: this.vectorStoreBuffer.size, + inactiveStoreCount: 0, + ttlHours: this.inactiveTtlMs > 0 ? this.inactiveTtlMs / (60 * 60 * 1000) : -1, // -1 indicates disabled + stores: {}, + }; + + // Add stats for each store + for (const [key, metadata] of this.storeMetadata.entries()) { + const store = this.vectorStoreBuffer.get(key); + + if (store) { + const lastAccessedTime = metadata.lastAccessed.getTime(); + const inactiveTimeMs = now - lastAccessedTime; + const isInactive = this.cleanupService.isStoreInactive(metadata); + + if (isInactive) { + inactiveStoreCount++; + } + + stats.stores[key] = { + sizeBytes: metadata.size, + sizeMB: Math.round((metadata.size / (1024 * 1024)) * 100) / 100, + percentOfTotal: Math.round((metadata.size / this.memoryUsageBytes) * 100) || 0, + vectors: store.memoryVectors?.length || 0, + createdAt: metadata.createdAt.toISOString(), + lastAccessed: metadata.lastAccessed.toISOString(), + inactive: isInactive, + inactiveForHours: Math.round(inactiveTimeMs / (60 * 60 * 1000)), + }; + } + } + + stats.inactiveStoreCount = inactiveStoreCount; + + return stats; + } +} diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/StoreCleanupService.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/StoreCleanupService.ts new file mode 100644 index 00000000000..3fb1d85e3a7 --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/StoreCleanupService.ts @@ -0,0 +1,157 @@ +import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; + +import type { VectorStoreMetadata, IStoreCleanupService } from './types'; + +/** + * Service for cleaning up vector stores based on inactivity or memory pressure + */ +export class StoreCleanupService implements IStoreCleanupService { + // Cache for oldest stores sorted by creation time + private oldestStoreKeys: string[] = []; + + private lastSortTime = 0; + + private readonly CACHE_TTL_MS = 5000; // 5 seconds + + constructor( + private readonly maxMemorySizeBytes: number, + private readonly inactiveTtlMs: number, + private readonly vectorStores: Map, + private readonly storeMetadata: Map, + private readonly onCleanup: ( + removedKeys: string[], + freedBytes: number, + reason: 'ttl' | 'memory', + ) => void, + ) {} + + /** + * Check if a store has been inactive for longer than the TTL + */ + isStoreInactive(metadata: VectorStoreMetadata): boolean { + // If TTL is disabled, nothing is considered inactive + if (this.inactiveTtlMs <= 0) { + return false; + } + + const now = Date.now(); + const lastAccessedTime = metadata.lastAccessed.getTime(); + return now - lastAccessedTime > this.inactiveTtlMs; + } + + /** + * Remove vector stores that haven't been accessed for longer than TTL + */ + cleanupInactiveStores(): void { + // Skip if TTL is disabled + if (this.inactiveTtlMs <= 0) { + return; + } + + let freedBytes = 0; + const removedStores: string[] = []; + + // Find and remove inactive stores + for (const [key, metadata] of this.storeMetadata.entries()) { + if (this.isStoreInactive(metadata)) { + // Remove this inactive store + this.vectorStores.delete(key); + freedBytes += metadata.size; + removedStores.push(key); + } + } + + // Remove from metadata after iteration to avoid concurrent modification + for (const key of removedStores) { + this.storeMetadata.delete(key); + } + + // Invalidate cache if we removed any stores + if (removedStores.length > 0) { + this.oldestStoreKeys = []; + this.onCleanup(removedStores, freedBytes, 'ttl'); + } + } + + /** + * Remove the oldest vector stores to free up memory + */ + cleanupOldestStores(requiredBytes: number): void { + // Skip if memory limit is disabled + if (this.maxMemorySizeBytes <= 0) { + return; + } + + // Calculate current total memory usage + let currentMemoryUsage = 0; + for (const metadata of this.storeMetadata.values()) { + currentMemoryUsage += metadata.size; + } + + // First, try to clean up inactive stores + this.cleanupInactiveStores(); + + // Recalculate memory usage after inactive cleanup + currentMemoryUsage = 0; + for (const metadata of this.storeMetadata.values()) { + currentMemoryUsage += metadata.size; + } + + // If no more cleanup needed, return early + if (currentMemoryUsage + requiredBytes <= this.maxMemorySizeBytes) { + return; + } + + const now = Date.now(); + + // Reuse cached ordering if available and not stale + if (this.oldestStoreKeys.length === 0 || now - this.lastSortTime > this.CACHE_TTL_MS) { + // Collect and sort store keys by age + const stores: Array<[string, number]> = []; + + for (const [key, metadata] of this.storeMetadata.entries()) { + stores.push([key, metadata.createdAt.getTime()]); + } + + // Sort by creation time (oldest first) + stores.sort((a, b) => a[1] - b[1]); + + // Extract just the keys + this.oldestStoreKeys = stores.map(([key]) => key); + this.lastSortTime = now; + } + + let freedBytes = 0; + const removedStores: string[] = []; + + // Remove stores in order until we have enough space + for (const key of this.oldestStoreKeys) { + // Skip if store no longer exists + if (!this.storeMetadata.has(key)) continue; + + // Stop if we've freed enough space + if (currentMemoryUsage - freedBytes + requiredBytes <= this.maxMemorySizeBytes) { + break; + } + + const metadata = this.storeMetadata.get(key); + if (metadata) { + this.vectorStores.delete(key); + freedBytes += metadata.size; + removedStores.push(key); + } + } + + // Remove from metadata after iteration to avoid concurrent modification + for (const key of removedStores) { + this.storeMetadata.delete(key); + } + + // Update our cache if we removed stores + if (removedStores.length > 0) { + // Filter out removed stores from cached keys + this.oldestStoreKeys = this.oldestStoreKeys.filter((key) => !removedStores.includes(key)); + this.onCleanup(removedStores, freedBytes, 'memory'); + } + } +} diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/config.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/config.ts new file mode 100644 index 00000000000..62af65b0fc1 --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/config.ts @@ -0,0 +1,51 @@ +import type { MemoryVectorStoreConfig } from './types'; + +// Defaults +const DEFAULT_MAX_MEMORY_MB = -1; +const DEFAULT_INACTIVE_TTL_HOURS = -1; + +/** + * Helper function to get the configuration from environment variables + */ +export function getConfig(): MemoryVectorStoreConfig { + // Get memory limit from env var or use default + let maxMemoryMB = DEFAULT_MAX_MEMORY_MB; + if (process.env.N8N_VECTOR_STORE_MAX_MEMORY) { + const parsed = parseInt(process.env.N8N_VECTOR_STORE_MAX_MEMORY, 10); + if (!isNaN(parsed)) { + maxMemoryMB = parsed; + } + } + + // Get TTL from env var or use default + let ttlHours = DEFAULT_INACTIVE_TTL_HOURS; + if (process.env.N8N_VECTOR_STORE_TTL_HOURS) { + const parsed = parseInt(process.env.N8N_VECTOR_STORE_TTL_HOURS, 10); + if (!isNaN(parsed)) { + ttlHours = parsed; + } + } + + return { + maxMemoryMB, + ttlHours, + }; +} + +/** + * Convert memory size from MB to bytes + */ +export function mbToBytes(mb: number): number { + // -1 - "unlimited" + if (mb <= 0) return -1; + return mb * 1024 * 1024; +} + +/** + * Convert TTL from hours to milliseconds + */ +export function hoursToMs(hours: number): number { + // -1 - "disabled" + if (hours <= 0) return -1; + return hours * 60 * 60 * 1000; +} diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryCalculator.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryCalculator.test.ts new file mode 100644 index 00000000000..e8438a1d58b --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryCalculator.test.ts @@ -0,0 +1,202 @@ +import { Document } from '@langchain/core/documents'; +import { mock } from 'jest-mock-extended'; +import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; + +import { MemoryCalculator } from '../MemoryCalculator'; + +function createTestEmbedding(dimensions = 1536, initialValue = 0.1, multiplier = 1): number[] { + return new Array(dimensions).fill(initialValue).map((value) => value * multiplier); +} + +describe('MemoryCalculator', () => { + let calculator: MemoryCalculator; + + beforeEach(() => { + calculator = new MemoryCalculator(); + }); + + describe('estimateBatchSize', () => { + it('should return 0 for empty document arrays', () => { + const size = calculator.estimateBatchSize([]); + expect(size).toBe(0); + }); + + it('should calculate size for simple documents', () => { + const documents = [ + new Document({ pageContent: 'Hello, world!', metadata: { simple: 'value' } }), + ]; + + const size = calculator.estimateBatchSize(documents); + + expect(size).toBeGreaterThan(0); + + // The size should account for the content, metadata, embedding size, and overhead + const simpleCase = calculator.estimateBatchSize([ + new Document({ pageContent: '', metadata: {} }), + ]); + const withContent = calculator.estimateBatchSize([ + new Document({ pageContent: 'test content', metadata: {} }), + ]); + const withMetadata = calculator.estimateBatchSize([ + new Document({ pageContent: '', metadata: { key: 'value' } }), + ]); + + // Content should increase size + expect(withContent).toBeGreaterThan(simpleCase); + + // Metadata should increase size + expect(withMetadata).toBeGreaterThan(simpleCase); + }); + + it('should account for content length in size calculation', () => { + const shortDoc = new Document({ + pageContent: 'Short content', + metadata: {}, + }); + + const longDoc = new Document({ + pageContent: 'A'.repeat(1000), + metadata: {}, + }); + + const shortSize = calculator.estimateBatchSize([shortDoc]); + const longSize = calculator.estimateBatchSize([longDoc]); + + // Long content should result in a larger size estimate + expect(longSize).toBeGreaterThan(shortSize); + expect(longSize - shortSize).toBeGreaterThan(1000); + }); + + it('should account for metadata complexity in size calculation', () => { + const simpleMetadata = new Document({ + pageContent: '', + metadata: { simple: 'value' }, + }); + + const complexMetadata = new Document({ + pageContent: '', + metadata: { + nested: { + objects: { + with: { + many: { + levels: [1, 2, 3, 4, 5], + andArray: ['a', 'b', 'c', 'd', 'e'], + }, + }, + }, + }, + moreKeys: 'moreValues', + evenMore: 'data', + }, + }); + + const simpleSize = calculator.estimateBatchSize([simpleMetadata]); + const complexSize = calculator.estimateBatchSize([complexMetadata]); + + // Complex metadata should result in a larger size estimate + expect(complexSize).toBeGreaterThan(simpleSize); + }); + + it('should scale with the number of documents', () => { + const doc = new Document({ pageContent: 'Sample content', metadata: { key: 'value' } }); + + const singleSize = calculator.estimateBatchSize([doc]); + const doubleSize = calculator.estimateBatchSize([doc, doc]); + const tripleSize = calculator.estimateBatchSize([doc, doc, doc]); + + // Size should scale roughly linearly with document count + expect(doubleSize).toBeGreaterThan(singleSize * 1.5); // Allow for some overhead + expect(tripleSize).toBeGreaterThan(doubleSize * 1.3); // Allow for some overhead + }); + }); + + describe('calculateVectorStoreSize', () => { + it('should return 0 for empty vector stores', () => { + const mockVectorStore = mock(); + + const size = calculator.calculateVectorStoreSize(mockVectorStore); + expect(size).toBe(0); + }); + + it('should calculate size for vector stores with content', () => { + const mockVectorStore = mock(); + mockVectorStore.memoryVectors = [ + { + embedding: createTestEmbedding(), // Using the helper function + content: 'Document content', + metadata: { simple: 'value' }, + }, + ]; + + const size = calculator.calculateVectorStoreSize(mockVectorStore); + + // Size should account for the embedding, content, metadata, and overhead + expect(size).toBeGreaterThan(1536 * 8); // At least the size of the embedding in bytes + }); + + it('should account for vector count in size calculation', () => { + const singleVector = mock(); + singleVector.memoryVectors = [ + { + embedding: createTestEmbedding(), + content: 'Content', + metadata: {}, + }, + ]; + + const multiVector = mock(); + multiVector.memoryVectors = [ + { + embedding: createTestEmbedding(), + content: 'Content', + metadata: {}, + }, + { + embedding: createTestEmbedding(), + content: 'Content', + metadata: {}, + }, + { + embedding: createTestEmbedding(), + content: 'Content', + metadata: {}, + }, + ]; + + const singleSize = calculator.calculateVectorStoreSize(singleVector); + const multiSize = calculator.calculateVectorStoreSize(multiVector); + + // Multi-vector store should be about 3x the size + expect(multiSize).toBeGreaterThan(singleSize * 2.5); + expect(multiSize).toBeLessThan(singleSize * 3.5); + }); + + it('should handle vectors with no content or metadata', () => { + const vectorStore = mock(); + vectorStore.memoryVectors = [ + { + embedding: createTestEmbedding(), + content: '', + metadata: {}, + }, + ]; + + const size = calculator.calculateVectorStoreSize(vectorStore); + + // Size should still be positive (at least the embedding size) + expect(size).toBeGreaterThan(1536 * 8); + }); + + it('should handle null or undefined vector arrays', () => { + const nullVectorStore = mock(); + nullVectorStore.memoryVectors = []; + + const undefinedVectorStore = mock(); + undefinedVectorStore.memoryVectors = []; + + expect(calculator.calculateVectorStoreSize(nullVectorStore)).toBe(0); + expect(calculator.calculateVectorStoreSize(undefinedVectorStore)).toBe(0); + }); + }); +}); diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryVectorStoreManager.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryVectorStoreManager.test.ts new file mode 100644 index 00000000000..f8e82217c0b --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/MemoryVectorStoreManager.test.ts @@ -0,0 +1,249 @@ +/* eslint-disable @typescript-eslint/dot-notation */ +import { Document } from '@langchain/core/documents'; +import type { OpenAIEmbeddings } from '@langchain/openai'; +import { mock } from 'jest-mock-extended'; +import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; +import type { Logger } from 'n8n-workflow'; + +import * as configModule from '../config'; +import { MemoryVectorStoreManager } from '../MemoryVectorStoreManager'; + +function createTestEmbedding(dimensions = 1536, initialValue = 0.1, multiplier = 1): number[] { + return new Array(dimensions).fill(initialValue).map((value) => value * multiplier); +} + +jest.mock('langchain/vectorstores/memory', () => { + return { + MemoryVectorStore: { + fromExistingIndex: jest.fn().mockImplementation(() => { + return { + embeddings: null, + addDocuments: jest.fn(), + memoryVectors: [], + }; + }), + }, + }; +}); + +describe('MemoryVectorStoreManager', () => { + let logger: Logger; + // Reset the singleton instance before each test + beforeEach(() => { + jest.clearAllMocks(); + logger = mock(); + MemoryVectorStoreManager['instance'] = null; + + jest.useFakeTimers(); + + // Mock the config + jest.spyOn(configModule, 'getConfig').mockReturnValue({ + maxMemoryMB: 100, + ttlHours: 168, + }); + }); + + afterEach(() => { + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + }); + + it('should create an instance of MemoryVectorStoreManager', () => { + const embeddings = mock(); + + const instance = MemoryVectorStoreManager.getInstance(embeddings, logger); + expect(instance).toBeInstanceOf(MemoryVectorStoreManager); + }); + + it('should return existing instance', () => { + const embeddings = mock(); + + const instance1 = MemoryVectorStoreManager.getInstance(embeddings, logger); + const instance2 = MemoryVectorStoreManager.getInstance(embeddings, logger); + expect(instance1).toBe(instance2); + }); + + it('should update embeddings in existing instance', () => { + const embeddings1 = mock(); + const embeddings2 = mock(); + + const instance = MemoryVectorStoreManager.getInstance(embeddings1, logger); + MemoryVectorStoreManager.getInstance(embeddings2, logger); + + expect(instance['embeddings']).toBe(embeddings2); + }); + + it('should update embeddings in existing vector store instances', async () => { + const embeddings1 = mock(); + const embeddings2 = mock(); + + const instance1 = MemoryVectorStoreManager.getInstance(embeddings1, logger); + await instance1.getVectorStore('test'); + + const instance2 = MemoryVectorStoreManager.getInstance(embeddings2, logger); + const vectorStoreInstance2 = await instance2.getVectorStore('test'); + + expect(vectorStoreInstance2.embeddings).toBe(embeddings2); + }); + + it('should set up the TTL cleanup interval', () => { + jest.spyOn(global, 'setInterval'); + const embeddings = mock(); + + MemoryVectorStoreManager.getInstance(embeddings, logger); + + expect(setInterval).toHaveBeenCalled(); + }); + + it('should not set up the TTL cleanup interval when TTL is disabled', () => { + jest.spyOn(configModule, 'getConfig').mockReturnValue({ + maxMemoryMB: 100, + ttlHours: -1, // TTL disabled + }); + + jest.spyOn(global, 'setInterval'); + const embeddings = mock(); + + MemoryVectorStoreManager.getInstance(embeddings, logger); + + expect(setInterval).not.toHaveBeenCalled(); + }); + + it('should track memory usage when adding documents', async () => { + const embeddings = mock(); + const instance = MemoryVectorStoreManager.getInstance(embeddings, logger); + + const calculatorSpy = jest + .spyOn(instance['memoryCalculator'], 'estimateBatchSize') + .mockReturnValue(1024 * 1024); // Mock 1MB size + + const documents = [new Document({ pageContent: 'test document', metadata: { test: 'value' } })]; + + await instance.addDocuments('test-key', documents); + + expect(calculatorSpy).toHaveBeenCalledWith(documents); + expect(instance.getMemoryUsage()).toBe(1024 * 1024); // Should be 1MB + }); + + it('should clear store metadata when clearing store', async () => { + const embeddings = mock(); + const instance = MemoryVectorStoreManager.getInstance(embeddings, logger); + + // Directly set memory usage to 0 to start with a clean state + instance['memoryUsageBytes'] = 0; + + // Add documents to create a store + const docs = [new Document({ pageContent: 'test', metadata: {} })]; + jest.spyOn(instance['memoryCalculator'], 'estimateBatchSize').mockReturnValue(1000); + + await instance.addDocuments('test-key', docs); + expect(instance.getMemoryUsage()).toBe(1000); + + // Directly access the metadata to verify clearing works + const metadataSizeBefore = instance['storeMetadata'].get('test-key')?.size; + expect(metadataSizeBefore).toBe(1000); + + // Now clear the store by calling the private method directly + instance['clearStoreMetadata']('test-key'); + + // Verify metadata was reset + const metadataSizeAfter = instance['storeMetadata'].get('test-key')?.size; + expect(metadataSizeAfter).toBe(0); + + // The memory usage should be reduced + expect(instance.getMemoryUsage()).toBe(0); + }); + + it('should request cleanup when adding documents that would exceed memory limit', async () => { + const embeddings = mock(); + const instance = MemoryVectorStoreManager.getInstance(embeddings, logger); + + // Spy on the cleanup service + const cleanupSpy = jest.spyOn(instance['cleanupService'], 'cleanupOldestStores'); + + // Set up a large document batch + const documents = [new Document({ pageContent: 'test', metadata: {} })]; + jest.spyOn(instance['memoryCalculator'], 'estimateBatchSize').mockReturnValue(50 * 1024 * 1024); // 50MB + + await instance.addDocuments('test-key', documents); + + expect(cleanupSpy).toHaveBeenCalledWith(50 * 1024 * 1024); + }); + + it('should recalculate memory usage periodically', async () => { + const embeddings = mock(); + const instance = MemoryVectorStoreManager.getInstance(embeddings, logger); + + // Mock methods and spies + const recalcSpy = jest.spyOn(instance, 'recalculateMemoryUsage'); + const mockVectorStore = mock(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + mockVectorStore.memoryVectors = new Array(100).fill({ + embedding: createTestEmbedding(), + content: 'test', + metadata: {}, + }); + + // Mock the getVectorStore to return our mock + jest.spyOn(instance, 'getVectorStore').mockResolvedValue(mockVectorStore); + jest.spyOn(instance['memoryCalculator'], 'estimateBatchSize').mockReturnValue(1000); + + // Add a large batch of documents + const documents = new Array(21).fill(new Document({ pageContent: 'test', metadata: {} })); + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + await instance.addDocuments('test-key', documents); + + expect(recalcSpy).toHaveBeenCalled(); + }); + + it('should provide accurate stats about vector stores', async () => { + const embeddings = mock(); + const instance = MemoryVectorStoreManager.getInstance(embeddings, logger); + + // Create mock vector stores + const mockVectorStore1 = mock(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + mockVectorStore1.memoryVectors = new Array(50).fill({ + embedding: createTestEmbedding(), + content: 'test1', + metadata: {}, + }); + + const mockVectorStore2 = mock(); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + mockVectorStore2.memoryVectors = new Array(30).fill({ + embedding: createTestEmbedding(), + content: 'test2', + metadata: {}, + }); + + // Mock internal state + instance['vectorStoreBuffer'].set('store1', mockVectorStore1); + instance['vectorStoreBuffer'].set('store2', mockVectorStore2); + + // Set metadata for the stores + instance['storeMetadata'].set('store1', { + size: 1024 * 1024, // 1MB + createdAt: new Date(Date.now() - 3600000), // 1 hour ago + lastAccessed: new Date(Date.now() - 1800000), // 30 minutes ago + }); + + instance['storeMetadata'].set('store2', { + size: 512 * 1024, // 0.5MB + createdAt: new Date(Date.now() - 7200000), // 2 hours ago + lastAccessed: new Date(Date.now() - 3600000), // 1 hour ago + }); + + // Set memory usage + instance['memoryUsageBytes'] = 1024 * 1024 + 512 * 1024; + + const stats = instance.getStats(); + + expect(stats.storeCount).toBe(2); + expect(stats.totalSizeBytes).toBeGreaterThan(0); + expect(Object.keys(stats.stores)).toContain('store1'); + expect(Object.keys(stats.stores)).toContain('store2'); + expect(stats.stores.store1.vectors).toBe(50); + expect(stats.stores.store2.vectors).toBe(30); + }); +}); diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/StoreCleanupService.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/StoreCleanupService.test.ts new file mode 100644 index 00000000000..206fa90296f --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/StoreCleanupService.test.ts @@ -0,0 +1,289 @@ +/* eslint-disable @typescript-eslint/dot-notation */ +import { mock } from 'jest-mock-extended'; +import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; + +import { StoreCleanupService } from '../StoreCleanupService'; +import type { VectorStoreMetadata } from '../types'; + +describe('StoreCleanupService', () => { + // Setup test data + let vectorStores: Map; + let storeMetadata: Map; + let onCleanupMock: jest.Mock; + + // Utility to add a test store with given age + const addTestStore = ( + key: string, + sizeBytes: number, + createdHoursAgo: number, + accessedHoursAgo: number, + ) => { + const mockStore = mock(); + vectorStores.set(key, mockStore); + + const now = Date.now(); + storeMetadata.set(key, { + size: sizeBytes, + createdAt: new Date(now - createdHoursAgo * 3600000), + lastAccessed: new Date(now - accessedHoursAgo * 3600000), + }); + }; + + beforeEach(() => { + vectorStores = new Map(); + storeMetadata = new Map(); + onCleanupMock = jest.fn(); + }); + + describe('TTL-based cleanup', () => { + it('should identify inactive stores correctly', () => { + const service = new StoreCleanupService( + 100 * 1024 * 1024, // 100MB max + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Create test metadata + const recentMetadata: VectorStoreMetadata = { + size: 1024, + createdAt: new Date(Date.now() - 48 * 3600 * 1000), // 48 hours ago + lastAccessed: new Date(Date.now() - 12 * 3600 * 1000), // 12 hours ago + }; + + const inactiveMetadata: VectorStoreMetadata = { + size: 1024, + createdAt: new Date(Date.now() - 48 * 3600 * 1000), // 48 hours ago + lastAccessed: new Date(Date.now() - 36 * 3600 * 1000), // 36 hours ago + }; + + // Test the inactive check + expect(service.isStoreInactive(recentMetadata)).toBe(false); + expect(service.isStoreInactive(inactiveMetadata)).toBe(true); + }); + + it('should never identify stores as inactive when TTL is disabled', () => { + const service = new StoreCleanupService( + 100 * 1024 * 1024, // 100MB max + -1, // TTL disabled + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Create very old metadata + const veryOldMetadata: VectorStoreMetadata = { + size: 1024, + createdAt: new Date(Date.now() - 365 * 24 * 3600 * 1000), // 1 year ago + lastAccessed: new Date(Date.now() - 365 * 24 * 3600 * 1000), // 1 year ago + }; + + // Should never be inactive when TTL is disabled + expect(service.isStoreInactive(veryOldMetadata)).toBe(false); + }); + + it('should clean up inactive stores', () => { + const service = new StoreCleanupService( + 100 * 1024 * 1024, // 100MB max + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Add active and inactive stores + addTestStore('active1', 1024 * 1024, 48, 12); // 48 hours old, accessed 12 hours ago + addTestStore('active2', 2048 * 1024, 72, 20); // 72 hours old, accessed 20 hours ago + addTestStore('inactive1', 3072 * 1024, 100, 30); // 100 hours old, accessed 30 hours ago + addTestStore('inactive2', 4096 * 1024, 120, 48); // 120 hours old, accessed 48 hours ago + + // Run cleanup + service.cleanupInactiveStores(); + + // Check which stores were cleaned up + expect(vectorStores.has('active1')).toBe(true); + expect(vectorStores.has('active2')).toBe(true); + expect(vectorStores.has('inactive1')).toBe(false); + expect(vectorStores.has('inactive2')).toBe(false); + + // Metadata should also be cleaned up + expect(storeMetadata.has('active1')).toBe(true); + expect(storeMetadata.has('active2')).toBe(true); + expect(storeMetadata.has('inactive1')).toBe(false); + expect(storeMetadata.has('inactive2')).toBe(false); + + // Check callback was called correctly + expect(onCleanupMock).toHaveBeenCalledWith( + expect.arrayContaining(['inactive1', 'inactive2']), + 7168 * 1024, // sum of inactive store sizes + 'ttl', + ); + }); + + it('should not run TTL cleanup when disabled', () => { + const service = new StoreCleanupService( + 100 * 1024 * 1024, // 100MB max + -1, // TTL disabled + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Add all "inactive" stores + addTestStore('store1', 1024 * 1024, 48, 30); + addTestStore('store2', 2048 * 1024, 72, 48); + + // Run cleanup + service.cleanupInactiveStores(); + + // Nothing should be cleaned up + expect(vectorStores.size).toBe(2); + expect(storeMetadata.size).toBe(2); + expect(onCleanupMock).not.toHaveBeenCalled(); + }); + }); + + describe('Memory-based cleanup', () => { + it('should clean up oldest stores to make room for new data', () => { + const maxMemoryBytes = 10 * 1024 * 1024; // 10MB + const service = new StoreCleanupService( + maxMemoryBytes, + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Add stores with different creation times + addTestStore('newest', 2 * 1024 * 1024, 1, 1); // 2MB, 1 hour old + addTestStore('newer', 3 * 1024 * 1024, 2, 1); // 3MB, 2 hours old + addTestStore('older', 3 * 1024 * 1024, 3, 1); // 3MB, 3 hours old + addTestStore('oldest', 2 * 1024 * 1024, 4, 1); // 2MB, 4 hours old + + // Current total: 10MB + + // Try to add 5MB more + service.cleanupOldestStores(5 * 1024 * 1024); + + // Should have removed oldest and older (5MB total) + expect(vectorStores.has('newest')).toBe(true); + expect(vectorStores.has('newer')).toBe(true); + expect(vectorStores.has('older')).toBe(false); + expect(vectorStores.has('oldest')).toBe(false); + + // Check callback + expect(onCleanupMock).toHaveBeenCalledWith( + expect.arrayContaining(['older', 'oldest']), + 5 * 1024 * 1024, + 'memory', + ); + }); + + it('should run TTL cleanup before memory cleanup', () => { + const maxMemoryBytes = 10 * 1024 * 1024; // 10MB + const service = new StoreCleanupService( + maxMemoryBytes, + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Add a mix of active and inactive stores + addTestStore('active-newest', 2 * 1024 * 1024, 1, 1); // 2MB, active + addTestStore('active-older', 3 * 1024 * 1024, 3, 12); // 3MB, active + addTestStore('inactive', 3 * 1024 * 1024, 3, 30); // 3MB, inactive (30h) + addTestStore('active-oldest', 2 * 1024 * 1024, 4, 20); // 2MB, active + + // Total: 10MB, with 3MB inactive + + // Try to add 5MB more + service.cleanupOldestStores(5 * 1024 * 1024); + + // Should have removed inactive first, then active-oldest (5MB total) + expect(vectorStores.has('active-newest')).toBe(true); + expect(vectorStores.has('active-older')).toBe(true); + expect(vectorStores.has('inactive')).toBe(false); + expect(vectorStores.has('active-oldest')).toBe(false); + + // Check callbacks + expect(onCleanupMock).toHaveBeenCalledTimes(2); + // First call for TTL cleanup + expect(onCleanupMock).toHaveBeenNthCalledWith(1, ['inactive'], 3 * 1024 * 1024, 'ttl'); + // Second call for memory cleanup + expect(onCleanupMock).toHaveBeenNthCalledWith( + 2, + ['active-oldest'], + 2 * 1024 * 1024, + 'memory', + ); + }); + + it('should not perform memory cleanup when limit is disabled', () => { + const service = new StoreCleanupService( + -1, // Memory limit disabled + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Add some stores + addTestStore('store1', 5 * 1024 * 1024, 1, 1); + addTestStore('store2', 10 * 1024 * 1024, 2, 1); + + // Try to add a lot more data + service.cleanupOldestStores(100 * 1024 * 1024); + + // Nothing should be cleaned up + expect(vectorStores.size).toBe(2); + expect(storeMetadata.size).toBe(2); + expect(onCleanupMock).not.toHaveBeenCalled(); + }); + + it('should handle empty stores during cleanup', () => { + const service = new StoreCleanupService( + 10 * 1024 * 1024, // 10MB + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + service.cleanupOldestStores(5 * 1024 * 1024); + service.cleanupInactiveStores(); + + expect(onCleanupMock).not.toHaveBeenCalled(); + }); + + it('should update the cache when stores are removed', () => { + const service = new StoreCleanupService( + 10 * 1024 * 1024, // 10MB + 24 * 3600 * 1000, // 24 hours TTL + vectorStores, + storeMetadata, + onCleanupMock, + ); + + // Add test stores + addTestStore('newest', 2 * 1024 * 1024, 1, 1); + addTestStore('middle', 3 * 1024 * 1024, 3, 1); + addTestStore('oldest', 4 * 1024 * 1024, 5, 1); + + // Trigger a cleanup that will remove only the oldest store + service.cleanupOldestStores(4 * 1024 * 1024); // 4MB + + // Verify removal + expect(vectorStores.has('oldest')).toBe(false); + expect(vectorStores.has('middle')).toBe(true); + expect(vectorStores.has('newest')).toBe(true); + + // Check that the cache was updated correctly + const cacheKeys = service['oldestStoreKeys']; + expect(cacheKeys.includes('oldest')).toBe(false); + expect(cacheKeys.includes('middle')).toBe(true); + expect(cacheKeys.includes('newest')).toBe(true); + }); + }); +}); diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/config.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/config.test.ts new file mode 100644 index 00000000000..5ef82c31778 --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/test/config.test.ts @@ -0,0 +1,74 @@ +import { getConfig, mbToBytes, hoursToMs } from '../config'; + +describe('Vector Store Config', () => { + // Store original environment + const originalEnv = { ...process.env }; + + // Restore original environment after each test + afterEach(() => { + process.env = { ...originalEnv }; + }); + + describe('getConfig', () => { + it('should return default values when no environment variables set', () => { + // Clear relevant environment variables + delete process.env.N8N_VECTOR_STORE_MAX_MEMORY; + delete process.env.N8N_VECTOR_STORE_TTL_HOURS; + + const config = getConfig(); + + expect(config.maxMemoryMB).toBe(-1); + expect(config.ttlHours).toBe(-1); + }); + + it('should use values from environment variables when set', () => { + process.env.N8N_VECTOR_STORE_MAX_MEMORY = '200'; + process.env.N8N_VECTOR_STORE_TTL_HOURS = '24'; + + const config = getConfig(); + + expect(config.maxMemoryMB).toBe(200); + expect(config.ttlHours).toBe(24); + }); + + it('should handle invalid environment variable values', () => { + // Set invalid values (non-numeric) + process.env.N8N_VECTOR_STORE_MAX_MEMORY = 'invalid'; + process.env.N8N_VECTOR_STORE_TTL_HOURS = 'notanumber'; + + const config = getConfig(); + + // Should use default values for invalid inputs + expect(config.maxMemoryMB).toBe(-1); + expect(config.ttlHours).toBe(-1); + }); + }); + + describe('mbToBytes', () => { + it('should convert MB to bytes', () => { + expect(mbToBytes(1)).toBe(1024 * 1024); + expect(mbToBytes(5)).toBe(5 * 1024 * 1024); + expect(mbToBytes(100)).toBe(100 * 1024 * 1024); + }); + + it('should handle zero and negative values', () => { + expect(mbToBytes(0)).toBe(-1); + expect(mbToBytes(-1)).toBe(-1); + expect(mbToBytes(-10)).toBe(-1); + }); + }); + + describe('hoursToMs', () => { + it('should convert hours to milliseconds', () => { + expect(hoursToMs(1)).toBe(60 * 60 * 1000); + expect(hoursToMs(24)).toBe(24 * 60 * 60 * 1000); + expect(hoursToMs(168)).toBe(168 * 60 * 60 * 1000); + }); + + it('should handle zero and negative values', () => { + expect(hoursToMs(0)).toBe(-1); + expect(hoursToMs(-1)).toBe(-1); + expect(hoursToMs(-24)).toBe(-1); + }); + }); +}); diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/types.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/types.ts new file mode 100644 index 00000000000..340cee889e6 --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryManager/types.ts @@ -0,0 +1,70 @@ +import type { Document } from '@langchain/core/documents'; +import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; + +/** + * Configuration options for the memory vector store + */ +export interface MemoryVectorStoreConfig { + /** + * Maximum memory size in MB, -1 to disable + */ + maxMemoryMB: number; + + /** + * TTL for inactive stores in hours, -1 to disable + */ + ttlHours: number; +} + +/** + * Vector store metadata for tracking usage + */ +export interface VectorStoreMetadata { + size: number; + createdAt: Date; + lastAccessed: Date; +} + +/** + * Per-store statistics for reporting + */ +export interface StoreStats { + sizeBytes: number; + sizeMB: number; + percentOfTotal: number; + vectors: number; + createdAt: string; + lastAccessed: string; + inactive?: boolean; + inactiveForHours?: number; +} + +/** + * Overall vector store statistics + */ +export interface VectorStoreStats { + totalSizeBytes: number; + totalSizeMB: number; + percentOfLimit: number; + maxMemoryMB: number; + storeCount: number; + inactiveStoreCount: number; + ttlHours: number; + stores: Record; +} + +/** + * Service for calculating memory usage + */ +export interface IMemoryCalculator { + estimateBatchSize(documents: Document[]): number; + calculateVectorStoreSize(vectorStore: MemoryVectorStore): number; +} + +/** + * Service for cleaning up vector stores + */ +export interface IStoreCleanupService { + cleanupInactiveStores(): void; + cleanupOldestStores(requiredBytes: number): void; +} diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.test.ts deleted file mode 100644 index 1088505a0a7..00000000000 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import type { OpenAIEmbeddings } from '@langchain/openai'; -import { mock } from 'jest-mock-extended'; - -import { MemoryVectorStoreManager } from './MemoryVectorStoreManager'; - -describe('MemoryVectorStoreManager', () => { - it('should create an instance of MemoryVectorStoreManager', () => { - const embeddings = mock(); - - const instance = MemoryVectorStoreManager.getInstance(embeddings); - expect(instance).toBeInstanceOf(MemoryVectorStoreManager); - }); - - it('should return existing instance', () => { - const embeddings = mock(); - - const instance1 = MemoryVectorStoreManager.getInstance(embeddings); - const instance2 = MemoryVectorStoreManager.getInstance(embeddings); - expect(instance1).toBe(instance2); - }); - - it('should update embeddings in existing instance', () => { - const embeddings1 = mock(); - const embeddings2 = mock(); - - const instance = MemoryVectorStoreManager.getInstance(embeddings1); - MemoryVectorStoreManager.getInstance(embeddings2); - - expect((instance as any).embeddings).toBe(embeddings2); - }); - - it('should update embeddings in existing vector store instances', async () => { - const embeddings1 = mock(); - const embeddings2 = mock(); - - const instance1 = MemoryVectorStoreManager.getInstance(embeddings1); - await instance1.getVectorStore('test'); - - const instance2 = MemoryVectorStoreManager.getInstance(embeddings2); - const vectorStoreInstance2 = await instance2.getVectorStore('test'); - - expect((vectorStoreInstance2 as any).embeddings).toBe(embeddings2); - }); -}); diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.ts deleted file mode 100644 index f92c8abd411..00000000000 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/MemoryVectorStoreManager.ts +++ /dev/null @@ -1,52 +0,0 @@ -import type { Document } from '@langchain/core/documents'; -import type { Embeddings } from '@langchain/core/embeddings'; -import { MemoryVectorStore } from 'langchain/vectorstores/memory'; - -export class MemoryVectorStoreManager { - private static instance: MemoryVectorStoreManager | null = null; - - private vectorStoreBuffer: Map; - - private constructor(private embeddings: Embeddings) { - this.vectorStoreBuffer = new Map(); - } - - static getInstance(embeddings: Embeddings): MemoryVectorStoreManager { - if (!MemoryVectorStoreManager.instance) { - MemoryVectorStoreManager.instance = new MemoryVectorStoreManager(embeddings); - } else { - // We need to update the embeddings in the existing instance. - // This is important as embeddings instance is wrapped in a logWrapper, - // which relies on supplyDataFunctions context which changes on each workflow run - MemoryVectorStoreManager.instance.embeddings = embeddings; - MemoryVectorStoreManager.instance.vectorStoreBuffer.forEach((vectorStoreInstance) => { - vectorStoreInstance.embeddings = embeddings; - }); - } - - return MemoryVectorStoreManager.instance; - } - - async getVectorStore(memoryKey: string): Promise { - let vectorStoreInstance = this.vectorStoreBuffer.get(memoryKey); - - if (!vectorStoreInstance) { - vectorStoreInstance = await MemoryVectorStore.fromExistingIndex(this.embeddings); - this.vectorStoreBuffer.set(memoryKey, vectorStoreInstance); - } - - return vectorStoreInstance; - } - - async addDocuments( - memoryKey: string, - documents: Document[], - clearStore?: boolean, - ): Promise { - if (clearStore) { - this.vectorStoreBuffer.delete(memoryKey); - } - const vectorStoreInstance = await this.getVectorStore(memoryKey); - await vectorStoreInstance.addDocuments(documents); - } -} From 305ea0fb324fd9a20f0183cc6478c7e6b130b4d3 Mon Sep 17 00:00:00 2001 From: Ricardo Espinoza Date: Thu, 20 Mar 2025 09:45:10 -0400 Subject: [PATCH 04/19] feat(core): Allow moving workflow to project root (no-changelog) (#14075) --- .../cli/src/workflows/workflow.service.ts | 8 ++++--- .../workflows/workflows.controller.test.ts | 23 +++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index bf71261278e..f2a497279bd 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -9,7 +9,7 @@ import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPar import omit from 'lodash/omit'; import pick from 'lodash/pick'; import { BinaryDataService, Logger } from 'n8n-core'; -import { NodeApiError } from 'n8n-workflow'; +import { NodeApiError, PROJECT_ROOT } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; @@ -281,8 +281,10 @@ export class WorkflowService { if (parentFolderId) { const project = await this.sharedWorkflowRepository.getWorkflowOwningProject(workflow.id); - await this.folderService.findFolderInProjectOrFail(parentFolderId, project?.id ?? ''); - updatePayload.parentFolder = { id: parentFolderId }; + if (parentFolderId !== PROJECT_ROOT) { + await this.folderService.findFolderInProjectOrFail(parentFolderId, project?.id ?? ''); + } + updatePayload.parentFolder = parentFolderId === PROJECT_ROOT ? null : { id: parentFolderId }; } await this.workflowRepository.update(workflowId, updatePayload); diff --git a/packages/cli/test/integration/workflows/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts index f26c4c728de..043b958d4ab 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -1,7 +1,7 @@ import { Container } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; import { DateTime } from 'luxon'; -import type { INode, IPinData, IWorkflowBase } from 'n8n-workflow'; +import { PROJECT_ROOT, type INode, type IPinData, type IWorkflowBase } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; @@ -2045,7 +2045,7 @@ describe('PATCH /workflows/:workflowId', () => { expect(updatedWorkflow.meta).toEqual(payload.meta); }); - test('should update workflow parent folder', async () => { + test('should move workflow to folder', async () => { const ownerPersonalProject = await projectRepository.getPersonalProjectForUserOrFail(owner.id); const folder1 = await createFolder(ownerPersonalProject, { name: 'folder1' }); @@ -2067,6 +2067,25 @@ describe('PATCH /workflows/:workflowId', () => { expect(updatedWorkflow.parentFolder?.id).toBe(folder1.id); }); + test('should move workflow to project root', async () => { + const workflow = await createWorkflow({}, owner); + const payload = { + versionId: workflow.versionId, + parentFolderId: PROJECT_ROOT, + }; + + const response = await authOwnerAgent.patch(`/workflows/${workflow.id}`).send(payload); + + expect(response.statusCode).toBe(200); + + const updatedWorkflow = await Container.get(WorkflowRepository).findOneOrFail({ + where: { id: workflow.id }, + relations: ['parentFolder'], + }); + + expect(updatedWorkflow.parentFolder).toBe(null); + }); + test('should fail if trying update workflow parent folder with a folder that does not belong to project', async () => { const ownerPersonalProject = await projectRepository.getPersonalProjectForUserOrFail(owner.id); const memberPersonalProject = await projectRepository.getPersonalProjectForUserOrFail( From 1f56a24bbd4fdffa76403923ab79dc3bef78b129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Thu, 20 Mar 2025 15:48:10 +0100 Subject: [PATCH 05/19] fix(editor): Addressing internal testing feedback for folders (no-changelog) (#13997) --- cypress/composables/folders.ts | 232 +++++++++++++- cypress/composables/ndv.ts | 6 +- .../e2e/30-editor-after-route-changes.cy.ts | 2 +- cypress/e2e/39-projects.cy.ts | 2 +- cypress/e2e/49-folders.cy.ts | 284 +++++++++++++++++- cypress/pages/workflows.ts | 4 +- .../src/components/N8nActionBox/ActionBox.vue | 2 +- .../components/N8nBreadcrumbs/Breadcrumbs.vue | 7 +- .../@n8n/design-system/src/css/_tokens.scss | 1 + .../@n8n/design-system/src/types/user.ts | 1 + packages/frontend/editor-ui/src/Interface.ts | 6 +- .../frontend/editor-ui/src/api/workflows.ts | 6 +- .../CommunityPlusEnrollmentModal.vue | 10 +- .../components/Folders/DeleteFolderModal.vue | 43 +-- .../components/Folders/FolderBreadcrumbs.vue | 7 + .../Folders/MoveToFolderDropdown.vue | 93 ++++-- .../components/Folders/MoveToFolderModal.vue | 4 +- .../components/MainHeader/WorkflowDetails.vue | 43 ++- .../src/components/PersonalizationModal.vue | 1 + .../src/components/Projects/ProjectHeader.vue | 3 +- .../editor-ui/src/components/WorkflowCard.vue | 89 +++++- .../forms/ResourceFiltersDropdown.vue | 4 +- .../layouts/ResourcesListLayout.vue | 16 +- .../src/composables/useFolders.test.ts | 122 ++++++++ .../editor-ui/src/composables/useFolders.ts | 46 +++ .../composables/useGlobalEntityCreation.ts | 1 + packages/frontend/editor-ui/src/constants.ts | 27 +- .../src/plugins/i18n/locales/en.json | 25 +- .../editor-ui/src/stores/folders.store.ts | 87 +++++- .../frontend/editor-ui/src/stores/ui.store.ts | 7 +- .../src/views/SettingsUsageAndPlan.test.ts | 4 +- .../src/views/SettingsUsageAndPlan.vue | 7 +- .../editor-ui/src/views/VariablesView.vue | 2 +- .../editor-ui/src/views/WorkflowsView.test.ts | 4 + .../editor-ui/src/views/WorkflowsView.vue | 224 ++++++++++---- 35 files changed, 1277 insertions(+), 145 deletions(-) create mode 100644 packages/frontend/editor-ui/src/composables/useFolders.test.ts create mode 100644 packages/frontend/editor-ui/src/composables/useFolders.ts diff --git a/cypress/composables/folders.ts b/cypress/composables/folders.ts index 7d604e71bcd..56085c0a3e2 100644 --- a/cypress/composables/folders.ts +++ b/cypress/composables/folders.ts @@ -22,6 +22,31 @@ export function getFolderCards() { export function getFolderCard(name: string) { return cy.getByTestId('folder-card-name').contains(name).closest('[data-test-id="folder-card"]'); } + +export function getWorkflowCards() { + return cy.getByTestId('resources-list-item-workflow'); +} + +export function getWorkflowCard(name: string) { + return cy + .getByTestId('workflow-card-name') + .contains(name) + .closest('[data-test-id="resources-list-item-workflow"]'); +} + +export function getWorkflowCardActions(name: string) { + return getWorkflowCard(name).find('[data-test-id="workflow-card-actions"]'); +} + +export function getWorkflowCardActionItem(workflowName: string, actionName: string) { + return getWorkflowCardActions(workflowName) + .find('span[aria-controls]') + .invoke('attr', 'aria-controls') + .then((popperId) => { + return cy.get(`#${popperId}`).find(`[data-test-id="action-${actionName}"]`); + }); +} + export function getAddFolderButton() { return cy.getByTestId('add-folder-button'); } @@ -34,6 +59,10 @@ export function getHomeProjectBreadcrumb() { return getListBreadcrumbs().findChildByTestId('home-project'); } +export function getListBreadcrumbItem(name: string) { + return getListBreadcrumbs().findChildByTestId('breadcrumbs-item').contains(name); +} + export function getVisibleListBreadcrumbs() { return getListBreadcrumbs().findChildByTestId('breadcrumbs-item'); } @@ -94,13 +123,14 @@ export function getFolderCardActionToggle(folderName: string) { return getFolderCard(folderName).find('[data-test-id="folder-card-actions"]'); } -export function getFolderCardActionItem(name: string) { - return cy - .getByTestId('folder-card-actions') +export function getFolderCardActionItem(folderName: string, actionName: string) { + return getFolderCard(folderName) + .findChildByTestId('folder-card-actions') + .filter(':visible') .find('span[aria-controls]') .invoke('attr', 'aria-controls') .then((popperId) => { - return cy.get(`#${popperId}`).find(`[data-test-id="action-${name}"]`); + return cy.get(`#${popperId}`).find(`[data-test-id="action-${actionName}"]`); }); } @@ -108,10 +138,18 @@ export function getFolderDeleteModal() { return cy.getByTestId('deleteFolder-modal'); } +export function getMoveFolderModal() { + return cy.getByTestId('moveFolder-modal'); +} + export function getDeleteRadioButton() { return cy.getByTestId('delete-content-radio'); } +export function getTransferContentRadioButton() { + return cy.getByTestId('transfer-content-radio'); +} + export function getConfirmDeleteInput() { return getFolderDeleteModal().findChildByTestId('delete-data-input').find('input'); } @@ -119,6 +157,61 @@ export function getConfirmDeleteInput() { export function getDeleteFolderModalConfirmButton() { return getFolderDeleteModal().findChildByTestId('confirm-delete-folder-button'); } + +export function getProjectEmptyState() { + return cy.getByTestId('list-empty-state'); +} + +export function getFolderEmptyState() { + return cy.getByTestId('empty-folder-container'); +} + +export function getProjectMenuItem(name: string) { + if (name.toLowerCase() === 'personal') { + return getPersonalProjectMenuItem(); + } + return cy.getByTestId('project-menu-item').contains(name); +} + +export function getMoveToFolderDropdown() { + return cy.getByTestId('move-to-folder-dropdown'); +} + +export function getMoveToFolderOption(name: string) { + return cy.getByTestId('move-to-folder-option').contains(name); +} + +export function getMoveToFolderInput() { + return getMoveToFolderDropdown().find('input'); +} + +export function getEmptyFolderDropdownMessage(text: string) { + return cy.get('.el-select-dropdown__empty').contains(text); +} + +export function getMoveFolderConfirmButton() { + return cy.getByTestId('confirm-move-folder-button'); +} + +export function getMoveWorkflowModal() { + return cy.getByTestId('moveFolder-modal'); +} + +export function getWorkflowCardBreadcrumbs(workflowName: string) { + return getWorkflowCard(workflowName).find('[data-test-id="workflow-card-breadcrumbs"]'); +} + +export function getWorkflowCardBreadcrumbsEllipsis(workflowName: string) { + return getWorkflowCardBreadcrumbs(workflowName).find('[data-test-id="ellipsis"]'); +} + +export function getNewFolderNameInput() { + return cy.get('.add-folder-modal').filter(':visible').find('input.el-input__inner'); +} + +export function getNewFolderModalErrorMessage() { + return cy.get('.el-message-box__errormsg').filter(':visible'); +} /** * Actions */ @@ -136,8 +229,46 @@ export function createFolderFromListHeaderButton(folderName: string) { createNewFolder(folderName); } +export function createWorkflowFromEmptyState(workflowName?: string) { + getFolderEmptyState().find('button').contains('Create Workflow').click(); + if (workflowName) { + cy.getByTestId('workflow-name-input').type(`{selectAll}{backspace}${workflowName}`, { + delay: 50, + }); + } + cy.getByTestId('workflow-save-button').click(); + successToast().should('exist'); +} + +export function createWorkflowFromProjectHeader(folderName?: string, workflowName?: string) { + cy.getByTestId('add-resource-workflow').click(); + if (workflowName) { + cy.getByTestId('workflow-name-input').type(`{selectAll}{backspace}${workflowName}`, { + delay: 50, + }); + } + cy.getByTestId('workflow-save-button').click(); + if (folderName) { + successToast().should( + 'contain.text', + `Workflow successfully created in "Personal", within "${folderName}"`, + ); + } +} + +export function createWorkflowFromListDropdown(workflowName?: string) { + getListActionsToggle().click(); + getListActionItem('create_workflow').click(); + if (workflowName) { + cy.getByTestId('workflow-name-input').type(`{selectAll}{backspace}${workflowName}`, { + delay: 50, + }); + } + cy.getByTestId('workflow-save-button').click(); + successToast().should('exist'); +} + export function createFolderFromProjectHeader(folderName: string) { - getPersonalProjectMenuItem().click(); getAddResourceDropdown().click(); cy.getByTestId('action-folder').click(); createNewFolder(folderName); @@ -151,7 +282,7 @@ export function createFolderFromListDropdown(folderName: string) { export function createFolderFromCardActions(parentName: string, folderName: string) { getFolderCardActionToggle(parentName).click(); - getFolderCardActionItem('create').click(); + getFolderCardActionItem(parentName, 'create').click(); createNewFolder(folderName); } @@ -164,7 +295,7 @@ export function renameFolderFromListActions(folderName: string, newName: string) export function renameFolderFromCardActions(folderName: string, newName: string) { getFolderCardActionToggle(folderName).click(); - getFolderCardActionItem('rename').click(); + getFolderCardActionItem(folderName, 'rename').click(); renameFolder(newName); } @@ -194,9 +325,63 @@ export function deleteFolderWithContentsFromListDropdown(folderName: string) { export function deleteFolderWithContentsFromCardDropdown(folderName: string) { getFolderCardActionToggle(folderName).click(); - getFolderCardActionItem('delete').click(); + getFolderCardActionItem(folderName, 'delete').click(); confirmFolderDelete(folderName); } + +export function deleteAndTransferFolderContentsFromCardDropdown( + folderName: string, + destinationName: string, +) { + getFolderCardActionToggle(folderName).click(); + getFolderCardActionItem(folderName, 'delete').click(); + deleteFolderAndMoveContents(folderName, destinationName); +} + +export function deleteAndTransferFolderContentsFromListDropdown(destinationName: string) { + getListActionsToggle().click(); + getListActionItem('delete').click(); + getCurrentBreadcrumb() + .find('span') + .invoke('text') + .then((currentFolderName) => { + deleteFolderAndMoveContents(currentFolderName, destinationName); + }); +} + +export function createNewProject(projectName: string, options: { openAfterCreate?: boolean } = {}) { + cy.getByTestId('universal-add').should('exist').click(); + cy.getByTestId('navigation-menu-item').contains('Project').click(); + cy.getByTestId('project-settings-name-input').type(projectName, { delay: 50 }); + cy.getByTestId('project-settings-save-button').click(); + successToast().should('exist'); + if (options.openAfterCreate) { + getProjectMenuItem(projectName).click(); + } +} + +export function moveFolderFromFolderCardActions(folderName: string, destinationName: string) { + getFolderCardActionToggle(folderName).click(); + getFolderCardActionItem(folderName, 'move').click(); + moveFolder(folderName, destinationName); +} + +export function moveFolderFromListActions(folderName: string, destinationName: string) { + getFolderCard(folderName).click(); + getListActionsToggle().click(); + getListActionItem('move').click(); + moveFolder(folderName, destinationName); +} + +export function moveWorkflowToFolder(workflowName: string, folderName: string) { + getWorkflowCardActions(workflowName).click(); + getWorkflowCardActionItem(workflowName, 'moveToFolder').click(); + getMoveFolderModal().should('be.visible'); + getMoveToFolderDropdown().click(); + getMoveToFolderInput().type(folderName, { delay: 50 }); + getMoveToFolderOption(folderName).should('be.visible').click(); + getMoveFolderConfirmButton().should('be.enabled').click(); +} /** * Utils */ @@ -240,3 +425,34 @@ function confirmFolderDelete(folderName: string) { cy.wait('@deleteFolder'); successToast().contains('Folder deleted').should('exist'); } + +function deleteFolderAndMoveContents(folderName: string, destinationName: string) { + cy.intercept('DELETE', '/rest/projects/**').as('deleteFolder'); + getFolderDeleteModal().should('be.visible'); + getFolderDeleteModal().find('h1').first().contains(`Delete "${folderName}"`); + getTransferContentRadioButton().should('be.visible').click(); + getMoveToFolderDropdown().click(); + getMoveToFolderInput().type(destinationName); + getMoveToFolderOption(destinationName).click(); + getDeleteFolderModalConfirmButton().should('be.enabled').click(); + cy.wait('@deleteFolder'); + successToast().should('contain.text', `Data transferred to "${destinationName}"`); +} + +function moveFolder(folderName: string, destinationName: string) { + cy.intercept('PATCH', '/rest/projects/**').as('moveFolder'); + getMoveFolderModal().should('be.visible'); + getMoveFolderModal().find('h1').first().contains(`Move "${folderName}" to another folder`); + getMoveToFolderDropdown().click(); + // Try to find current folder in the dropdown + getMoveToFolderInput().type(folderName, { delay: 50 }); + // Should not be available + getEmptyFolderDropdownMessage('No folders found').should('exist'); + // Select destination folder + getMoveToFolderInput().type(`{selectall}{backspace}${destinationName}`, { + delay: 50, + }); + getMoveToFolderOption(destinationName).should('be.visible').click(); + getMoveFolderConfirmButton().should('be.enabled').click(); + cy.wait('@moveFolder'); +} diff --git a/cypress/composables/ndv.ts b/cypress/composables/ndv.ts index 36ae10669b4..1423801a5e6 100644 --- a/cypress/composables/ndv.ts +++ b/cypress/composables/ndv.ts @@ -105,11 +105,13 @@ export function getNodeOutputHint() { } export function getWorkflowCards() { - return cy.getByTestId('resources-list-item'); + return cy.getByTestId('resources-list-item-workflow'); } export function getWorkflowCard(workflowName: string) { - return getWorkflowCards().contains(workflowName).parents('[data-test-id="resources-list-item"]'); + return getWorkflowCards() + .contains(workflowName) + .parents('[data-test-id="resources-list-item-workflow"]'); } export function getWorkflowCardContent(workflowName: string) { diff --git a/cypress/e2e/30-editor-after-route-changes.cy.ts b/cypress/e2e/30-editor-after-route-changes.cy.ts index 89c64e1156c..6b069dfbf18 100644 --- a/cypress/e2e/30-editor-after-route-changes.cy.ts +++ b/cypress/e2e/30-editor-after-route-changes.cy.ts @@ -21,7 +21,7 @@ const switchBetweenEditorAndWorkflowlist = () => { cy.getByTestId('menu-item').first().click(); cy.wait(['@getUsers', '@getWorkflows', '@getActiveWorkflows', '@getProjects']); - cy.getByTestId('resources-list-item').first().click(); + cy.getByTestId('resources-list-item-workflow').first().click(); workflowPage.getters.canvasNodes().first().should('be.visible'); workflowPage.getters.canvasNodes().last().should('be.visible'); diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index b6dcb449da1..12881739076 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -514,7 +514,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowsPage.getters.workflowCards().should('have.length', 3); workflowsPage.getters .workflowCards() - .filter(':has(.n8n-badge:contains("Project"))') + .filter(':has([data-test-id="workflow-card-breadcrumbs"]:contains("Project"))') .should('have.length', 2); workflowsPage.getters.workflowCardActions('Workflow in Home project').click(); workflowsPage.getters.workflowMoveButton().click(); diff --git a/cypress/e2e/49-folders.cy.ts b/cypress/e2e/49-folders.cy.ts index 8591daaaa04..5208c5bb206 100644 --- a/cypress/e2e/49-folders.cy.ts +++ b/cypress/e2e/49-folders.cy.ts @@ -4,6 +4,12 @@ import { createFolderFromListHeaderButton, createFolderFromProjectHeader, createFolderInsideFolder, + createNewProject, + createWorkflowFromEmptyState, + createWorkflowFromListDropdown, + createWorkflowFromProjectHeader, + deleteAndTransferFolderContentsFromCardDropdown, + deleteAndTransferFolderContentsFromListDropdown, deleteEmptyFolderFromCardDropdown, deleteEmptyFolderFromListDropdown, deleteFolderWithContentsFromCardDropdown, @@ -14,14 +20,27 @@ import { getFolderCardActionItem, getFolderCardActionToggle, getFolderCards, + getFolderEmptyState, getHomeProjectBreadcrumb, + getListBreadcrumbItem, getListBreadcrumbs, getMainBreadcrumbsEllipsis, getMainBreadcrumbsEllipsisMenuItems, + getNewFolderModalErrorMessage, + getNewFolderNameInput, getOverviewMenuItem, getPersonalProjectMenuItem, + getProjectEmptyState, + getProjectMenuItem, getVisibleListBreadcrumbs, + getWorkflowCard, + getWorkflowCardBreadcrumbs, + getWorkflowCardBreadcrumbsEllipsis, + getWorkflowCards, goToPersonalProject, + moveFolderFromFolderCardActions, + moveFolderFromListActions, + moveWorkflowToFolder, renameFolderFromCardActions, renameFolderFromListActions, } from '../composables/folders'; @@ -44,6 +63,7 @@ describe('Folders', () => { describe('Create and navigate folders', () => { it('should create folder from the project header', () => { + getPersonalProjectMenuItem().click(); createFolderFromProjectHeader('My Folder'); getFolderCards().should('have.length.greaterThan', 0); // Clicking on the success toast should navigate to the folder @@ -51,6 +71,33 @@ describe('Folders', () => { getCurrentBreadcrumb().should('contain.text', 'My Folder'); }); + it('should not allow illegal folder names', () => { + // Validation logic is thoroughly tested in unit tests + // Here we just make sure everything is working in the full UI + const ILLEGAL_CHARACTERS_NAME = 'hello['; + const ONLY_DOTS_NAME = '...'; + const REGULAR_NAME = 'My Folder'; + + getPersonalProjectMenuItem().click(); + getAddResourceDropdown().click(); + cy.getByTestId('action-folder').click(); + getNewFolderNameInput().type(ILLEGAL_CHARACTERS_NAME, { delay: 50 }); + getNewFolderModalErrorMessage().should( + 'contain.text', + 'Folder name cannot contain the following characters', + ); + getNewFolderNameInput().clear(); + getNewFolderNameInput().type(ONLY_DOTS_NAME, { delay: 50 }); + getNewFolderModalErrorMessage().should( + 'contain.text', + 'Folder name cannot contain only dots', + ); + getNewFolderNameInput().clear(); + getNewFolderModalErrorMessage().should('contain.text', 'Folder name cannot be empty'); + getNewFolderNameInput().type(REGULAR_NAME, { delay: 50 }); + getNewFolderModalErrorMessage().should('not.exist'); + }); + it('should create folder from the list header button', () => { goToPersonalProject(); // First create a folder so list appears @@ -78,9 +125,9 @@ describe('Folders', () => { getFolderCard('Created from card dropdown').should('exist'); createFolderFromCardActions('Created from card dropdown', 'Child Folder'); successToast().should('exist'); - // Open parent folder to see the new child folder - getFolderCard('Created from card dropdown').click(); + // Should be automatically navigated to the new folder getFolderCard('Child Folder').should('exist'); + getCurrentBreadcrumb().should('contain.text', 'Created from card dropdown'); }); it('should navigate folders using breadcrumbs and dropdown menu', () => { @@ -88,7 +135,7 @@ describe('Folders', () => { createFolderFromProjectHeader('Navigate Test'); // Open folder using menu item getFolderCardActionToggle('Navigate Test').click(); - getFolderCardActionItem('open').click(); + getFolderCardActionItem('Navigate Test', 'open').click(); getCurrentBreadcrumb().should('contain.text', 'Navigate Test'); // Create new child folder and navigate to it createFolderFromListHeaderButton('Child Folder'); @@ -165,12 +212,72 @@ describe('Folders', () => { // In personal, we should see previously created folders getPersonalProjectMenuItem().click(); + getAddResourceDropdown().click(); cy.getByTestId('action-folder').should('exist'); createFolderFromProjectHeader('Personal Folder'); getFolderCards().should('exist'); }); }); + describe('Empty State', () => { + it('should show project empty state when no folders exist', () => { + createNewProject('Test empty project', { openAfterCreate: true }); + getProjectEmptyState().should('exist'); + }); + + it('should toggle folder empty state correctly', () => { + createNewProject('Test empty folder', { openAfterCreate: true }); + createFolderFromProjectHeader('My Folder'); + getProjectEmptyState().should('not.exist'); + getFolderCard('My Folder').should('exist'); + getFolderCard('My Folder').click(); + getFolderEmptyState().should('exist'); + // Create a new workflow from the empty state + createWorkflowFromEmptyState('My Workflow'); + // Toast should inform that the workflow was created in the folder + successToast().should( + 'contain.text', + 'Workflow successfully created in "Test empty folder", within "My Folder"', + ); + // Go back to the folder + getProjectMenuItem('Test empty folder').click(); + getFolderCard('My Folder').should('exist'); + getFolderCard('My Folder').click(); + // Should not show empty state anymore + getFolderEmptyState().should('not.exist'); + getWorkflowCards().should('have.length.greaterThan', 0); + // Also when filtering and there are no results, empty state CTA should not show + cy.getByTestId('resources-list-search').type('non-existing', { delay: 20 }); + getWorkflowCards().should('not.exist'); + getFolderEmptyState().should('not.exist'); + // But there should be a message saying that no results were found + cy.getByTestId('resources-list-empty').should('exist'); + }); + }); + + describe('Create workflows inside folders', () => { + it('should create workflows in folders in all supported ways', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Workflows go here'); + // 1. From empty state + getFolderCard('Workflows go here').should('exist').click(); + createWorkflowFromEmptyState('Created from empty state'); + goToPersonalProject(); + getFolderCard('Workflows go here').click(); + getWorkflowCard('Created from empty state').should('exist'); + // 2. From the project header + createWorkflowFromProjectHeader('Workflows go here', 'Created from project header'); + goToPersonalProject(); + getFolderCard('Workflows go here').click(); + getWorkflowCard('Created from project header').should('exist'); + // 3. From list breadcrumbs + createWorkflowFromListDropdown('Created from list breadcrumbs'); + goToPersonalProject(); + getFolderCard('Workflows go here').click(); + getWorkflowCard('Created from list breadcrumbs').should('exist'); + }); + }); + describe('Rename and delete folders', () => { it('should rename folder from main dropdown', () => { goToPersonalProject(); @@ -224,6 +331,175 @@ describe('Folders', () => { deleteFolderWithContentsFromCardDropdown('I also have family'); }); - // TODO: Once we have backend endpoint that lists project folders, test transfer when deleting + it('should transfer contents when deleting non-empty folder - from card dropdown', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Move my contents'); + createFolderFromProjectHeader('Destination'); + createFolderInsideFolder('Child 1', 'Move my contents'); + getHomeProjectBreadcrumb().click(); + getFolderCard('Move my contents').should('exist'); + deleteAndTransferFolderContentsFromCardDropdown('Move my contents', 'Destination'); + getFolderCard('Destination').click(); + // Should show the contents of the moved folder + getFolderCard('Child 1').should('exist'); + }); + + it('should transfer contents when deleting non-empty folder - from list breadcrumbs', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Move me too'); + createFolderFromProjectHeader('Destination 2'); + createFolderInsideFolder('Child 1', 'Move me too'); + deleteAndTransferFolderContentsFromListDropdown('Destination 2'); + getFolderCard('Destination').click(); + // Should show the contents of the moved folder + getFolderCard('Child 1').should('exist'); + }); + }); + + describe('Move folders and workflows', () => { + it('should move empty folder to another folder - from folder card action', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Move me - I am empty'); + createFolderFromProjectHeader('Destination 3'); + moveFolderFromFolderCardActions('Move me - I am empty', 'Destination 3'); + getFolderCard('Destination 3').click(); + getFolderCard('Move me - I am empty').should('exist'); + getFolderCard('Move me - I am empty').click(); + getFolderEmptyState().should('exist'); + successToast().should('contain.text', 'Move me - I am empty has been moved to Destination 3'); + // Breadcrumbs should show the destination folder + getListBreadcrumbItem('Destination 3').should('exist'); + }); + + it('should move folder with contents to another folder - from folder card action', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Move me - I have family'); + createFolderFromProjectHeader('Destination 4'); + // Create a workflow and a folder inside the folder + createFolderInsideFolder('Child 1', 'Move me - I have family'); + createWorkflowFromProjectHeader('Move me - I have family'); + goToPersonalProject(); + // Move the folder + moveFolderFromFolderCardActions('Move me - I have family', 'Destination 4'); + successToast().should( + 'contain.text', + 'Move me - I have family has been moved to Destination 4', + ); + // Go to destination folder and check if contents are there + getFolderCard('Destination 4').click(); + // Moved folder should be there + getFolderCard('Move me - I have family').should('exist').click(); + // Both the workflow and the folder should be there + getFolderCards().should('have.length', 1); + getWorkflowCards().should('have.length', 1); + // Breadcrumbs should show the destination folder + getListBreadcrumbItem('Destination 4').should('exist'); + }); + + it('should move empty folder to another folder - from list breadcrumbs', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Move me too - I am empty'); + createFolderFromProjectHeader('Destination 5'); + moveFolderFromListActions('Move me too - I am empty', 'Destination 5'); + // Since we moved the current folder, we should be in the destination folder + getCurrentBreadcrumb().should('contain.text', 'Destination 5'); + }); + + it('should move folder with contents to another folder - from list dropdown', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Move me - I have family 2'); + createFolderFromProjectHeader('Destination 6'); + // Create a workflow and a folder inside the folder + createFolderInsideFolder('Child 1', 'Move me - I have family 2'); + createWorkflowFromProjectHeader('Move me - I have family 2'); + // Navigate back to folder + goToPersonalProject(); + getFolderCard('Move me - I have family 2').should('exist'); + // Move the folder + moveFolderFromListActions('Move me - I have family 2', 'Destination 6'); + // Since we moved the current folder, we should be in the destination folder + getCurrentBreadcrumb().should('contain.text', 'Destination 6'); + // Moved folder should be there + getFolderCard('Move me - I have family 2').should('exist').click(); + // After navigating to the moved folder, both the workflow and the folder should be there + getFolderCards().should('have.length', 1); + getWorkflowCards().should('have.length', 1); + // Breadcrumbs should show the destination folder + getListBreadcrumbItem('Destination 6').should('exist'); + }); + + it('should move folder to project root - from folder card action', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Test parent'); + createFolderInsideFolder('Move me to root', 'Test parent'); + moveFolderFromFolderCardActions('Move me to root', 'Personal'); + // Parent folder should be empty + getFolderEmptyState().should('exist'); + // Child folder should be in the root + goToPersonalProject(); + getFolderCard('Move me to root').should('exist'); + // Navigate to the moved folder and check breadcrumbs + getFolderCard('Move me to root').click(); + getHomeProjectBreadcrumb().should('contain.text', 'Personal'); + getListBreadcrumbs().findChildByTestId('breadcrumbs-item').should('not.exist'); + getCurrentBreadcrumb().should('contain.text', 'Move me to root'); + }); + + it('should move workflow from project root to folder', () => { + goToPersonalProject(); + createWorkflowFromProjectHeader(undefined, 'Move me'); + goToPersonalProject(); + createFolderFromProjectHeader('Workflow destination'); + moveWorkflowToFolder('Move me', 'Workflow destination'); + successToast().should('contain.text', 'Move me has been moved to Workflow destination'); + // Navigate to the destination folder + getFolderCard('Workflow destination').click(); + // Moved workflow should be there + getWorkflowCards().should('have.length', 1); + getWorkflowCard('Move me').should('exist'); + }); + + it('should move workflow to another folder', () => { + goToPersonalProject(); + createFolderFromProjectHeader('Moving workflow from here'); + createFolderFromProjectHeader('Moving workflow to here'); + getFolderCard('Moving workflow from here').click(); + createWorkflowFromProjectHeader(undefined, 'Move me'); + goToPersonalProject(); + getFolderCard('Moving workflow from here').click(); + getWorkflowCard('Move me').should('exist'); + moveWorkflowToFolder('Move me', 'Moving workflow to here'); + // Now folder should be empty + getFolderEmptyState().should('exist'); + // Navigate to the destination folder + getHomeProjectBreadcrumb().click(); + getFolderCard('Moving workflow to here').click(); + // Moved workflow should be there + getWorkflowCards().should('have.length', 1); + getWorkflowCard('Move me').should('exist'); + }); + }); + + describe('Workflow card breadcrumbs', () => { + it('should correctly show workflow card breadcrumbs', () => { + createNewProject('Test workflow breadcrumbs', { openAfterCreate: true }); + createFolderFromProjectHeader('Parent Folder'); + createFolderInsideFolder('Child Folder', 'Parent Folder'); + getFolderCard('Child Folder').click(); + createFolderFromListHeaderButton('Child Folder 2'); + getFolderCard('Child Folder 2').click(); + createWorkflowFromEmptyState('Breadcrumbs Test'); + // Go to overview page + getOverviewMenuItem().click(); + getWorkflowCard('Breadcrumbs Test').should('exist'); + getWorkflowCardBreadcrumbs('Breadcrumbs Test').should('exist'); + getWorkflowCardBreadcrumbsEllipsis('Breadcrumbs Test').should('exist'); + getWorkflowCardBreadcrumbsEllipsis('Breadcrumbs Test').realHover({ position: 'topLeft' }); + cy.get('[role=tooltip]').should('exist'); + cy.get('[role=tooltip]').should( + 'contain.text', + 'est workflow breadcrumbs / Parent Folder / Child Folder / Child Folder 2', + ); + }); }); }); diff --git a/cypress/pages/workflows.ts b/cypress/pages/workflows.ts index 5e7a298890c..a57d9d22eaf 100644 --- a/cypress/pages/workflows.ts +++ b/cypress/pages/workflows.ts @@ -19,12 +19,12 @@ export class WorkflowsPage extends BasePage { cy.getByTestId('add-resource-workflow').should('be.visible'); return cy.getByTestId('add-resource-workflow'); }, - workflowCards: () => cy.getByTestId('resources-list-item'), + workflowCards: () => cy.getByTestId('resources-list-item-workflow'), workflowCard: (workflowName: string) => this.getters .workflowCards() .contains(workflowName) - .parents('[data-test-id="resources-list-item"]'), + .parents('[data-test-id="resources-list-item-workflow"]'), workflowTags: (workflowName: string) => this.getters.workflowCard(workflowName).findChildByTestId('workflow-card-tags'), workflowCardContent: (workflowName: string) => diff --git a/packages/frontend/@n8n/design-system/src/components/N8nActionBox/ActionBox.vue b/packages/frontend/@n8n/design-system/src/components/N8nActionBox/ActionBox.vue index 75b5fa2607c..0c9c4e07735 100644 --- a/packages/frontend/@n8n/design-system/src/components/N8nActionBox/ActionBox.vue +++ b/packages/frontend/@n8n/design-system/src/components/N8nActionBox/ActionBox.vue @@ -37,7 +37,7 @@ withDefaults(defineProps(), { {{ heading }} -
+
diff --git a/packages/frontend/@n8n/design-system/src/components/N8nBreadcrumbs/Breadcrumbs.vue b/packages/frontend/@n8n/design-system/src/components/N8nBreadcrumbs/Breadcrumbs.vue index 1c82fe2cdc5..38a7bb9c89a 100644 --- a/packages/frontend/@n8n/design-system/src/components/N8nBreadcrumbs/Breadcrumbs.vue +++ b/packages/frontend/@n8n/design-system/src/components/N8nBreadcrumbs/Breadcrumbs.vue @@ -19,6 +19,7 @@ type Props = { loadingSkeletonRows?: number; separator?: string; highlightLastItem?: boolean; + hiddenItemsTrigger?: 'hover' | 'click'; // Setting this to true will show the ellipsis even if there are no hidden items pathTruncated?: boolean; }; @@ -40,6 +41,7 @@ const props = withDefaults(defineProps(), { separator: '/', highlightLastItem: true, isPathTruncated: false, + hiddenItemsTrigger: 'click', }); const loadedHiddenItems = ref([]); @@ -170,7 +172,8 @@ const handleTooltipClose = () => { v-else :popper-class="$style.tooltip" :disabled="dropdownDisabled" - trigger="click" + :trigger="hiddenItemsTrigger" + placement="bottom" @before-show="handleTooltipShow" @hide="handleTooltipClose" > @@ -313,6 +316,7 @@ const handleTooltipClose = () => { .tooltip { padding: var(--spacing-xs) var(--spacing-2xs); + text-align: center; & > div { color: var(--color-text-lighter); span { @@ -352,6 +356,7 @@ const handleTooltipClose = () => { color: var(--color-text-base); font-size: var(--font-size-2xs); font-weight: var(--font-weight-bold); + line-height: var(--font-line-heigh-xsmall); } .item a:hover * { diff --git a/packages/frontend/@n8n/design-system/src/css/_tokens.scss b/packages/frontend/@n8n/design-system/src/css/_tokens.scss index 223f548b887..6aa16e21526 100644 --- a/packages/frontend/@n8n/design-system/src/css/_tokens.scss +++ b/packages/frontend/@n8n/design-system/src/css/_tokens.scss @@ -616,6 +616,7 @@ --font-size-xl: 1.25rem; --font-size-2xl: 1.75rem; + --font-line-heigh-xsmall: 1; --font-line-height-compact: 1.25; --font-line-height-regular: 1.3; --font-line-height-loose: 1.35; diff --git a/packages/frontend/@n8n/design-system/src/types/user.ts b/packages/frontend/@n8n/design-system/src/types/user.ts index b5b7f512cfa..4af8b52dbc0 100644 --- a/packages/frontend/@n8n/design-system/src/types/user.ts +++ b/packages/frontend/@n8n/design-system/src/types/user.ts @@ -16,6 +16,7 @@ export interface UserAction { value: string; disabled: boolean; type?: 'external-link'; + tooltip?: string; guard?: (user: IUser) => boolean; } diff --git a/packages/frontend/editor-ui/src/Interface.ts b/packages/frontend/editor-ui/src/Interface.ts index 98e87b56436..c44867141dc 100644 --- a/packages/frontend/editor-ui/src/Interface.ts +++ b/packages/frontend/editor-ui/src/Interface.ts @@ -326,6 +326,7 @@ export interface IWorkflowDb { versionId: string; usedCredentials?: IUsedCredential[]; meta?: WorkflowMetadata; + parentFolder?: { id: string; name: string }; } // For workflow list we don't need the full workflow data @@ -339,7 +340,6 @@ export type WorkflowListItem = Omit< 'nodes' | 'connections' | 'settings' | 'pinData' | 'usedCredentials' | 'meta' > & { resource: 'workflow'; - parentFolder?: { id: string; name: string }; }; export type FolderShortInfo = { @@ -363,6 +363,10 @@ export interface FolderListItem extends BaseFolderItem { resource: 'folder'; } +export interface ChangeLocationSearchResult extends BaseFolderItem { + resource: 'folder' | 'project'; +} + export type FolderPathItem = PathItem & { parentFolder?: string }; export type WorkflowListResource = WorkflowListItem | FolderListItem; diff --git a/packages/frontend/editor-ui/src/api/workflows.ts b/packages/frontend/editor-ui/src/api/workflows.ts index 9d2bd5c0c64..c3d23a22ef1 100644 --- a/packages/frontend/editor-ui/src/api/workflows.ts +++ b/packages/frontend/editor-ui/src/api/workflows.ts @@ -1,6 +1,6 @@ import type { + ChangeLocationSearchResult, FolderCreateResponse, - FolderListItem, FolderTreeResponseItem, IExecutionResponse, IExecutionsCurrentSummaryExtended, @@ -146,8 +146,8 @@ export async function getProjectFolders( excludeFolderIdAndDescendants?: string; name?: string; }, -): Promise { - const res = await getFullApiResponse( +): Promise { + const res = await getFullApiResponse( context, 'GET', `/projects/${projectId}/folders`, diff --git a/packages/frontend/editor-ui/src/components/CommunityPlusEnrollmentModal.vue b/packages/frontend/editor-ui/src/components/CommunityPlusEnrollmentModal.vue index 6db5e704264..fddfe8e3fd5 100644 --- a/packages/frontend/editor-ui/src/components/CommunityPlusEnrollmentModal.vue +++ b/packages/frontend/editor-ui/src/components/CommunityPlusEnrollmentModal.vue @@ -15,6 +15,7 @@ const props = defineProps<{ modalName: string; data?: { closeCallback?: () => void; + customHeading?: string; }; }>(); @@ -89,7 +90,7 @@ const confirm = async () => { {{ i18n.baseText('communityPlusModal.badge') }}

{{ - i18n.baseText('communityPlusModal.title') + data?.customHeading ?? i18n.baseText('communityPlusModal.title') }} {{ i18n.baseText('communityPlusModal.description') }}
    @@ -114,6 +115,13 @@ const confirm = async () => { {{ i18n.baseText('communityPlusModal.features.third.description') }} +
  • + 📁 + + {{ i18n.baseText('communityPlusModal.features.fourth.title') }} + {{ i18n.baseText('communityPlusModal.features.fourth.description') }} + +
(null); -const projectFolders = ref([]); - -const currentFolder = computed(() => { - return projectFolders.value.find((folder) => folder.id === props.activeId); -}); +const selectedFolder = ref<{ id: string; name: string; type: 'folder' | 'project' } | null>(null); const folderToDelete = computed(() => { if (!props.activeId) return null; @@ -72,6 +67,14 @@ const enabled = computed(() => { return false; }); +const currentProjectName = computed(() => { + const currentProject = projectsStore.currentProject; + if (currentProject?.type === ProjectTypes.Personal) { + return i18n.baseText('projects.menu.personal'); + } + return currentProject?.name; +}); + const folderContentWarningMessage = computed(() => { const folderCount = props.data.content.subFolderCount ?? 0; const workflowCount = props.data.content.workflowCount ?? 0; @@ -102,11 +105,10 @@ async function onSubmit() { try { loading.value = true; - await foldersStore.deleteFolder( - route.params.projectId as string, - props.activeId, - selectedFolder.value?.id ?? undefined, - ); + const newParentId = + selectedFolder.value?.type === 'project' ? '0' : (selectedFolder.value?.id ?? undefined); + + await foldersStore.deleteFolder(route.params.projectId as string, props.activeId, newParentId); let message = ''; if (selectedFolder.value) { @@ -132,7 +134,7 @@ async function onSubmit() { } } -const onFolderSelected = (payload: { id: string; name: string }) => { +const onFolderSelected = (payload: { id: string; name: string; type: 'folder' | 'project' }) => { selectedFolder.value = payload; }; @@ -142,7 +144,7 @@ const onFolderSelected = (payload: { id: string; name: string }) => { :name="modalName" :title="title" :center="true" - width="520" + width="600" :event-bus="modalBus" @enter="onSubmit" > @@ -163,7 +165,14 @@ const onFolderSelected = (payload: { id: string; name: string }) => { label="transfer" @update:model-value="operation = 'transfer'" > - {{ i18n.baseText('folders.transfer.action') }} + {{ + i18n.baseText('folders.transfer.action', { + interpolate: { projectName: currentProjectName }, + }) + }} + {{ + i18n.baseText('folders.transfer.action.noProject') + }}
{{ @@ -173,8 +182,8 @@ const onFolderSelected = (payload: { id: string; name: string }) => { v-if="projectsStore.currentProject" :current-folder-id="props.activeId" :current-project-id="projectsStore.currentProject?.id" - :parent-folder-id="currentFolder?.parentFolder?.id" - @folder:selected="onFolderSelected" + :parent-folder-id="folderToDelete?.parentFolder" + @location:selected="onFolderSelected" />
{ { align-items: center; } +.action-toggle { + span[role='button'] { + color: var(--color-text-base); + } +} + .home-project { display: flex; align-items: center; diff --git a/packages/frontend/editor-ui/src/components/Folders/MoveToFolderDropdown.vue b/packages/frontend/editor-ui/src/components/Folders/MoveToFolderDropdown.vue index d8d9ca7553a..ebdcbf1b78f 100644 --- a/packages/frontend/editor-ui/src/components/Folders/MoveToFolderDropdown.vue +++ b/packages/frontend/editor-ui/src/components/Folders/MoveToFolderDropdown.vue @@ -1,9 +1,11 @@ @@ -71,22 +117,35 @@ const onFolderSelected = (folderId: string) => { v-model="selectedFolderId" :filterable="true" :remote="true" - :remote-method="fetchAvailableFolders" + :remote-method="fetchAvailableLocations" :loading="loading" :placeholder="i18n.baseText('folders.move.modal.select.placeholder')" + :no-data-text="i18n.baseText('folders.move.modal.no.data.label')" option-label="name" option-value="id" @update:model-value="onFolderSelected" > +
- - {{ folder.name }} + + + {{ location.name }}
diff --git a/packages/frontend/editor-ui/src/components/Folders/MoveToFolderModal.vue b/packages/frontend/editor-ui/src/components/Folders/MoveToFolderModal.vue index c5d32d0ea68..1bbdd8181a8 100644 --- a/packages/frontend/editor-ui/src/components/Folders/MoveToFolderModal.vue +++ b/packages/frontend/editor-ui/src/components/Folders/MoveToFolderModal.vue @@ -48,7 +48,7 @@ const currentFolder = computed(() => { }; }); -const onFolderSelected = (payload: { id: string; name: string }) => { +const onFolderSelected = (payload: { id: string; name: string; type: string }) => { selectedFolder.value = payload; }; @@ -81,7 +81,7 @@ const onSubmit = () => { :current-project-id="projectsStore.currentProject?.id" :parent-folder-id="props.data.resource.parentFolderId" :exclude-only-parent="props.data.resourceType === 'workflow'" - @folder:selected="onFolderSelected" + @location:selected="onFolderSelected" />

{ return (props.tags ?? []).map((tag) => (typeof tag === 'string' ? tag : tag.id)); }); +const currentFolder = computed(() => { + if (props.id === PLACEHOLDER_EMPTY_WORKFLOW_ID) { + return undefined; + } + + const workflow = workflowsStore.getWorkflowById(props.id); + if (!workflow) { + return undefined; + } + + return workflow.parentFolder; +}); + +const currentProjectName = computed(() => { + if (projectsStore.currentProject?.type === ProjectTypes.Personal) { + return locale.baseText('projects.menu.personal'); + } + return projectsStore.currentProject?.name; +}); + watch( () => props.id, () => { @@ -533,16 +554,22 @@ function showCreateWorkflowSuccessToast(id?: string) { let toastTitle = locale.baseText('workflows.create.personal.toast.title'); let toastText = locale.baseText('workflows.create.personal.toast.text'); - if ( - projectsStore.currentProject && - projectsStore.currentProject.id !== projectsStore.personalProject?.id - ) { - toastTitle = locale.baseText('workflows.create.project.toast.title', { - interpolate: { projectName: projectsStore.currentProject.name ?? '' }, - }); + if (projectsStore.currentProject) { + if (currentFolder.value) { + toastTitle = locale.baseText('workflows.create.folder.toast.title', { + interpolate: { + projectName: currentProjectName.value ?? '', + folderName: currentFolder.value.name ?? '', + }, + }); + } else if (projectsStore.currentProject.id !== projectsStore.personalProject?.id) { + toastTitle = locale.baseText('workflows.create.project.toast.title', { + interpolate: { projectName: currentProjectName.value ?? '' }, + }); + } toastText = locale.baseText('workflows.create.project.toast.text', { - interpolate: { projectName: projectsStore.currentProject.name ?? '' }, + interpolate: { projectName: currentProjectName.value ?? '' }, }); } diff --git a/packages/frontend/editor-ui/src/components/PersonalizationModal.vue b/packages/frontend/editor-ui/src/components/PersonalizationModal.vue index ea45b373240..84051691fb7 100644 --- a/packages/frontend/editor-ui/src/components/PersonalizationModal.vue +++ b/packages/frontend/editor-ui/src/components/PersonalizationModal.vue @@ -567,6 +567,7 @@ const closeDialog = () => { name: COMMUNITY_PLUS_ENROLLMENT_MODAL, data: { closeCallback, + customHeading: undefined, }, }); } else { diff --git a/packages/frontend/editor-ui/src/components/Projects/ProjectHeader.vue b/packages/frontend/editor-ui/src/components/Projects/ProjectHeader.vue index b0d053a3a54..b7d26432b23 100644 --- a/packages/frontend/editor-ui/src/components/Projects/ProjectHeader.vue +++ b/packages/frontend/editor-ui/src/components/Projects/ProjectHeader.vue @@ -56,6 +56,7 @@ const showSettings = computed( ); const homeProject = computed(() => projectsStore.currentProject ?? projectsStore.personalProject); + const showFolders = computed(() => { return settingsStore.isFoldersFeatureEnabled && route.name !== VIEWS.WORKFLOWS; }); @@ -189,7 +190,7 @@ const onSelect = (action: string) => { } .actions { - padding: var(--spacing-2xs) 0 var(--spacing-l); + padding: var(--spacing-2xs) 0 var(--spacing-xs); } @include mixins.breakpoint('xs-only') { diff --git a/packages/frontend/editor-ui/src/components/WorkflowCard.vue b/packages/frontend/editor-ui/src/components/WorkflowCard.vue index 8503bb4dbc6..b39c0f8a069 100644 --- a/packages/frontend/editor-ui/src/components/WorkflowCard.vue +++ b/packages/frontend/editor-ui/src/components/WorkflowCard.vue @@ -1,5 +1,5 @@