Move JSON-related JPA tests to their own module

[marko-bekhta]

fixes #37498

[quarkus-bot]

/cc @Sanne (hibernate-orm), @gsmet (hibernate-orm), @yrodiere (hibernate-orm)

[gsmet]

Hmm, could we stand by a bit?

I'm not exactly a big fan of adding yet another native module (it wasn't declared as one but probably should be).

Also I'm a bit worried that just adding Jackson makes our ORM optimization go away.

Because, well, most people add Jackson so this optimization is then completely useless in a lot of cases.

[yrodiere]

Because, well, most people add Jackson so this optimization is then completely useless in a lot of cases.

... True. Especially since it seems we don't support XML serialization with Jackson; at least I couldn't find that in the docs.

So, I guess the problem becomes that Jackson (hopefully) or Hibernate ORM (I hope not!) leads to XML parsers being detected as used by GraalVM.

I can't work on this right now, so either this will wait, or someone will have to work on native reports... fun stuff.

[quarkus-bot]

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[gsmet]

@zakkak any chance you could help us to understand what's going on and if the Jackson extension needs some tuning?

[zakkak]

Sure, I will have a look, but it might take a while.

[galderz]

So, I guess the problem becomes that Jackson (hopefully) or Hibernate ORM (I hope not!) leads to XML parsers being detected as used by GraalVM.

I'm not sure the issue is really related to Jackson, but more the fact that when JSON integration was added, XML integration was also included and JAXB was added as an optional dependency: https://github.com/quarkusio/quarkus/pull/35113/files#diff-af9399740cc05097b1e4d8ae864622d81b6ab8f927375a047f81bbf8e9d1477bR146-R160

Seems to me you need different modules for Hibernate and JAXB integration, separate from those that integrate Hibernate and JSON (Jackson). You already have that differentation in the integration test when you test postgresql with xml and the standard postgresql integration test.

[yrodiere]

Thanks for having a look.

Seems to me you need different modules for Hibernate and JAXB integration, separate from those that integrate Hibernate and JSON (Jackson).

I'd rather avoid that if possible... It's not very intuitive, especially since upstream bundles it all in a single module.

I'm not sure the issue is really related to Jackson, but more the fact that when JSON integration was added, XML integration was also included and JAXB was added as an optional dependency: https://github.com/quarkusio/quarkus/pull/35113/files#diff-af9399740cc05097b1e4d8ae864622d81b6ab8f927375a047f81bbf8e9d1477bR146-R160

Alright, but it's optional; I hope an optional dependency, by itself, won't trigger code from that dependency to be added to the native image... ? The very point of an optional dependency is that it's ignored at runtime unless someone else adds a non-optional dependency to the same artifact...

Anyway, if that's not the case, I imagine that PR did add some code that might confuse GraalVM into thinking we use JAXB. This looks like a prime suspect:

quarkus/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/customized/FormatMapperKind.java

Lines 31 to 34 in 47ecd0a

	@Override
	public FormatMapper create() {
	return new JaxbXmlFormatMapper();
	}

Maybe that code, or org.hibernate.type.format.jaxb.JaxbXmlFormatMapper, should be eliminated when JAXB is not present in the classpath (the runtime one, ignoring optional dependencies)?

Would that make sense @galderz?

[galderz]

Alright, but it's optional; I hope an optional dependency, by itself, won't trigger code from that dependency to be added to the native image... ? The very point of an optional dependency is that it's ignored at runtime unless someone else adds a non-optional dependency to the same artifact...

Optional dependencies are just a concept of Maven. Neither JDK nor GraalVM really has such a concept so anything you bring in through that is considered part of the universe. I wrote about the issues these have here.

Anyway, if that's not the case, I imagine that PR did add some code that might confuse GraalVM into thinking we use JAXB. This looks like a prime suspect:

quarkus/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/customized/FormatMapperKind.java

Lines 31 to 34 in 47ecd0a

	@Override
	public FormatMapper create() {
	return new JaxbXmlFormatMapper();
	}

Maybe that code, or org.hibernate.type.format.jaxb.JaxbXmlFormatMapper, should be eliminated when JAXB is not present in the classpath (the runtime one, ignoring optional dependencies)?

Would that make sense @galderz?

I think it's more nuanced than that, because the last GraalVM and/or Mandrel releases work fine with this. So this is not an immediate issue. The problem arises when you try with the very latest GraalVM code along with a newer JDK (the issue seems to appear with both latest JDK 21 and 22). Either of these two parts is causing this to break now but I don't know yet.

All my investigations so far lead up to this entry point which is a bit of a dead end:

├── entry com.oracle.svm.core.code.FactoryMethodHolder.DOMSerializer_constructor_ed8d8ed1c0ba7927c499a8c3ec42227bb4ec6043():com.fasterxml.jackson.databind.ext.DOMSerializer id=633 
│   └── directly calls com.fasterxml.jackson.databind.ext.DOMSerializer.<init>():void id=12495 @bci=1 

I'll see if I can git bisect on either the GraalVM or JDK code to try to narrow down the cause.

[galderz]

FYI I've located the commit that triggers this. I'm trying to understand its effects and how it affects us and will ping the GraalVM author shortly to discuss options. I'll post more details on #37498 when I have them. For now let's leave this PR as is pending a resolution to my investigation.

[galderz]

This has been fixed in GraalVM upstream so this PR is no longer needed.

[marko-bekhta]

Nice! thank you @galderz 🎉