Skip to content

Inconsitent way of choosing request factory implementation for RestClient #42086

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
matsior opened this issue Sep 2, 2024 · 1 comment
Closed
Labels
status: duplicate A duplicate of another issue

Comments

@matsior
Copy link

matsior commented Sep 2, 2024

Used Spring Boot version: 3.3.2

There is inconsistency with creating org.springframework.web.client.RestClient.
When using org.springframework.web.client.RestClient#builder() method there is possibility that org.springframework.http.client.JdkClientHttpRequestFactory will be used if it's present. Code fragment from org.springframework.web.client.DefaultRestClientBuilder#build method:

@Override
	public RestClient build() {
		ClientHttpRequestFactory requestFactory = initRequestFactory();
		UriBuilderFactory uriBuilderFactory = initUriBuilderFactory();
		HttpHeaders defaultHeaders = copyDefaultHeaders();
		List<HttpMessageConverter<?>> messageConverters = (this.messageConverters != null ?
				this.messageConverters : initMessageConverters());
		return new DefaultRestClient(requestFactory,
				this.interceptors, this.initializers, uriBuilderFactory,
				defaultHeaders,
				this.defaultRequest,
				this.statusHandlers,
				messageConverters,
				this.observationRegistry,
				this.observationConvention,
				new DefaultRestClientBuilder(this)
				);
	}

	private ClientHttpRequestFactory initRequestFactory() {
		if (this.requestFactory != null) {
			return this.requestFactory;
		}
		else if (httpComponentsClientPresent) {
			return new HttpComponentsClientHttpRequestFactory();
		}
		else if (jettyClientPresent) {
			return new JettyClientHttpRequestFactory();
		}
		else if (jdkClientPresent) {
			// java.net.http module might not be loaded, so we can't default to the JDK HttpClient
			return new JdkClientHttpRequestFactory();
		}
		else {
			return new SimpleClientHttpRequestFactory();
		}
	}

So the priority of request factory looks following: HttpComponentsClientHttpRequestFactory > JettyClientHttpRequestFactory > JdkClientHttpRequestFactory > SimpleClientHttpRequestFactory.

On the other side using builder from org.springframework.boot.autoconfigure.web.client.RestClientAutoConfiguration looks different. Code snippet from org.springframework.boot.autoconfigure.web.client.RestClientAutoConfiguration#restClientBuilder

  @Bean
  @Scope("prototype")
  @ConditionalOnMissingBean
  RestClient.Builder restClientBuilder(RestClientBuilderConfigurer restClientBuilderConfigurer) {
    RestClient.Builder builder = RestClient.builder().requestFactory(ClientHttpRequestFactories.get(ClientHttpRequestFactorySettings.DEFAULTS));
    return restClientBuilderConfigurer.configure(builder);
  }

and org.springframework.boot.web.client.ClientHttpRequestFactories#get(org.springframework.boot.web.client.ClientHttpRequestFactorySettings)

  public static ClientHttpRequestFactory get(ClientHttpRequestFactorySettings settings) {
    Assert.notNull(settings, "Settings must not be null");
    if (APACHE_HTTP_CLIENT_PRESENT) {
      return ClientHttpRequestFactories.HttpComponents.get(settings);
    } else if (JETTY_CLIENT_PRESENT) {
      return ClientHttpRequestFactories.Jetty.get(settings);
    } else {
      return (ClientHttpRequestFactory)(OKHTTP_CLIENT_PRESENT ? ClientHttpRequestFactories.OkHttp.get(settings) : ClientHttpRequestFactories.Simple.get(settings));
    }
  }

In this case priority of request factory is different: HttpComponentsClientHttpRequestFactory > JettyClientHttpRequestFactory > OkHttp3ClientHttpRequestFactory > SimpleClientHttpRequestFactory.

I think it's confusing. In first example there is attempt to use JdkClientHttpRequestFactory, in second there is not. On the other side there is attempt to use OkHttp3ClientHttpRequestFactory in second example, while in first not. Are there any reasons why there are differences in choosing request factory implementation? If not, my proposition is to make it consistent for both cases.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 2, 2024
@wilkinsona
Copy link
Member

Duplicates #38856.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2024
@wilkinsona wilkinsona added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 2, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants