quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.54k stars 2.61k forks source link

Automatically add dependency for jackson-module-kotlin #11974

Open laguiar opened 4 years ago

laguiar commented 4 years ago

As pointed in the Kotlin documentation: https://quarkus.io/guides/kotlin#kotlin-and-jackson

The jackson-module-kotlin has to be manually added when there is the need to register the Kotlin module on Jackson, basically, when using kotlin + jackson.

Description When bootstrapping an application, when any Jackson specific extensions were added (as mentioned in the documentation above), with combination with quarkus-kotlin, then jackson-module-kotlin might also be automatically added as a dependency.

It may be simple for someone already in touch with Quarkus code to add this, I can try to do it myself otherwise, with some directions of any team member.

quarkusbot commented 4 years ago

/cc @evanchooly

evanchooly commented 4 years ago

Is it safe to assume we would always need the jackson kotlin module? On the one hand, it's just a jar probably few would notice but some might quibble at the unnecessary jar. I'm thinking of, say, kafka projects. Do they need/use jackson for serialization or would it just be the RESTy front end type apps. For that matter, even REST wouldn't necessarily need jackson if the payloads were simple.

In general, I'm not opposed to the idea but it warrants a little thought on the wider impact. @cescoffier? @geoand? @kenfinnigan

geoand commented 4 years ago

The reason we don't add this automatically, is because we don't have a kotlin + jackson extension - so there is no natural place to put this dependency that would make sense. For me it isn't safe to assume that all Kotlin projects will include Jackson - and the reverse is of course even more unreasonable.

What we do do (as mentioned in the documentation), is activate the Kotlin Jackson module if it's on the classpath and both Kotlin and Jackson extensions are being used.

evanchooly commented 4 years ago

Ah. So this sounds like a non-issue then and it's already being handled behind the scenes. Is there anything to done here, then?

geoand commented 4 years ago

I personally don't see anything else we can do here - theoretically we can add the dependency on our own, but I really don't think we want to start doing that kind of thing...

That's just my opinion though

evanchooly commented 4 years ago

I tend to agree. what about if we enhanced the add-extension plugin to detect the need and silently include it then?

geoand commented 4 years ago

That does sound like a good idea to me.

@ia3andy what do you think?

laguiar commented 4 years ago

Hi @evanchooly @geoand My initial idea was exactly to detect automatically on https://code.quarkus.io that both kotlin + jackson were added to the project dependencies and place the extra jackson-module-kotlin on gradle/maven file.

I had to figure out the missing dependency when some json parsing was failing already in my tests, because my first expectation was that it was already on classhpath somehow.

Just as an example, please I'm not comparing anything here... both spring-boot and micronaut add jackson-module-kotlin on their bootstrap tool, at least for me, it kinda makes sense to expect this behaviour.

The add-extension plugin would be very handy too.

maxandersen commented 4 years ago

this should not be limited to code.quarkus.io - this should behave consistently across our tool chain.

do we have examples on other similar implicitly needed dependencies (that users must add and we can't add for them elsewhere) ?

wondering if we should add a metadata to extensions to act as hint what could be recommended to add.

geoand commented 4 years ago

@maxandersen this one is the only such case I can think of

ia3andy commented 4 years ago

There is also Qute RESTEasy and RESTEasy Qute for example, and I think there are some other. I think we need to have a new requires-extensions field in the extensions metadata. Then we would just have to auto-add the extension when the require-extensions are all in the set of selected extensions. In the other direction also, if RESTEasy Qute is selected, it will auto-add Qute and RESTEasy:

// io.quarkus:quarkus-resteasy-qute metadata:

requires-extensions:
  - io.quarkus:quarkus-qute
  - io.quarkus:quarkus-resteasy

Maybe there is a better label than requires-extensions? Any thoughts?

machi1990 commented 4 years ago

There is also Qute RESTEasy and RESTEasy Qute for example, and I think there are some other. I think we need to have a new requires-extensions field in the extensions metadata. Then we would just have to auto-add the extension when the require-extensions are all in the set of selected extensions. In the other direction also, if RESTEasy Qute is selected, it will auto-add Qute and RESTEasy:

// io.quarkus:quarkus-resteasy-qute metadata:

requires-extensions:
  - io.quarkus:quarkus-qute
  - io.quarkus:quarkus-resteasy

Maybe there is a better label than requires-extensions? Any thoughts?

@ia3andy interesting. I'd thought that the RESTEasy Qute extension came with the RESTEasy and Qute extensions automatic.

gsmet commented 4 years ago

We don't have a mechanism that automatically adds extensions. IIIRC, @Sanne wanted that in the beginning and others weren't excited about it thus why it was never done.

BTW, requires-extensions is very misleading, it's more something like extensions-triggering-inclusion that you want (name is equally bad but that's the idea) or something like that. And you would need the ability to add several different combinations of extensions triggering the inclusion.

Personally, I would really like to have a mechanism suggesting extensions but I wouldn't add them automatically because good luck with keeping your build tools sane after that.

This is something that should be discussed on the list IMHO.

gsmet commented 4 years ago

Also, I don't think I would put it in the metadata but I would leverage the build steps and have a SuggestedExtensionBuildItem that would be collected and then displayed nicely in a unified way.

That would allow for much more flexibility.

maxandersen commented 4 years ago

@gsmet wouldn't that mean it gets printed on every build ?

maxandersen commented 4 years ago

also - tools should be able to reason about this without running quarkus build imo. that doesn't exclude us doing some builditem thing additional but just seems like for this usecase its "too late" to only do this at quarkus buildtime.

Sanne commented 4 years ago

Thanks for pinging me! Indeed the lack of automatcally controlling additional dependencies has been bothering me for a while, and I believe the time could be right. I'll send a proposal to the list.