Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

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

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

Open
carusology wants to merge 2 commits into jknack:master
base: master
Choose a base branch
Loading
from carusology:issue-964

Conversation

@carusology
Copy link
Collaborator

@carusology carusology commented Apr 10, 2022 •
edited
Loading

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.

hanguyen0 reacted with thumbs up emoji
Copy link
Collaborator Author

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

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
Copy link
Collaborator Author

( •_•)σ @jknack

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

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?

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?

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 202303.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@DavidWei252 DavidWei252 DavidWei252 approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /