Expose consumers API and recursive export option#7309
Expose consumers API and recursive export option#7309sfmskywalker merged 14 commits intorelease/3.6.0from
Conversation
…rkflows option
- New endpoint: GET /workflow-definitions/{definitionId}/consumers
- Export request model: added IncludeConsumingWorkflows boolean property
- Export endpoint: recursive consumer discovery and inclusion in exports
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
…shSet, add version comment, remove unused params - Extract WriteZipResponseAsync helper to avoid double-fetching definitions from DB - Remove redundant existingDefinitionIds HashSet in IncludeConsumersAsync - Add XML doc comment documenting that consumers are resolved at VersionOptions.Latest - Remove unused constructor parameters (workflowDefinitionService, variableDefinitionMapper) Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
…stic ZIP response, parallel BFS - Consumers endpoint now returns 404 when the definition ID doesn't exist - Single-workflow export always returns ZIP when includeConsumingWorkflows=true - BFS traversal processes each frontier level concurrently via Task.WhenAll Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Greptile SummaryThis PR adds a new Key Changes:
Note: The PR description mentions "parallel BFS" but the implementation uses sequential depth-first traversal with cycle detection, which is appropriate for this use case. Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Consumers/Endpoint.cs | New endpoint to list consuming workflows. Properly validates existence with 404, uses correct permissions. |
| src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Endpoint.cs | Adds recursive consumer export. Filename collision issue already noted. Deterministic ZIP generation looks good. |
| src/modules/Elsa.Workflows.Management/Services/WorkflowReferenceGraphBuilder.cs | New depth-first traversal with cycle detection and limits. Not parallel BFS as PR claims, but implementation is sound. |
| src/modules/Elsa.Workflows.Management/Services/WorkflowReferenceUpdater.cs | Refactored to use WorkflowReferenceGraphBuilder instead of inline recursion. Cleaner implementation. |
| src/modules/Elsa.Workflows.Management/Models/WorkflowReferenceGraph.cs | Well-designed graph data structure with pre-computed lookups for efficient queries. |
| test/unit/Elsa.Workflows.Management.UnitTests/Services/WorkflowReferenceGraphBuilderTests.cs | Comprehensive unit tests cover cycles, diamonds, depth limits, and multi-root scenarios. |
Sequence Diagram
sequenceDiagram
participant Client
participant ConsumersEndpoint
participant ExportEndpoint
participant GraphBuilder
participant Query as IWorkflowReferenceQuery
participant Store as IWorkflowDefinitionStore
Note over Client,Store: Consumers API Flow
Client->>ConsumersEndpoint: GET /workflow-definitions/{id}/consumers
ConsumersEndpoint->>Store: FindAsync(definitionId, Latest)
Store-->>ConsumersEndpoint: WorkflowDefinition or null
alt Definition not found
ConsumersEndpoint-->>Client: 404 Not Found
else Definition found
ConsumersEndpoint->>GraphBuilder: BuildGraphAsync(definitionId)
loop Recursive DFS
GraphBuilder->>Query: ExecuteAsync(definitionId)
Query-->>GraphBuilder: List of consumer IDs
Note over GraphBuilder: Builds edges, checks cycles<br/>and depth/definition limits
end
GraphBuilder-->>ConsumersEndpoint: WorkflowReferenceGraph
ConsumersEndpoint-->>Client: 200 OK with consumer IDs
end
Note over Client,Store: Export with Consumers Flow
Client->>ExportEndpoint: GET /workflow-definitions/{id}/export?includeConsumingWorkflows=true
ExportEndpoint->>Store: FindAsync(definitionId)
Store-->>ExportEndpoint: WorkflowDefinition
ExportEndpoint->>GraphBuilder: BuildGraphAsync(definitionId)
GraphBuilder-->>ExportEndpoint: WorkflowReferenceGraph
ExportEndpoint->>Store: FindManyAsync(consumerIds, Latest)
Store-->>ExportEndpoint: Consumer WorkflowDefinitions
Note over ExportEndpoint: Generate ZIP with all<br/>definitions (deterministic)
ExportEndpoint-->>Client: ZIP file with all workflows
Last reviewed commit: 897de6d
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Endpoint.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Endpoint.cs
Outdated
Show resolved
Hide resolved
- Added `IWorkflowReferenceGraphBuilder` interface for building complete graphs of workflow references. - Created `WorkflowReferenceGraph` and `WorkflowReferenceEdge` models. - Implemented `WorkflowReferenceGraphBuilder` service for recursive graph building. - Replaced previous workflow reference query logic with the new graph-based approach in consumers.
… for retrieving consumer definitions.
…kflows and update nullable default values across endpoints and models.
…iguration - Introduced `WorkflowReferenceGraphOptions` to configure max depth and definition limits for reference graph building. - Updated `WorkflowManagementFeature` to support configuring these options. - Enhanced `WorkflowReferenceGraphBuilder` to respect configuration limits during graph construction process.
…expressions for cleaner code.
Introduced JSON files for parent-child-grandchild workflow hierarchy tests and implemented unit tests for `WorkflowReferenceGraphBuilder`. Also added component tests for workflow export and consumers endpoint validation. Updated project configuration for workflow JSON to always copy to output directory.
…with `SetupGraph`, replace repeated assertions with helper methods.
|
@greptile please do another review. |
src/modules/Elsa.Workflows.Management/Services/WorkflowReferenceGraphBuilder.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This pull request introduces reverse dependency analysis and enhanced export capabilities for workflow definitions in Elsa Workflows. It adds a new API endpoint to discover consuming workflows and extends the export functionality to recursively include all workflows that depend on a given workflow.
Changes:
- Introduces
IWorkflowReferenceGraphBuilderwith graph-based workflow reference discovery - Adds
/workflow-definitions/{definitionId}/consumersendpoint with 404 handling for unknown workflows - Enhances export endpoints with
IncludeConsumingWorkflowsflag for recursive consumer inclusion - Refactors
WorkflowReferenceUpdaterto use the new graph builder, eliminating duplicate traversal logic
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/Elsa.Workflows.Management/Services/WorkflowReferenceGraphBuilder.cs |
Implements recursive DFS traversal to build consumer reference graphs with configurable depth/size limits |
src/modules/Elsa.Workflows.Management/Models/WorkflowReferenceGraph.cs |
Defines graph data structure with edges, lookups, and helper methods for dependency analysis |
src/modules/Elsa.Workflows.Management/Options/WorkflowReferenceGraphOptions.cs |
Configuration options for traversal limits (MaxDepth=100, MaxDefinitions=1000) |
src/modules/Elsa.Workflows.Management/Contracts/IWorkflowReferenceGraphBuilder.cs |
Interface for building single-root and multi-root reference graphs |
src/modules/Elsa.Workflows.Management/Features/WorkflowManagementFeature.cs |
Registers graph builder service and configuration options |
src/modules/Elsa.Workflows.Management/Services/WorkflowReferenceUpdater.cs |
Refactored to use graph builder instead of custom recursion, leverages OutboundEdges for topological sort |
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Consumers/Endpoint.cs |
New endpoint returning transitive consumers with 404 for unknown definitions |
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Consumers/Models.cs |
Request/response models for consumers endpoint |
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Endpoint.cs |
Enhanced with IncludeConsumersAsync method, refactored ZIP generation, improved file naming with definitionId suffix |
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Models.cs |
Added IncludeConsumingWorkflows flag, changed default! to null! for nullable properties |
src/clients/Elsa.Api.Client/Resources/WorkflowDefinitions/Contracts/IWorkflowDefinitionsApi.cs |
Added GetConsumersAsync method and includeConsumingWorkflows parameter to export methods |
src/clients/Elsa.Api.Client/Resources/WorkflowDefinitions/Models/BulkExportWorkflowDefinitionsRequest.cs |
Added IncludeConsumingWorkflows parameter |
src/clients/Elsa.Api.Client/Resources/WorkflowDefinitions/Responses/GetConsumingWorkflowDefinitionsResponse.cs |
Response model for consumers endpoint |
test/unit/Elsa.Workflows.Management.UnitTests/Services/WorkflowReferenceGraphBuilderTests.cs |
Comprehensive unit tests covering direct/transitive consumers, diamond graphs, cycles, and traversal limits |
test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowReferenceGraph/WorkflowReferenceGraphTests.cs |
Component tests for consumers endpoint with parent→child→grandchild hierarchy |
test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowDefinitionExport/WorkflowDefinitionExportTests.cs |
Component tests verifying export with/without consumers for single and bulk operations |
test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowReferenceGraph/Workflows/*.json |
Test workflow definitions forming parent→child→grandchild reference chain |
test/component/Elsa.Workflows.ComponentTests/Elsa.Workflows.ComponentTests.csproj |
Added test workflow files to build output |
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Endpoint.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Export/Models.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Management/Services/WorkflowReferenceGraphBuilder.cs
Show resolved
Hide resolved
...lsa.Workflows.ComponentTests/Scenarios/WorkflowReferenceGraph/WorkflowReferenceGraphTests.cs
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the unresolved comments in this thread |
|
@sfmskywalker I've opened a new pull request, #7312, to work on those changes. Once the pull request is ready, I'll request review from you. |
…#7312) * Initial plan * Fix deterministic ZIP export and add 404 test for consumers endpoint Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
lukhipolito-nexxbiz
left a comment
There was a problem hiding this comment.
Looking good
Summary
Introduces a workflow reference graph to reliably discover consuming workflows, exposes a new Consumers API endpoint, and extends workflow export to optionally include transitive consuming workflows.
This replaces the previous recursive, query-based approach with a graph-based model and improves determinism, test coverage, and export behavior.
Key Changes
1. Workflow Reference Graph
IWorkflowReferenceGraphBuilderWorkflowReferenceGraph,WorkflowReferenceEdgeWorkflowReferenceGraphOptionsWorkflowReferenceGraphBuilder(implementation)WorkflowManagementFeatureWorkflowReferenceUpdaterwith graph-based dependency mapping.2. Consumers API
New endpoint:
IWorkflowDefinitionsApi.GetConsumersAsyncGetConsumingWorkflowDefinitionsResponse3. Export Enhancements
IncludeConsumingWorkflowsflag to:ExportrequestBulkExportWorkflowDefinitionsRequestExportAsync(query param, defaultfalse)VersionOptions.LatestAdditional improvements:
{Name}-{DefinitionId}.json)WriteZipResponseAsync4. API & Signature Adjustments
defaultparameter defaults withnullwhere appropriate in client APIsincludeConsumingWorkflowsquery parameter (defaultfalse)5. Tests
WorkflowReferenceGraphBuilderBehavioral Impact
Performance Considerations
WorkflowReferenceGraphOptions(MaxDepth,MaxDefinitions)Breaking Changes
None at the HTTP level.
Minor client signature default adjustments (
default→null) are non-breaking for typical usage.Internal DI now requires
IWorkflowReferenceGraphBuilder(registered viaWorkflowManagementFeature).