search

FasterXML / jackson-core

Core part of Jackson that defines Streaming API as well as basic shared abstractions
Apache License 2.0
2.27k stars 799 forks source link

Fix #1340 by adding missing "provides" in `module-info.java` #1339

Closed sdyura closed 2 months ago

sdyura commented 2 months ago

(Fixes #1340)

if we have a src/main/resources/META-INF/services/com.fasterxml.jackson.core.JsonFactory this works in projects that have no module-info.java, but if a project does have module-info.java then we need to also add the java modules way of provides

this fixes ServiceLoader<JsonFactory> sl = ServiceLoader.load(JsonFactory.class); not working (returning empty) for projects that have a module-info.java

pjfanning commented 2 months ago

Would it be possible to provide something to test this with? Even a standalone project.

pjfanning commented 2 months ago

Or even links to docs that describe this issue and/or fix generally.

sdyura commented 2 months ago

to test you can run ServiceLoader<JsonFactory> sl = ServiceLoader.load(JsonFactory.class); in a project, it will work fine (return 1 item) if the project has no module-info.java, but as soon as you add module-info.java to the project, it will break (return 0 items)

here is a simple project that fails when run: https://github.com/user-attachments/files/17117454/arangodb-java-driver-demo.zip

sdyura commented 2 months ago

it is standard java to Always put the ServiceLoader loaded class in BOTH the "META-INF/services" AND module-info.java, you can see in the other jackson module this is the case:

rashtao commented 2 months ago

Ideally this should be backported also to older branches.

pjfanning commented 2 months ago

This will likely be merged but can't users who need this today create their own jar that has a module-info.class in like described in #1341. The workaround jar might also need the META-INF/services/com.fasterxml.jackson.core.JsonFactory file.

The same users who think this is needed yesterday could also build their own jackson-core with this amendment in it.

cowtowncoder commented 2 months ago

I can backport to, say, 2.16 and 2.17 but unlikely to release new patches for pre-2.17.

I assume this has been tested to work? (even if just manually)

cowtowncoder commented 2 months ago

Backport in 2.16 and 2.17 in case new versions might be released. But the next version that has the fix will be 2.18.0.

rashtao commented 2 months ago

Hi @cowtowncoder , I have manually checked that the changes work when building from 2.18. Thanks!