quarkusio / quarkus

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

Error with HTTP POST using "REST Data with Panache" extension #9510

Closed jclingan closed 4 years ago

jclingan commented 4 years ago

Describe the bug HTTP POST with results in an error, but GET, PUT, and DELETE work.

Expected behavior HTTP POST should add entity to H2 database.

Actual behavior Error on HTTP POST:

RESTEASY012020: Discovery failed for method org.acme.rest.PersonControllerImpl_772bba7f3925f61a49c0d93159012a40b2de28d6.delete: RESTEASY012005: Cannot guess resource type for service discovery

To Reproduce Reproducer here.

Environment (please complete the following information): java -version output:

openjdk version "11.0.6" 2020-01-14
OpenJDK Runtime Environment GraalVM CE 20.0.0 (build 11.0.6+9-jvmci-20.0-b02)
OpenJDK 64-Bit Server VM GraalVM CE 20.0.0 (build 11.0.6+9-jvmci-20.0-b02, mixed mode, sharing)

Additional context I'm a really nice guy.

geoand commented 4 years ago

cc @gytis

gytis commented 4 years ago

This line doesn't work as expected https://github.com/resteasy/Resteasy/blob/master/resteasy-links/src/main/java/org/jboss/resteasy/links/impl/AbstractLinksProvider.java#L84. It calls Class.forName("org.acme.rest.Person") but gets an exception.

PUT with a new entity also doesn't work.

We call this RESTEasy class to get a URL of a newly created object in order to include it in a Location header. RESTEasy looks up all the possible endpoints( hence .delete) and we pick only the get out of the returned map.

I'll look into it.

geoand commented 4 years ago

What kind of exception do you get from that method? Seems weird to get an exception there...

Although I do have a suspicion about what might be going on... Does the exception only happen in dev mode?

gytis commented 4 years ago

Considering that it continues the execution it has to be ClassNotFoundException because it's the only one being caught by a try block.

The error that John sees is given a bit later in the same method https://github.com/resteasy/Resteasy/blob/master/resteasy-links/src/main/java/org/jboss/resteasy/links/impl/AbstractLinksProvider.java#L92

geoand commented 4 years ago

Is this a dev-mode only problem? If it is, then I think I know the cause and what needs to be done to fix it (it will have to be in RESTEasy)

gytis commented 4 years ago

Let me try. But I guess it could be because it never came up in the integration tests.

gytis commented 4 years ago

Yeah it works fine with a runner jar.

geoand commented 4 years ago

OK, then we need a change in RESTEasy. We need to add a getServiceType method that will take an additional ClassLoader argument. When we call that method, the CL we pass in will be the TCCL.

Obviously once that fix is in RESTEasy, we'll need to add a Quarkus DevModeTest.

Does that make sense?

gytis commented 4 years ago

Yeah I assumed it's a CL issue. Just wondered why didn't it show up in a regular execution mode.

geoand commented 4 years ago

Because in regular execution mode there is only one ClassLoader.

gytis commented 4 years ago

I thought that. Didn't know it's different from the application point of view in a dev mode.

gytis commented 4 years ago

Anyway, I'll fix it. And I've already been planing to reorganise the tests, so will add dev-mode tests at the same time.

geoand commented 4 years ago

Great thanks!

gytis commented 4 years ago

RESTEasy JIRA ticket https://issues.redhat.com/browse/RESTEASY-2595 and a PR to fix it https://github.com/resteasy/Resteasy/pull/2424

gytis commented 4 years ago

A test to verify the fix https://github.com/gytis/quarkus/commit/2b18281373fc420aa649f2ebc137386436efe187#diff-8b31f68e73bea3aaa11b328a7caabac4R32. It of course needs a patched RESTEasy version, so cannot raise a PR for it just yet.

gytis commented 4 years ago

Funny enough with QuarkusDevModeTest the failure is different (although same fix applies). In a test, a call to Class.forName succeeds. However, a moment later type.isInstance(entity) fails here https://github.com/resteasy/Resteasy/blob/master/resteasy-links/src/main/java/org/jboss/resteasy/links/ObjectLinksProvider.java#L49.

geoand commented 4 years ago

Yeah, doesn't seem weird that you would need to have the same fix throughout

lordofthejars commented 4 years ago

Happened to me as well this in Dev mode. Just in case you need it as well.

https://groups.google.com/forum/#!topic/quarkus-dev/v1ysGv1JWm4

gytis commented 4 years ago

@lordofthejars I have a fix for it. But unfortunately it needs a change in RESTEasy code, so it might take a while until it trickles down here.

lordofthejars commented 4 years ago

oh :( I was thinking on demoing this awesome addition. Now I'll do something like, let's do a native compilation so I do not need to show dev mode.

geoand commented 4 years ago

@gytis has your RESTEasy fix been included in the latest release that was merged in Quarkus?

gytis commented 4 years ago

Seems that it wasn't. I think it will be part of 4.6.0. It would have failed the Quarkus extension test, because the API exposed by RESTEasy changed.

geoand commented 4 years ago

Understood, thanks

ge0ffrey commented 4 years ago

Any progress on this? I am also running into it for optaplanner's quarkus-school-timetabling-quickstart.

image

ge0ffrey commented 4 years ago

Summary: resteasy fix is released in 4.5.7, but quarkus 1.8 still uses resteasy 4.5.6. Hopefully quarkus 1.9 can upgrade resteasy and fix this?

More info: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/PanacheRepositoryResource.20add

ge0ffrey commented 4 years ago

Quarkus 1.9 will probably fix it if it contains this PR https://github.com/quarkusio/quarkus/pull/12232/files

gytis commented 4 years ago

To fully fix this we'll need to merge https://github.com/quarkusio/quarkus/pull/10431 but it depends on the RESTEasy update that @ge0ffrey mentioned.

anhldbk commented 4 years ago

I tried Quarkus v1.9.0.CR1. This issue is fully fixed. Thanks guys!