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

fix(jackson): replace ObjectMapper with lower-level APIs in FEEL deserializers #1724

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

chillleader
Copy link
Member

@chillleader chillleader commented Jan 15, 2024

Description

This PR replaces the temporary hotfix we had in place to fix the broken FEEL context aware deserialization.

To recap, the issue was that we are calling Jackson deserialization not only on ObjectMapper, but also on ObjectReader in case of the polling connector where we need to inject process variables into FEEL expression evaluation. We previously fetched the ObjectMapper instance from the deserialization context, but it turned out that Jackson doesn't necessarily return an ObjectMapper, but actually whatever was used to call the deserialization initially.

The proposed solution is to partially switch to lower-level APIs (such as deserializationContext.readValue(...).

  • The important part was to make sure that custom modules registered with the original object reader / mapper and any custom configs will also be used in the FEEL deserializer. To check that, I added a test that requires Jackson's Java time module to be preserved in the deserializer.
  • The fix also includes some refactoring of the FeelEngineWrapper for improved type cast support (moved from the deserializer) and readability.

Related issues

closes #1552

@chillleader chillleader self-assigned this Jan 15, 2024
@chillleader chillleader marked this pull request as ready for review January 16, 2024 08:59
@chillleader chillleader requested a review from a team as a code owner January 16, 2024 08:59
@chillleader chillleader added this pull request to the merge queue Jan 16, 2024
Merged via the queue into main with commit c80b73a Jan 16, 2024
3 checks passed
@chillleader chillleader deleted the 1552-replace-object-mapper-in-feel-deserializer branch January 16, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken FEEL context aware deserialization
2 participants