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

[WFLY-19298] Update fasterxml to 2.17.0 #17856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spyrkob
Copy link
Contributor

@spyrkob spyrkob commented Apr 29, 2024

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Apr 29, 2024
@bstansberry
Copy link
Contributor

This requires approval from the various leads responsible for FasterXML use:

@jamezp (RESTEasy)
@rhusar (com.microsoft.azure.storage module)
@yrodiere (Hibernate Search stuff)
@scottmarlow (Hibernate ORM)
@fjuma (org.wildfly.security.elytron-http-oidc and org.wildfly.security.elytron-jose-*)
@jasondlee (OpenTelemetry)
@pferraro (OpenAPI)
@kabir (VertX and Kafka)

@scottmarlow
Copy link
Contributor

scottmarlow commented Apr 29, 2024

Hibernate ORM 6.4.4.Final is currently using 2.14.1 as is the ORM main branch.

Let's see what @yrodiere has to say about Search but I think 7.1.0.Final is on 2.15.2 and main looks to be on 2.17.0.

I think the underlying question is whether there could be Search/ORM releases on fasterxml 2.17.0 if that could work for some future versions of all components?

Copy link
Contributor

@fjuma fjuma left a comment

Choose a reason for hiding this comment

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

Thanks @spyrkob!

@yrodiere
Copy link
Contributor

yrodiere commented Apr 30, 2024

Hibernate ORM 6.4.4.Final is currently using 2.14.1 as is the ORM main branch.

WildFly should probably upgrade to ORM 6.5 though, and that's not the main branch. Did you have a look at ORM 6.5?
EDIT: Well that was a dumb suggestion, sorry. If main is on 2.14.1, 6.5 probably is too.

Let's see what @yrodiere has to say about Search but I think 7.1.0.Final is on 2.15.2 and main looks to be on 2.17.0.

I'll defer to @marko-bekhta for Search.

I think the underlying question is whether there could be Search/ORM releases on fasterxml 2.17.0 if that could work for some future versions of all components?

We could upgrade in ORM 6.6, sure, though obviously that'll take a few weeks/months to reach WildFly.

We need to have a look, though. I suspect ORM is not actually that deeply coupled with Jackson, at least if it's only about JSON columns, but I might be missing something. If coupling is indeed loose, it might be enough to just run the ORM test suite with the dependency upgraded.

@sebersole or @beikov may have an answer ready for you, otherwise I'll have a closer look, but not before next week unfortunately.

@beikov
Copy link
Contributor

beikov commented Apr 30, 2024

I suspect ORM is not actually that deeply coupled with Jackson, at least if it's only about JSON columns, but I might be missing something. If coupling is indeed loose, it might be enough to just run the ORM test suite with the dependency upgraded.

You're right, it's an "optional" dependency. Upgrading Jackson should not be a problem, so +1 from my side on upgrading in Wildfly.

@scottmarlow
Copy link
Contributor

I suspect ORM is not actually that deeply coupled with Jackson, at least if it's only about JSON columns, but I might be missing something. If coupling is indeed loose, it might be enough to just run the ORM test suite with the dependency upgraded.

You're right, it's an "optional" dependency. Upgrading Jackson should not be a problem, so +1 from my side on upgrading in Wildfly.

Optional dependencies or not, I think the Hibernate ORM testsuite needs to pass with the fasterxml upgrade before WildFly upgrades to it to ensure that the ORM integration fasterxml works as expected.

Copy link
Member

@rhusar rhusar left a comment

Choose a reason for hiding this comment

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

Fine for com.microsoft.azure.storage module. 👍🏼

@marko-bekhta
Copy link
Contributor

For Hibernate Search:

@yrodiere
Copy link
Contributor

yrodiere commented May 2, 2024

Thanks @marko-bekhta @beikov @scottmarlow .

I created:

@scottmarlow Will you please make the upgrade to Hibernate ORM 6.5 in WildFly?

@beikov
Copy link
Contributor

beikov commented May 2, 2024

It's a good first step for Wildfly to try out ORM 6.5, but I would like to ask you to make sure that version doesn't end up in a release, because ORM 6.6 is around the corner and it would be best for Wildfly to use that ORM version in its next release.

@yrodiere
Copy link
Contributor

yrodiere commented May 2, 2024

It's a good first step for Wildfly to try out ORM 6.5, but I would like to ask you to make sure that version doesn't end up in a release, because ORM 6.6 is around the corner and it would be best for Wildfly to use that ORM version in its next release.

Clarifying: ideally WildFly's next version would use Hibernate ORM 6.6 if it's ready by the feature freeze date, but failing that shipping Hibernate ORM 6.5 would definitely be better than the current Hibernate ORM 6.4. So yes, please upgrade @scottmarlow :)

We're going on a tangent here though, let's move to Zulip if more discussion is necessary.

Back to discussing Jackson here :)

Copy link
Contributor

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Hibernate ORM 6.5 works fine with Jackson 2.17: hibernate/hibernate-orm#8284
I'll leave it to @scottmarlow to upgrade to Hibernate ORM 6.5 in WildFly, as WildFly is currently using Hibernate ORM 6.4 (but shouldn't).

@scottmarlow
Copy link
Contributor

Hibernate ORM 6.5 works fine with Jackson 2.17: hibernate/hibernate-orm#8284 I'll leave it to @scottmarlow to upgrade to Hibernate ORM 6.5 in WildFly, as WildFly is currently using Hibernate ORM 6.4 (but shouldn't).

Created https://issues.redhat.com/browse/WFLY-19306 for trying to upgrade to ORM 6.5 in WildFly (note that WildFly Preview is already including ORM 6.5).

@jamezp
Copy link
Member

jamezp commented May 3, 2024

There is an issue with RESTEasy with this upgrade. Previously java.util.Optional would serialize to something like:

{"empty":false,"present":true}

This happened when the com.fasterxml.jackson.datatype.jdk8.Jdk8Module was not registered with the ObjectMapper. In 2.17 this now throws an exception instead because the Jdk8Module is not installed by default. It could be related to FasterXML/jackson-modules-java8#294.

One could argue that RESTEasy should register some default modules in a default ObjectMapper. However, there could be users relying on this old behavior. IMO the old behavior is wrong, but I'll need to make changes in RESTEasy to fix the support for this. If we feel this is needed please let me know so we can file an issue and fix this.

@jamezp
Copy link
Member

jamezp commented May 7, 2024

I've filed RESTEASY-3502 for the RESTEasy issue. I can disable the tests that fail in RESTEasy if a newer version of Jackson is found. However, this could be considered a behavior change and if we proceed with the upgrade we should document that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
10 participants