Skip to content

Move jackson-datatype-jsr310 into jackson-databind #5032

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

Merged
merged 27 commits into from
Apr 4, 2025

Conversation

pjfanning
Copy link
Member

@pjfanning pjfanning commented Mar 18, 2025

  • I have already moved the enable/disable methods for the JavaTimeFeatures to MapperBuilder so they are enabled/disabled similarly to other features
  • I have removed the old databind code that warns users when JavaTimeModule is not enabled (no longer needed)
  • I have checked that no jackson-datatype-jsr310 code has been missed - including checking for new changes in master branch

@cowtowncoder if you have time, would you be able to review the change from a high level?

  • the package structure
  • the changes to MapperBuilder and JsonMapper to facilitate registering JavaTImeFeatures and automatically adding the JavaTimeModule serializers/deserializers/etc.

…t enabled
…the box
@@ -0,0 +1,226 @@
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good -- I like this idea -- making it work much like modules, without having to be one.

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far -- aside from maybe changing package I think this would work.

@pjfanning pjfanning changed the title [WIP] Move jackson-datatype-jsr310 in jackson-databind Move jackson-datatype-jsr310 into jackson-databind Mar 19, 2025
@cowtowncoder
Copy link
Member

LGTM: what do you think @JooHyukKim? (since you are module maintainer)

@JooHyukKim
Copy link
Member

@cowtowncoder yes! LGTM at high level.
I like how datetime related classes are isloated in tools.jackson.databind.ext.datetime.
Though my Chrome browser keeps on timing out 😅

Work in progress

@pjfanning you are saying its WIP, may I ask what the remaining pieces are?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@pjfanning
Copy link
Member Author

@cowtowncoder yes! LGTM at high level. I like how datetime related classes are isloated in tools.jackson.databind.ext.datetime. Though my Chrome browser keeps on timing out 😅

Work in progress

@pjfanning you are saying its WIP, may I ask what the remaining pieces are?

I'm happy to treat this as ready to merge at this stage.

@cowtowncoder cowtowncoder added this to the 3.0.0-rc2 milestone Mar 21, 2025
@JooHyukKim
Copy link
Member

Just one concern I have is how easy it would be to forward merge changes in Jackson 2 version of jsr310 module, good thing is that the module itself does not have frequent changes

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@cowtowncoder
Copy link
Member

Just one concern I have is how easy it would be to forward merge changes in Jackson 2 version of jsr310 module, good thing is that the module itself does not have frequent changes

That is a valid concern. Not sure we can do a lot about it.
But there will be some fixes in 2.x that need to be ported so at least need to be aware of the extra work.

I feel merging still makes sense, tho, overall. WDYT?

@pjfanning
Copy link
Member Author

I think the JSR310 support is pretty stable. Forward fitting to master (3.0) branches is fairly complicated already. This change doesn't really even make it much more complicated.

@cowtowncoder
Copy link
Member

Forward fitting to master (3.0) branches is fairly complicated already.

While there are some kinks to be sure (git seems to lose tracking of some heavily changed renamed classes), overall for modules forward merging has been doable.
But sometimes quite a hassle.

So I agree, it's just a degree of additional complexity. So I think we can live with it.

WDYT @JooHyukKim ?

Copy link
Member

@JooHyukKim JooHyukKim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
*
* @since 2.16
*/
public enum JavaTimeFeature implements JacksonFeature
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, I think we should actually change the idea here to create new DateTimeFeature that should ideally control ALL date/time handling (mostly concerning then old java.util Date/Calendar and Joda date/time).

It would contain entries we have here, initially, but then see if we could move existing DeserializationFeatures/SerializationFeatures that are date/time specific into this new enum.

This would then be a building block for

https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP-5

Finally, renamed DateTimeFeature should go in tools.jackson.databind.cfg along with existing EnumFeature and JsonNodeFeature.

… date/time types
@cowtowncoder
Copy link
Member

@pjfanning @JooHyukKim I did last tweaking, renaming and am ready to merge this in. Renamed so that

  1. "module" is labelled Java Time (just to align with references I have seen) -- so ext.javatime.*
  2. general-purpose on/off features: DateTimeFeature

Beyond merging this big PR I will start filing issues for

  1. Moving date/time specific DeserializationFeatures/SerializationFeatures into DateTimeFeature
  2. Changing some of feature defaults (one known: 1-based Months; others not yet sure)

and hopefully we can address (most of) these for 3.0.0-rc3.

@cowtowncoder cowtowncoder merged commit 766923c into FasterXML:master Apr 4, 2025
6 checks passed
cowtowncoder added a commit that referenced this pull request Apr 4, 2025
@cowtowncoder cowtowncoder changed the title Move jackson-datatype-jsr310 into jackson-databind Move jackson-datatype-jsr310 into jackson-databind Apr 12, 2025
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 10, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 10, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 10, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 10, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 23, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 23, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 24, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 24, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
rursprung added a commit to rursprung/opensearch-java that referenced this pull request Jun 24, 2025

Verified

This commit was signed with the committer’s verified signature.
rursprung Ralph Ursprung
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
Xtansia pushed a commit to opensearch-project/opensearch-java that referenced this pull request Jun 24, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR #251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032
opensearch-trigger-bot bot pushed a commit to opensearch-project/opensearch-java that referenced this pull request Jun 24, 2025
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR #251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

(cherry picked from commit c1ae512)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Xtansia pushed a commit to opensearch-project/opensearch-java that referenced this pull request Jun 24, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR #251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).



[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

(cherry picked from commit c1ae512)

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Xtansia pushed a commit to Xtansia/opensearch-java that referenced this pull request Jun 25, 2025

Partially verified

This commit is signed with the committer’s verified signature.
Xtansia’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

(cherry picked from commit c1ae512)
Signed-off-by: Thomas Farr <[email protected]>
reta pushed a commit to reta/opensearch-java that referenced this pull request Jun 25, 2025

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

(cherry picked from commit c1ae512)
Xtansia added a commit to opensearch-project/opensearch-java that referenced this pull request Jun 25, 2025

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR #251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).



[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

(cherry picked from commit c1ae512)

Signed-off-by: Thomas Farr <[email protected]>
Co-authored-by: Ralph Ursprung <[email protected]>
001mertiya pushed a commit to 001mertiya/opensearch-java that referenced this pull request Jul 9, 2025

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

Signed-off-by: psingh3 <[email protected]>
001mertiya pushed a commit to 001mertiya/opensearch-java that referenced this pull request Jul 9, 2025

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

Signed-off-by: psingh3 <[email protected]>
001mertiya pushed a commit to 001mertiya/opensearch-java that referenced this pull request Jul 9, 2025

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
jackson 2.x still supports java 7 and thus does not automatically
support java 8 classes. this will change with jackson 3.x (see e.g.
this [PR adding JSR310 date support] to 3.x).

PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be
registered - but there's no reason why we can't just enable
auto-detection centrally. the docs mention that performance should be
considered, but this is IMHO not relevant here since we only construct
it once and then keep the same `ObjectMapper` (which in turn the docs
mention as best practices).

Signed-off-by: Ralph Ursprung <[email protected]>

[PR adding JSR310 date support]: FasterXML/jackson-databind#5032

Signed-off-by: psingh3 <[email protected]>
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

Successfully merging this pull request may close these issues.

None yet

3 participants