Fix Output Persistence of Async Activities #6542
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e as when run synchronously
Introduced `GetPersistableOutputAsync` in `IActivityExecutionMapper` to streamline output persistence logic. Refactored the handling of activity persistence properties, replacing repetitive code with reusable methods. Removed unused dependencies and redundant methods, optimizing code readability and maintainability.
The docker-compose-datadog.yml file is no longer included in the solution structure. This change cleans up unused references to ensure the solution remains consistent and up-to-date.
…t-captured' into bug/6227
Split and reorganize workflow-related extension methods into `RunActivityExtensions` and `RunWorkflowExtensions` for better modularity. Removed deprecated methods from `ServiceProviderExtensions`. Updated tests and usages to reflect these changes.
sfmskywalker
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues with output persistence in asynchronous activities by refactoring how activity outputs are captured. Key changes include:
- Introducing the GetPersistableOutputAsync (and related GetPersistableInputAsync) methods in IActivityExecutionMapper and updating callers accordingly.
- Removing the legacy StorePropertyUsingPersistenceMode method and renaming it to FilterPropertiesUsingPersistenceMode.
- Removing unused dependencies and cleaning up older test helper code for improved maintainability.
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/integration/Elsa.Workflows.IntegrationTests/Scenarios/RunAsynchronousActivityOutput/Tests.cs | Added tests validating the output persistence for both sequential and parallel activities. |
| test/integration/Elsa.Workflows.IntegrationTests/Scenarios/RunAsynchronousActivityOutput/Activities/SampleActivity.cs | Introduced a new SampleActivity to support async output persistence tests. |
| src/modules/Elsa.Workflows.Runtime/Services/DefaultActivityExecutionMapper.cs | Extracted output/input persistence logic into dedicated methods, replacing repetitive code. |
| src/modules/Elsa.Workflows.Runtime/Services/BackgroundActivityInvoker.cs | Updated to use the new GetPersistableOutputAsync method for output extraction. |
| src/modules/Elsa.Workflows.Runtime/Middleware/Activities/BackgroundActivityInvokerMiddleware.cs | Modified how output values are set by passing an additional parameter. |
| src/modules/Elsa.Workflows.Runtime/Contracts/IActivityExecutonMapper.cs | Extended the interface with the GetPersistableOutputAsync method. |
| src/common/Elsa.Testing.Shared.Integration/* | Various cleanup and extension method updates to support the new persistence logic. |
| src/common/Elsa.Testing.Shared.Integration/DispatchWorkflowExtensions.cs | Added a new extension to dispatch workflows and wait for completion using notifications. |
Files not reviewed (1)
- src/apps/Elsa.Server.Web/Elsa.Server.Web.csproj: Language not supported
Comments suppressed due to low confidence (1)
src/modules/Elsa.Workflows.Runtime/Middleware/Activities/BackgroundActivityInvokerMiddleware.cs:127
- [nitpick] The updated call to context.Set now includes an additional parameter (outputDescriptor.Name). Please confirm that the consuming implementations of Set are updated accordingly and that this change is intentional.
context.Set(output, outputEntry.Value, outputDescriptor.Name);
src/common/Elsa.Testing.Shared.Integration/DispatchWorkflowExtensions.cs
Show resolved
Hide resolved
MariusVuscanNx
approved these changes
Apr 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduced
GetPersistableOutputAsyncinIActivityExecutionMapperto streamline output persistence logic. Refactored the handling of activity persistence properties, replacing repetitive code with reusable methods. Removed unused dependencies and redundant methods, optimizing code readability and maintainability.This change is