Skip to content

Commit 76e075d

Browse files
philwebbphilwebb
committed
Polish "Use request factory to support Basic Authentication"
Reduce the surface area of the public API by making the `BasicAuthentication` and `BasicAuthenticationClientHttpRequestFactory` class package private. This commit also attempts to simplify `TestRestTemplate` by keeping the `RestTemplateBuilder` and reusing it, rather than needing to deal only with a `RestTemplate` instance. See gh-17010
1 parent 18a0a7a commit 76e075d

File tree

7 files changed

+128
-224
lines changed

7 files changed

+128
-224
lines changed

‎spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java

+22-79
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@
1717
package org.springframework.boot.test.web.client;
1818

1919
import java.io.IOException;
20-
import java.lang.reflect.Field;
2120
import java.net.URI;
2221
import java.util.Arrays;
2322
import java.util.HashSet;
2423
import java.util.Map;
2524
import java.util.Set;
26-
import java.util.function.Supplier;
2725

2826
import org.apache.http.client.HttpClient;
2927
import org.apache.http.client.config.CookieSpecs;
@@ -36,11 +34,6 @@
3634
import org.apache.http.protocol.HttpContext;
3735
import org.apache.http.ssl.SSLContextBuilder;
3836

39-
import org.springframework.beans.BeanInstantiationException;
40-
import org.springframework.beans.BeanUtils;
41-
import org.springframework.boot.web.client.BasicAuthentication;
42-
import org.springframework.boot.web.client.BasicAuthenticationClientHttpRequestFactory;
43-
import org.springframework.boot.web.client.ClientHttpRequestFactorySupplier;
4437
import org.springframework.boot.web.client.RestTemplateBuilder;
4538
import org.springframework.boot.web.client.RootUriTemplateHandler;
4639
import org.springframework.core.ParameterizedTypeReference;
@@ -49,13 +42,10 @@
4942
import org.springframework.http.HttpMethod;
5043
import org.springframework.http.RequestEntity;
5144
import org.springframework.http.ResponseEntity;
52-
import org.springframework.http.client.AbstractClientHttpRequestFactoryWrapper;
5345
import org.springframework.http.client.ClientHttpRequestFactory;
5446
import org.springframework.http.client.ClientHttpResponse;
5547
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
56-
import org.springframework.http.client.InterceptingClientHttpRequestFactory;
5748
import org.springframework.util.Assert;
58-
import org.springframework.util.ReflectionUtils;
5949
import org.springframework.web.client.DefaultResponseErrorHandler;
6050
import org.springframework.web.client.RequestCallback;
6151
import org.springframework.web.client.ResponseExtractor;
@@ -89,10 +79,12 @@
8979
*/
9080
public class TestRestTemplate {
9181

92-
private final RestTemplate restTemplate;
82+
private final RestTemplateBuilder builder;
9383

9484
private final HttpClientOption[] httpClientOptions;
9585

86+
private final RestTemplate restTemplate;
87+
9688
/**
9789
* Create a new {@link TestRestTemplate} instance.
9890
* @param restTemplateBuilder builder used to configure underlying
@@ -124,64 +116,30 @@ public TestRestTemplate(String username, String password,
124116

125117
/**
126118
* Create a new {@link TestRestTemplate} instance with the specified credentials.
127-
* @param restTemplateBuilder builder used to configure underlying
128-
* {@link RestTemplate}
119+
* @param builder builder used to configure underlying {@link RestTemplate}
129120
* @param username the username to use (or {@code null})
130121
* @param password the password (or {@code null})
131122
* @param httpClientOptions client options to use if the Apache HTTP Client is used
132123
* @since 2.0.0
133124
*/
134-
public TestRestTemplate(RestTemplateBuilder restTemplateBuilder, String username,
135-
String password, HttpClientOption... httpClientOptions) {
136-
this((restTemplateBuilder != null) ? restTemplateBuilder.build() : null, username,
137-
password, httpClientOptions);
138-
}
139-
140-
private TestRestTemplate(RestTemplate restTemplate, String username, String password,
125+
public TestRestTemplate(RestTemplateBuilder builder, String username, String password,
141126
HttpClientOption... httpClientOptions) {
142-
Assert.notNull(restTemplate, "RestTemplate must not be null");
127+
Assert.notNull(builder, "Builder must not be null");
128+
this.builder = builder;
143129
this.httpClientOptions = httpClientOptions;
144-
if (getRequestFactoryClass(restTemplate)
145-
.isAssignableFrom(HttpComponentsClientHttpRequestFactory.class)) {
146-
restTemplate.setRequestFactory(
147-
new CustomHttpComponentsClientHttpRequestFactory(httpClientOptions));
148-
}
149-
addAuthentication(restTemplate, username, password);
150-
restTemplate.setErrorHandler(new NoOpResponseErrorHandler());
151-
this.restTemplate = restTemplate;
152-
}
153-
154-
private Class<? extends ClientHttpRequestFactory> getRequestFactoryClass(
155-
RestTemplate restTemplate) {
156-
return getRequestFactory(restTemplate).getClass();
157-
}
158-
159-
private ClientHttpRequestFactory getRequestFactory(RestTemplate restTemplate) {
160-
ClientHttpRequestFactory requestFactory = restTemplate.getRequestFactory();
161-
while (requestFactory instanceof InterceptingClientHttpRequestFactory
162-
|| requestFactory instanceof BasicAuthenticationClientHttpRequestFactory) {
163-
requestFactory = unwrapRequestFactory(
164-
((AbstractClientHttpRequestFactoryWrapper) requestFactory));
130+
if (httpClientOptions != null) {
131+
ClientHttpRequestFactory requestFactory = builder.buildRequestFactory();
132+
if (requestFactory instanceof HttpComponentsClientHttpRequestFactory) {
133+
builder = builder.requestFactory(
134+
() -> new CustomHttpComponentsClientHttpRequestFactory(
135+
httpClientOptions));
136+
}
165137
}
166-
return requestFactory;
167-
}
168-
169-
private ClientHttpRequestFactory unwrapRequestFactory(
170-
AbstractClientHttpRequestFactoryWrapper requestFactory) {
171-
Field field = ReflectionUtils.findField(
172-
AbstractClientHttpRequestFactoryWrapper.class, "requestFactory");
173-
ReflectionUtils.makeAccessible(field);
174-
return (ClientHttpRequestFactory) ReflectionUtils.getField(field, requestFactory);
175-
}
176-
177-
private void addAuthentication(RestTemplate restTemplate, String username,
178-
String password) {
179-
if (username == null || password == null) {
180-
return;
138+
if (username != null || password != null) {
139+
builder = builder.basicAuthentication(username, password);
181140
}
182-
ClientHttpRequestFactory requestFactory = getRequestFactory(restTemplate);
183-
restTemplate.setRequestFactory(new BasicAuthenticationClientHttpRequestFactory(
184-
new BasicAuthentication(username, password), requestFactory));
141+
this.restTemplate = builder.build();
142+
this.restTemplate.setErrorHandler(new NoOpResponseErrorHandler());
185143
}
186144

187145
/**
@@ -1038,25 +996,10 @@ public RestTemplate getRestTemplate() {
1038996
* @since 1.4.1
1039997
*/
1040998
public TestRestTemplate withBasicAuth(String username, String password) {
1041-
RestTemplate restTemplate = new RestTemplateBuilder()
1042-
.requestFactory(getRequestFactorySupplier())
1043-
.messageConverters(getRestTemplate().getMessageConverters())
1044-
.interceptors(getRestTemplate().getInterceptors())
1045-
.uriTemplateHandler(getRestTemplate().getUriTemplateHandler()).build();
1046-
return new TestRestTemplate(restTemplate, username, password,
999+
TestRestTemplate template = new TestRestTemplate(this.builder, username, password,
10471000
this.httpClientOptions);
1048-
}
1049-
1050-
private Supplier<ClientHttpRequestFactory> getRequestFactorySupplier() {
1051-
return () -> {
1052-
try {
1053-
return BeanUtils
1054-
.instantiateClass(getRequestFactoryClass(getRestTemplate()));
1055-
}
1056-
catch (BeanInstantiationException ex) {
1057-
return new ClientHttpRequestFactorySupplier().get();
1058-
}
1059-
};
1001+
template.setUriTemplateHandler(getRestTemplate().getUriTemplateHandler());
1002+
return template;
10601003
}
10611004

10621005
@SuppressWarnings({ "rawtypes", "unchecked" })
@@ -1078,7 +1021,7 @@ private URI applyRootUriIfNecessary(URI uri) {
10781021
}
10791022

10801023
/**
1081-
* Options used to customize the Apache Http Client if it is used.
1024+
* Options used to customize the Apache HTTP Client.
10821025
*/
10831026
public enum HttpClientOption {
10841027

‎spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java

+27-42
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@
2020
import java.lang.reflect.Method;
2121
import java.lang.reflect.Modifier;
2222
import java.net.URI;
23+
import java.util.List;
24+
import java.util.stream.Collectors;
2325

2426
import org.apache.http.client.config.RequestConfig;
2527
import org.junit.jupiter.api.Test;
2628

2729
import org.springframework.boot.test.web.client.TestRestTemplate.CustomHttpComponentsClientHttpRequestFactory;
2830
import org.springframework.boot.test.web.client.TestRestTemplate.HttpClientOption;
29-
import org.springframework.boot.web.client.BasicAuthenticationClientHttpRequestFactory;
3031
import org.springframework.boot.web.client.RestTemplateBuilder;
3132
import org.springframework.core.ParameterizedTypeReference;
3233
import org.springframework.http.HttpEntity;
@@ -99,25 +100,11 @@ public void useTheSameRequestFactoryClassWithBasicAuth() {
99100
TestRestTemplate testRestTemplate = new TestRestTemplate(builder)
100101
.withBasicAuth("test", "test");
101102
RestTemplate restTemplate = testRestTemplate.getRestTemplate();
103+
assertThat(restTemplate.getRequestFactory().getClass().getName())
104+
.contains("BasicAuth");
102105
Object requestFactory = ReflectionTestUtils
103106
.getField(restTemplate.getRequestFactory(), "requestFactory");
104-
assertThat(requestFactory).isNotEqualTo(customFactory)
105-
.hasSameClassAs(customFactory);
106-
}
107-
108-
@Test
109-
public void withBasicAuthWhenRequestFactoryTypeCannotBeInstantiatedShouldFallback() {
110-
TestClientHttpRequestFactory customFactory = new TestClientHttpRequestFactory(
111-
"my-request-factory");
112-
RestTemplateBuilder builder = new RestTemplateBuilder()
113-
.requestFactory(() -> customFactory);
114-
TestRestTemplate testRestTemplate = new TestRestTemplate(builder)
115-
.withBasicAuth("test", "test");
116-
RestTemplate restTemplate = testRestTemplate.getRestTemplate();
117-
Object requestFactory = ReflectionTestUtils
118-
.getField(restTemplate.getRequestFactory(), "requestFactory");
119-
assertThat(requestFactory).isNotEqualTo(customFactory)
120-
.isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class);
107+
assertThat(requestFactory).isEqualTo(customFactory).hasSameClassAs(customFactory);
121108
}
122109

123110
@Test
@@ -145,9 +132,10 @@ public void getRootUriRootUriNotSet() {
145132

146133
@Test
147134
public void authenticated() {
148-
assertThat(new TestRestTemplate("user", "password").getRestTemplate()
149-
.getRequestFactory())
150-
.isInstanceOf(BasicAuthenticationClientHttpRequestFactory.class);
135+
RestTemplate restTemplate = new TestRestTemplate("user", "password")
136+
.getRestTemplate();
137+
ClientHttpRequestFactory factory = restTemplate.getRequestFactory();
138+
assertThat(factory.getClass().getName()).contains("BasicAuthentication");
151139
}
152140

153141
@Test
@@ -225,43 +213,40 @@ private Object mockArgument(Class<?> type) throws Exception {
225213

226214
@Test
227215
public void withBasicAuthAddsBasicAuthClientFactoryWhenNotAlreadyPresent() {
228-
TestRestTemplate originalTemplate = new TestRestTemplate();
229-
TestRestTemplate basicAuthTemplate = originalTemplate.withBasicAuth("user",
230-
"password");
231-
assertThat(basicAuthTemplate.getRestTemplate().getMessageConverters())
232-
.containsExactlyElementsOf(
233-
originalTemplate.getRestTemplate().getMessageConverters());
234-
assertThat(basicAuthTemplate.getRestTemplate().getRequestFactory())
235-
.isInstanceOf(BasicAuthenticationClientHttpRequestFactory.class);
216+
TestRestTemplate original = new TestRestTemplate();
217+
TestRestTemplate basicAuth = original.withBasicAuth("user", "password");
218+
assertThat(getConverterClasses(original))
219+
.containsExactlyElementsOf(getConverterClasses(basicAuth));
220+
assertThat(basicAuth.getRestTemplate().getRequestFactory().getClass().getName())
221+
.contains("BasicAuth");
236222
assertThat(ReflectionTestUtils.getField(
237-
basicAuthTemplate.getRestTemplate().getRequestFactory(),
238-
"requestFactory"))
223+
basicAuth.getRestTemplate().getRequestFactory(), "requestFactory"))
239224
.isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class);
240-
assertThat(basicAuthTemplate.getRestTemplate().getUriTemplateHandler())
241-
.isSameAs(originalTemplate.getRestTemplate().getUriTemplateHandler());
242-
assertThat(basicAuthTemplate.getRestTemplate().getInterceptors()).isEmpty();
243-
assertBasicAuthorizationCredentials(basicAuthTemplate, "user", "password");
225+
assertThat(basicAuth.getRestTemplate().getInterceptors()).isEmpty();
226+
assertBasicAuthorizationCredentials(basicAuth, "user", "password");
244227
}
245228

246229
@Test
247230
public void withBasicAuthReplacesBasicAuthClientFactoryWhenAlreadyPresent() {
248231
TestRestTemplate original = new TestRestTemplate("foo", "bar")
249232
.withBasicAuth("replace", "replace");
250233
TestRestTemplate basicAuth = original.withBasicAuth("user", "password");
251-
assertThat(basicAuth.getRestTemplate().getMessageConverters())
252-
.containsExactlyElementsOf(
253-
original.getRestTemplate().getMessageConverters());
254-
assertThat(basicAuth.getRestTemplate().getRequestFactory())
255-
.isInstanceOf(BasicAuthenticationClientHttpRequestFactory.class);
234+
assertThat(getConverterClasses(basicAuth))
235+
.containsExactlyElementsOf(getConverterClasses(original));
236+
assertThat(basicAuth.getRestTemplate().getRequestFactory().getClass().getName())
237+
.contains("BasicAuth");
256238
assertThat(ReflectionTestUtils.getField(
257239
basicAuth.getRestTemplate().getRequestFactory(), "requestFactory"))
258240
.isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class);
259-
assertThat(basicAuth.getRestTemplate().getUriTemplateHandler())
260-
.isSameAs(original.getRestTemplate().getUriTemplateHandler());
261241
assertThat(basicAuth.getRestTemplate().getInterceptors()).isEmpty();
262242
assertBasicAuthorizationCredentials(basicAuth, "user", "password");
263243
}
264244

245+
private List<Class<?>> getConverterClasses(TestRestTemplate testRestTemplate) {
246+
return testRestTemplate.getRestTemplate().getMessageConverters().stream()
247+
.map(Object::getClass).collect(Collectors.toList());
248+
}
249+
265250
@Test
266251
public void withBasicAuthShouldUseNoOpErrorHandler() throws Exception {
267252
TestRestTemplate originalTemplate = new TestRestTemplate("foo", "bar");

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/BasicAuthentication.java

+9-40
Original file line numberDiff line numberDiff line change
@@ -18,69 +18,38 @@
1818

1919
import java.nio.charset.Charset;
2020

21+
import org.springframework.http.HttpHeaders;
22+
import org.springframework.http.client.ClientHttpRequest;
2123
import org.springframework.util.Assert;
2224

2325
/**
2426
* Basic authentication properties which are used by
2527
* {@link BasicAuthenticationClientHttpRequestFactory}.
2628
*
2729
* @author Dmytro Nosan
28-
* @since 2.2.0
2930
* @see BasicAuthenticationClientHttpRequestFactory
3031
*/
31-
public class BasicAuthentication {
32+
class BasicAuthentication {
3233

3334
private final String username;
3435

3536
private final String password;
3637

3738
private final Charset charset;
3839

39-
/**
40-
* Create a new {@link BasicAuthentication}.
41-
* @param username the username to use
42-
* @param password the password to use
43-
*/
44-
public BasicAuthentication(String username, String password) {
45-
this(username, password, null);
46-
}
47-
48-
/**
49-
* Create a new {@link BasicAuthentication}.
50-
* @param username the username to use
51-
* @param password the password to use
52-
* @param charset the charset to use
53-
*/
54-
public BasicAuthentication(String username, String password, Charset charset) {
40+
BasicAuthentication(String username, String password, Charset charset) {
5541
Assert.notNull(username, "Username must not be null");
5642
Assert.notNull(password, "Password must not be null");
5743
this.username = username;
5844
this.password = password;
5945
this.charset = charset;
6046
}
6147

62-
/**
63-
* The username to use.
64-
* @return the username, never {@code null} or {@code empty}.
65-
*/
66-
public String getUsername() {
67-
return this.username;
68-
}
69-
70-
/**
71-
* The password to use.
72-
* @return the password, never {@code null} or {@code empty}.
73-
*/
74-
public String getPassword() {
75-
return this.password;
76-
}
77-
78-
/**
79-
* The charset to use.
80-
* @return the charset, or {@code null}.
81-
*/
82-
public Charset getCharset() {
83-
return this.charset;
48+
void applyTo(ClientHttpRequest request) {
49+
HttpHeaders headers = request.getHeaders();
50+
if (!headers.containsKey(HttpHeaders.AUTHORIZATION)) {
51+
headers.setBasicAuth(this.username, this.password, this.charset);
52+
}
8453
}
8554

8655
}

0 commit comments

Comments
 (0)