You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@reef.apache.org by we...@apache.org on 2016/08/03 18:25:00 UTC
reef git commit: [REEF-967] Handle null in RequiredImpl
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 <sm...@gmail.com>
Authored: Wed Jul 27 15:54:25 2016 +0900
Committer: Markus Weimer <we...@apache.org>
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);