Revert "Revisit resource cleanup in ClientHttpRequestFactory"

Closes gh-29370
This commit is contained in:
Brian Clozel 2022-10-24 10:52:07 +02:00
parent 5e808ad018
commit 13c0c242b3
6 changed files with 27 additions and 53 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2022 the original author or authors. * Copyright 2002-2016 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,7 +16,6 @@
package org.springframework.http.client; package org.springframework.http.client;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.net.URI; import java.net.URI;
@ -30,7 +29,7 @@ import org.springframework.http.HttpMethod;
* @since 3.0 * @since 3.0
*/ */
@FunctionalInterface @FunctionalInterface
public interface ClientHttpRequestFactory extends Closeable { public interface ClientHttpRequestFactory {
/** /**
* Create a new {@link ClientHttpRequest} for the specified URI and HTTP method. * Create a new {@link ClientHttpRequest} for the specified URI and HTTP method.
@ -43,8 +42,4 @@ public interface ClientHttpRequestFactory extends Closeable {
*/ */
ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException; ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException;
@Override
default void close() throws IOException {
}
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2022 the original author or authors. * Copyright 2002-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -327,11 +327,6 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
*/ */
@Override @Override
public void destroy() throws Exception { public void destroy() throws Exception {
close();
}
@Override
public void close() throws IOException {
HttpClient httpClient = getHttpClient(); HttpClient httpClient = getHttpClient();
if (httpClient instanceof Closeable) { if (httpClient instanceof Closeable) {
((Closeable) httpClient).close(); ((Closeable) httpClient).close();

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2022 the original author or authors. * Copyright 2002-2021 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -107,11 +107,6 @@ public class OkHttp3ClientHttpRequestFactory implements ClientHttpRequestFactory
@Override @Override
public void destroy() throws IOException { public void destroy() throws IOException {
close();
}
@Override
public void close() throws IOException {
if (this.defaultClient) { if (this.defaultClient) {
// Clean up the client if we created it in the constructor // Clean up the client if we created it in the constructor
Cache cache = this.client.cache(); Cache cache = this.client.cache();
@ -123,6 +118,7 @@ public class OkHttp3ClientHttpRequestFactory implements ClientHttpRequestFactory
} }
} }
static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method) static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
throws MalformedURLException { throws MalformedURLException {

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2022 the original author or authors. * Copyright 2002-2019 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,7 +16,6 @@
package org.springframework.http.client.support; package org.springframework.http.client.support;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.util.ArrayList; import java.util.ArrayList;
@ -49,7 +48,7 @@ import org.springframework.util.Assert;
* @see ClientHttpRequestFactory * @see ClientHttpRequestFactory
* @see org.springframework.web.client.RestTemplate * @see org.springframework.web.client.RestTemplate
*/ */
public abstract class HttpAccessor implements Closeable { public abstract class HttpAccessor {
/** Logger available to subclasses. */ /** Logger available to subclasses. */
protected final Log logger = HttpLogging.forLogName(getClass()); protected final Log logger = HttpLogging.forLogName(getClass());
@ -67,7 +66,7 @@ public abstract class HttpAccessor implements Closeable {
* Configure the Apache HttpComponents or OkHttp request factory to enable PATCH.</b> * Configure the Apache HttpComponents or OkHttp request factory to enable PATCH.</b>
* @see #createRequest(URI, HttpMethod) * @see #createRequest(URI, HttpMethod)
* @see SimpleClientHttpRequestFactory * @see SimpleClientHttpRequestFactory
* @see org.springframework.http.client.HttpComponentsClientHttpRequestFactory * @see org.springframework.http.client.HttpComponentsAsyncClientHttpRequestFactory
* @see org.springframework.http.client.OkHttp3ClientHttpRequestFactory * @see org.springframework.http.client.OkHttp3ClientHttpRequestFactory
*/ */
public void setRequestFactory(ClientHttpRequestFactory requestFactory) { public void setRequestFactory(ClientHttpRequestFactory requestFactory) {
@ -134,15 +133,4 @@ public abstract class HttpAccessor implements Closeable {
this.clientHttpRequestInitializers.forEach(initializer -> initializer.initialize(request)); this.clientHttpRequestInitializers.forEach(initializer -> initializer.initialize(request));
} }
/**
* Close the underlying {@link ClientHttpRequestFactory}.
* <p>This should not be called if the factory has been {@link #setRequestFactory(ClientHttpRequestFactory) set}
* and is externally managed.
* @throws IOException if the request factory cannot be closed properly
*/
@Override
public void close() throws IOException {
this.requestFactory.close();
}
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2022 the original author or authors. * Copyright 2002-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -25,6 +25,7 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.DisposableBean;
import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.InitializingBean;
import org.springframework.http.HttpMethod; import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
@ -46,15 +47,17 @@ abstract class AbstractHttpRequestFactoryTests extends AbstractMockWebServerTest
@BeforeEach @BeforeEach
final void createFactory() throws Exception { final void createFactory() throws Exception {
this.factory = createRequestFactory(); factory = createRequestFactory();
if (this.factory instanceof InitializingBean) { if (factory instanceof InitializingBean) {
((InitializingBean) this.factory).afterPropertiesSet(); ((InitializingBean) factory).afterPropertiesSet();
} }
} }
@AfterEach @AfterEach
final void destroyFactory() throws Exception { final void destroyFactory() throws Exception {
this.factory.close(); if (factory instanceof DisposableBean) {
((DisposableBean) factory).destroy();
}
} }

View File

@ -55,14 +55,12 @@ public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpReq
@Test @Test
public void assertCustomConfig() throws Exception { public void assertCustomConfig() throws Exception {
HttpClient httpClient = HttpClientBuilder.create().build(); HttpClient httpClient = HttpClientBuilder.create().build();
HttpComponentsClientHttpRequest request; HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient);
try (HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient)) {
hrf.setConnectTimeout(1234); hrf.setConnectTimeout(1234);
hrf.setConnectionRequestTimeout(4321); hrf.setConnectionRequestTimeout(4321);
URI uri = new URI(baseUrl + "/status/ok"); URI uri = new URI(baseUrl + "/status/ok");
request = (HttpComponentsClientHttpRequest) HttpComponentsClientHttpRequest request = (HttpComponentsClientHttpRequest) hrf.createRequest(uri, HttpMethod.GET);
hrf.createRequest(uri, HttpMethod.GET);
Object config = request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG); Object config = request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG);
assertThat(config).as("Request config should be set").isNotNull(); assertThat(config).as("Request config should be set").isNotNull();
@ -71,7 +69,6 @@ public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpReq
assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(Timeout.of(1234, MILLISECONDS)); assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(Timeout.of(1234, MILLISECONDS));
assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(Timeout.of(4321, MILLISECONDS)); assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(Timeout.of(4321, MILLISECONDS));
} }
}
@Test @Test
public void defaultSettingsOfHttpClientMergedOnExecutorCustomization() throws Exception { public void defaultSettingsOfHttpClientMergedOnExecutorCustomization() throws Exception {