Repository: flink Updated Branches: refs/heads/master fab61a195 -> d0cd1c7d4
[FLINK-2426] [core] Cleanup/improve unmodifiable configuration. Project: http://git-wip-us.apache.org/repos/asf/flink/repo Commit: http://git-wip-us.apache.org/repos/asf/flink/commit/5bf2197b Tree: http://git-wip-us.apache.org/repos/asf/flink/tree/5bf2197b Diff: http://git-wip-us.apache.org/repos/asf/flink/diff/5bf2197b Branch: refs/heads/master Commit: 5bf2197b148f2b9857d9fcdb5859f37ee8997100 Parents: 8eb9cbf Author: Stephan Ewen <[email protected]> Authored: Mon Aug 3 13:52:12 2015 +0200 Committer: Stephan Ewen <[email protected]> Committed: Mon Aug 3 18:48:07 2015 +0200 ---------------------------------------------------------------------- .../flink/configuration/Configuration.java | 27 ++++++-- .../UnmodifiableConfiguration.java | 70 +++++--------------- .../flink/configuration/ConfigurationTest.java | 21 +++++- .../UnmodifiableConfigurationTest.java | 67 ++++++++++++++++--- 4 files changed, 113 insertions(+), 72 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/flink/blob/5bf2197b/flink-core/src/main/java/org/apache/flink/configuration/Configuration.java ---------------------------------------------------------------------- diff --git a/flink-core/src/main/java/org/apache/flink/configuration/Configuration.java b/flink-core/src/main/java/org/apache/flink/configuration/Configuration.java index e9d7621..da42958 100644 --- a/flink-core/src/main/java/org/apache/flink/configuration/Configuration.java +++ b/flink-core/src/main/java/org/apache/flink/configuration/Configuration.java @@ -34,10 +34,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Lightweight configuration object which can store key/value pairs. + * Lightweight configuration object which stores key/value pairs. */ -@SuppressWarnings("EqualsBetweenInconvertibleTypes") -public class Configuration extends ExecutionConfig.GlobalJobParameters implements IOReadableWritable, java.io.Serializable, Cloneable { +public class Configuration extends ExecutionConfig.GlobalJobParameters + implements IOReadableWritable, java.io.Serializable, Cloneable { private static final long serialVersionUID = 1L; @@ -54,11 +54,25 @@ public class Configuration extends ExecutionConfig.GlobalJobParameters implement /** Stores the concrete key/value pairs of this configuration object. */ - private final Map<String, Object> confData = new HashMap<String, Object>(); + private final HashMap<String, Object> confData; // -------------------------------------------------------------------------------------------- - - public Configuration() {} + + /** + * Creates a new empty configuration. + */ + public Configuration() { + this.confData = new HashMap<String, Object>(); + } + + /** + * Creates a new configuration with the copy of the given configuration. + * + * @param other The configuration to copy the entries from. + */ + public Configuration(Configuration other) { + this.confData = new HashMap<String, Object>(other.confData); + } // -------------------------------------------------------------------------------------------- @@ -362,6 +376,7 @@ public class Configuration extends ExecutionConfig.GlobalJobParameters implement * The default value which is returned in case there is no value associated with the given key. * @return the (default) value associated with the given key. */ + @SuppressWarnings("EqualsBetweenInconvertibleTypes") public byte[] getBytes(String key, byte[] defaultValue) { Object o = getRawValue(key); http://git-wip-us.apache.org/repos/asf/flink/blob/5bf2197b/flink-core/src/main/java/org/apache/flink/configuration/UnmodifiableConfiguration.java ---------------------------------------------------------------------- diff --git a/flink-core/src/main/java/org/apache/flink/configuration/UnmodifiableConfiguration.java b/flink-core/src/main/java/org/apache/flink/configuration/UnmodifiableConfiguration.java index b436a53..5d92cf0 100644 --- a/flink-core/src/main/java/org/apache/flink/configuration/UnmodifiableConfiguration.java +++ b/flink-core/src/main/java/org/apache/flink/configuration/UnmodifiableConfiguration.java @@ -18,67 +18,28 @@ package org.apache.flink.configuration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** - * Unmodifiable version of the Configuration class + * Unmodifiable version of the Configuration class. */ public class UnmodifiableConfiguration extends Configuration { - - /** The log object used for debugging. */ - private static final Logger LOG = LoggerFactory.getLogger(UnmodifiableConfiguration.class); - + + private static final long serialVersionUID = -8151292629158972280L; + + /** + * Creates a new UnmodifiableConfiguration, which holds a copy of the given configuration + * that cannot be altered. + * + * @param config The configuration with the original contents. + */ public UnmodifiableConfiguration(Configuration config) { - super(); - super.addAll(config); + super(config); } // -------------------------------------------------------------------------------------------- - // All setter methods must fail. + // All mutating methods must fail // -------------------------------------------------------------------------------------------- @Override - public final void setClass(String key, Class<?> klazz) { - error(); - } - - @Override - public final void setString(String key, String value) { - error(); - } - - @Override - public final void setInteger(String key, int value) { - error(); - } - - @Override - public final void setLong(String key, long value) { - error(); - } - - @Override - public final void setBoolean(String key, boolean value) { - error(); - } - - @Override - public final void setFloat(String key, float value) { - error(); - } - - @Override - public final void setDouble(String key, double value) { - error(); - } - - @Override - public final void setBytes(String key, byte[] bytes) { - error(); - } - - @Override public final void addAll(Configuration other) { error(); } @@ -89,12 +50,11 @@ public class UnmodifiableConfiguration extends Configuration { } @Override - <T> void setValueInternal(String key, T value){ + final <T> void setValueInternal(String key, T value){ error(); } - private final void error(){ - throw new UnsupportedOperationException("The unmodifiable configuration object doesn't allow set methods."); + private void error(){ + throw new UnsupportedOperationException("The configuration is unmodifiable; its contents cannot be changed."); } - } http://git-wip-us.apache.org/repos/asf/flink/blob/5bf2197b/flink-core/src/test/java/org/apache/flink/configuration/ConfigurationTest.java ---------------------------------------------------------------------- diff --git a/flink-core/src/test/java/org/apache/flink/configuration/ConfigurationTest.java b/flink-core/src/test/java/org/apache/flink/configuration/ConfigurationTest.java index e131892..33dde3d 100644 --- a/flink-core/src/test/java/org/apache/flink/configuration/ConfigurationTest.java +++ b/flink-core/src/test/java/org/apache/flink/configuration/ConfigurationTest.java @@ -23,8 +23,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import org.apache.flink.configuration.Configuration; import org.apache.flink.core.testutils.CommonTestUtils; + import org.junit.Test; /** @@ -175,4 +175,23 @@ public class ConfigurationTest { fail(e.getMessage()); } } + + @Test + public void testCopyConstructor() { + try { + final String key = "theKey"; + + Configuration cfg1 = new Configuration(); + cfg1.setString(key, "value"); + + Configuration cfg2 = new Configuration(cfg1); + cfg2.setString(key, "another value"); + + assertEquals("value", cfg1.getString(key, "")); + } + catch (Exception e) { + e.printStackTrace(); + fail(e.getMessage()); + } + } } http://git-wip-us.apache.org/repos/asf/flink/blob/5bf2197b/flink-core/src/test/java/org/apache/flink/configuration/UnmodifiableConfigurationTest.java ---------------------------------------------------------------------- diff --git a/flink-core/src/test/java/org/apache/flink/configuration/UnmodifiableConfigurationTest.java b/flink-core/src/test/java/org/apache/flink/configuration/UnmodifiableConfigurationTest.java index 302b72b..3ea52b8 100644 --- a/flink-core/src/test/java/org/apache/flink/configuration/UnmodifiableConfigurationTest.java +++ b/flink-core/src/test/java/org/apache/flink/configuration/UnmodifiableConfigurationTest.java @@ -18,29 +18,76 @@ package org.apache.flink.configuration; - import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.junit.Test; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; /** * This class verifies that the Unmodifiable Configuration class overrides all setter methods in * Configuration. */ public class UnmodifiableConfigurationTest { - - private static Configuration pc = new Configuration(); - private static UnmodifiableConfiguration unConf = new UnmodifiableConfiguration(pc); - private static Class clazz = unConf.getClass(); - + @Test - public void testOverride() throws Exception{ - for(Method m : clazz.getMethods()){ - if(m.getName().indexOf("set") == 0 || m.getName().indexOf("add") == 0 ) { - assertEquals(clazz, m.getDeclaringClass()); + public void testOverrideAddMethods() { + try { + Class<UnmodifiableConfiguration> clazz = UnmodifiableConfiguration.class; + for (Method m : clazz.getMethods()) { + if (m.getName().startsWith("add")) { + assertEquals(clazz, m.getDeclaringClass()); + } } } + catch (Exception e) { + e.printStackTrace(); + fail(e.getMessage()); + } + } + + @Test + public void testExceptionOnSet() { + try { + Map<Class<?>, Object> parameters = new HashMap<Class<?>, Object>(); + parameters.put(byte[].class, new byte[0]); + parameters.put(Class.class, Object.class); + parameters.put(int.class, 0); + parameters.put(long.class, 0L); + parameters.put(float.class, 0.0f); + parameters.put(double.class, 0.0); + parameters.put(String.class, ""); + parameters.put(boolean.class, false); + + Class<UnmodifiableConfiguration> clazz = UnmodifiableConfiguration.class; + UnmodifiableConfiguration config = new UnmodifiableConfiguration(new Configuration()); + + for (Method m : clazz.getMethods()) { + if (m.getName().startsWith("set")) { + + Class<?> parameterClass = m.getParameterTypes()[1]; + Object parameter = parameters.get(parameterClass); + assertNotNull("method " + m + " not covered by test", parameter); + + try { + m.invoke(config, "key", parameter); + fail("should fail with an exception"); + } + catch (InvocationTargetException e) { + assertTrue(e.getTargetException() instanceof UnsupportedOperationException); + } + } + } + } + catch (Exception e) { + e.printStackTrace(); + fail(e.getMessage()); + } } }
