Problem
The current design of spring.http.reactiveclient and spring.http.client introduces unnecessary duplication and complexity.
These two configurations serve blocking clients (RestClient) and reactive clients (WebClient) respectively. Their configuration options are nearly identical, with the only difference being the underlying implementations (factory vs connector).
spring:
http:
client:
connect-timeout: 1s
read-timeout: 10s
factory: jdk
service:
group:
service-a:
base-url: service-a:8080
connect-timeout: 2s
read-timeout: 30s
reactiveclient:
connect-timeout: 1s
read-timeout: 10s
connector: jdk
service:
group:
service-b:
base-url: service-b:8080
connect-timeout: 2s
read-timeout: 30s
This configuration structure is unnecessarily verbose and difficult to reason about. We can unify spring.http.reactiveclient and spring.http.client into a single spring.http.client configuration.
Proposed Solution
spring:
http:
client:
connect-timeout: 1s
read-timeout: 10s
service:
group:
service-a:
base-url: service-a:8080
connect-timeout: 2s
read-timeout: 30s
client-type: rest_client
service-b:
base-url: service-b:8080
connect-timeout: 2s
read-timeout: 30s
client-type: web_client
From a user's perspective, this unified configuration is clearer. The trade-off is that we cannot provide factory/connector configuration in properties, which is acceptable compared to the benefit of clearer configuration. Users can easily provide customization through RestClientCustomizer/WebClientCustomizer beans, making this a worthwhile trade-off.
In terms of implementation, we can merge the spring-boot-restclient and spring-boot-webclient modules into spring-boot-http-client, while keeping the spring-boot-starter-restclient and spring-boot-starter-webclient starters.
Overall, we unify the configuration for blocking and reactive clients, sacrificing the ability to configure factory/connector in exchange for more consistent and ergonomic configuration and cleaner code implementation.
Comment From: bclozel
In terms of implementation, we can merge the spring-boot-restclient and spring-boot-webclient modules into spring-boot-http-client, while keeping the spring-boot-starter-restclient and spring-boot-starter-webclient starters.
I think this means that anyone depending on "spring-boot-http-client" would get both reactive and non-reactive clients in the same application. Modularizing the Spring Boot codebase was driven partly because clients and servers were not enough separated from a user perspective because of classpath signals. Here, this would introduce a new regression. Developers expecting a RestClient would also get WebClient and the entire reactive stack that goes with it.
Overall, we unify the configuration for blocking and reactive clients, sacrificing the ability to configure factory/connector in exchange for more consistent and ergonomic configuration and cleaner code implementation.
Given my comment above, I believe that the tradeoff is clearly in favor of separation. While developer ergonomics is a core value of the project, a consistent dependency management is a must have. Bringing both reactive and traditional stacks with a single dependency is the wrong move.
Comment From: DanielLiu1123
I think this means that anyone depending on "spring-boot-http-client" would get both reactive and non-reactive clients in the same application.
No, both reactive and non-reactive clients are optional (compile-only) dependencies for spring-boot-http-client. Users still use starters:
- spring-boot-starter-restclient brings in the dependencies required by non-reactive clients.
- spring-boot-starter-webclient brings in the dependencies required by reactive clients.
First, I fully understand why boot wants to split modules, it’s a change I’ve been hoping to see for years, so I would never propose a design where a single dependency includes both reactive and non-reactive, which would obviously be wrong. (BTW, not a fan of reactive.)
Essentially, both non-reactive and reactive clients are implementing the same functionality (HTTP clients), they are just different implementations. Therefore, they should share the same “facade.” I’m not someone who overuses abstraction, but in this case, abstraction is exactly what makes sense.
Comment From: DanielLiu1123
The following configuration is totally enough, it’s simpler and easier to use:
@ConfigurationProperties("spring.http.client")
public class HttpClientProperties extends AbstractHttpClientProperties {
private Map<String, Group> groups = new LinkedHashMap<>();
public static class Group extends AbstractHttpClientProperties {
private ClientType clientType = ClientType.UNSPECIFIED;
}
}
public abstract class AbstractHttpClientProperties {
private @Nullable String baseUrl;
private Map<String, List<String>> defaultHeader = new LinkedHashMap<>();
private ApiversionProperties apiversion = new ApiversionProperties();
private @Nullable HttpRedirects redirects;
private @Nullable Duration connectTimeout;
private @Nullable Duration readTimeout;
private final Ssl ssl = new Ssl();
}
This configuration is also very easy to implement, and it allows us to remove many classes that feel redundant. - ClientHttpRequestFactorySettings / ClientHttpConnectorSettings - HttpReactiveClientSettingsProperties - ...
The current approach of separating spring.http.reactiveclient and spring.http.client feels really odd and awkward to use, it honestly gives off a bit of a code smell.
Comment From: philwebb
Thanks for the suggestion, but I don't think we should make this change for a few reasons. The main one is that the two sets of properties are for different implementations and if we try to unify them we can only ever expose the things that they have in common. As you've already identified factory/connector is one such property and one that I'd rather not sacrifice.
The other issue is that we're pushing implementation specifics into the spring-boot-http module which we'd rather not do. The client-type property you're suggesting would need to know about rest_client and web_client. Ideally spring-boot-http should not know about these concepts.
Thanks anyway for the suggestion.
Comment From: DanielLiu1123
Let me stand my ground one last time, as I believe this is the right direction. I sincerely hope the Spring Boot team can have a discussion about this issue.
As you’ve already identified, factory/connector is one such property and one that I’d rather not sacrifice.
If exposing configurations for different implementations (factory/connector) comes at the cost of doubling the configuration set, that’s not a worthwhile trade-off. Suppose Spring 10 introduces a new "UltimateHttpClient" (very fancy, very modern). We’d then need yet another configuration set, spring.http.ultimatehttpclient. This is just not right.
Ideally spring-boot-http should not know about these concepts.
spring-boot-http-client should serve as the auto-configuration module for HTTP clients. Whether the underlying implementation is RestClient, WebClient, or a future "UltimateHttpClient", it’s perfectly reasonable for this module to be aware of those implementations. Developers can simply use the starter that corresponds to their preferred client.
Comment From: philwebb
We discussed this again today and decided to look again at the properties to see if we can rationalize/simplify them. We're still not convinced about merging the modules together, but we agree there's a lot of complexity with the properties.
Comment From: philwebb
Thanks for trying the milestones and providing feedback @DanielLiu1123. I've just pushed an update that I hope helps.
Comment From: DanielLiu1123
Awesome!
I still have a few optimization suggestions:
-
Changing
spring.http.serviceclienttospring.http.clients.groupsmight be better. The configurations underspring.http.clients.groups, such asconnectTimeoutandreadTimeout, would “inherit” fromspring.http.clients. Placing them underspring.http.clientsbetter expresses that relationship. Also, thegroupsconfiguration aligns naturally with thegroupattribute in@ImportHttpServices, whereasserviceclientcould be confusing. -
Merging
HttpClientsProperties,ReactiveHttpClientsProperties, andBlockingHttpClientsPropertiesinto a singleHttpClientsProperties. Looking at the current code:spring-boot-http-clientimports bothBlockingHttpClientsPropertiesandReactiveHttpClientsProperties.spring-boot-restclientandspring-boot-webclientboth importHttpServiceClientProperties.
The issue is that all three (
HttpServiceClientProperties,BlockingHttpClientsProperties, andReactiveHttpClientsProperties) live in the same module (spring-boot-http-client), so users can already see all related configurations (spring.http.serviceclient,spring.http.clients,spring.http.clients.blocking,spring.http.clients.reactive) no matter they are usingspring-boot-starter-restclientorspring-boot-starter-webclient. Splitting them into multiple properties classes feels a bit over-engineered.If you do want to keep them separate, then
BlockingHttpClientsPropertiesshould go intospring-boot-restclient, andReactiveHttpClientsPropertiesshould go intospring-boot-webclient. That way: - When users depend onspring-boot-starter-restclient, they won’t seespring.http.clients.reactive. - When users depend onspring-boot-starter-webclient, they won’t seespring.http.clients.blocking.Still, I’d strongly recommend unifying everything under
HttpClientsProperties, it’s just much cleaner.
Comment From: vpavic
Regarding spring.http.clients.blocking and spring.http.clients.reactive, wouldn't it be more consistent with the terminology used across Spring to use imperative instead of blocking? For example, configuration properties such as spring.data.*.repositories.type whose values bind to org.springframework.boot.autoconfigure.data.RepositoryType offer imperative and reactive (among other) values.
Comment From: philwebb
Changing spring.http.serviceclient to spring.http.clients.groups might be better. The configurations under spring.http.clients.groups, such as connectTimeout and readTimeout, would “inherit” from spring.http.clients. Placing them under spring.http.clients better expresses that relationship. Also, the groups configuration aligns naturally with the group attribute in @ImportHttpServices, whereas serviceclient could be confusing.
I don't want to put configuration for an individual item under spring.http.clients as those properties are supposed to apply to all clients. I'm trying to follow the already established pattern we have (e.g. management.endpoints.... vs management.endpoint.<name>...)
I toyed with spring.http.client.service.group, but that felt a bit too long. Perhaps reintroducing spring.http.serviceclient.group might be a good idea, but it seems a little unnecessary.
I generally like the spring.http.serviceclient name as it aligns with the term "HTTP Service Client" which is used in the Spring Framework docs.
I also want to think about future expansion where we might support multiple WebClient and RestClient beans.
Those would fit naturally under spring.http.webclient.<name> and spring.http.restclient.<name>.
Merging HttpClientsProperties, ReactiveHttpClientsProperties, and BlockingHttpClientsProperties into a single HttpClientsProperties.
I very much don't want to do this. Although the spring-boot-http-client knows about both reactive and blocking clients I want to keep the package structure distinct. This follows the pattern we use elsewhere in code.
Splitting them into multiple properties classes feels a bit over-engineered.
It allows us to keep the concepts in distinct packages so I don't think it's over-engineered.
If you do want to keep them separate, then BlockingHttpClientsProperties should go into spring-boot-restclient, and ReactiveHttpClientsProperties should go into spring-boot-webclient. That way:
The properties are at a lower level than RestClient and WebClient so they shouldn't move out. If you look at the spring-boot-webservices module you'll see it uses spring-boot-http-client but doesn't know about spring-boot-restclient.
wouldn't it be more consistent with the terminology used across Spring to use imperative instead of blocking?
Yeah, we're not too happy with the name. I'll reopen to discuss.
Perhaps we can follow package structure in Spring Web and drop the prefix for blocking code. That would also reduce the over-engineering concerns
spring.http.client.factory=...
spring.http.client.reactive.connector=...
Comment From: philwebb
We've updated the term blocking to imperative.