Add Default Workflow and Activity Commit Strategy Support#7148
Add Default Workflow and Activity Commit Strategy Support#7148KnibbsyMan merged 3 commits intoelsa-workflows:mainfrom
Conversation
…sage examples - Introduced methods to set default workflow and activity commit strategies in CommitStrategiesFeature. - Updated CommitStateOptions to include properties for default strategies. - Added extension methods for configuring default strategies in WorkflowsFeature. - Created usage examples demonstrating how to set and utilize default commit strategies. - Implemented tests to verify default strategy behavior in various scenarios.
…cy; remove unused example files and add a new CommitTracker helper for testing.
…st method signature for consistency
|
@KnibbsyMan I aborted my other PR due to the force push. Prior PR: #7137 for reference |
There was a problem hiding this comment.
Pull request overview
This PR introduces optional global default commit strategies for workflows and activities, providing application-wide defaults while maintaining override capabilities at the workflow/activity level. The implementation adds two new properties to CommitStateOptions, extension methods for configuration, and updates the middleware resolution logic to check for default strategies when no explicit strategy is specified.
Key Changes:
- Added default workflow and activity commit strategy support with proper resolution fallback chain
- Updated middleware to resolve default strategies when no explicit strategy is configured
- Added comprehensive integration tests (11 tests) validating all scenarios including commit counts
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
CommitTracker.cs |
New test helper to track commit invocations in integration tests |
DefaultWorkflowCommitStrategy/Tests.cs |
5 integration tests validating workflow-level default strategy behavior |
DefaultWorkflowCommitStrategy/*.cs |
Test workflow classes with and without explicit strategies |
DefaultActivityCommitStrategy/Tests.cs |
6 integration tests validating activity-level default strategy behavior and fallback to workflow defaults |
DefaultActivityCommitStrategy/*.cs |
Test workflow classes demonstrating activity-level strategy resolution |
CommitStateOptions.cs |
Added DefaultWorkflowCommitStrategy and DefaultActivityCommitStrategy properties with documentation |
ModuleExtensions.cs |
Added WithDefaultWorkflowCommitStrategy and WithDefaultActivityCommitStrategy extension methods |
CommitStrategiesFeature.cs |
Added SetDefaultWorkflowCommitStrategy and SetDefaultActivityCommitStrategy configuration methods |
DefaultActivitySchedulerMiddleware.cs |
Updated to fallback to default workflow strategy when no explicit strategy is set |
DefaultActivityInvokerMiddleware.cs |
Updated with full resolution chain: activity strategy → default activity strategy → workflow strategy → default workflow strategy |
BackgroundActivityInvokerMiddleware.cs |
Updated constructor to accept IOptions<CommitStateOptions> parameter and minor whitespace cleanup |
src/modules/Elsa.Workflows.Core/CommitStates/Extensions/ModuleExtensions.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Core/CommitStates/Options/CommitStateOptions.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Core/CommitStates/Options/CommitStateOptions.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Core/CommitStates/Extensions/ModuleExtensions.cs
Show resolved
Hide resolved
|
Hi @dwoldo, if you close out the Copilot comments, agree or disagree with them as you see fit, I’ll give this a look over this week and try get it merged🚀 |
|
Thank you for your patience @KnibbsyMan. I've resolved the comments with no changes. |
|
Hi @dwoldo, Not a problem, it's that time of year! I'll take a look at it over the next few days but at a glance it seems to make sense! |
|
Hi @dwoldo, LGTM👍🏼 PS: Great to see test coverage, they are always a welcomed addition to any new feature! |
|
@KnibbsyMan The tests are important, thank you! J/W, could this be part of the 3.6 release coming out soon? |
|
Yes, it should be part of either the 3.6.0-rc2 or the 3.6.0 release, whatever happens first! |
Add Default Workflow and Activity Commit Strategy Support
Summary
This PR introduces optional global default commit strategies for both workflows and activities, providing a fallback mechanism when no explicit commit strategy is configured. This feature allows developers to set application-wide defaults while still maintaining the ability to override them at the workflow or activity level.
Addresses #7135 #7135
Problem
Currently, if a workflow or activity doesn't specify a commit strategy, no commits occur during execution (except for the final commit). This requires developers to explicitly configure commit strategies for every workflow and activity, even when they want consistent behavior across their application.
Solution
Implemented two complementary features:
1. Default Workflow Commit Strategy
WithDefaultWorkflowCommitStrategy()extension method2. Default Activity Commit Strategy
WithDefaultActivityCommitStrategy()extension methodResolution Order
For Activities:
For Workflows:
Usage Example
Key Features
IWorkflowCommitStrategyandIActivityCommitStrategyinstances directlyChanges
Core Implementation
CommitStateOptions.cs: AddedDefaultWorkflowCommitStrategyandDefaultActivityCommitStrategypropertiesCommitStrategiesFeature.cs: AddedSetDefaultWorkflowCommitStrategy()andSetDefaultActivityCommitStrategy()configuration methodsModuleExtensions.cs: AddedWithDefaultWorkflowCommitStrategy()andWithDefaultActivityCommitStrategy()extension methodsDefaultActivitySchedulerMiddleware.cs: Modified to fall back to default workflow strategyDefaultActivityInvokerMiddleware.cs: Modified to implement full resolution chain with default strategiesTesting
DefaultWorkflowCommitStrategy/Tests.cs: 5 integration tests covering workflow-level defaultsDefaultActivityCommitStrategy/Tests.cs: 6 integration tests covering activity-level defaultsTest Coverage
Workflow Default Strategy Tests:
Activity Default Strategy Tests:
Breaking Changes
None. This is a purely additive feature that maintains backward compatibility.
Additional Notes
Sequence) trigger commit strategy evaluation when processing child activity completions, which is why some tests show 6 commits instead of 3 (3 WriteLine activities + 3 Sequence composite completions)Documentation
Usage examples and API documentation have been added via XML comments on all new public methods.
This change is