Closed spring-projects-issues closed 7 years ago
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:
webClient.exchange(GET(uri).build())
vs preparing the ClientRequest
outside.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.
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.
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));
Toshiaki Maki commented
looks much greater than my proposal :D
Toshiaki Maki commented
I found a small bug https://github.com/spring-projects/spring-framework/pull/1309
Rossen Stoyanchev commented
We've simplified things a bit further and I've updated the sample from my last comment.
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:I would like to propose adding the following methods in
org.springframework.web.reactive.function.client.ClientRequest
With this method, the example code above can be re-written as follows:
Affects: 5.0 M4
Issue Links:
19765 RestTemplate drops trailing / from request URI