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

Replace wrapUp and wrap() methods with @JacksonInject #599

Open
bitwiseman opened this issue Nov 8, 2019 · 2 comments
Open

Replace wrapUp and wrap() methods with @JacksonInject #599

bitwiseman opened this issue Nov 8, 2019 · 2 comments
Labels

Comments

@bitwiseman
Copy link
Member

This is very much a code smells issue, not a user facing feature.

There's quite a bit of code and complexity devoted to added root references to objects after they are received. I've thought about various class structures to clean the up, but really the right thing to do seems to be using the @JacksonInject annotation. We'd add the root instance to the mapper when reading and let the injector handle the assignment.

See https://fasterxml.github.io/jackson-databind/javadoc/2.9/com/fasterxml/jackson/databind/ObjectMapper.html#reader-com.fasterxml.jackson.databind.InjectableValues-

@bitwiseman bitwiseman added the task label Nov 8, 2019
@bitwiseman bitwiseman mentioned this issue Nov 8, 2019
3 tasks
@bitwiseman
Copy link
Member Author

After some attempt at this I found that some parts of the project depend on the late binding behavior or provided by the current wrap* methods.

The root field is generally safe to bind early (it alway is preset before the query starts) but aside from that many owners have scenarios in which they are not available until late or are even optional.

It might be possible to create a Binding class that would hold these reference an allow them to be resolved later, but I'm not sure how useful that is.

Also, the @JacksonInject entries must preset in the InjectableValues or else the reader throws, making optional/late binding even more of a pain. This may not be worth the effort.

@bitwiseman
Copy link
Member Author

If nothing else changing the name of wrapUp() and wrap() to lateBind()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant