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

BeanDeserializerBuilder Protected Factory Method for Extension #2487

Closed
vjkoskela opened this issue Oct 3, 2019 · 5 comments
Closed

BeanDeserializerBuilder Protected Factory Method for Extension #2487

vjkoskela opened this issue Oct 3, 2019 · 5 comments
Milestone

Comments

@vjkoskela
Copy link
Contributor

In #1870 we were able to switch from creating custom builder deserializers using encapsulation to using inheritance. This greatly simplified our code and allowed it to move from com.fasterxml into our package (since we no longer required access to package private methods). (Thank you!)

The next step in allowing builder deserializers to be customized would be to simplify their creation in BeanDeserializerBuilder.java#L442.

Our custom builder deserializer has a corresponding builder deserializer builder, but currently we need to override the entire buildBuilderBased method in our class when all we want to change is the type of builder deserializer created. Since our builder deserializer is a subclass of BeanDeserializerBuilder it has a constructor which accepts at least the same arguments as BeanDeserializerBuilder.

For our use case at least, delegating the construction of BeanDeserializerBuilder to a separate protected ("factory") method would decouple the logic in buildBuilderBased from the type of builder deserializer created.

Further generalizations would be possible by registering the type of builder to be created although I am unclear on the ROI of such a change and how it fits architecturally with the project.

Let me know what you think! I can definitely submit a patch for the simple refactoring of BeanDeserializerBuilder, but I'm curious if you have some insight into how to make this more flexible.

Jackson Version: 2.10.0

@cowtowncoder
Copy link
Member

Too bad this did not quite make it in 2.10.0, but I hope to make 2.11 dev cycle much shorter than 2.10, release candidates getting released by Dec 2019.

@cowtowncoder
Copy link
Member

Oh, also: yes, further simplifications would also be welcome, targeted for 2.11.
Support for builder-based construction is becoming more and more important so it'd be good to improve Jackson's handling. I haven't had time to focus on it myself so really appreciate your help here.

@cowtowncoder
Copy link
Member

Decided that since this is backwards-compatible after all, I can (and did) add this in 2.10 for 2.10.1.
So the "official" addition is in 2.11(.0), but it will technically be included in 2.10.1 and after.

@vjkoskela
Copy link
Contributor Author

Thank you! If you have some thoughts on strategies for builder deserializer generalization please let me know.

@cowtowncoder
Copy link
Member

Definitely, will do!

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

No branches or pull requests

2 participants