Quarkus REST client fails to process all path variables

[dhoffer]

Describe the bug

I am creating a REST client programmatically using Quarkus. E.g.

service= RestClientBuilder.newBuilder()
.baseUrl(new URL(URL))
.build(SomeService.class);

It seems there is a problem with it reading path params. Actually two separate problems.

Problem 1:
public interface SomeService {
@path("{currentUserId}/{fileName}")
Response sendFile( String currentUserId,
String fileName,
@BeanParam() MultipartBody body);
}
Here it thinks I have no path variables but from the docs my understanding is that if my variable name matches the Path annotation name then I don't need to annotate with @PathParam.

Problem 2:
public interface SomeService {
@path("{currentUserId}/{fileName}")
Response sendFile( @PathParam(value = "currentUserId") String currentUserId,
@PathParam(value = "fileName") String fileName,
@BeanParam() MultipartBody body);
}
Here I go ahead and add @PathParam to solve the prior problem but it still fails because the QuarkusRestClientBuilder#verifyInterface method only files one path variable in @path. It only files the last one (fileName)

As a workaround I had to define the two path variables as one variable and then just concatenate them with a / between.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

openjdk 11.0.11 2021-04-20

GraalVM version (if different from Java)

No response

Quarkus version or git rev

I am using 2.1.4.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @michalszynkiewicz

[geoand]

@michalszynkiewicz was this fixed by #20056?

[michalszynkiewicz]

@geoand nope, this is the old rest client.

@radcortez do you by any chance have some time to fix this?

[radcortez]

@radcortez do you by any chance have some time to fix this?

Yes, I'll have a look.

[radcortez]

@dhoffer

1 - @dhoffer You can't ignore the annotation, because we don't know the parameter type. It can be a path, query, header, etc., so you still need to annotate the parameter to state the type. Using javax.ws.rs.PathParam won't work, because the value is mandatory, so you need to use org.jboss.resteasy.annotations.jaxrs.PathParam, which doesn't require you to set the value. Finally, you need to tell the compiler to enable debug info (-g) or parameter names (-parameters). We need to discuss if we want to keep it this way to remove the reflection calls to retrieve the parameter names.

@michalszynkiewicz We may be able to support the parameter names via a build step, but it is going to require some major rewriting on the builder side. We also need to consider what we want to do with the current builder. Right now, it is a copy of the RESTEasy one, which uses reflection and JDK proxies. Should we rewrite it with a more Quarkus like implementation?

2 - @dhoffer I was not able to replicate it. Can you please provide me a reproducer?

[dhoffer]

@radcortez

1. The Quarkus documentation says you can ignore the parameter annotation. https://quarkus.io/guides/rest-client-reactive#programmatic-client-creation-with-restclientbuilder. Specifically No @PathParam annotation is necessary on countryName as the name of the parameter matches the value used in the @Path annotation. Furthermore the example shown in that same section uses javax.ws.rs.PathParam. But now your saying that that one does not work and must use org.jboss.resteasy.annotations.jaxrs.PathParam instead? IMO that's not an acceptable rule as both the documentation disagrees and there is no way developers can know that the standard javax.ws.rs.PathParam does not work. Also can't require that developers use -g else things don't work. All of that is too cumbersome/fragile/etc.
2. But I did switch to using org.jboss.resteasy.annotations.jaxrs.PathParam and I don't see the error now so I switched back to javax.ws.rs.PathParam and still no errors. So I don't know what is going on. It could be the Path error occurs when the body parameter is specified a particular way. Now I am just passing the body as InputStream without any annotations. Previously I was using various body param annotations/etc.

What I have is working now but something here is very fragile.

[michalszynkiewicz]

@dhoffer

1 - @dhoffer You can't ignore the annotation, because we don't know the parameter type. It can be a path, query, header, etc., so you still need to annotate the parameter to state the type. Using javax.ws.rs.PathParam won't work, because the value is mandatory, so you need to use org.jboss.resteasy.annotations.jaxrs.PathParam, which doesn't require you to set the value. Finally, you need to tell the compiler to enable debug info (-g) or parameter names (-parameters). We need to discuss if we want to keep it this way to remove the reflection calls to retrieve the parameter names.

@michalszynkiewicz We may be able to support the parameter names via a build step, but it is going to require some major rewriting on the builder side. We also need to consider what we want to do with the current builder. Right now, it is a copy of the RESTEasy one, which uses reflection and JDK proxies. Should we rewrite it with a more Quarkus like implementation?

We have RESTEasy Reactive, I don't think such a rewrite is of high priority.

2 - @dhoffer I was not able to replicate it. Can you please provide me a reproducer?
👍🏻

If everything works now, let's close the issue. @dhoffer if you find what was causing the error, please reopen (or create a new issue) with a reproducer.

[michalszynkiewicz]

@radcortez thanks a lot for investigating this!

[radcortez]

@dhoffer I believe the issue is that you were trying to use the MP REST Client API (and not the reactive one, like your link in the documentation), which is a little bit different and the documentation is here: https://quarkus.io/guides/rest-client.

Ideally, both clients should behave in the same way, but it requires some rewrite in the non-reactive version. I guess we will need to align them at some point.

[dhoffer]

Yes I was using the MP REST Client API. I thought that is the docs I was using. In any case, the 2 path params could not be parsed with the original code I posted. I see then I was using @BeanParam() MultipartBody for the body and now am just using InputStream w/o any annotations and now it works. No idea why.

[radcortez]

If you can provide me a reproducer for that second case, I can look at it. Thanks!