Introduce request attributes in RestClient

[evkaky]

Hi there!
When spring boot 3.2 came out, I considered switching from WebClient to RestClient as the first one needs all interceptors to be written in reactive style which makes them harder to support and understand.
It turns out RestClient doesn't provide any alternates to 2 pretty important features which are present in WebClient and which we heavily use.

1. no attribute(key, value) method for RestClient. That means it's not possible to pass a value from the call site into the interceptor code. Dozen of auth interceptors become impossible to implement with RestClient. Here is how we use them with WebClient:

myWebClient
    .get()
    .uri("/smth")
    .attribute("token" token)
    .retrieve()
    // ...

// the interceptor code
public ExchangeFilterFunction authenticate() {
    return (request, next) -> {
        String token = request.attribute("token").get().toString();
        // ...
    };
}

2. No single option to configure timeouts. With WebClient there are lots of them. An example

@Bean
WebClientCustomizer webClientGlobalTimeoutCustomizer() {
    return (WebClient.Builder webClientBuilder) -> {
        ConnectionProvider provider = ConnectionProvider.builder("common")
            .maxIdleTime(Duration.ofSeconds(10))
            .maxLifeTime(Duration.ofSeconds(60))
            .pendingAcquireTimeout(Duration.ofSeconds(30))
            .evictInBackground(Duration.ofSeconds(60))
            .build();

        HttpClient httpClient = HttpClient
            .create(provider)
            .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, timeoutInMillis)
            .doOnConnected(conn -> conn
                .addHandlerLast(new ReadTimeoutHandler(5000, TimeUnit.MILLISECONDS))
                .addHandlerLast(new WriteTimeoutHandler(5000, TimeUnit.MILLISECONDS)));

        webClientBuilder.clientConnector(new ReactorClientHttpConnector(httpClient));
    };
}

Are those things something which is planned to be implemented in spring boot 3.3+ ? Or there some fundamental issues with RestClient underlying implementation so no attribute() and no timeouts?

[sbrannen]

• related to Remove leftover RestClient attribute methods #31625

[ThomasKasene]

Regarding the attribute feature: I'm attempting a PR towards spring-projects/spring-security#13588 and the solution I've come up with so far is calling RequestContextHolder.currentRequestAttributes() and then adding whatever attributes I need to the resulting RequestAttributes object. I don't actually know if this is the correct way of doing it or not, and I suspect I'll encounter major issues once I try to implement a version of the interceptor that doesn't have a servlet request context for me to use (the RestClient version of ServerOAuth2AuthorizedClientExchangeFilterFunction).

For both of these reasons it would be nice to have a uniform way of passing attributes to the interceptors, instead of every interceptor having to re-invent the wheel.

[poutsma]

In the future, please do not list multiple feature requests in a single issue, but create separate issues instead. This makes it easier to track the state of each.

Attributes

With regard to attributes, it is important to understand that they were introduced into WebClient to solve a particular problem of reactive environments: the lack of dependable thread-locals. Since its introduction in Spring Framework 3.0, the non-reactive RestTemplate never needed attributes, or at least such a feature request was not made (so far).

The RestClient is based on the existing infrastructure of RestTemplate (i.e. ClientHttpRequestFactory, ClientHttpRequestInterceptor , etc.), so without any support for attributes. We initially considered adding them (some leftovers even made it into 6.1.0, see #31625), but in the end decided to drop attribute support for RestClient, because adding them would require making many changes in the existing infrastructure, while it was not yet clear if they were even needed. After all, RestTemplate had done fine so far without attributes!

We can of course consider introducing support for attributes in RestClient for Spring Framework 6.2, but I'd like to know if you still consider them essential, given the information above.

Timeouts

I am not entirely sure what you mean with the lack of options to configure timeouts. For RestClient (and RestTemplate), these are configured on the particular ClientHttpRequestFactory you decide to use. For instance, for the JettyClientHttpRequestFactory, there is a connection timeout and a read timeout. Other request factories have similar timeout properties you can configure.

Are these configuration options sufficient, or is there something that is lacking?

[ThomasKasene]

We initially considered adding them (some leftovers even made it into 6.1.0, see #31625), but in the end decided to drop attribute support for RestClient, because adding them would require making many changes in the existing infrastructure, while it was not yet clear if they were even needed. After all, RestTemplate had done fine so far without attributes!

True enough, it's possible that some of us have become too preoccupied with keeping the APIs/usage of RestClient as close to that of WebClient as possible, without considering what's best in the long run. 😃At the moment, my most pressing need is being able to customize the Authorization header in a simple manner, on a per-request basis.

There are helper methods in the RestClient API for adding certain headers such as accept(MediaType...) - is there any particular reason why there isn't an authorization(Supplier<String>) method in there as well, apart from nobody having requested one yet? It would enable us to make something that looks pretty succinct:

restClient
    .get()
    .uri("/rest/person")
    .authorization(oauth2
        .clientRegistrationId("my-client-registration")
        .principal("johndoe"))
    .retrieve()
    // ...

as opposed to the monstrosity we'd have to write with today's API:

restClient
    .get()
    .uri("/rest/person")
    .headers(headers -> headers
        .setBearerAuth(oauth2
            .clientRegistrationId("my-client-registration")
            .principal("johndoe")
            .build()))
    .retrieve()
    // ...

Note the mention of "bearer" (which should be an implementation detail, not up to the caller) and an explicit call to some sort of terminating method (build() in this case).

[poutsma]

Re-added feedback label as an answer from @evkaky is still forthcoming.

[evkaky]

Hi everyone.
Thanks a lot for your support!
@poutsma , on timeouts - yeah, totally makes sense, I somehow missed that requestFactory method on rest client builder, when I was exploring the api.

On attribute() replacement for rest client - I still have doubts.
Is the approach Thomas suggested (with RequestContextHolder.currentRequestAttributes()) is an idiomatic way to dynamically pass data to interceptors in rest template / rest client world?
Is this how it should be used? (assuming I have to decouple rest client creation from the code where I actually make an http call. This rest client should be reusable across entire app in my case)

@Bean
RestClient myRestClient(RestClient.Builder builder) {
  builder
    .baseUrl("...")
    .requestInterceptor((request, body, execution) -> {
        String someId = RequestContextHolder.getRequestAttributes().getAttribute("someId", 0).toString();
        // I need this someId to make a request to a 3rd party API,
        // that 3rd party API will return a `token`, which I will attach to the header of the initial request, like this
        String token = getTokenFromAuthService(someId);
        request.getHeaders().setBearerAuth(token);
    })
    .build();
}

and then, somewhere on the call site (in my case "call site" is a message handler of some queue):

@Autowired
@Qualifier("myRestClient")
RestClient myRestClient;

void onDataReceive(String someId) {
   RequestContextHolder.getRequestAttributes().setAttribute("someId", someId, 0);
   myRestClient
     .get()
     .retrieve()
     // ...
}

[davebarda]

+1 for the request attributes, for my case, I want to transfer some data to the ClientRequestObservationConvention, but there's no way of adding some additional data to the request nor to the ClientRequestObservationContext as i've explained at #32153.
In my context, the http request might be sent from multiple contexts(kafka,rabbit,rest,grpc, etc..) and I don't feel like populating RequestContextHolder is the right way.

Regarding the question about RestTemplate, I can say that it was solved by wrapping all of our request and sending our own metrics instead of using the ones supplied by spring, due to lack of context. Now that we're investing time in using the new client, I hoped I could improve the experience and the robustness of the solution.

[evkaky]

It seems like RequestContextHolder can be used only within servlet environment (spring-web-mvc). If you have some queue handler (like @davebarda has) RequestContextHolder.getRequestAttributes() seems to always return null, doesn't it?
If so, I would also vote for adding attribute method (or smth similar) to RestClient

[poutsma]

Thanks for the feedback, everybody, I have scheduled this issue for 6.2.

[snicoll]

When handling this issue, please take #32231 and #32232 into account as it will require an update.

[KrishnaST]

Use of ThreadLocals via RequestContextHolder is simply bad design. There is extra burdon of cleaning up after. +1 from me for request attributes.

[bclozel]

@KrishnaST this is already done. Is there something missing? Please advise.

[KrishnaST]

@bclozel am I missing something here? How to pass attributes to interceptors?

[bclozel]

See https://docs.spring.io/spring-framework/docs/6.2.0-SNAPSHOT/javadoc-api/org/springframework/web/client/RestClient.RequestHeadersSpec.html#attribute(java.lang.String,java.lang.Object) and https://docs.spring.io/spring-framework/docs/6.2.0-SNAPSHOT/javadoc-api/org/springframework/http/HttpRequest.html#getAttributes().

For further questions, please use StackOverflow.