Closed fedinskiy closed 2 years ago
/cc @geoand, @gsmet
Indeed. We will likely switch the generator to include RESTEasy Reactive by default. When that happens, there will no longer be such a problem with generated projects.
@geoand this problem is not specific to generated projects. Any project, which has quarkus-rest-data-panache as a transitive dependency cannot use non-reactive jackson(and, probably non-reactive resteasy, too). It's especially surprising(in a bad sense), since neither quarkus-spring-data-rest nor quarkus-hibernate-orm-rest-data-panache is reactive.
@aloubyansky it seems that although both RESTEasy Reactive and RESTEasy Classic declare that they provide io.quarkus.rest
, when a build is done with both those extensions, we don't get the expected error
@geoand this problem is not specific for generated projects. Any project, which has quarkus-rest-data-panache as a transitive dependency cannot use non-reactive jackson(and, probably non-reactive resteasy, too). It's especially surprising(in a bad sense), since neither quarkus-spring-data-rest nor quarkus-hibernate-orm-rest-data-panache is reactive.
@fedinskiy I sense a misconception here... RESTEasy Reactive does not force you to use reactive programming, see this for more details.
What happened with #19801 is that quarkus-spring-data-rest
and quarkus-hibernate-orm-rest-data-panache
are now based on RESTEasy Reactive instead of RESTEasy Classic.
From a user's point of view mostlikely all that needs to be done is to switch the explicitly declared quarkus-resteasy*
dependencies to quarkus-resteasy-reactive*
.
@aloubyansky it seems that although both RESTEasy Reactive and RESTEasy Classic declare that they provide
io.quarkus.rest
, when a build is done with both those extensions, we don't get the expected error
Sure, that's a known issue, I think I raised it before for the RESTEasy classic and reactive. The config validation is happening very early, before build steps are actually executed. The capability check is performed in a build step. I originally had it at the AppModel build time, which is earlier than the config init. But then Max wanted the dev mode to be able to recover from a capability conflict and I moved to a build step and we hit this.
Hm... Maybe then it makes sense to add the check into BuildMojo
as well (or have BuildMojo
enable the AppModel build time check)?
Because when fixing the dev mode case, we have now lost the proper message for the prod build.
But then dev mode would fail with the reported error?
That will also mean two capability checks. It's currently based on CapabilityBuildItem
s . There actually are two ways to provide capabilities: in an extension descriptor and producing CapabilityBuildItem
s directly (which is the legacy way and discouraged but still possible). If we do the [first] check before the config init, it will not take into account all the possible CapabilityBuildItem
s.
But then dev mode would fail with the reported error?
Ah, I thought that dev-mode did display the proper error, but I see that is not the case.
So if you ask me, the idea of allowing dev-mode to recover is doing more harm than good in this case, because the proper error is never displayed and the user has no way of knowing how to recover.
A similar issue was reported in https://github.com/quarkusio/quarkus/issues/19868. Unless someone is close to Quarkus development, they have no way of knowing what action to take.
Oh, I think Kogito should not be dictating the RESTEasy flavor.
AFAIR, when a conflicting extension was added while running in dev mode, it would simply exit with an error. Which wasn't good.
AFAICS, dev mode can recover from the config error. Which means theoretically we could move the capability check right before the config init. But that still won't take into account all the CapabilityBuildItem
s. Should we check twice then? Or completely forbid producing CapabilityBuildItem
s directly.
I would say let's forbid producing CapabilityBuildItem
. I don't recall there being a valid use case for producing it programmatically.
As for Kogito, I definitely agree.
Oh, I think Kogito should not be dictating the RESTEasy flavor.
+1
Same error popped up in latest run of OptaPlanner QS
https://github.com/quarkusio/quarkus-quickstarts/tree/main/optaplanner-quickstart
java.lang.RuntimeException: java.lang.RuntimeException: java.lang.IllegalArgumentException: Multiple matching properties for name "security.jaxrs.deny-unannotated-endpoints" property was matched by both public boolean io.quarkus.resteasy.reactive.common.runtime.JaxRsSecurityConfig.denyJaxRs and public boolean io.quarkus.resteasy.runtime.JaxRsSecurityConfig.denyJaxRs. This is likely because you have an incompatible combination of extensions that both define the same properties (e.g. including both reactive and blocking database extensions)
I assigned it to myself but I was mostly looking into the proper error reporting, which I realized is quite tricky because there is a part of the capability production evaluation which requires the config to be built. But anyway, about the actual issue, @geoand are we going to look into adding the links support to the resteasy classic?
RESTEasy Classic does already have links support in the form of the org.jboss.resteasy:resteasy-links
module. It doesn't need any dedicated Quarkus support
The question then is whether we want to refactor the rest-data-panache
to use the links
impl that's available on the classpath instead of declaring a direct dependency on the reactive impl.
The question then is whether we want to refactor the
rest-data-panache
to use thelinks
impl that's available on the classpath instead of declaring a direct dependency on the reactive impl.
I don't think so, no. rest-data-panache
is super opinionated (and to mention experimental).
Here we go then. rest-data-panache
is not going to be compatible with RESTEasy classic. We need to make sure Kogito and OptaPlanner (and the rest of the ecosystem that is supposed to be working with reactive and classic impl) don't directly depend on any specific flavor of RESTEasy and probably fix our quickstarts, because they will keep failing.
Yeah, for sure.
cc @gsmet
quarkus-spring-data-rest
is TP on RHBQ side, "not compatible with RESTEasy classic" sounds like a strong message as there were some tests working fine before the change in main / for 2.3.
Sure. For 2.2 things work fine. But if we want to move to RESTEasy Reactive as the default, then things need to adapt
If this is going to be permanent, we should update the guides. I've provided a PR for this: https://github.com/quarkusio/quarkus/pull/19995
Moreover, it should be noted in the migration guide.
I think @geoand fixed this one. I am not sure in which version exactly though.
Right, this should now be fixed (I think in 2.5
)
Describe the bug
If project uses both quarkus-resteasy-jackson and some of the ORMm libraries(e.g. quarkus-spring-data-rest or quarkus-hibernate-orm-rest-data-panache) then any attempt to run tests or quarkus:dev in this project will fail with an exception.
Expected behavior
Project with both of these dependencies should be working correctly.
Actual behavior
An exception(for quarkus:dev):
How to Reproduce?
mvn clean quarkus:dev -Dquarkus.platform.version=999-SNAPSHOT -Dquarkus.platform.group-id=io.quarkus
Output of
uname -a
orver
Linux 5.13.12-200.fc34.x86_64
Output of
java -version
11.0.12
GraalVM version (if different from Java)
No response
Quarkus version or git rev
d4084fbde27c0fffaaff9e58b1cc155b66d48b62
Build tool (ie. output of
mvnw --version
orgradlew --version
)Apache Maven 3.6.3
Additional information
It looks like an underlying cause is a library quarkus-rest-data-panache which depends on quarkus-resteasy-reactive-links and the latter is not compatible with quarkus-reateasy, which is a dependency for quarkus-resteasy-jackson.
It looks like this behaviour is a side effect of https://github.com/quarkusio/quarkus/pull/19801.
The workaround is to use quarkus-resteasy-reactive-jackson instead of quarkus-resteasy-jackson. It doesn't look like a good solution, since it forces projects to use reactive libraries even if the do not require this functionality.