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

Add JsonFactoryBuilder.configureForJackson2(), JsonFactoryBuilder builderWithJackson2Defaults() #1411

Merged
merged 8 commits into from
Mar 6, 2025

Conversation

pjfanning
Copy link
Member

@pjfanning pjfanning commented Mar 5, 2025

@pjfanning pjfanning marked this pull request as draft March 5, 2025 09:40
@pjfanning pjfanning added the 3.0 Issue planned for initial 3.0 release label Mar 5, 2025
cowtowncoder added a commit that referenced this pull request Mar 5, 2025
@pjfanning pjfanning marked this pull request as ready for review March 5, 2025 21:55
@cowtowncoder
Copy link
Member

Makes sense as a starting point, but if the idea is to add same functionality for other backends, should probably implement in a way that can share general (non-JSON-specific) settings.
So parts in TokenStreamFactory/TSFBuilder, parts in JsonFactory/JsonFactoryBuilder.

And in fact, instead of (or in addition to?) having all of these in Builder factory method, could perhaps have builder member method like TSFBuilder.configureForJackson2().
This could be called separately, as well as part of factory method?

@pjfanning
Copy link
Member Author

Makes sense as a starting point, but if the idea is to add same functionality for other backends, should probably implement in a way that can share general (non-JSON-specific) settings. So parts in TokenStreamFactory/TSFBuilder, parts in JsonFactory/JsonFactoryBuilder.

And in fact, instead of (or in addition to?) having all of these in Builder factory method, could perhaps have builder member method like TSFBuilder.configureForJackson2(). This could be called separately, as well as part of factory method?

I thought about having it on the builder but my worry is that users will then use annoying ordering in chaining their commands to configure the builder. Someone is bound to come along and create a build, set some config then call configureForJackson2() and that could undo some of their pre-set config. I guess you could say that about any command chaining on the builder but the actions of configureForJackson2() are a bit opaque - you need to read the code to see hat it does.

@cowtowncoder
Copy link
Member

@pjfanning I still suggest adding configure method in Builder instance even if called from static factory method that creates Builders. That allows composition of TSFBuilder (format-agnostic settings) and format-specific actually builder (like JsonFactoryBuilder).

Configure method can then be alternatively called by someone being only handed Builder, as opposed to only being available from Factory.

@pjfanning
Copy link
Member Author

configureForJackson2

I added configureForJackson2 in latest commit.

@cowtowncoder cowtowncoder changed the title Jackson3: add setJackson2Defaults() to JsonFactoryBuilder Add setJackson2Defaults() to JsonFactoryBuilder Mar 6, 2025
@cowtowncoder cowtowncoder changed the title Add setJackson2Defaults() to JsonFactoryBuilder Add JsonFactoryBuilder.configureForJackson2(), JsonFactoryBuilder builderWithJackson2Defaults() Mar 6, 2025
@cowtowncoder cowtowncoder merged commit ba083e5 into FasterXML:master Mar 6, 2025
6 checks passed
@pjfanning pjfanning deleted the jackson2-like-factory branch March 6, 2025 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0 Issue planned for initial 3.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants