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 KotlinFeature for configuration settings? #442

Closed
cowtowncoder opened this issue May 9, 2021 · 8 comments
Closed

Add KotlinFeature for configuration settings? #442

cowtowncoder opened this issue May 9, 2021 · 8 comments
Labels
enhancement good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project
Milestone

Comments

@cowtowncoder
Copy link
Member

(note: inspired by #439 work)

As things are, KotlinModule has a few configuration options, and at least 4 are boolean-valued (plus #439 would add one more, at least temporarily). Since they have to be passed separately, adding a new option requires more changes than is ideal, and technically also reduces cross-version compatibility (although constructors should not really be called from outside).

Most Jackson components offer Feature style on/off settings, in which we can have up to 31 options (or, if using long, 63 -- and of course with diff. backends even more) and make it much easier to add new options, only adding enum entries but no need to add new methods, constructors etc.
Implementation for int-backed variant -- which I think is sufficient for Kotlin module, with a smallish set of on/off things -- is done by implementing JacksonFeature. I don't deeply care if that is used or something else, but I think the general idea is good.
(note: the reason I chose not to use BitSet from JDK originally was because that is not immutable so copies were needed; any similar data struct is fine with me).

@dinomite
Copy link
Member

Great idea. What's the timeline like for Jackson 2.13? I'm wondering whether to scope this for then or later.

@cowtowncoder
Copy link
Member Author

I have been bit swamped at work lately, so we are not yet close to RC phase. I think that if all goes well, we might get first RC in early July? So there is quite a bit of time I think.
I think this would be great to have for 2.13.

@dinomite dinomite added this to the 2.13.0 milestone May 13, 2021
@dinomite dinomite added the good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project label May 14, 2021
@dinomite
Copy link
Member

This is a great case for someone new to Jackson or to open source work to tackle. Other Jackson components provide a good model and the area where this work needs to happen is fairly contained & straightforward. I'm happy to give pointers and help someone get started on this.

@efenderbosch
Copy link

What should this look like in use?

KotlinModule()
    .enable(SingletonSupport)
    .disable(SomeEnabledByDefaultFeature)

@cowtowncoder
Copy link
Member Author

I think use of Builder-style would be good (since Jackson 3.0 moves to that for ObjectMapper and TokenStreamFactory), but I defer to @dinomite here.

@efenderbosch
Copy link

efenderbosch commented May 28, 2021

So like this?

KotlinModule.Builder()
    .withReflectionCacheSize(1024)
    .enable(SingletonSupport)
    .disable(SomeEnabledByDefaultFeature)
    .build()

@cowtowncoder
Copy link
Member Author

@efenderbosch exactly

@dinomite
Copy link
Member

dinomite commented Jun 7, 2021

Complete!

@dinomite dinomite closed this as completed Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project
Projects
None yet
Development

No branches or pull requests

3 participants