Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude Activity from being serialized in instance and execution log #4464

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

sfmskywalker
Copy link
Member

@sfmskywalker sfmskywalker commented Sep 21, 2023

This PR fixes an issue where the HTTP Request activity contains input properties containing IActivity instances, which shouldn't be serialized as part of the workflow instance or eecution log records.

Ideally, we could add a custom JSON converter that ignores IActivity instances, but since I need more time to figure that one out properly, I added an extensibility point to the WorkflowStateSerializer and SafeSerializer services to allow modules to add specialized custom converters, which fixes the issue with HTTP Request at hand.

=== auto-pr-body ===

Summary:
This Pull Request adds a new Elsa project ("Elsa.Samples.AspNet.DocumentApproval") to the solution, updates GlobalSection(SolutionConfigurationPlatforms) and GlobalSection(NestedProjects) in the Elsa.sln file, and refactors the DapperActivityExecutionRecordStore.cs file by converting synchronous methods to asynchronous methods. Additionally, methods in ActivityExecutionRecord, EFCoreActivityExecutionStore, WorkflowExecutionLogStore, ConfigureWorkflowStateSerialization.cs, HttpStatusCodeCaseForWorkflowInstanceConverter.cs, InputDescriptor, JsonIgnoreCompositeRootConverter.cs, ActivityDescriber.cs, JsonWorkflowStateSerializer.cs, and SafeSerializer.cs are updated to include the use of asynchronous methods and the addition of boolean variables IsSerializable in their implementations.

List of Changes:

  • Added a new project ("Elsa.Samples.AspNet.DocumentApproval") to the Elsa.sln file
  • Updated GlobalSection(SolutionConfigurationPlatforms) with configurations for the new project
  • Updated GlobalSection(NestedProjects) with a nested project for the new project
  • Updated the DapperActivityExecutionRecordStore with asynchronous methods instead of synchronous methods
  • Updated Read method in HttpStatusCodeCaseConverter to deserialize the HTTP status code
  • Updated InputAttribute and OutputAttribute by adding boolean variables IsSerializable
  • Updated ISafeSerializer to include SerializeAsync and DeserializeAsync functions with CancellationToken parameter
  • Updated ISafeSerializerConfigurator to configure the serializer
  • Updated ActivityExecutionContextExtensions to include IsSerializable when evaluating and completing input/output properties and outcomes
  • Added boolean variable IsSerializable in InputDescriptor class
  • Added using clauses for Elsa.Mediator.Contracts, Elsa.Workflows.Core.Notifications
  • Changed public string Serialize(object? value) to public async ValueTask<string> SerializeAsync(object? value, CancellationToken cancellationToken = default)
  • Added notification related code before serialization
  • Changed public T Deserialize<T>(string json) to public async ValueTask<T> DeserializeAsync<T>(string json, CancellationToken cancellationToken = default)
  • Added notification related code before deserialization

Refactoring Targets:

  • Improve efficiency of the method Write in HttpStatusCodeCaseConverter by using a dictionary instead of an array to store the property values
  • Replace Serialize and Deserialize methods in the ISafeSerializer interface with their async counterparts
  • Replace 'if-else' statements in ActivityExecutionContextExtensions with ternary operator to improve readability and reduce the number of lines of code
  • Refactor JsonWorkflowStateSerializer.cs to be more DRY and move shared logic to helper functions.
  • Move SerializingWorkflowState.cs notification record to Notifications folder, which could increase the readability and organization of the code.
  • Move notification related code out of the serialization and deserialization methods, so they can be used in other places as well.
  • Add a cancellationToken parameter to the serialization and deserialization interfaces.
  • Move creation of JsonSerializerOptions to its own method, instead of it being inside CreateOptions.
  • Consider introducing Immutable Array of Configurators instead of IEnumerable.

@sfmskywalker sfmskywalker added the elsa 3 This issue is specific to Elsa 3 label Sep 21, 2023
@sfmskywalker sfmskywalker merged commit fc14cfb into v3 Sep 21, 2023
2 checks passed
@sfmskywalker sfmskywalker deleted the v3-http-status-code-case-serialization-issue branch September 21, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elsa 3 This issue is specific to Elsa 3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants