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

Enhanced ObjectConverter to Return JsonObject Instead of JsonElement #4686

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

sfmskywalker
Copy link
Member

@sfmskywalker sfmskywalker commented Dec 8, 2023

This pull request enhances the ObjectConverter, resolving a critical issue that users encountered when interacting with JSON objects in specific scenarios.

Key changes include:

  • Streamlined the deserialization handling using a switch expression, which has not only improved code readability but also maintained performance efficiency.
  • Fine-tuned the deserialization process where the targetType is not an object.
  • Crucially, when converting JsonObject to object, we now return the JsonObject itself rather than a JsonElement. This change is significant because previously this conversion would produce a JsonElement value, breaking users' code where they employ JavaScript expressions and use index notation, such as myValue["Foo"]. Index notation works with JsonObject but not with JsonElement. Now users can use this notation without encountering issues.

These enhancements enhance the versatility and robustness of JSON object deserialization, increasing the user-friendliness of our framework, particularly for those using JavaScript expressions. We expect this to significantly aid users in their daily development work and make our codebase more maintainable and straightforward.

=== auto-pr-body ===

Summary:
This pull request updates various portions of the codebase to remove duplicate information, improve readability, and add additional functionality. Changes introduced include the addition of the TestWorkflow class and associated objects, modification to the SerializationTest class, and modification to the WriteLine activity. Additionally, suggested refactorings are included to address clarity, naming, and code standard compliance.

List of Changes:

  • Added class TestWorkflow and associated objects
  • Modified SerializationTests class and the WriteLine activity

Suggested Refactorings:

  • Refactor Test_Serialization_read method
  • Replace File.ReadAllText($@"...") with File.ReadAllText(@$"...") in Test_Serialization_create_Island
  • Consider returning an interface (or abstract type) from GetContent, GetExpected methods to avoid implementation details leaking through the class interface

This commit includes a significant rework of JSON object handling by ensuring produced JsonObject remains as JsonObject. Previously unimplemented customer and order workflow context providers have been removed. The test cases have been renamed and updated to confirm the new behavior is working as expected.
@sfmskywalker sfmskywalker added bug Something isn't working elsa 3 This issue is specific to Elsa 3 labels Dec 8, 2023
@sfmskywalker sfmskywalker merged commit faf8fd1 into v3 Dec 8, 2023
2 checks passed
@sfmskywalker sfmskywalker deleted the jsonobject-jint branch December 8, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working elsa 3 This issue is specific to Elsa 3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant