You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2016/06/15 21:36:12 UTC

calcite git commit: [CALCITE-1263] Case-insensitive match and null default value for enum properties

Repository: calcite
Updated Branches:
  refs/heads/master eb291b029 -> 240cee445


[CALCITE-1263] Case-insensitive match and null default value for enum properties


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/240cee44
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/240cee44
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/240cee44

Branch: refs/heads/master
Commit: 240cee44579e25f87ac5de0c2e676278ed474e5b
Parents: eb291b0
Author: Julian Hyde <jh...@apache.org>
Authored: Sat May 28 22:44:58 2016 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Wed Jun 15 13:51:37 2016 -0700

----------------------------------------------------------------------
 .../avatica/BuiltInConnectionProperty.java      | 13 +++-
 .../calcite/avatica/ConnectionConfig.java       |  2 +
 .../calcite/avatica/ConnectionConfigImpl.java   | 29 ++++++++-
 .../calcite/avatica/ConnectionProperty.java     | 39 ++++++++++--
 .../remote/AvaticaRemoteConnectionProperty.java |  8 ++-
 .../calcite/avatica/test/AvaticaUtilsTest.java  | 64 ++++++++++++++++++++
 6 files changed, 144 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/240cee44/avatica/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java b/avatica/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java
index 0533e7b..c408f24 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/BuiltInConnectionProperty.java
@@ -71,6 +71,7 @@ public enum BuiltInConnectionProperty implements ConnectionProperty {
   private final String camelName;
   private final Type type;
   private final Object defaultValue;
+  private Class valueClass;
   private final boolean required;
 
   /** Deprecated; use {@link #TIME_ZONE}. */
@@ -95,11 +96,17 @@ public enum BuiltInConnectionProperty implements ConnectionProperty {
 
   BuiltInConnectionProperty(String camelName, Type type, Object defaultValue,
       boolean required) {
+    this(camelName, type, defaultValue, type.defaultValueClass(), required);
+  }
+
+  BuiltInConnectionProperty(String camelName, Type type, Object defaultValue,
+      Class valueClass, boolean required) {
     this.camelName = camelName;
     this.type = type;
     this.defaultValue = defaultValue;
+    this.valueClass = valueClass;
     this.required = required;
-    assert defaultValue == null || type.valid(defaultValue);
+    assert type.valid(defaultValue, valueClass);
   }
 
   public String camelName() {
@@ -118,6 +125,10 @@ public enum BuiltInConnectionProperty implements ConnectionProperty {
     return required;
   }
 
+  public Class valueClass() {
+    return valueClass;
+  }
+
   public PropEnv wrap(Properties properties) {
     return new PropEnv(parse(properties, NAME_TO_PROPS), this);
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/240cee44/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java b/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java
index cd48243..032fafc 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfig.java
@@ -41,7 +41,9 @@ public interface ConnectionConfig {
   String avaticaUser();
   /** @see BuiltInConnectionProperty#AVATICA_PASSWORD */
   String avaticaPassword();
+  /** @see BuiltInConnectionProperty#HTTP_CLIENT_FACTORY */
   AvaticaHttpClientFactory httpClientFactory();
+  /** @see BuiltInConnectionProperty#HTTP_CLIENT_IMPL */
   String httpClientClass();
   /** @see BuiltInConnectionProperty#PRINCIPAL */
   String kerberosPrincipal();

http://git-wip-us.apache.org/repos/asf/calcite/blob/240cee44/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java b/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java
index ee47ebe..9ef1e66 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionConfigImpl.java
@@ -139,6 +139,14 @@ public class ConnectionConfigImpl implements ConnectionConfig {
       return converter.apply(property, defaultValue);
     }
 
+    private <T> T getDefaultNull(Converter<T> converter) {
+      final String s = map.get(property);
+      if (s != null) {
+        return converter.apply(property, s);
+      }
+      return null;
+    }
+
     /** Returns the string value of this property, or null if not specified and
      * no default. */
     public String getString() {
@@ -214,9 +222,18 @@ public class ConnectionConfigImpl implements ConnectionConfig {
     /** Returns the enum value of this property. Throws if not set and no
      * default. */
     public <E extends Enum<E>> E getEnum(Class<E> enumClass, E defaultValue) {
-      assert property.type() == ConnectionProperty.Type.ENUM;
-      //noinspection unchecked
-      return get_(enumConverter(enumClass), defaultValue.name());
+      if (property.type() != ConnectionProperty.Type.ENUM) {
+        throw new AssertionError("not an enum");
+      }
+      if (enumClass != property.valueClass()) {
+        throw new AssertionError("wrong value class; expected "
+            + property.valueClass());
+      }
+      if (defaultValue == null) {
+        return getDefaultNull(enumConverter(enumClass));
+      } else {
+        return get_(enumConverter(enumClass), defaultValue.name());
+      }
     }
 
     /** Returns an instance of a plugin.
@@ -308,6 +325,12 @@ public class ConnectionConfigImpl implements ConnectionConfig {
         try {
           return (E) Enum.valueOf(enumClass, s);
         } catch (IllegalArgumentException e) {
+          // Case insensitive match is OK too.
+          for (E c : enumClass.getEnumConstants()) {
+            if (c.name().equalsIgnoreCase(s)) {
+              return c;
+            }
+          }
           throw new RuntimeException("Property '" + s + "' not valid for enum "
               + enumClass.getName());
         }

http://git-wip-us.apache.org/repos/asf/calcite/blob/240cee44/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionProperty.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionProperty.java b/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionProperty.java
index 4f14d5f..b21b862 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionProperty.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/ConnectionProperty.java
@@ -45,6 +45,10 @@ public interface ConnectionProperty {
   /** Whether the property is mandatory. */
   boolean required();
 
+  /** Class of values that this property can take. Most useful for
+   * {@link Type#ENUM} properties. */
+  Class valueClass();
+
   /** Data type of property. */
   enum Type {
     BOOLEAN,
@@ -53,17 +57,42 @@ public interface ConnectionProperty {
     ENUM,
     PLUGIN;
 
-    public boolean valid(Object defaultValue) {
+    /** Returns whether a default value and value types are valid for this
+     * kind of property. */
+    public boolean valid(Object defaultValue, Class clazz) {
+      switch (this) {
+      case BOOLEAN:
+        return clazz == Boolean.class
+            && (defaultValue == null || defaultValue instanceof Boolean);
+      case NUMBER:
+        return Number.class.isAssignableFrom(clazz)
+            && (defaultValue == null || defaultValue instanceof Number);
+      case STRING:
+        return clazz == String.class
+            && (defaultValue == null || defaultValue instanceof String);
+      case PLUGIN:
+        return clazz != null
+            && (defaultValue == null || defaultValue instanceof String);
+      case ENUM:
+        return Enum.class.isAssignableFrom(clazz)
+            && (defaultValue == null || clazz.isInstance(defaultValue));
+      default:
+        throw new AssertionError();
+      }
+    }
+
+    public Class defaultValueClass() {
       switch (this) {
       case BOOLEAN:
-        return defaultValue instanceof Boolean;
+        return Boolean.class;
       case NUMBER:
-        return defaultValue instanceof Number;
+        return Number.class;
       case STRING:
+        return String.class;
       case PLUGIN:
-        return defaultValue instanceof String;
+        return Object.class;
       default:
-        return defaultValue instanceof Enum;
+        throw new AssertionError("must specify value class for an ENUM");
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/240cee44/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaRemoteConnectionProperty.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaRemoteConnectionProperty.java b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaRemoteConnectionProperty.java
index 5e755f8..e0d9bb1 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaRemoteConnectionProperty.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaRemoteConnectionProperty.java
@@ -39,7 +39,7 @@ public enum AvaticaRemoteConnectionProperty implements ConnectionProperty {
   private static final Map<String, AvaticaRemoteConnectionProperty> NAME_TO_PROPS;
 
   static {
-    NAME_TO_PROPS = new HashMap<String, AvaticaRemoteConnectionProperty>();
+    NAME_TO_PROPS = new HashMap<>();
     for (AvaticaRemoteConnectionProperty p
         : AvaticaRemoteConnectionProperty.values()) {
       NAME_TO_PROPS.put(p.camelName.toUpperCase(), p);
@@ -53,7 +53,7 @@ public enum AvaticaRemoteConnectionProperty implements ConnectionProperty {
     this.camelName = camelName;
     this.type = type;
     this.defaultValue = defaultValue;
-    assert defaultValue == null || type.valid(defaultValue);
+    assert type.valid(defaultValue, type.defaultValueClass());
   }
 
   public String camelName() {
@@ -68,6 +68,10 @@ public enum AvaticaRemoteConnectionProperty implements ConnectionProperty {
     return type;
   }
 
+  public Class valueClass() {
+    return type.defaultValueClass();
+  }
+
   public PropEnv wrap(Properties properties) {
     return new PropEnv(parse(properties, NAME_TO_PROPS), this);
   }

http://git-wip-us.apache.org/repos/asf/calcite/blob/240cee44/avatica/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java b/avatica/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
index 8905508..5d653d6 100644
--- a/avatica/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
+++ b/avatica/core/src/test/java/org/apache/calcite/avatica/test/AvaticaUtilsTest.java
@@ -30,6 +30,7 @@ import java.util.Properties;
 import java.util.Set;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
@@ -147,6 +148,46 @@ public class AvaticaUtilsTest {
     properties.setProperty(s.name, "  ");
     env = s.wrap(properties);
     assertThat(env.getString(), is("  "));
+
+    try {
+      final ConnectionPropertyImpl t =
+          new ConnectionPropertyImpl("T", null, ConnectionProperty.Type.ENUM);
+      fail("should throw if you specify an enum property without a class, got "
+          + t);
+    } catch (AssertionError e) {
+      assertThat(e.getMessage(), is("must specify value class for an ENUM"));
+      // ok
+    }
+
+    // An enum with a default value
+    final ConnectionPropertyImpl t = new ConnectionPropertyImpl("T", Size.BIG,
+        ConnectionProperty.Type.ENUM, Size.class);
+    env = t.wrap(properties);
+    assertThat(env.getEnum(Size.class), is(Size.BIG));
+    assertThat(env.getEnum(Size.class, Size.SMALL), is(Size.SMALL));
+    assertThat(env.getEnum(Size.class, null), nullValue());
+    try {
+      final Weight envEnum = env.getEnum(Weight.class, null);
+      fail("expected error, got " + envEnum);
+    } catch (AssertionError e) {
+      assertThat(e.getMessage(),
+          is("wrong value class; expected " + Size.class));
+    }
+
+    // An enum with no default value
+    final ConnectionPropertyImpl u = new ConnectionPropertyImpl("U", null,
+        ConnectionProperty.Type.ENUM, Size.class);
+    env = u.wrap(properties);
+    assertThat(env.getEnum(Size.class), nullValue());
+    assertThat(env.getEnum(Size.class, Size.SMALL), is(Size.SMALL));
+    assertThat(env.getEnum(Size.class, null), nullValue());
+    try {
+      final Weight envEnum = env.getEnum(Weight.class, null);
+      fail("expected error, got " + envEnum);
+    } catch (AssertionError e) {
+      assertThat(e.getMessage(),
+          is("wrong value class; expected " + Size.class));
+    }
   }
 
   @Test public void testLongToIntegerTranslation() {
@@ -162,12 +203,22 @@ public class AvaticaUtilsTest {
   private static class ConnectionPropertyImpl implements ConnectionProperty {
     private final String name;
     private final Object defaultValue;
+    private final Class<?> valueClass;
     private Type type;
 
     ConnectionPropertyImpl(String name, Object defaultValue, Type type) {
+      this(name, defaultValue, type, type.defaultValueClass());
+    }
+
+    ConnectionPropertyImpl(String name, Object defaultValue, Type type,
+        Class valueClass) {
       this.name = name;
       this.defaultValue = defaultValue;
       this.type = type;
+      this.valueClass = valueClass;
+      if (!type.valid(defaultValue, valueClass)) {
+        throw new AssertionError(name);
+      }
     }
 
     public String name() {
@@ -186,6 +237,10 @@ public class AvaticaUtilsTest {
       return type;
     }
 
+    public Class valueClass() {
+      return valueClass;
+    }
+
     public ConnectionConfigImpl.PropEnv wrap(Properties properties) {
       final HashMap<String, ConnectionProperty> map = new HashMap<>();
       map.put(name, this);
@@ -198,6 +253,15 @@ public class AvaticaUtilsTest {
     }
   }
 
+  /** How large? */
+  private enum Size {
+    BIG, SMALL
+  }
+
+  /** How heavy? */
+  private enum Weight {
+    HEAVY, LIGHT
+  }
 }
 
 // End AvaticaUtilsTest.java