#19 Add error to error recorder if value is invalid for map/collection

AuthMe · Dec 26, 2019 · 90e5c35 · 90e5c35
1 parent ec9b392
commit 90e5c35
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 4 deletions.
diff --git a/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java
@@ -220,6 +220,7 @@ protected Object handleSpecialTypes(MappingContext context, Object value) {
      * @return Collection property from the value, or null if not applicable
      */
     @Nullable
+    @SuppressWarnings("unchecked")
     protected Collection createCollection(MappingContext context, Object value) {
         if (value instanceof Iterable<?>) {
             TypeInformation entryType = context.getGenericTypeInfoOrFail(0);
@@ -228,7 +229,9 @@ protected Collection createCollection(MappingContext context, Object value) {
             int index = 0;
             for (Object entry : (Iterable) value) {
                 Object convertedEntry = convertValueForType(context.createChild("[" + index + "]", entryType), entry);
-                if (convertedEntry != null) {
+                if (convertedEntry == null) {
+                    context.registerError("Cannot convert value at index " + index);
+                } else {
                     result.add(convertedEntry);
                 }
             }
@@ -264,6 +267,7 @@ protected Collection createCollectionMatchingType(MappingContext mappingContext)
      * @return Map property, or null if not applicable
      */
     @Nullable
+    @SuppressWarnings("unchecked")
     protected Map createMap(MappingContext context, Object value) {
         if (value instanceof Map<?, ?>) {
             if (context.getGenericTypeInfoOrFail(0).getSafeToWriteClass() != String.class) {
@@ -276,7 +280,9 @@ protected Map createMap(MappingContext context, Object value) {
             for (Map.Entry<String, ?> entry : entries.entrySet()) {
                 Object mappedValue = convertValueForType(
                     context.createChild("[k=" + entry.getKey() + "]", mapValueType), entry.getValue());
-                if (mappedValue != null) {
+                if (mappedValue == null) {
+                    context.registerError("Cannot map value for key " + entry.getKey());
+                } else {
                     result.put(entry.getKey(), mappedValue);
                 }
             }

diff --git a/src/main/java/ch/jalu/configme/beanmapper/MappingContextImpl.java b/src/main/java/ch/jalu/configme/beanmapper/MappingContextImpl.java
@@ -49,7 +49,7 @@ public String createDescription() {
 
     @Override
     public void registerError(String reason) {
-        errorRecorder.setHasError(reason);
+        errorRecorder.setHasError("At path '" + path + "': " + reason);
     }
 
     @Override

diff --git a/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/MapperImplTest.java
@@ -127,7 +127,7 @@ void shouldSkipInvalidEntry() {
             reader.getObject(""), WorldGroupConfig.class, errorRecorder);
 
         // then
-        assertThat(errorRecorder.isFullyValid(), equalTo(true)); // todo #19: Should be false
+        assertThat(errorRecorder.isFullyValid(), equalTo(false));
         assertThat(config, not(nullValue()));
         assertThat(config.getGroups().keySet(), contains("creative"));
     }

diff --git a/src/test/java/ch/jalu/configme/beanmapper/MappingContextImplTest.java b/src/test/java/ch/jalu/configme/beanmapper/MappingContextImplTest.java
@@ -1,5 +1,6 @@
 package ch.jalu.configme.beanmapper;
 
+import ch.jalu.configme.properties.convertresult.ConvertErrorRecorder;
 import ch.jalu.configme.utils.TypeInformation;
 import org.junit.jupiter.api.Test;
 
@@ -8,6 +9,8 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 /**
  * Test for {@link MappingContextImpl}.
@@ -42,4 +45,18 @@ void shouldCreateDescription() {
         // then
         assertThat(description, equalTo("Path: 'oh.em.gee', type: '" + ArrayList.class + "'"));
     }
+
+    @Test
+    void shouldForwardErrorToErrorRecorder() {
+        // given
+        ConvertErrorRecorder errorRecorder = mock(ConvertErrorRecorder.class);
+        MappingContext root = MappingContextImpl.createRoot(new TypeInformation(String.class), errorRecorder);
+        MappingContext context = root.createChild("bar", new TypeInformation(Double.class));
+
+        // when
+        context.registerError("Not a valid value");
+
+        // then
+        verify(errorRecorder).setHasError("At path 'bar': Not a valid value");
+    }
 }