You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by li...@apache.org on 2022/10/25 02:25:25 UTC

[calcite-avatica] branch main updated: [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields

This is an automated email from the ASF dual-hosted git repository.

libenchao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git


The following commit(s) were added to refs/heads/main by this push:
     new 644d8218e [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
644d8218e is described below

commit 644d8218eb7bd0f558955c6d0c6a84a53393a5ed
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Wed Sep 21 15:27:32 2022 -0700

    [CALCITE-5295] Read the values of plugins (such as connect string properties) from ThreadLocal fields
    
    This closes #183
---
 .../org/apache/calcite/avatica/AvaticaUtils.java   |  32 ++++--
 .../calcite/avatica/test/AvaticaUtilsTest.java     | 121 +++++++++++++++++++--
 2 files changed, 137 insertions(+), 16 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java b/core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java
index 770b8e29f..000ebd0d3 100644
--- a/core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java
+++ b/core/src/main/java/org/apache/calcite/avatica/AvaticaUtils.java
@@ -197,10 +197,18 @@ public class AvaticaUtils {
     return clazz;
   }
 
-  /** Creates an instance of a plugin class. First looks for a static
-   * member called INSTANCE, then calls a public default constructor.
+  /** Creates an instance of a plugin class.
    *
-   * <p>If className contains a "#" instead looks for a static field.
+   * <p>First looks for a static member called "{@code INSTANCE}",
+   * then calls a public default constructor.
+   *
+   * <p>If {@code className} contains a "#", instead looks for a static field.
+   *
+   * <p>In the "#" case, if the static field is a {@link ThreadLocal}, this
+   * method dereferences the {@code ThreadLocal} by calling
+   * {@link ThreadLocal#get()}. This behavior allows, for example, client code
+   * to pass an object to a JDBC driver. The JDBC driver needs to be running in
+   * the same JVM and the same thread as the client.
    *
    * @param pluginClass Class (or interface) to instantiate
    * @param className Name of implementing class
@@ -211,6 +219,7 @@ public class AvaticaUtils {
       String className) {
     String right = null;
     String left = null;
+    Object value = null;
     try {
       // Given a static field, say "com.example.MyClass#FOO_INSTANCE", return
       // the value of that static field.
@@ -222,7 +231,13 @@ public class AvaticaUtils {
         final Class<T> clazz = (Class) Class.forName(left);
         final Field field;
         field = clazz.getField(right);
-        return pluginClass.cast(field.get(null));
+        final Object fieldValue = field.get(null);
+        if (fieldValue instanceof ThreadLocal) {
+          value = ((ThreadLocal<?>) fieldValue).get();
+        } else {
+          value = fieldValue;
+        }
+        return pluginClass.cast(value);
       }
       //noinspection unchecked
       final Class<T> clazz = (Class) Class.forName(className);
@@ -230,7 +245,8 @@ public class AvaticaUtils {
         // We assume that if there is an INSTANCE field it is static and
         // has the right type.
         final Field field = clazz.getField("INSTANCE");
-        return pluginClass.cast(field.get(null));
+        value = field.get(null);
+        return pluginClass.cast(value);
       } catch (NoSuchFieldException e) {
         // ignore
       }
@@ -248,8 +264,10 @@ public class AvaticaUtils {
           + "' not valid as there is no '" + right + "' field in the class of '"
           + left + "'", e);
     } catch (ClassCastException e) {
-      throw new RuntimeException(
-          "Property '" + className + "' not valid as " + e.getMessage(), e);
+      throw new RuntimeException("Property '" + className
+          + "' not valid as cannot convert "
+          + (value == null ? "null" : value.getClass().getName())
+          + " to " + pluginClass.getCanonicalName(), e);
     } catch (NoSuchMethodException e) {
       throw new RuntimeException("Property '" + className + "' not valid as "
           + "the default constructor is necessary, "
diff --git a/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java b/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
index 3d10b2498..d57912c29 100644
--- a/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
+++ b/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
@@ -50,24 +50,33 @@ import static org.junit.Assert.fail;
  * Unit test for Avatica utilities.
  */
 public class AvaticaUtilsTest {
+  /** Used by {@link #testInstantiatePlugin()}. */
+  public static final ThreadLocal<String> STRING_THREAD_LOCAL =
+      ThreadLocal.withInitial(() -> "not initialized");
+
+  /** Also used by {@link #testInstantiatePlugin()}. */
+  public static final ThreadLocal<Float> FLOAT_THREAD_LOCAL =
+      ThreadLocal.withInitial(() -> Float.MIN_VALUE);
+
+  /** Tests {@link AvaticaUtils#instantiatePlugin(Class, String)}. */
   @Test public void testInstantiatePlugin() {
     final String s =
         AvaticaUtils.instantiatePlugin(String.class, "java.lang.String");
     assertThat(s, is(""));
 
     final BigInteger b =
-            AvaticaUtils.instantiatePlugin(BigInteger.class, "java.math.BigInteger#ONE");
+        AvaticaUtils.instantiatePlugin(BigInteger.class, "java.math.BigInteger#ONE");
     assertThat(b, is(BigInteger.ONE));
 
     // Class not found
     try {
       final BigInteger b2 =
-              AvaticaUtils.instantiatePlugin(BigInteger.class, "org.apache.calcite.Abc");
+          AvaticaUtils.instantiatePlugin(BigInteger.class, "org.apache.calcite.Abc");
       fail("expected error, got " + b2);
     } catch (Throwable e) {
       assertThat(e.getMessage(),
-              is("Property 'org.apache.calcite.Abc' not valid as "
-                      + "'org.apache.calcite.Abc' not found in the classpath"));
+          is("Property 'org.apache.calcite.Abc' not valid as "
+              + "'org.apache.calcite.Abc' not found in the classpath"));
     }
 
     // No instance of ABC
@@ -76,8 +85,8 @@ public class AvaticaUtilsTest {
       fail("expected error, got " + s2);
     } catch (Throwable e) {
       assertThat(e.getMessage(),
-              is("Property 'java.lang.String#ABC' not valid as "
-                      + "there is no 'ABC' field in the class of 'java.lang.String'"));
+          is("Property 'java.lang.String#ABC' not valid as "
+              + "there is no 'ABC' field in the class of 'java.lang.String'"));
     }
 
     // The instance type is not the plugin type
@@ -86,8 +95,8 @@ public class AvaticaUtilsTest {
       fail("expected error, got " + s2);
     } catch (Throwable e) {
       assertThat(e.getMessage(),
-              is("Property 'java.math.BigInteger#ONE' not valid as "
-                      + "Cannot cast java.math.BigInteger to java.lang.String"));
+          is("Property 'java.math.BigInteger#ONE' not valid as "
+              + "cannot convert java.math.BigInteger to java.lang.String"));
     }
 
     // No default constructor or INSTANCE member
@@ -98,7 +107,7 @@ public class AvaticaUtilsTest {
     } catch (Throwable e) {
       assertThat(e.getMessage(),
           is("Property 'java.lang.Integer' not valid as the default constructor is necessary,"
-                  + " but not found in the class of 'java.lang.Integer'"));
+              + " but not found in the class of 'java.lang.Integer'"));
     }
 
     // Not valid for plugin type
@@ -110,6 +119,100 @@ public class AvaticaUtilsTest {
       assertThat(e.getMessage(),
           is("Property 'java.lang.String' not valid for plugin type java.math.BigInteger"));
     }
+
+    // Read from thread-local
+    try {
+      STRING_THREAD_LOCAL.set("abc");
+      final String s2 =
+          AvaticaUtils.instantiatePlugin(String.class,
+              AvaticaUtilsTest.class.getName() + "#STRING_THREAD_LOCAL");
+      assertThat(s2, is("abc"));
+    } finally {
+      STRING_THREAD_LOCAL.remove();
+    }
+
+    // Read from thread-local, wrong type
+    try {
+      STRING_THREAD_LOCAL.set("abc");
+      final Integer i =
+          AvaticaUtils.instantiatePlugin(Integer.class,
+              AvaticaUtilsTest.class.getName() + "#STRING_THREAD_LOCAL");
+      fail("expected error, got " + i);
+    } catch (Throwable e) {
+      assertThat(e.getMessage(),
+          is("Property 'org.apache.calcite.avatica.test.AvaticaUtilsTest"
+              + "#STRING_THREAD_LOCAL' not valid as cannot convert java.lang.String "
+              + "to java.lang.Integer"));
+    } finally {
+      STRING_THREAD_LOCAL.remove();
+    }
+
+    // Read from thread-local, wrong type (array type, because it's tricky to
+    // print correctly).
+    try {
+      STRING_THREAD_LOCAL.set("abc");
+      final BigDecimal[] bigDecimals =
+          AvaticaUtils.instantiatePlugin(BigDecimal[].class,
+              AvaticaUtilsTest.class.getName() + "#STRING_THREAD_LOCAL");
+      fail("expected error, got " + Arrays.toString(bigDecimals));
+    } catch (Throwable e) {
+      assertThat(e.getMessage(),
+          is("Property 'org.apache.calcite.avatica.test.AvaticaUtilsTest"
+              + "#STRING_THREAD_LOCAL' not valid as cannot convert "
+              + "java.lang.String to java.math.BigDecimal[]"));
+    } finally {
+      STRING_THREAD_LOCAL.remove();
+    }
+
+    // Read from thread-local, wrong type (private enum type, because it's
+    // tricky to print correctly).
+    try {
+      STRING_THREAD_LOCAL.set("abc");
+      final Weight weight =
+          AvaticaUtils.instantiatePlugin(Weight.class,
+              AvaticaUtilsTest.class.getName() + "#STRING_THREAD_LOCAL");
+      fail("expected error, got " + weight);
+    } catch (Throwable e) {
+      assertThat(e.getMessage(),
+          is("Property 'org.apache.calcite.avatica.test.AvaticaUtilsTest"
+              + "#STRING_THREAD_LOCAL' not valid as cannot convert "
+              + "java.lang.String to "
+              + "org.apache.calcite.avatica.test.AvaticaUtilsTest.Weight"));
+    } finally {
+      STRING_THREAD_LOCAL.remove();
+    }
+
+    // Read from thread-local, wrong type (primitive type).
+    try {
+      STRING_THREAD_LOCAL.set("abc");
+      final float f =
+          AvaticaUtils.instantiatePlugin(float.class,
+              AvaticaUtilsTest.class.getName() + "#STRING_THREAD_LOCAL");
+      fail("expected error, got " + f);
+    } catch (Throwable e) {
+      assertThat(e.getMessage(),
+          is("Property 'org.apache.calcite.avatica.test.AvaticaUtilsTest"
+              + "#STRING_THREAD_LOCAL' not valid as cannot convert "
+              + "java.lang.String to float"));
+    } finally {
+      STRING_THREAD_LOCAL.remove();
+    }
+
+    // Read from thread-local, primitive type.
+    try {
+      FLOAT_THREAD_LOCAL.set(2.5f);
+      final float f =
+          AvaticaUtils.instantiatePlugin(float.class,
+              AvaticaUtilsTest.class.getName() + "#FLOAT_THREAD_LOCAL");
+      fail("expected error, got " + f);
+    } catch (Throwable e) {
+      assertThat(e.getMessage(),
+          is("Property 'org.apache.calcite.avatica.test.AvaticaUtilsTest"
+              + "#FLOAT_THREAD_LOCAL' not valid as cannot convert "
+              + "java.lang.Float to float"));
+    } finally {
+      FLOAT_THREAD_LOCAL.remove();
+    }
   }
 
   /** Unit test for