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 <[email protected]> Authored: Sat May 28 22:44:58 2016 -0700 Committer: Julian Hyde <[email protected]> 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
