From 809a5715eb8f9e8c8332a9888889073815ad4560 Mon Sep 17 00:00:00 2001 From: alvarosabu Date: Wed, 4 Jun 2025 08:53:15 +0200 Subject: [PATCH 1/2] fix(components): enhance circular dependency handling in resource fetching - Added a mechanism to track visited components during resource fetching to prevent circular dependencies. - Updated the `findRelatedResources` function to filter out components that have already been visited, ensuring that only new components are processed. - Improved error handling by skipping components that would create circular dependencies, enhancing the robustness of the component operations. --- src/commands/components/push/operations.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/commands/components/push/operations.ts b/src/commands/components/push/operations.ts index 6e63048..884bcce 100644 --- a/src/commands/components/push/operations.ts +++ b/src/commands/components/push/operations.ts @@ -16,6 +16,7 @@ import { delay } from '../../../utils/fetch'; function findRelatedResources( components: SpaceComponent[], spaceData: SpaceData, + visitedComponents = new Set(), // Track visited components to detect cycles ): { groups: SpaceComponentGroup[]; presets: SpaceComponentPreset[]; @@ -147,7 +148,16 @@ function findRelatedResources( }; if (whitelistedComponents.length > 0) { - additionalResources = findRelatedResources(whitelistedComponents, spaceData); + // Filter out components that would create circular dependencies + const newComponents = whitelistedComponents.filter(component => !visitedComponents.has(component.name)); + + if (newComponents.length > 0) { + // Add current components to visited set + components.forEach(component => visitedComponents.add(component.name)); + + // Only process components we haven't seen before + additionalResources = findRelatedResources(newComponents, spaceData, visitedComponents); + } } const result = { @@ -574,13 +584,8 @@ export async function handleWhitelists( return; } - // Check for circular dependencies + // Skip if we detect a circular dependency if (visited.has(componentName)) { - failedComponents.add(componentName); - results.failed.push({ - name: componentName, - error: new Error(`Circular dependency detected for component ${componentName}`), - }); return; } From 5c9c05831537125b1d35aa0947d7953c1666883b Mon Sep 17 00:00:00 2001 From: alvarosabu Date: Wed, 4 Jun 2025 08:58:37 +0200 Subject: [PATCH 2/2] fix(tests): update circular dependency handling tests for improved accuracy - Mocked successful responses in the test for circular dependency handling to ensure accurate verification of component processing. - Adjusted assertions to reflect that circular dependencies are now handled by skipping, resulting in no failures and confirming that only valid components are processed. - This change enhances the robustness of the tests and ensures they align with the updated logic for handling circular dependencies. --- .vscode/launch.json | 11 +++++++++++ src/commands/components/push/operations.test.ts | 14 +++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 6564178..c5c777e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -134,6 +134,17 @@ "sourceMaps": true, "outFiles": ["${workspaceFolder}/dist/**/*.js"] }, + { + "type": "node", + "request": "launch", + "name": "Debug circular dependencies", + "program": "${workspaceFolder}/dist/index.mjs", + "args": ["components", "push", "component-component-whitelist", "--space", "295018", "--from", "295017", "--verbose"], + "cwd": "${workspaceFolder}", + "console": "integratedTerminal", + "sourceMaps": true, + "outFiles": ["${workspaceFolder}/dist/**/*.js"] + }, { "type": "node", "request": "launch", diff --git a/src/commands/components/push/operations.test.ts b/src/commands/components/push/operations.test.ts index 9bc3f19..c4424a4 100644 --- a/src/commands/components/push/operations.test.ts +++ b/src/commands/components/push/operations.test.ts @@ -931,13 +931,17 @@ describe('operations', () => { ], }; + // Mock successful responses + vi.mocked(upsertComponent).mockImplementation(async (_space, component) => ({ + ...component, + id: component.id + 1000, + })); + const results = await handleWhitelists(mockSpace, mockPassword, mockRegion, circularData); - // Verify that circular dependency was detected and handled - expect(results.failed).toHaveLength(1); - expect(results.failed[0].error).toMatchObject({ - message: expect.stringContaining('Circular dependency detected'), - }); + // Verify that circular dependency was handled by skipping + expect(results.failed).toHaveLength(0); // No failures + expect(results.successful).toHaveLength(2); // Only two components were processed }); it('should handle missing dependencies gracefully', async () => {