-
Notifications
You must be signed in to change notification settings - Fork 54
feat: add to json(any): string
conversion function
#1038
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
Conversation
This PR adds a new `to json(any): string` as the inverse of `from Json(json)` built-in conversion function. Converts FEEL values (contexts, lists, primitives, temporal values, durations) into JSON strings. Uses convertToJsonValue helper to recursively map FEEL types to JSON-compatible Java/Scala types, with temporal values formatted as ISO-8601. Returns ValString containing the serialized JSON, or ValError if serialization fails. Note: All FEEL types are currently supported, so the error branch cannot be triggered in normal usage.
src/main/scala/org/camunda/feel/impl/builtin/ConversionBuiltinFunctions.scala
Show resolved
Hide resolved
@rnewton-bp3 thank you for your contribution. 🎉 I'll review your changes soon. 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new built-in FEEL function to json(object): string
that serializes FEEL values into JSON strings, providing the inverse functionality to the existing from json()
function. This enables FEEL expressions to convert contexts, lists, primitives, temporal values, and durations into JSON format for integration scenarios.
- Implements
to json()
function with support for all FEEL data types including temporal values and durations - Adds comprehensive test coverage for various data types and edge cases
- Uses ISO-8601 formatting for temporal values and toString representation for durations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/main/scala/org/camunda/feel/impl/builtin/ConversionBuiltinFunctions.scala | Implements the to json() function with recursive value conversion logic |
src/test/scala/org/camunda/feel/impl/builtin/BuiltinConversionFunctionsTest.scala | Adds comprehensive test cases covering all supported data types |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rnewton-bp3 looks good. 👍 I have a few comments to improve and complete the PR. Please have a look. 🍪
src/test/scala/org/camunda/feel/impl/builtin/BuiltinConversionFunctionsTest.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/camunda/feel/impl/builtin/ConversionBuiltinFunctions.scala
Outdated
Show resolved
Hide resolved
src/test/scala/org/camunda/feel/impl/builtin/BuiltinConversionFunctionsTest.scala
Outdated
Show resolved
Hide resolved
src/test/scala/org/camunda/feel/impl/builtin/BuiltinConversionFunctionsTest.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/camunda/feel/impl/builtin/ConversionBuiltinFunctions.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/camunda/feel/impl/builtin/ConversionBuiltinFunctions.scala
Outdated
Show resolved
Hide resolved
- Remove duplicate tests - Include ValFunction and ValRange conversions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rnewton-bp3 looks very good. 👍
I have just one minor improvement. Please take a look. 🍰
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rnewton-bp3 looks great. 🎉
Thank you for contributing this valuable new feature. 🚀
Co-authored-by: Philipp Ossler <philipp.ossler@gmail.com>
Description
This PR adds a new built-in FEEL function
to Json(object): string
that performs the inverse of the existingfrom Json(json):any
function. It allows FEEL expressions to serialize values (contexts, lists, primitives, temporal values, and durations) into JSON strings, similar toJSON.stringify(any)
function in JavaScript. This is especially useful for integration scenarios where FEEL values need to be passed to systems expecting JSON.Implementation details:
Introduced helper method
convertToJsonValue(Val)
to recursively map FEEL Val types into JSON-compatible Java/Scala values.Supports all FEEL data types, including temporal values (ValDate, ValTime, ValDateTime, ValLocalTime, ValLocalDateTime) formatted using ISO-8601 standards.
Durations are serialized using their toString representation.
The resulting JSON string is wrapped in a ValString to be fully FEEL-compatible.
Added safety net to return ValError when serialization fails (though in practice all FEEL types are supported).
I could not create a meaningful test that triggers the
ValError(s"Failed to convert object to JSON: ${obj.getClass.getSimpleName}")
case because all FEEL data types are fully mapped in theconvertToJsonValue(Val)
function. Triggering this branch would require artificially passing in unsupported or non-FEEL types, which isn’t a real usage scenario.Finally, I’m a complete Scala noob, so I appreciate any suggestions for making the code cleaner or more idiomatic.
Thanks to Alex Trukhny (@sashtran) 🙏 for the initial review.
Related issues
closes #602