spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.41k stars 40.51k forks source link

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

Open williamsuane opened 8 months ago

williamsuane commented 8 months ago

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.

Washingtonwei commented 8 months ago

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 commented 8 months ago

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);
  }
Washingtonwei commented 8 months ago

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 commented 8 months ago

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 commented 1 month ago

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: https://github.com/spring-projects/spring-framework/issues/32338

LinkedList commented 1 month ago

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?