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

Provide convenient ways to specify query parameters to WebClient [SPR-15124] #19691

Closed
spring-projects-issues opened this issue Jan 10, 2017 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 10, 2017

Toshiaki Maki opened SPR-15124 and commented

In my understanding, when specifying query parameters to WebClient, ClientRequest#method(HttpMethod, URI) or building url string can be used.
I'd like to have more convenient way.

For example, when http://api.example.com is a externalized property(api.path) and an application accesses $(api.path)/v1/foo?bar=$(bar)&baz=$(baz), we need to write code such as following:

int bar = 100;
String baz = aaa;
ClientRequest<Void> req = ClientRequest
		.method(GET, UriComponentsBuilder.fromHttpUrl(apiPath)
		  .pathSegment("v1", "foo")
		  .queryParam("bar", bar)
		  .queryParam("baz", baz).build().encode().toUri());

// or ClientRequest<Void> req = ClientRequest.GET(apiPath + "/v1/foo?bar=" + bar + "&baz=" + baz);  // I don't like this style

I would like to propose adding the following methods in org.springframework.web.reactive.function.client.ClientRequest

// though rough design
static BodyBuilder method(HttpMethod method, String url,
		Function<UriComponentsBuilder, UriComponents> f) {
	UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(url);
	URI uri = f.apply(builder).toUri();
	return new DefaultClientRequestBuilder(method, uri);
}

static HeadersBuilder<?> GET(String url,
		Function<UriComponentsBuilder, UriComponents> f) {
	return method(HttpMethod.GET, url, f);
}

// POST, PUT, ...

With this method, the example code above can be re-written as follows:

ClientRequest<Void> req = ClientRequest
		.GET(apiHost, b -> b.pathSegment("v1", "foo")
                                                 .queryParam("bar", bar)
                                                 .queryParam("baz", baz)
                                                 .build().encode())
		.build();

Affects: 5.0 M4

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 19, 2017

Rossen Stoyanchev commented

Toshiaki Maki thanks for raising this question. You could also do:

ClientRequest.GET(apiPath + "/v1/foo?bar={bar}&baz={baz}", bar, baz).build();

That said there are bigger issues at play with this and it's more than a question of preference for URI template expansion vs builder method style. The overloaded methods with String + URI variables are provided for convenience but they are also too basic, limiting and inflexible. Essentially it amounts to using UriTemplate vs the UriComponentsBuilder which was added later to provide more advanced options and control. Hence your preference to gravitate towards that.

The issue doesn't stop there however. In the RestTemplate where we also take String + URI variables, it is now possible to configure a UriTemplateHandler which customizes how URI templates are expanded, essentially implementing different ways to use UriComponentsBuilder. It provides such useful properties as a base URL, default URI variable values, different encoding models, and you can even completely replace that with a 3rd party, URI template mechanism (e.g. for RFC 6570).

The problem here is that the static builder methods of ClientRequest provide zero options for customizing how URI templates are expanded. We could allow using a UriComponentsBuilder in some fashion as you have suggested and that would provide more control but that's still only marginally better vs using UriComponentsBuilder outside, and more importantly it doesn't provide a path to solving the kinds of issues that UriTemplateHandler solves such as for example having a baseUrl, like your apiPath, i.e. it doesn't provide recipes, driven through configurable options for building URLs in different ways through UriComponentsBuilder.

We actually have the same issue in many places where URIs need to be accepted like the reactive WebSocketClient, the reactive MockServerHttpRequest builders. Even the RestTemplate and RequestEntity, the existing WebSocketClient , they all have methods that accept a URI.

So my suggestion is two-fold:

  1. First get rid of the String + URI variable arguments in APIs where we still can do that since they're too limiting and inflexible. Simply accept URI instead and that would encourage applications to separate the preparation of a URI and as a consequence more likely on inline like this webClient.exchange(GET(uri).build()) vs preparing the ClientRequest outside.
  2. Create a UriBuilder that exposes a simplified subset of what UriComponentsBuilder does (URI template, variables, and builder methods) along with the same benefits as UriTemplateHandler but in a more stateful, standalone fashion.

I realize a more concrete proposal would be needed to to understand better. I will work on that next. Hopefully on a high level this direction makes sense. It's an improvement we need to experiment with now as we introduce a whole bunch of new APIs where URIs are taken as input.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 20, 2017

Sébastien Deleuze commented

I see a lot of value to the approach you suggested Rossen Stoyanchev, but I would suggest to keep the String variant (no strong need to keep the varags parameter) because this will be used a lot by Kotlin users.

Kotlin allows string interpolation, allowing to use variable in strings with a very concise syntax. Imagine you have already base, owner and repo variables, in Kotlin you can just write client.exchange(GET("$base/repos/$owner/$repo/issues?state=open").accept(VND_GITHUB_V3)) and the relevant variable will be expanded automatically.

For the common use case where you deal with URL friendly variable, that's very handy and since this could have some usage in Java for very simple use case, I would just keep the String variant without varargs.

For use case where encoding is needed, the URI builder approach should be used in both Kotlin and Java.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 27, 2017

Rossen Stoyanchev commented

Commit 7b67b5 introduces a UriBuilderFactory + UriBuilder for configuring URI building preferences (e.g. base URI) once and then building many URIs. This can now be plugged into WebClient which provides instance-based methods for building and performing exchanges with the help of the configured UriBuilderFactory. See WebClientIntegrationTests for an example.

The above example becomes:

WebClient client = WebClient.create(apiPath);

Mono<String> result = client.get()
        .uri(builder -> builder.path("/foo").queryParam("bar", bar).queryParam("baz", baz).build())
        .exchange()
        .then(response -> response.bodyToMono(String.class));

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 29, 2017

Toshiaki Maki commented

looks much greater than my proposal :D

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 29, 2017

Toshiaki Maki commented

I found a small bug
#1309

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 1, 2017

Rossen Stoyanchev commented

We've simplified things a bit further and I've updated the sample from my last comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants