Skip to content

Commit eeb1cb5

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

File tree

3 files changed

+57
-45
lines changed

3 files changed

+57
-45
lines changed

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

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.hc.client5.http.auth.KerberosCredentials;
2929
import org.apache.hc.client5.http.auth.StandardAuthScheme;
3030
import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
31+
import org.apache.hc.client5.http.classic.methods.HttpGet;
3132
import org.apache.hc.client5.http.classic.methods.HttpPost;
3233
import org.apache.hc.client5.http.config.RequestConfig;
3334
import org.apache.hc.client5.http.impl.auth.BasicAuthCache;
@@ -36,16 +37,17 @@
3637
import org.apache.hc.client5.http.impl.auth.DigestSchemeFactory;
3738
import org.apache.hc.client5.http.impl.auth.SPNegoSchemeFactory;
3839
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
39-
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
4040
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
4141
import org.apache.hc.client5.http.impl.classic.HttpClients;
4242
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
4343
import org.apache.hc.client5.http.protocol.HttpClientContext;
44-
import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
44+
import org.apache.hc.client5.http.routing.RoutingSupport;
45+
import org.apache.hc.core5.http.ClassicHttpResponse;
4546
import org.apache.hc.core5.http.ContentType;
47+
import org.apache.hc.core5.http.HttpException;
48+
import org.apache.hc.core5.http.HttpHost;
4649
import org.apache.hc.core5.http.NoHttpResponseException;
4750
import org.apache.hc.core5.http.config.Lookup;
48-
import org.apache.hc.core5.http.config.Registry;
4951
import org.apache.hc.core5.http.config.RegistryBuilder;
5052
import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
5153
import org.apache.hc.core5.http.io.entity.EntityUtils;
@@ -84,9 +86,9 @@ public class AvaticaCommonsHttpClientImpl implements AvaticaHttpClient, HttpClie
8486
private static AuthScope anyAuthScope = new AuthScope(null, -1);
8587

8688
protected final URI uri;
89+
protected HttpHost httpHost;
8790
protected BasicAuthCache authCache;
8891
protected CloseableHttpClient client;
89-
protected Registry<ConnectionSocketFactory> socketFactoryRegistry;
9092
protected PoolingHttpClientConnectionManager pool;
9193

9294
protected UsernamePasswordCredentials credentials = null;
@@ -126,6 +128,12 @@ protected void initializeClient(PoolingHttpClientConnectionManager pool,
126128

127129
}
128130

131+
private org.apache.hc.client5.http.config.ConnectionConfig createConnectionConfig() {
132+
return org.apache.hc.client5.http.config.ConnectionConfig.custom()
133+
.setConnectTimeout(this.connectTimeout, TimeUnit.MILLISECONDS)
134+
.build();
135+
}
136+
129137
// This is needed because we initialize the client object too early.
130138
private RequestConfig createRequestConfig() {
131139
RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
@@ -151,14 +159,21 @@ private RequestConfig createRequestConfig() {
151159
}
152160

153161
@Override public byte[] send(byte[] request) {
162+
try {
163+
// Doing this earlier would break API backwards compatibility
164+
determineHost();
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+
}
154169
while (true) {
155170
ByteArrayEntity entity = new ByteArrayEntity(request, ContentType.APPLICATION_OCTET_STREAM);
156171

157172
// Create the client with the AuthSchemeRegistry and manager
158173
HttpPost post = new HttpPost(uri);
159174
post.setEntity(entity);
160175

161-
try (CloseableHttpResponse response = execute(post, context)) {
176+
try (ClassicHttpResponse response = executeOpen(httpHost, post, context)) {
162177
final int statusCode = response.getCode();
163178
if (HttpURLConnection.HTTP_OK == statusCode
164179
|| HttpURLConnection.HTTP_INTERNAL_ERROR == statusCode) {
@@ -184,10 +199,17 @@ private RequestConfig createRequestConfig() {
184199
}
185200
}
186201

202+
private void determineHost() throws HttpException {
203+
if (httpHost == null) {
204+
HttpGet dummy = new HttpGet(uri);
205+
this.httpHost = RoutingSupport.determineHost(dummy);
206+
}
207+
}
208+
187209
// Visible for testing
188-
CloseableHttpResponse execute(HttpPost post, HttpClientContext context)
210+
ClassicHttpResponse executeOpen(HttpHost httpHost, HttpPost post, HttpClientContext context)
189211
throws IOException, ClientProtocolException {
190-
return client.execute(post, context);
212+
return client.executeOpen(httpHost, post, context);
191213
}
192214

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

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

Lines changed: 20 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

@@ -37,6 +35,7 @@
3735
import java.nio.file.Files;
3836
import java.nio.file.attribute.BasicFileAttributes;
3937
import java.util.concurrent.ConcurrentHashMap;
38+
import java.util.concurrent.TimeUnit;
4039
import javax.net.ssl.HostnameVerifier;
4140
import javax.net.ssl.SSLContext;
4241

@@ -71,36 +70,30 @@ public static PoolingHttpClientConnectionManager getPool(ConnectionConfig config
7170
}
7271

7372
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
73+
final String maxCnxns = System.getProperty(MAX_POOLED_CONNECTIONS_KEY,
74+
MAX_POOLED_CONNECTIONS_DEFAULT);
8075
final String maxCnxnsPerRoute = System.getProperty(MAX_POOLED_CONNECTION_PER_ROUTE_KEY,
8176
MAX_POOLED_CONNECTION_PER_ROUTE_DEFAULT);
82-
pool.setDefaultMaxPerRoute(Integer.parseInt(maxCnxnsPerRoute));
77+
org.apache.hc.client5.http.config.ConnectionConfig connectionConfig =
78+
org.apache.hc.client5.http.config.ConnectionConfig
79+
.custom()
80+
.setConnectTimeout(config.getHttpConnectionTimeout(), TimeUnit.MILLISECONDS)
81+
.build();
82+
PoolingHttpClientConnectionManager pool = PoolingHttpClientConnectionManagerBuilder.create()
83+
.setDefaultConnectionConfig(connectionConfig)
84+
.setTlsSocketStrategy(createTlsSocketStrategy(config))
85+
.setMaxConnTotal(Integer.parseInt(maxCnxns))
86+
.setMaxConnPerRoute(Integer.parseInt(maxCnxnsPerRoute)).build();
8387
LOG.debug("Created new pool {}", pool);
8488
return pool;
8589
}
8690

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) {
91+
private static TlsSocketStrategy createTlsSocketStrategy(ConnectionConfig config) {
9792
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);
93+
return new DefaultClientTlsStrategy(getSSLContext(config),
94+
getHostnameVerifier(config.hostnameVerification()));
10295
} catch (Exception e) {
103-
LOG.error("HTTPS registry configuration failed");
96+
LOG.error("HTTPS TlsSocketStrategy configuration failed");
10497
throw new RuntimeException(e);
10598
}
10699
}
@@ -134,11 +127,6 @@ private static void loadTrustStore(SSLContextBuilder sslContextBuilder, Connecti
134127
LOG.info("Trustore loaded from: {}", config.truststore());
135128
}
136129

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

‎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)