[CALCITE-5136] Elevate deprecation warnings to error and handle resul…

…tant failures. Fixes applied are: * move `new URL(...)` to `new URI(...).toUrl()` * move `Assert.assertThat(...)` to `MatcherAssert.assertThat(...)` * move `ExpectedException` to manual try-catch to please both JUnit 4.12 and 4.13 (Guava version influences resolution here) * in all other cases either propagate the deprecation or suppress it.
apache · Jan 28, 2025 · 8b1efc6 · 8b1efc6
1 parent b656437
commit 8b1efc6
Show file tree

Hide file tree

Showing 26 changed files with 129 additions and 87 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -404,6 +404,7 @@ allprojects {
 
             withType<JavaCompile>().configureEach {
                 options.encoding = "UTF-8"
+                options.compilerArgs.addAll(listOf("-Xlint:deprecation,-options", "-Werror"))
             }
             withType<Test>().configureEach {
                 testLogging {

diff --git a/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java b/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java
@@ -18,7 +18,6 @@
 
 import org.apache.calcite.avatica.ha.ShuffledRoundRobinLBStrategy;
 import org.apache.calcite.avatica.remote.AvaticaHttpClientFactoryImpl;
-import org.apache.calcite.avatica.remote.HostnameVerificationConfigurable.HostnameVerification;
 
 import org.apache.hc.core5.util.Timeout;
 
@@ -92,8 +91,13 @@ public enum BuiltInConnectionProperty implements ConnectionProperty {
   /** Password for the key inside keystore */
   KEY_PASSWORD("key_password", Type.STRING, "", false),
 
-  HOSTNAME_VERIFICATION("hostname_verification", Type.ENUM, HostnameVerification.STRICT,
-      HostnameVerification.class, false),
+  @SuppressWarnings("deprecation")
+  HOSTNAME_VERIFICATION("hostname_verification", Type.ENUM,
+      org.apache.calcite.avatica.remote.
+          HostnameVerificationConfigurable.HostnameVerification.STRICT,
+      org.apache.calcite.avatica.remote.
+          HostnameVerificationConfigurable.HostnameVerification.class,
+      false),
 
   TRANSPARENT_RECONNECTION("transparent_reconnection", Type.BOOLEAN, Boolean.FALSE, false),
 

diff --git a/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java b/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java
@@ -18,7 +18,6 @@
 
 import org.apache.calcite.avatica.ha.LBStrategy;
 import org.apache.calcite.avatica.remote.AvaticaHttpClientFactory;
-import org.apache.calcite.avatica.remote.HostnameVerificationConfigurable.HostnameVerification;
 import org.apache.calcite.avatica.remote.Service;
 
 import java.io.File;
@@ -64,7 +63,9 @@ public interface ConnectionConfig {
   /** @see BuiltInConnectionProperty#KEY_PASSWORD */
   String keyPassword();
   /** @see BuiltInConnectionProperty#HOSTNAME_VERIFICATION */
-  HostnameVerification hostnameVerification();
+  @SuppressWarnings("deprecation")
+  org.apache.calcite.avatica.remote.
+      HostnameVerificationConfigurable.HostnameVerification hostnameVerification();
   /** @see BuiltInConnectionProperty#TRANSPARENT_RECONNECTION */
   boolean transparentReconnectionEnabled();
   /** @see BuiltInConnectionProperty#FETCH_SIZE */

diff --git a/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java b/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java
@@ -18,7 +18,6 @@
 
 import org.apache.calcite.avatica.ha.LBStrategy;
 import org.apache.calcite.avatica.remote.AvaticaHttpClientFactory;
-import org.apache.calcite.avatica.remote.HostnameVerificationConfigurable.HostnameVerification;
 import org.apache.calcite.avatica.remote.Service;
 
 import java.io.File;
@@ -130,9 +129,12 @@ public String keyPassword() {
 
   }
 
-  public HostnameVerification hostnameVerification() {
+  @SuppressWarnings("deprecation")
+  public org.apache.calcite.avatica.remote.
+      HostnameVerificationConfigurable.HostnameVerification hostnameVerification() {
     return BuiltInConnectionProperty.HOSTNAME_VERIFICATION.wrap(properties)
-        .getEnum(HostnameVerification.class);
+        .getEnum(org.apache.calcite.avatica.remote.
+            HostnameVerificationConfigurable.HostnameVerification.class);
   }
 
   @Override public boolean transparentReconnectionEnabled() {

diff --git a/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java b/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImpl.java
@@ -24,8 +24,6 @@
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.Credentials;
 import org.apache.hc.client5.http.auth.CredentialsProvider;
-import org.apache.hc.client5.http.auth.KerberosConfig;
-import org.apache.hc.client5.http.auth.KerberosCredentials;
 import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
@@ -34,14 +32,12 @@
 import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
 import org.apache.hc.client5.http.impl.auth.BasicSchemeFactory;
 import org.apache.hc.client5.http.impl.auth.DigestSchemeFactory;
-import org.apache.hc.client5.http.impl.auth.SPNegoSchemeFactory;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
 import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
 import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
-import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
 import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.NoHttpResponseException;
 import org.apache.hc.core5.http.config.Lookup;
@@ -77,16 +73,20 @@ public class AvaticaCommonsHttpClientImpl implements AvaticaHttpClient, HttpClie
   private static final boolean USE_CANONICAL_HOSTNAME = Boolean
       .parseBoolean(System.getProperty("avatica.http.spnego.use_canonical_hostname", "true"));
   private static final boolean STRIP_PORT_ON_SERVER_LOOKUP = true;
-  private static final KerberosConfig KERBEROS_CONFIG =
-          KerberosConfig.custom().setStripPort(STRIP_PORT_ON_SERVER_LOOKUP)
+  @SuppressWarnings("deprecation")
+  private static final org.apache.hc.client5.http.auth.KerberosConfig KERBEROS_CONFIG =
+      org.apache.hc.client5.http.auth.KerberosConfig.custom()
+          .setStripPort(STRIP_PORT_ON_SERVER_LOOKUP)
           .setUseCanonicalHostname(USE_CANONICAL_HOSTNAME)
           .build();
   private static AuthScope anyAuthScope = new AuthScope(null, -1);
 
   protected final URI uri;
   protected BasicAuthCache authCache;
   protected CloseableHttpClient client;
-  protected Registry<ConnectionSocketFactory> socketFactoryRegistry;
+  @SuppressWarnings("deprecation")
+  protected Registry<org.apache.hc.client5.http.socket.
+      ConnectionSocketFactory> socketFactoryRegistry;
   protected PoolingHttpClientConnectionManager pool;
 
   protected UsernamePasswordCredentials credentials = null;
@@ -101,6 +101,10 @@ public AvaticaCommonsHttpClientImpl(URL url) {
     this.uri = toURI(Objects.requireNonNull(url));
   }
 
+  public AvaticaCommonsHttpClientImpl(URI uri) {
+    this.uri = uri;
+  }
+
   protected void initializeClient(PoolingHttpClientConnectionManager pool,
                                   ConnectionConfig config) {
     this.authCache = new BasicAuthCache();
@@ -127,6 +131,7 @@ protected void initializeClient(PoolingHttpClientConnectionManager pool,
   }
 
   // This is needed because we initialize the client object too early.
+  @SuppressWarnings("deprecation")
   private RequestConfig createRequestConfig() {
     RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
     requestConfigBuilder
@@ -185,6 +190,7 @@ private RequestConfig createRequestConfig() {
   }
 
   // Visible for testing
+  @SuppressWarnings("deprecation")
   CloseableHttpResponse execute(HttpPost post, HttpClientContext context)
       throws IOException, ClientProtocolException {
     return client.execute(post, context);
@@ -215,19 +221,23 @@ CloseableHttpResponse execute(HttpPost post, HttpClientContext context)
     context.setRequestConfig(createRequestConfig());
   }
 
+  @SuppressWarnings("deprecation")
   @Override public void setGSSCredential(GSSCredential credential) {
 
     this.authRegistry = RegistryBuilder.<AuthSchemeFactory>create()
         .register(StandardAuthScheme.SPNEGO,
-                new SPNegoSchemeFactory(KERBEROS_CONFIG, SystemDefaultDnsResolver.INSTANCE))
+                new org.apache.hc.client5.http.impl.auth.SPNegoSchemeFactory(
+                    KERBEROS_CONFIG,
+                    SystemDefaultDnsResolver.INSTANCE))
         .build();
 
     this.credentialsProvider = new BasicCredentialsProvider();
     if (null != credential) {
       // Non-null credential should be used directly with KerberosCredentials.
       // This is never set by the JDBC driver, nor the tests
       ((BasicCredentialsProvider) this.credentialsProvider)
-              .setCredentials(anyAuthScope, new KerberosCredentials(credential));
+              .setCredentials(anyAuthScope,
+                  new org.apache.hc.client5.http.auth.KerberosCredentials(credential));
     } else {
       // A null credential implies that the user is logged in via JAAS using the
       // java.security.auth.login.config system property
@@ -245,6 +255,7 @@ CloseableHttpResponse execute(HttpPost post, HttpClientContext context)
   private static class EmptyCredentials implements Credentials {
     public static final EmptyCredentials INSTANCE = new EmptyCredentials();
 
+    @Deprecated
     @Override public char[] getPassword() {
       return null;
     }

diff --git a/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryImpl.java b/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryImpl.java
@@ -53,8 +53,9 @@ public static AvaticaHttpClientFactoryImpl getInstance() {
     return INSTANCE;
   }
 
+  @SuppressWarnings("deprecation")
   @Override public AvaticaHttpClient getClient(URL url, ConnectionConfig config,
-      KerberosConnection kerberosUtil) {
+                                               KerberosConnection kerberosUtil) {
     String className = config.httpClientClass();
     if (null == className) {
       className = HTTP_CLIENT_IMPL_DEFAULT;

diff --git a/core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java b/core/src/main/java/org/apache/calcite/avatica/remote/CommonsHttpClientPoolCache.java
@@ -17,14 +17,10 @@
 package org.apache.calcite.avatica.remote;
 
 import org.apache.calcite.avatica.ConnectionConfig;
-import org.apache.calcite.avatica.remote.HostnameVerificationConfigurable.HostnameVerification;
 
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
-import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
-import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory;
 import org.apache.hc.client5.http.ssl.HttpsSupport;
 import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
-import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
 import org.apache.hc.core5.http.config.Registry;
 import org.apache.hc.core5.http.config.RegistryBuilder;
 import org.apache.hc.core5.ssl.SSLContextBuilder;
@@ -70,8 +66,10 @@ public static PoolingHttpClientConnectionManager getPool(ConnectionConfig config
     return CACHED_POOLS.computeIfAbsent(sslDisc, k -> setupPool(config));
   }
 
+  @SuppressWarnings("deprecation")
   private static PoolingHttpClientConnectionManager setupPool(ConnectionConfig config) {
-    Registry<ConnectionSocketFactory> csfr = createCSFRegistry(config);
+    Registry<org.apache.hc.client5.http.socket.ConnectionSocketFactory> csfr
+        = createCSFRegistry(config);
     PoolingHttpClientConnectionManager pool = new PoolingHttpClientConnectionManager(csfr);
     final String maxCnxns =
         System.getProperty(MAX_POOLED_CONNECTIONS_KEY, MAX_POOLED_CONNECTIONS_DEFAULT);
@@ -84,20 +82,26 @@ private static PoolingHttpClientConnectionManager setupPool(ConnectionConfig con
     return pool;
   }
 
-  private static Registry<ConnectionSocketFactory> createCSFRegistry(ConnectionConfig config) {
-    RegistryBuilder<ConnectionSocketFactory> registryBuilder = RegistryBuilder.create();
+  @SuppressWarnings("deprecation")
+  private static Registry<org.apache.hc.client5.http.socket.ConnectionSocketFactory>
+      createCSFRegistry(ConnectionConfig config) {
+    RegistryBuilder<org.apache.hc.client5.http.socket.ConnectionSocketFactory> registryBuilder
+        = RegistryBuilder.create();
     configureHttpRegistry(registryBuilder);
     configureHttpsRegistry(registryBuilder, config);
 
     return registryBuilder.build();
   }
 
+  @SuppressWarnings("deprecation")
   private static void configureHttpsRegistry(
-      RegistryBuilder<ConnectionSocketFactory> registryBuilder, ConnectionConfig config) {
+      RegistryBuilder<org.apache.hc.client5.http.socket.ConnectionSocketFactory> registryBuilder,
+      ConnectionConfig config) {
     try {
       SSLContext sslContext = getSSLContext(config);
       final HostnameVerifier verifier = getHostnameVerifier(config.hostnameVerification());
-      SSLConnectionSocketFactory sslFactory = new SSLConnectionSocketFactory(sslContext, verifier);
+      org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory sslFactory
+          = new org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory(sslContext, verifier);
       registryBuilder.register("https", sslFactory);
     } catch (Exception e) {
       LOG.error("HTTPS registry configuration failed");
@@ -134,9 +138,11 @@ private static void loadTrustStore(SSLContextBuilder sslContextBuilder, Connecti
     LOG.info("Trustore loaded from: {}", config.truststore());
   }
 
+  @SuppressWarnings("deprecation")
   private static void configureHttpRegistry(
-      RegistryBuilder<ConnectionSocketFactory> registryBuilder) {
-    registryBuilder.register("http", PlainConnectionSocketFactory.getSocketFactory());
+      RegistryBuilder<org.apache.hc.client5.http.socket.ConnectionSocketFactory> registryBuilder) {
+    registryBuilder.register("http",
+        org.apache.hc.client5.http.socket.PlainConnectionSocketFactory.getSocketFactory());
   }
 
   /**
@@ -147,11 +153,15 @@ private static void configureHttpRegistry(
    * @throws IllegalArgumentException if the provided verification cannot be
    *                                  handled.
    */
-  private static HostnameVerifier getHostnameVerifier(HostnameVerification verification) {
+  @SuppressWarnings("deprecation")
+  private static HostnameVerifier getHostnameVerifier(
+      org.apache.calcite.avatica.remote.
+          HostnameVerificationConfigurable.HostnameVerification verification) {
     // Normally, the configuration logic would give us a default of STRICT if it was
     // not provided by the user. It's easy for us to do a double-check.
     if (verification == null) {
-      verification = HostnameVerification.STRICT;
+      verification = org.apache.calcite.avatica.remote.
+          HostnameVerificationConfigurable.HostnameVerification.STRICT;
     }
     switch (verification) {
     case STRICT:

diff --git a/core/src/main/java/org/apache/calcite/avatica/remote/Driver.java b/core/src/main/java/org/apache/calcite/avatica/remote/Driver.java
@@ -28,6 +28,8 @@
 import org.slf4j.LoggerFactory;
 
 import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.sql.Connection;
 import java.sql.SQLException;
@@ -156,8 +158,8 @@ AvaticaHttpClient getHttpClient(AvaticaConnection connection, ConnectionConfig c
       urlStr = config.url();
     }
     try {
-      url = new URL(urlStr);
-    } catch (MalformedURLException e) {
+      url = new URI(urlStr).toURL();
+    } catch (MalformedURLException | URISyntaxException e) {
       throw new RuntimeException(e);
     }
 

diff --git a/core/src/main/java/org/apache/calcite/avatica/util/Sources.java b/core/src/main/java/org/apache/calcite/avatica/util/Sources.java
@@ -59,8 +59,8 @@ public static Source file(File baseDirectory, String fileName) {
 
   public static Source url(String url) {
     try {
-      return of(new URL(url));
-    } catch (MalformedURLException | IllegalArgumentException e) {
+      return of(new URI(url).toURL());
+    } catch (MalformedURLException | IllegalArgumentException | URISyntaxException e) {
       throw new RuntimeException("Malformed URL: '" + url + "'", e);
     }
   }
@@ -136,8 +136,7 @@ private static URL fileToUrl(File file) {
           // We need to encode path. For instance, " " should become "%20"
           // That is why java.net.URLEncoder.encode(java.lang.String, java.lang.String) is not
           // suitable because it replaces " " with "+".
-          String encodedPath = new URI(null, null, filePath, null).getRawPath();
-          return new URL("file", null, 0, encodedPath);
+          return new URI("file", null, filePath, null).toURL();
         } catch (MalformedURLException | URISyntaxException e) {
           throw new IllegalArgumentException("Unable to create URL for file " + filePath, e);
         }

diff --git a/core/src/test/java/org/apache/calcite/avatica/MetaImplCreateIterableTest.java b/core/src/test/java/org/apache/calcite/avatica/MetaImplCreateIterableTest.java
@@ -52,6 +52,7 @@ private static class MetaImplWithHardCodedResult extends MetaImpl {
       return null;
     }
 
+    @Deprecated
     @Override public ExecuteResult prepareAndExecute(StatementHandle h, String sql,
         long maxRowCount,
         PrepareCallback callback) {
@@ -87,6 +88,7 @@ private static class MetaImplWithHardCodedResult extends MetaImpl {
       return new Frame(offset, done, next);
     }
 
+    @Deprecated
     @Override public ExecuteResult execute(StatementHandle h, List<TypedValue> parameterValues,
         long maxRowCount)
         throws NoSuchStatementException {

diff --git a/core/src/test/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImplTest.java b/core/src/test/java/org/apache/calcite/avatica/remote/AvaticaCommonsHttpClientImplTest.java
@@ -32,7 +32,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.net.HttpURLConnection;
-import java.net.URL;
+import java.net.URI;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
@@ -71,7 +71,7 @@ public class AvaticaCommonsHttpClientImplTest {
     };
 
     final AvaticaCommonsHttpClientImpl client =
-            spy(new AvaticaCommonsHttpClientImpl(new URL("http://127.0.0.1")));
+            spy(new AvaticaCommonsHttpClientImpl(new URI("http://127.0.0.1")));
     client.setHttpClientPool(mock(PoolingHttpClientConnectionManager.class), mock(
         ConnectionConfig.class));
 
@@ -105,7 +105,7 @@ public class AvaticaCommonsHttpClientImplTest {
     };
 
     final AvaticaCommonsHttpClientImpl client =
-            spy(new AvaticaCommonsHttpClientImpl(new URL("http://127.0.0.1")));
+            spy(new AvaticaCommonsHttpClientImpl(new URI("http://127.0.0.1")));
     client.setHttpClientPool(mock(PoolingHttpClientConnectionManager.class), mock(
         ConnectionConfig.class));
 
@@ -124,7 +124,7 @@ public class AvaticaCommonsHttpClientImplTest {
   @Test
   public void testPersistentContextReusedAcrossRequests() throws Exception {
     final AvaticaCommonsHttpClientImpl client =
-        spy(new AvaticaCommonsHttpClientImpl(new URL("http://127.0.0.1")));
+        spy(new AvaticaCommonsHttpClientImpl(new URI("http://127.0.0.1")));
     client.setHttpClientPool(mock(PoolingHttpClientConnectionManager.class), mock(
         ConnectionConfig.class));
 
@@ -149,7 +149,7 @@ public void testPersistentContextReusedAcrossRequests() throws Exception {
   @Test
   public void testPersistentContextThreadSafety() throws Exception {
     final AvaticaCommonsHttpClientImpl client =
-        spy(new AvaticaCommonsHttpClientImpl(new URL("http://127.0.0.1")));
+        spy(new AvaticaCommonsHttpClientImpl(new URI("http://127.0.0.1")));
     client.setHttpClientPool(mock(PoolingHttpClientConnectionManager.class), mock(
         ConnectionConfig.class));