Closes GH-34189

Comment From: bclozel

I'm declining this PR because I don't think we should take this approach to solve this problem.

The UriComponents infrastructure is shared for both RestClient and WebClient. They're both declaring defaultUriVariables(Map<String, ?> defaultUriVariables) methods that provide static values as a Map for the entire lifetime of the client. Supporting Function or Supplier means that such values will be calculated at runtime, possibly depending on the execution context. The background information in #34189 shows MDC usage which is a typical example of this. This will create concurrency and lifecycle problems as URLs might be built on a different thread.

I think this use case should be supported by ExchangeFilterFunction + reactor context for WebClient, and ClientHttpRequestInterceptor + request attributes for RestTemplate and RestClient.

Comment From: quaff

This will create concurrency and lifecycle problems as URLs might be built on a different thread.

It's safe for RestClient and RestTemplate, It may be a problem for WebClient, but it's the developer's choice to or not to use Supplier/Function as default variable value.

I think this use case should be supported by ExchangeFilterFunction + reactor context for WebClient, and ClientHttpRequestInterceptor + request attributes for RestTemplate and RestClient.

Could you reopen #34189 or create new issue to address it officially?

Comment From: quaff

Another thought, introduce a subclass of UriComponentsBuilder to accept Supplier/Function, let application construct their own DefaultUriBuilderFactory, for example:

UriComponentsBuilder ucb = UriComponentsBuilder.fromUriString("http://{partition}.163.com").acceptFunctions();
DefaultUriBuilderFactory  uriBuilderFactory = new DefaultUriBuilderFactory(ucb);
uriBuilderFactory.setDefaultUriVariables(defaultUriVariables);
RestClient restClient = RestClient.builder().uriBuilderFactory(uriBuilderFactory).build();

WDYT @bclozel

Comment From: bclozel

Could you reopen #34189 or create new issue to address it officially?

I meant that this should be already possible with an interceptor and a better for for this use case.

Comment From: quaff

Could you reopen #34189 or create new issue to address it officially?

I meant that this should be already possible with an interceptor and a better for for this use case.

I tried with ClientHttpRequestInterceptor:

        RestClient restClient = RestClient.builder().baseUrl("http://{partition}.example.com")
                .requestInterceptor(new ClientHttpRequestInterceptor() {
                    @Override
                    public ClientHttpResponse intercept(HttpRequest request, byte[] body,
                            ClientHttpRequestExecution execution) throws IOException {
                        return execution.execute(new HttpRequestWrapper(request) {
                            @Override
                            public URI getURI() {
                                return UriComponentsBuilder.fromUri(request.getURI())
                                        .buildAndExpand(Map.of("partition", MDC.get("partition"))).toUri();
                            }
                        }, body);
                    }
                }).build();


        restClient.get().uri("/index.html").retrieve().body(String.class)

It's not working, and not elegant, please rethink the solution I proposed.

Exception in thread "main" java.lang.IllegalArgumentException: Not enough variable values available to expand 'partition'
    at org.springframework.web.util.UriComponents$VarArgsTemplateVariables.getValue(UriComponents.java:370)
    at org.springframework.web.util.UriComponents.expandUriComponent(UriComponents.java:263)
    at org.springframework.web.util.HierarchicalUriComponents.expandInternal(HierarchicalUriComponents.java:438)
    at org.springframework.web.util.HierarchicalUriComponents.expandInternal(HierarchicalUriComponents.java:53)
    at org.springframework.web.util.UriComponents.expand(UriComponents.java:172)
    at org.springframework.web.util.DefaultUriBuilderFactory$DefaultUriBuilder.build(DefaultUriBuilderFactory.java:459)
    at org.springframework.web.util.DefaultUriBuilderFactory.expand(DefaultUriBuilderFactory.java:204)
    at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.uri(DefaultRestClient.java:317)
    at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.uri(DefaultRestClient.java:288)

Comment From: bclozel

I don't think the uri template should leave a path variable in. The interceptor should change the host instead.

Comment From: quaff

I don't think the uri template should leave a path variable in. The interceptor should change the host instead.

OK, it will works, but not elegant.

@Override
public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException {
    return execution.execute(new HttpRequestWrapper(request) {
        @Override
        public URI getURI() {
            return UriComponentsBuilder.fromUri(request.getURI()).host(MDC.get("partition") + "." + request.getURI().getHost()).build().toUri();
        }
    }, body);
}

Comment From: bclozel

I'll discuss https://github.com/spring-projects/spring-framework/issues/34189 with the team.