Preserve null in ValueMapper except for primitive types

Author: adrienlauer

src/main/java/org/seedstack/coffig/mapper/ValueMapper.java

@@ -7,16 +7,17 @@
  */
 package org.seedstack.coffig.mapper;
 
-import java.lang.reflect.Type;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.Function;
 import org.seedstack.coffig.TreeNode;
 import org.seedstack.coffig.internal.ConfigurationErrorCode;
 import org.seedstack.coffig.internal.ConfigurationException;
 import org.seedstack.coffig.node.ValueNode;
 import org.seedstack.coffig.spi.ConfigurationMapper;
 
+import java.lang.reflect.Type;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
 public class ValueMapper implements ConfigurationMapper {
     private final Map<Type, Function<String, ?>> converters = new HashMap<>();
 
@@ -47,7 +48,18 @@ public boolean canHandle(Type type) {
 
     @Override
     public Object map(TreeNode value, Type type) {
-        return converters.get(type).apply(value.value());
+        String innerValue = value.value();
+        if (type instanceof Class && ((Class<?>) type).isPrimitive()) {
+            // Don't return a null value for primitive types (would cause NPE)
+            return converters.get(type).apply(innerValue);
+        } else {
+            // For other types it's ok to return null
+            if (innerValue == null) {
+                return null;
+            } else {
+                return converters.get(type).apply(innerValue);
+            }
+        }
     }
 
     @Override

src/main/java/org/seedstack/coffig/node/AbstractTreeNode.java

@@ -7,9 +7,10 @@
  */
 package org.seedstack.coffig.node;
 
+import org.seedstack.coffig.TreeNode;
+
 import java.util.Arrays;
 import java.util.stream.Collectors;
-import org.seedstack.coffig.TreeNode;
 
 abstract class AbstractTreeNode implements TreeNode {
     static String HIDDEN_PLACEHOLDER = "***";
@@ -28,8 +29,4 @@ public void hide() {
     String indent(String s) {
         return Arrays.stream(s.split("\n")).map(line -> "  " + line).collect(Collectors.joining("\n"));
     }
-
-    String quote(String s) {
-        return s == null ? null : s.replace("\\", "\\\\").replace("\"", "\\\"");
-    }
 }

src/main/java/org/seedstack/coffig/node/ArrayNode.java

@@ -182,13 +182,7 @@ public String toMappedString(ConfigurationMapper mapper) {
         if (isHidden()) {
             return "\"" + HIDDEN_PLACEHOLDER + "\"";
         } else if (children.size() > 0 && children.get(0).type() == Type.VALUE_NODE) {
-            return children.stream().map(item -> {
-                if (mapper != null) {
-                    return "- " + mapper.map(item, String.class);
-                } else {
-                    return "- " + item.toString();
-                }
-            }).collect(joining("\n"));
+            return children.stream().map(item -> "- " + item.toMappedString(mapper)).collect(joining("\n"));
         } else {
             return children.stream().map(item -> "-\n" + indent(item.toMappedString(mapper))).collect(joining("\n"));
         }

src/main/java/org/seedstack/coffig/node/MapNode.java

@@ -7,19 +7,19 @@
  */
 package org.seedstack.coffig.node;
 
+import org.seedstack.coffig.TreeNode;
+import org.seedstack.coffig.internal.ConfigurationErrorCode;
+import org.seedstack.coffig.internal.ConfigurationException;
+import org.seedstack.coffig.internal.PropertyNotFoundException;
+import org.seedstack.coffig.spi.ConfigurationMapper;
+
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import org.seedstack.coffig.TreeNode;
-import org.seedstack.coffig.internal.ConfigurationErrorCode;
-import org.seedstack.coffig.internal.ConfigurationException;
-import org.seedstack.coffig.internal.PropertyNotFoundException;
-import org.seedstack.coffig.spi.ConfigurationMapper;
-
 public class MapNode extends AbstractTreeNode {
     private final Map<String, TreeNode> children;
 
@@ -177,11 +177,7 @@ public String toMappedString(ConfigurationMapper mapper) {
         } else {
             return children.entrySet().stream().map(entry -> {
                 if (entry.getValue().type() == Type.VALUE_NODE) {
-                    if (mapper != null) {
-                        return entry.getKey() + ": " + mapper.map(entry.getValue(), String.class);
-                    } else {
-                        return entry.getKey() + ": " + entry.getValue().toString();
-                    }
+                    return entry.getKey() + ": " + entry.getValue().toMappedString(mapper);
                 } else {
                     return entry.getKey() + ":\n" + indent(entry.getValue().toMappedString(mapper));
                 }

src/main/java/org/seedstack/coffig/node/ValueNode.java

@@ -121,22 +121,25 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        return value == null ? "~" : "\"" + (isHidden() ? HIDDEN_PLACEHOLDER : quote(value)) + "\"";
+        return toMappedString(null);
     }
 
     @Override
     public String toMappedString(ConfigurationMapper mapper) {
-        if (value == null) {
-            return "~";
+        if (isHidden()) {
+            return "\"" + HIDDEN_PLACEHOLDER + "\"";
         } else {
-            if (isHidden()) {
-                return "\"" + HIDDEN_PLACEHOLDER + "\"";
+            String s;
+            if (mapper != null) {
+                s = (String) mapper.map(this, String.class);
+            } else {
+                s = value;
+            }
+
+            if (s == null) {
+                return "~";
             } else {
-                if (mapper != null) {
-                    return "\"" + quote(String.valueOf(mapper.map(this, String.class))) + "\"";
-                } else {
-                    return "\"" + quote(value) + "\"";
-                }
+                return "\"" + s.replace("\\", "\\\\").replace("\"", "\\\"") + "\"";
             }
         }
     }