Skip to content

Commit 7093292

Browse files
stotystoty
committed
[CALCITE-6811] Refactor deprecated httpclient API usage in Avatica
1 parent 72ce94a commit 7093292

File tree

3 files changed

+40
-47
lines changed

3 files changed

+40
-47
lines changed

‎core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,17 @@
3636
import org.apache.hc.client5.http.impl.auth.DigestSchemeFactory;
3737
import org.apache.hc.client5.http.impl.auth.SPNegoSchemeFactory;
3838
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
39-
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
4039
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
4140
import org.apache.hc.client5.http.impl.classic.HttpClients;
4241
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
4342
import org.apache.hc.client5.http.protocol.HttpClientContext;
44-
import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
43+
import org.apache.hc.client5.http.routing.RoutingSupport;
44+
import org.apache.hc.core5.http.ClassicHttpResponse;
4545
import org.apache.hc.core5.http.ContentType;
46+
import org.apache.hc.core5.http.HttpException;
47+
import org.apache.hc.core5.http.HttpHost;
4648
import org.apache.hc.core5.http.NoHttpResponseException;
4749
import org.apache.hc.core5.http.config.Lookup;
48-
import org.apache.hc.core5.http.config.Registry;
4950
import org.apache.hc.core5.http.config.RegistryBuilder;
5051
import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
5152
import org.apache.hc.core5.http.io.entity.EntityUtils;
@@ -84,9 +85,9 @@ public class AvaticaCommonsHttpClientImpl implements AvaticaHttpClient, HttpClie
8485
private static AuthScope anyAuthScope = new AuthScope(null, -1);
8586

8687
protected final URI uri;
88+
protected HttpHost httpHost;
8789
protected BasicAuthCache authCache;
8890
protected CloseableHttpClient client;
89-
protected Registry<ConnectionSocketFactory> socketFactoryRegistry;
9091
protected PoolingHttpClientConnectionManager pool;
9192

9293
protected UsernamePasswordCredentials credentials = null;
@@ -130,6 +131,8 @@ protected void initializeClient(PoolingHttpClientConnectionManager pool,
130131
private RequestConfig createRequestConfig() {
131132
RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
132133
requestConfigBuilder
134+
// We cannot avoid this. If the timeout were defined on the pool, then
135+
// it couldn't be overridden later
133136
.setConnectTimeout(this.connectTimeout, TimeUnit.MILLISECONDS)
134137
.setResponseTimeout(this.responseTimeout, TimeUnit.MILLISECONDS);
135138
List<String> preferredSchemes = new ArrayList<>();
@@ -153,12 +156,19 @@ private RequestConfig createRequestConfig() {
153156
@Override public byte[] send(byte[] request) {
154157
while (true) {
155158
ByteArrayEntity entity = new ByteArrayEntity(request, ContentType.APPLICATION_OCTET_STREAM);
156-
157-
// Create the client with the AuthSchemeRegistry and manager
158159
HttpPost post = new HttpPost(uri);
159160
post.setEntity(entity);
160161

161-
try (CloseableHttpResponse response = execute(post, context)) {
162+
if (httpHost == null) {
163+
try {
164+
httpHost = RoutingSupport.determineHost(post);
165+
} catch (HttpException e) {
166+
LOG.debug("Failed to execute HTTP request", e);
167+
throw new RuntimeException("Could not determine Http Host from URI", e);
168+
}
169+
}
170+
171+
try (ClassicHttpResponse response = executeOpen(httpHost, post, context)) {
162172
final int statusCode = response.getCode();
163173
if (HttpURLConnection.HTTP_OK == statusCode
164174
|| HttpURLConnection.HTTP_INTERNAL_ERROR == statusCode) {
@@ -185,9 +195,9 @@ private RequestConfig createRequestConfig() {
185195
}
186196

187197
// Visible for testing
188-
CloseableHttpResponse execute(HttpPost post, HttpClientContext context)
198+
ClassicHttpResponse executeOpen(HttpHost httpHost, HttpPost post, HttpClientContext context)
189199
throws IOException, ClientProtocolException {
190-
return client.execute(post, context);
200+
return client.executeOpen(httpHost, post, context);
191201
}
192202

193203
@Override public void setUsernamePassword(AuthenticationType authType, String username,

‎core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java‎

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,11 @@
2020
import org.apache.calcite.avatica.remote.HostnameVerificationConfigurable.HostnameVerification;
2121

2222
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
23-
import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
24-
import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
23+
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
24+
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
2525
import org.apache.hc.client5.http.ssl.HttpsSupport;
2626
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
27-
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
28-
import org.apache.hc.core5.http.config.Registry;
29-
import org.apache.hc.core5.http.config.RegistryBuilder;
27+
import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
3028
import org.apache.hc.core5.ssl.SSLContextBuilder;
3129
import org.apache.hc.core5.ssl.SSLContexts;
3230

@@ -71,36 +69,24 @@ public static PoolingHttpClientConnectionManager getPool(ConnectionConfig config
7169
}
7270

7371
private static PoolingHttpClientConnectionManager setupPool(ConnectionConfig config) {
74-
Registry<ConnectionSocketFactory> csfr = createCSFRegistry(config);
75-
PoolingHttpClientConnectionManager pool = new PoolingHttpClientConnectionManager(csfr);
76-
final String maxCnxns =
77-
System.getProperty(MAX_POOLED_CONNECTIONS_KEY, MAX_POOLED_CONNECTIONS_DEFAULT);
78-
pool.setMaxTotal(Integer.parseInt(maxCnxns));
79-
// Increase default max connection per route to 25
72+
final String maxCnxns = System.getProperty(MAX_POOLED_CONNECTIONS_KEY,
73+
MAX_POOLED_CONNECTIONS_DEFAULT);
8074
final String maxCnxnsPerRoute = System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
8175
MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
82-
pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
76+
PoolingHttpClientConnectionManager pool = PoolingHttpClientConnectionManagerBuilder.create()
77+
.setTlsSocketStrategy(createTlsSocketStrategy(config))
78+
.setMaxConnTotal(Integer.parseInt(maxCnxns))
79+
.setMaxConnPerRoute(Integer.parseInt(maxCnxnsPerRoute)).build();
8380
LOG.debug("Created new pool {}", pool);
8481
return pool;
8582
}
8683

87-
private static Registry<ConnectionSocketFactory> createCSFRegistry(ConnectionConfig config) {
88-
RegistryBuilder<ConnectionSocketFactory> registryBuilder = RegistryBuilder.create();
89-
configureHttpRegistry(registryBuilder);
90-
configureHttpsRegistry(registryBuilder, config);
91-
92-
return registryBuilder.build();
93-
}
94-
95-
private static void configureHttpsRegistry(
96-
RegistryBuilder<ConnectionSocketFactory> registryBuilder, ConnectionConfig config) {
84+
private static TlsSocketStrategy createTlsSocketStrategy(ConnectionConfig config) {
9785
try {
98-
SSLContext sslContext = getSSLContext(config);
99-
final HostnameVerifier verifier = getHostnameVerifier(config.hostnameVerification());
100-
SSLConnectionSocketFactory sslFactory = new SSLConnectionSocketFactory(sslContext, verifier);
101-
registryBuilder.register("https", sslFactory);
86+
return new DefaultClientTlsStrategy(getSSLContext(config),
87+
getHostnameVerifier(config.hostnameVerification()));
10288
} catch (Exception e) {
103-
LOG.error("HTTPS registry configuration failed");
89+
LOG.error("HTTPS TlsSocketStrategy configuration failed");
10490
throw new RuntimeException(e);
10591
}
10692
}
@@ -134,11 +120,6 @@ private static void loadTrustStore(SSLContextBuilder sslContextBuilder, Connecti
134120
LOG.info("Trustore loaded from: {}", config.truststore());
135121
}
136122

137-
private static void configureHttpRegistry(
138-
RegistryBuilder<ConnectionSocketFactory> registryBuilder) {
139-
registryBuilder.register("http", PlainConnectionSocketFactory.getSocketFactory());
140-
}
141-
142123
/**
143124
* Creates the {@code HostnameVerifier} given the provided {@code verification}.
144125
*

‎core/src/test/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImplTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.hc.client5.http.classic.methods.HttpPost;
2323
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
2424
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
25+
import org.apache.hc.core5.http.HttpHost;
2526
import org.apache.hc.core5.http.NoHttpResponseException;
2627
import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
2728
import org.apache.hc.core5.http.io.entity.StringEntity;
@@ -76,7 +77,7 @@ public class AvaticaCommonsHttpClientImplTest {
7677
ConnectionConfig.class));
7778

7879
doAnswer(failThenSucceed).when(client)
79-
.execute(any(HttpPost.class), eq(client.context));
80+
.executeOpen(any(HttpHost.class), any(HttpPost.class), eq(client.context));
8081

8182
when(badResponse.getCode()).thenReturn(HttpURLConnection.HTTP_UNAVAILABLE);
8283

@@ -110,7 +111,7 @@ public class AvaticaCommonsHttpClientImplTest {
110111
ConnectionConfig.class));
111112

112113
doAnswer(failThenSucceed).when(client)
113-
.execute(any(HttpPost.class), eq(client.context));
114+
.executeOpen(any(HttpHost.class), any(HttpPost.class), eq(client.context));
114115

115116
when(badResponse.getCode()).thenReturn(HttpURLConnection.HTTP_UNAVAILABLE);
116117

@@ -136,13 +137,13 @@ public void testPersistentContextReusedAcrossRequests() throws Exception {
136137
when(response.getEntity()).thenReturn(entity);
137138

138139
doReturn(response).when(client)
139-
.execute(any(HttpPost.class), eq(client.context));
140+
.executeOpen(any(HttpHost.class), any(HttpPost.class), eq(client.context));
140141

141142
client.send(new byte[0]);
142143
client.send(new byte[0]);
143144

144145
// Verify that the persistent context was reused and not created again
145-
verify(client, times(2)).execute(any(HttpPost.class),
146+
verify(client, times(2)).executeOpen(any(HttpHost.class), any(HttpPost.class),
146147
eq(client.context));
147148
}
148149

@@ -154,7 +155,7 @@ public void testPersistentContextThreadSafety() throws Exception {
154155
ConnectionConfig.class));
155156

156157
doReturn(mock(CloseableHttpResponse.class)).when(client)
157-
.execute(any(HttpPost.class), eq(client.context));
158+
.executeOpen(any(HttpHost.class), any(HttpPost.class), eq(client.context));
158159

159160
Runnable requestTask = () -> {
160161
try {
@@ -175,7 +176,8 @@ public void testPersistentContextThreadSafety() throws Exception {
175176
thread.join();
176177
}
177178

178-
verify(client, times(threadCount)).execute(any(HttpPost.class), eq(client.context));
179+
verify(client, times(threadCount)).executeOpen(any(HttpHost.class), any(HttpPost.class),
180+
eq(client.context));
179181
}
180182

181183
}

0 commit comments

Comments
 (0)