Repository: reef Updated Branches: refs/heads/master cfdb2bf5d -> cf2f146a1
[REEF-967] Handle null in RequiredImpl This addressed the issue by * throwing IllegalStateException * when null is passed as Required Implementation JIRA: [REEF-967] (https://issues.apache.org/jira/browse/REEF-967) Pull Request: This closes #1079 Project: http://git-wip-us.apache.org/repos/asf/reef/repo Commit: http://git-wip-us.apache.org/repos/asf/reef/commit/cf2f146a Tree: http://git-wip-us.apache.org/repos/asf/reef/tree/cf2f146a Diff: http://git-wip-us.apache.org/repos/asf/reef/diff/cf2f146a Branch: refs/heads/master Commit: cf2f146a19f54a976ab8b593badb3a00b7d8bf4c Parents: cfdb2bf Author: JiEun Lee <[email protected]> Authored: Wed Jul 27 15:54:25 2016 +0900 Committer: Markus Weimer <[email protected]> Committed: Wed Aug 3 11:22:27 2016 -0700 ---------------------------------------------------------------------- .../reef/tang/formats/ConfigurationModule.java | 37 ++++++++++++++++---- .../tang/formats/TestConfigurationModule.java | 35 ++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/reef/blob/cf2f146a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/formats/ConfigurationModule.java ---------------------------------------------------------------------- diff --git a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/formats/ConfigurationModule.java b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/formats/ConfigurationModule.java index 5f9f15f..09f2604 100644 --- a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/formats/ConfigurationModule.java +++ b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/formats/ConfigurationModule.java @@ -220,19 +220,42 @@ public class ConfigurationModule { for (final Class<?> clazz : c.builder.freeImpls.keySet()) { final Impl<?> i = c.builder.freeImpls.get(clazz); + boolean foundOne = false; if (c.setImpls.containsKey(i)) { - c.builder.b.bind(clazz, c.setImpls.get(i)); + if (c.setImpls.get(i) != null) { + c.builder.b.bind(clazz, c.setImpls.get(i)); + foundOne = true; + } } else if (c.setLateImpls.containsKey(i)) { - c.builder.b.bind(ReflectionUtilities.getFullName(clazz), c.setLateImpls.get(i)); + if (c.setLateImpls.get(i) != null) { + c.builder.b.bind(ReflectionUtilities.getFullName(clazz), c.setLateImpls.get(i)); + foundOne = true; + } } else if (c.setImplSets.containsKey(i) || c.setLateImplSets.containsKey(i)) { - for (final Class<?> clz : c.setImplSets.getValuesForKey(i)) { - c.builder.b.bindSetEntry((Class) clazz, (Class) clz); + if (c.setImplSets.getValuesForKey(i) != null) { + for (final Class<?> clz : c.setImplSets.getValuesForKey(i)) { + c.builder.b.bindSetEntry((Class) clazz, (Class) clz); + } + foundOne = true; } - for (final String s : c.setLateImplSets.getValuesForKey(i)) { - c.builder.b.bindSetEntry((Class) clazz, s); + if (c.setLateImplSets.getValuesForKey(i) != null) { + for (final String s : c.setLateImplSets.getValuesForKey(i)) { + c.builder.b.bindSetEntry((Class) clazz, s); + } + foundOne = true; } } else if (c.setImplLists.containsKey(i)) { - c.builder.b.bindList((Class) clazz, c.setImplLists.get(i)); + if (c.setImplLists.get(i) != null) { + c.builder.b.bindList((Class) clazz, c.setImplLists.get(i)); + foundOne = true; + } + } + if(!foundOne && !(i instanceof OptionalImpl)) { + final IllegalStateException e = + new IllegalStateException("Cannot find the value for the RequiredImplementation of the " + clazz + + ". Check that you don't pass null as an implementation value."); + LOG.log(Level.SEVERE, "Failed to build configuration", e); + throw e; } } for (final Class<? extends Name<?>> clazz : c.builder.freeParams.keySet()) { http://git-wip-us.apache.org/repos/asf/reef/blob/cf2f146a/lang/java/reef-tang/tang/src/test/java/org/apache/reef/tang/formats/TestConfigurationModule.java ---------------------------------------------------------------------- diff --git a/lang/java/reef-tang/tang/src/test/java/org/apache/reef/tang/formats/TestConfigurationModule.java b/lang/java/reef-tang/tang/src/test/java/org/apache/reef/tang/formats/TestConfigurationModule.java index ce064ba..d4910e3 100644 --- a/lang/java/reef-tang/tang/src/test/java/org/apache/reef/tang/formats/TestConfigurationModule.java +++ b/lang/java/reef-tang/tang/src/test/java/org/apache/reef/tang/formats/TestConfigurationModule.java @@ -38,6 +38,8 @@ import java.io.File; import java.io.IOException; import java.util.Set; +import static junit.framework.TestCase.fail; + /* * Define a configuration module that explains how Foo should be injected. * @@ -213,6 +215,39 @@ public class TestConfigurationModule { } @Test + public void nullInRequiredImplementationValueRaisesIllegalStateExceptionTest() { + final String nullString = null; + final Class<? extends TestConfigurationModule.Foo> nullClass = null; + try { + //case 1: string + MyConfigurationModule.CONF + .set(MyConfigurationModule.THE_FOO, nullString) + .set(MyConfigurationModule.FOO_STRING_NESS, "abc") + .build(); + fail(); + } catch (IllegalStateException e) { + nullInRequiredImplementationValueRaisesIllegalStateExceptionTestAssert(e); + } + try { + //case 2: class + MyConfigurationModule.CONF + .set(MyConfigurationModule.THE_FOO, nullClass) + .set(MyConfigurationModule.FOO_STRING_NESS, "abc") + .build(); + fail(); + } catch (IllegalStateException e) { + nullInRequiredImplementationValueRaisesIllegalStateExceptionTestAssert(e); + } + } + + private void nullInRequiredImplementationValueRaisesIllegalStateExceptionTestAssert(final Exception e) { + Assert.assertTrue(e instanceof IllegalStateException); + Assert.assertEquals(e.getMessage(), "Cannot find the value for the RequiredImplementation of the interface" + + " org.apache.reef.tang.formats.TestConfigurationModule$Foo." + + " Check that you don't pass null as an implementation value."); + } + + @Test @SuppressWarnings("checkstyle:avoidhidingcauseexception") public void badConfTest() throws Throwable { thrown.expect(ClassHierarchyException.class);
