Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow us to configure dynamic query parameters in the rest client #24764

Closed
u6f6o opened this issue Apr 5, 2022 · 6 comments · Fixed by #24783
Closed

Allow us to configure dynamic query parameters in the rest client #24764

u6f6o opened this issue Apr 5, 2022 · 6 comments · Fixed by #24783
Assignees
Labels
Milestone

Comments

@u6f6o
Copy link

u6f6o commented Apr 5, 2022

Description

In certain scenarios it's not possible to know all query parameters in advance and to configure these accordingly with the @QueryParam("verbose") annotation. Other frameworks offer solutions for this, like annotating a map parameter with a @RequestParam annotation:

@RequestMapping(
    path = "/{id}/assets/with-options",
    method = RequestMethod.GET
)
public ResponseEntity<Collection<Asset>> getAssetsWithOptions(
    @PathVariable("id") String name,
    @RequestParam Map<String, String> params
) {
    ...
}

It would be nice if we would have an option to configure dynamic query paramters in quarkus as well. There is a workaround for this by adding a ClientRequestFilter, i.e:

@Provider
public class GetMessageBodyFilter implements ClientRequestFilter {
    @Override
    public void filter(ClientRequestContext requestContext) throws IOException {
        if (requestContext.getEntity() instanceof Map && requestContext.getMethod().equals(HttpMethod.GET)) {
            UriBuilder uriBuilder = UriBuilder.fromUri(requestContext.getUri());
            Map allParam = (Map)requestContext.getEntity();
            for (Object key : allParam.keySet()) {
                uriBuilder.queryParam(key.toString(), allParam.get(key));
            }
            requestContext.setUri(uriBuilder.build());
            requestContext.setEntity(null);
        }
    }
}

This method is a bit cumbersome though imo.

Implementation ideas

No response

@u6f6o u6f6o added the kind/enhancement New feature or request label Apr 5, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 5, 2022

/cc @michalszynkiewicz

@geoand
Copy link
Contributor

geoand commented Apr 5, 2022

We should definitely allow this for query params and I would assume also form params and header params.

We should probably use the @RestQuery (and friends) annotation to mark these parameters as they don't require a name (which in this case doesn't make sense anyway).

@michalszynkiewicz does that sound good? If so, I'll pick this up.

@michalszynkiewicz
Copy link
Member

yes, thank you! :)

@sberyozkin
Copy link
Member

How about using @UriInfo injection in the endpoint without any query parameters in the method signature ?

@sberyozkin
Copy link
Member

Never mind, sorry, forgot you are talking about the rest client

@u6f6o
Copy link
Author

u6f6o commented Apr 5, 2022

Never mind, sorry, forgot you are talking about the rest client

👍 Np, my first example code is a bit miss-leading, as it shows the controller method instead of the rest-client method.

@geoand geoand self-assigned this Apr 5, 2022
geoand added a commit to geoand/quarkus that referenced this issue Apr 6, 2022
@quarkus-bot quarkus-bot bot added this to the 2.9 - main milestone Apr 6, 2022
geoand added a commit that referenced this issue Apr 6, 2022
Add support for sending query params as using a Map in Reactive Rest Client
@gsmet gsmet modified the milestones: 2.9 - main, 2.8.2.Final Apr 25, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Apr 25, 2022
@gsmet gsmet modified the milestones: 2.8.2.Final, 2.7.6.Final May 24, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants