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

Unwrap Jackson2 Array and Object Nodes in JsonNodeValueResolver (#964, #969) #965

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

carusology
Copy link
Collaborator

@carusology carusology commented Apr 10, 2022

Background

This is a fix for self-reported issue #964. Now the JsonNodeValueResolver resolves a Jackson2 ArrayNode as a List<Object> with its items being recursively resolved.

I wrote it in the same style as the JsonNodeValueResolver.toMap() method, meaning:

  • It uses a closure as a reference the original JsonNode.
  • It uses an abstract collection as the base implementation of the list.
  • It only implements the retrieval methods on the abstract collection.
  • It performs the recursive resolution lazily.

Testing

I wrote a few unit tests to verify the behavior works as expected. All of the existing unit tests pass without issue.

@carusology
Copy link
Collaborator Author

@jknack: What do you think of this update to the JsonNodeValueResolver?

@carusology
Copy link
Collaborator Author

Added a fix for #969 as well since it's basically the same problem but on JSON objects instead of JSON arrays. After a JSON object is converted to a Map<String, Object>, its entrySet() wasn't recursively resolving the value properties (Source).

I fixed it in a distinct commit following the same "lazy" pattern of resolution and added a test.

@carusology carusology changed the title Unwrap Jackson2 ArrayNode in JsonNodeValueResolver (#964) Unwrap Jackson2 Array and Object Nodes in JsonNodeValueResolver (#964, #969) Apr 21, 2022
@carusology
Copy link
Collaborator Author

( •_•)σ @jknack

Any objections? If not, is there a release guide? I don't mind going through the machinations.

@mynameisjeoff
Copy link

@jknack would we be able to get an update on this pull request or is there someone else who would be able to review these changes before they can be merged in?

@carusology
Copy link
Collaborator Author

( •_•)σ @jknack

Any objections? If not, is there a release guide? I don't mind going through the machinations.

👋 I noticed you prepared the next release cycle, @jknack. Can this merge request be included?

@mimranfaruqi
Copy link

Hi, any updates as to when this PR could be merged? I have somewhat similar problem that #unless is not working when I use it in a nested setup. I will paste the code if someone is interested to have a look at? this only happens when I used the newest version 4.4.0 handlebars and 4.3.1 for handlebars with Jackson2. Because we recently updated our project to JDK 21 LTS and Spring boot 3.3 with Spring Cloud 2023.0.3.

Thank you

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.

4 participants