Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

RestClient Unit Test with @RestClientTest and MockRestServiceServer fails when using request factory JdkClientHttpRequestFactory #38832

Open
williamsuane opened this issue Dec 15, 2023 · 8 comments
Labels
theme: http-client-config Issues related to configuration of HTTP clients type: bug A general bug
Milestone

Comments

@williamsuane
Copy link

Description

When creating RestClient Bean with JdkClientHttpRequestFactory, like below

@Bean
  public RestClient restClient(RestClient.Builder builder) {
    return builder
        .requestFactory(new JdkClientHttpRequestFactory()) 
        .baseUrl(<baseUrl>)
        .build();
  }

Tests using @RestClientTest and MockRestServiceServer fail to connect.

Sample Application

https://github.com/williamsuane/failing-cat-fact/tree/master

Just execute the following test, as is

@RestClientTest({CatFactService.class})
class CatFactServiceTest {

  @Autowired
  private MockRestServiceServer server;
  @Autowired
  private CatFactService catFactService;

  @Test
  void test() {
    server.expect(MockRestRequestMatchers.requestTo("http://localhost:8080/fact"))
        .andRespond(MockRestResponseCreators.withSuccess("""
            	{"fact":"In 1987 cats overtook dogs as the number one pet in America."}
            """, MediaType.APPLICATION_JSON));

    var catFact = catFactService.getFact();
    assertEquals("In 1987 cats overtook dogs as the number one pet in America.", catFact.fact());
  }
}

it blows with the following exception

org.springframework.web.client.ResourceAccessException: I/O error on GET request for "http://localhost:8080/fact": null

	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.createResourceAccessException(DefaultRestClient.java:534)
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.exchangeInternal(DefaultRestClient.java:459)
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.retrieve(DefaultRestClient.java:424)
	at com.example.demo.CatFactService.getFact(CatFactService.java:17)
	at com.example.demo.CatFactServiceTest.test(CatFactServiceTest.java:28)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: java.net.ConnectException
	at java.net.http/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:951)
	at java.net.http/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:133)
	at org.springframework.http.client.JdkClientHttpRequest.executeInternal(JdkClientHttpRequest.java:94)
	at org.springframework.http.client.AbstractStreamingClientHttpRequest.executeInternal(AbstractStreamingClientHttpRequest.java:70)
	at org.springframework.http.client.AbstractClientHttpRequest.execute(AbstractClientHttpRequest.java:66)
	at org.springframework.web.client.DefaultRestClient$DefaultRequestBodyUriSpec.exchangeInternal(DefaultRestClient.java:453)
	... 6 more
Caused by: java.net.ConnectException
	at java.net.http/jdk.internal.net.http.common.Utils.toConnectException(Utils.java:1028)
	at java.net.http/jdk.internal.net.http.PlainHttpConnection.connectAsync(PlainHttpConnection.java:227)
	at java.net.http/jdk.internal.net.http.PlainHttpConnection.checkRetryConnect(PlainHttpConnection.java:280)
	at java.net.http/jdk.internal.net.http.PlainHttpConnection.lambda$connectAsync$2(PlainHttpConnection.java:238)
	at java.base/java.util.concurrent.CompletableFuture.uniHandle(CompletableFuture.java:934)
	at java.base/java.util.concurrent.CompletableFuture$UniHandle.tryFire(CompletableFuture.java:911)
	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run$$$capture(CompletableFuture.java:1773)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.nio.channels.ClosedChannelException
	at java.base/sun.nio.ch.SocketChannelImpl.ensureOpen(SocketChannelImpl.java:202)
	at java.base/sun.nio.ch.SocketChannelImpl.beginConnect(SocketChannelImpl.java:786)
	at java.base/sun.nio.ch.SocketChannelImpl.connect(SocketChannelImpl.java:874)
	at java.net.http/jdk.internal.net.http.PlainHttpConnection.lambda$connectAsync$1(PlainHttpConnection.java:210)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:571)
	at java.net.http/jdk.internal.net.http.PlainHttpConnection.connectAsync(PlainHttpConnection.java:212)
	... 10 more

By removing the .requestFactory(new JdkClientHttpRequestFactory()) from

@Bean
  public RestClient restClient(RestClient.Builder builder) {
    return builder
        .baseUrl(<baseUrl>)
        .build();
  }

The test works perfectly.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 15, 2023
@Washingtonwei
Copy link

Washingtonwei commented Dec 18, 2023

I am having the same problem. The requestFactory(new JdkClientHttpRequestFactory()) method replaced the original MockClientHttpRequestFactory in the injected Mock RestClient.Builder.

Here is a simple solution: just declare a bean to override the auto-configured RestClient.Builder.

@Configuration
public class RestClientBuilderConfiguration {

    @Bean
    public RestClient.Builder restClientBuilder() {
        return RestClient.builder()
                .requestFactory(new JdkClientHttpRequestFactory());
    }

}

@scottfrederick
Copy link
Contributor

Thanks for trying the new @RestClientTest support for RestClient. The MockRestServiceServer mocking works by setting the request factory to a MockClientHttpRequestFactory that keeps track of the request being made so that they can be verified later. Calling RestClient.Builder.requestFactory() directly is overriding this and breaking the mocking.

You can work around this for now by using a RestClientCustomizer instead of directly modifying the builder, something like this:

  @Bean
  public RestClient restClient(RestClient.Builder builder) {
    return builder.build();
  }

  @Bean
  public RestClientCustomizer restClientCustomizer() {
    return (restClientBuilder) -> restClientBuilder
            .requestFactory(new JdkClientHttpRequestFactory())
            .baseUrl(baseUrl);
  }

@scottfrederick scottfrederick added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 18, 2023
@scottfrederick scottfrederick added this to the 3.2.x milestone Dec 18, 2023
@Washingtonwei
Copy link

Thanks for trying the new @RestClientTest support for RestClient. The MockRestServiceServer mocking works by setting the request factory to a MockClientHttpRequestFactory that keeps track of the request being made so that they can be verified later. Calling RestClient.Builder.requestFactory() directly is overriding this and breaking the mocking.

You can work around this for now by using a RestClientCustomizer instead of directly modifying the builder, something like this:

  @Bean
  public RestClient restClient(RestClient.Builder builder) {
    return builder.build();
  }

  @Bean
  public RestClientCustomizer restClientCustomizer() {
    return (restClientBuilder) -> restClientBuilder
            .requestFactory(new JdkClientHttpRequestFactory())
            .baseUrl(baseUrl);
  }

Thank you, Scott. It works for me.

I would like to ask why not make JdkClientHttpRequestFactory the default request factory for autowired RestClient.Builder bean. In my case, the default SimpleClientHttpRequestFactory returns an empty response body.

@scottfrederick
Copy link
Contributor

I would like to ask why not make JdkClientHttpRequestFactory the default request factory for autowired RestClient.Builder

Thanks @Washingtonwei. I've created #38856 for us to take a look at the defaults for the Spring Boot auto-configuration.

@snicoll
Copy link
Member

snicoll commented Jul 17, 2024

This doesn't look like specific to RestClient and there is an issue in Spring Framework under consideration to provide a hook point to avoid this problem: spring-projects/spring-framework#32338

@LinkedList
Copy link

Hello,

I have a very similar problem but from the other angle.

I am trying to test timeouts and retries using MockRestServiceServer but because it replaces the original factory with MockClientHttpRequestFactory all the settings are lost.

Here's the example code:

@Configuration
class RestClientConfiguration {

    @Bean
    fun clientHttpRequestFactory(
        @Value("\${restClient.connectionTimeout:30}") connectionTimeout: Int,
        @Value("\${restClient.maxRetryAttempts:3}") maxRetryAttempts: Int,
        @Value("\${restClient.retryDelayMilliseconds:300}") retryDelayMilliseconds: Long
    ): ClientHttpRequestFactory {
        val retryDelay = TimeValue.ofMilliseconds(retryDelayMilliseconds)
        val client =
            HttpClients.custom().setRetryStrategy(DefaultHttpRequestRetryStrategy(maxRetryAttempts, retryDelay)).build()

        return HttpComponentsClientHttpRequestFactory(client).apply { setConnectTimeout(connectionTimeout) }
    }

    @Bean
    fun restClientBuilder(clientHttpRequestFactory: ClientHttpRequestFactory): RestClient.Builder =
        RestClient.builder().requestFactory(clientHttpRequestFactory).baseUrl("http://www.google.com")
}

And here is the setup of the MockRestServiceServer

@RunWith(SpringRunner::class)
@RestClientTest(ServiceRestClient::class)
@Import(RestClientConfiguration::class)
class ServiceRestClientTest

    @Autowired
    private lateinit var builder: RestClient.Builder

    private lateinit var client: ServiceRestClient

    private lateinit var server: MockRestServiceServer

    @Autowired lateinit var objectMapper: ObjectMapper


    @BeforeEach
    fun before() {
        val mockServer = MockRestServiceServer.bindTo(builder).build()
        server = mockServer

        client = ServiceRestClient(builder, objectMapper)
    }

...

Is there a way I can test the delays and retries using this setup?

@theMyth721
Copy link

Thanks for trying the new @RestClientTest support for RestClient. The MockRestServiceServer mocking works by setting the request factory to a MockClientHttpRequestFactory that keeps track of the request being made so that they can be verified later. Calling RestClient.Builder.requestFactory() directly is overriding this and breaking the mocking.

You can work around this for now by using a RestClientCustomizer instead of directly modifying the builder, something like this:

  @Bean
  public RestClient restClient(RestClient.Builder builder) {
    return builder.build();
  }

  @Bean
  public RestClientCustomizer restClientCustomizer() {
    return (restClientBuilder) -> restClientBuilder
            .requestFactory(new JdkClientHttpRequestFactory())
            .baseUrl(baseUrl);
  }

I have not changed any default implementations, I have two API's, one for the same JSON placeholder, and another contacting another Spring Boot server running locally.
I got the Json placeholder to work, but locally, I am getting the same error without any customisation

@Washingtonwei

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: http-client-config Issues related to configuration of HTTP clients type: bug A general bug
Projects
None yet
Development

No branches or pull requests

8 participants