#56 Fix export of empty Optionals, #55 improve tests for nested bean …

…serialization

- Empty optionals were being exported as empty maps because there was no way during the export value generation to signal that null should be used as the value. Also while gathering values to put into maps null values were not being skipped.
- Add more complicated examples of beans nested into each other to fully cover #55. The actual bug has been fixed during the large refactoring of #56

src/main/java/ch/jalu/configme/beanmapper/MapperImpl.java

@@ -49,6 +49,9 @@
  */
 public class MapperImpl implements Mapper {
 
+    /** Marker object to signal that null is meant to be used as value. */
+    public static final Object RETURN_NULL = new Object();
+
     // ---------
     // Fields and general configurable methods
     // ---------
@@ -86,22 +89,23 @@ protected MappingContext createRootMappingContext(String path, TypeInformation b
     public Object toExportValue(Object value) {
         // Step 1: attempt simple value transformation
         Object simpleValue = leafValueHandler.toExportValue(value);
-        if (simpleValue != null) {
-            return simpleValue;
-        } else if (value == null) {
-            return null;
+        if (simpleValue != null || value == null) {
+            return unwrapReturnNull(simpleValue);
         }
 
         // Step 2: handle special cases like Collection
         simpleValue = createExportValueForSpecialTypes(value);
         if (simpleValue != null) {
-            return simpleValue;
+            return unwrapReturnNull(simpleValue);
         }
 
         // Step 3: treat as bean
         Map<String, Object> mappedBean = new LinkedHashMap<>();
         for (BeanPropertyDescription property : beanDescriptionFactory.getAllProperties(value.getClass())) {
-            mappedBean.put(property.getName(), toExportValue(property.getValue(value)));
+            Object exportValueOfProperty = toExportValue(property.getValue(value));
+            if (exportValueOfProperty != null) {
+                mappedBean.put(property.getName(), exportValueOfProperty);
+            }
         }
         return mappedBean;
     }
@@ -124,12 +128,15 @@ protected Object createExportValueForSpecialTypes(Object value) {
 
         if (value instanceof Optional<?>) {
             Optional<?> optional = (Optional<?>) value;
-            return optional.map(this::toExportValue).orElse(null);
+            return optional.map(this::toExportValue).orElse(RETURN_NULL);
         }
 
         return null;
     }
 
+    protected static Object unwrapReturnNull(Object o) {
+        return o == RETURN_NULL ? null : o;
+    }
 
     // ---------
     // Bean mapping

src/main/java/ch/jalu/configme/beanmapper/leafvaluehandler/LeafValueHandler.java

@@ -31,6 +31,9 @@ public interface LeafValueHandler {
      * Converts the given value to a type more suitable for exporting. Used by the mapper in
      * when {@link ch.jalu.configme.properties.Property#toExportValue} is called on a bean property.
      * Returns null if the leaf value handler cannot handle the value.
+     * <p>
+     * Return {@link ch.jalu.configme.beanmapper.MapperImpl#RETURN_NULL} to signal that null should be used
+     * as the export value (returning {@code null} itself means this leaf value handler cannot handle it).
      *
      * @param value the value to convert to an export value, if possible
      * @return value to use in the export, or null if not applicable

src/test/java/ch/jalu/configme/beanmapper/BeanWithCollectionOfBeanTypeTest.java

@@ -5,45 +5,144 @@
 import ch.jalu.configme.SettingsManagerBuilder;
 import ch.jalu.configme.TestUtils;
 import ch.jalu.configme.properties.Property;
-import org.junit.Ignore;
+import org.hamcrest.Matchers;
+import org.hamcrest.core.CombinableMatcher;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 
 import static ch.jalu.configme.properties.PropertyInitializer.newBeanProperty;
-import static org.junit.Assert.fail;
+import static org.hamcrest.Matchers.anEmptyMap;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasProperty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.Assert.assertThat;
 
 /**
  * Test for bean types which have a property which is a collection of another bean type.
  *
  * @see <a href="https://github.com/AuthMe/ConfigMe/issues/55">#55: Nested bean serialization</a>
  */
-@Ignore // TODO #55: Add support for nested beans
 public class BeanWithCollectionOfBeanTypeTest {
 
+    private static final String NESTED_CHAT_COMPONENT_YML = "/beanmapper/nested_chat_component.yml";
+
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+    @Test
+    public void shouldLoadValue() {
+        // given
+        File file = TestUtils.copyFileFromResources(NESTED_CHAT_COMPONENT_YML, temporaryFolder);
+        SettingsManager settingsManager = SettingsManagerBuilder.withYamlFile(file)
+            .configurationData(PropertyHolder.class).create();
+
+        // when
+        ChatComponent value = settingsManager.getProperty(PropertyHolder.TEST);
+
+        // then
+        assertThat(value, hasColorAndText("blue", "outside"));
+        assertThat(value.getExtra(), hasSize(2));
+        assertThat(value.getExtra().get(0), hasColorAndText("green", "inner1"));
+        assertThat(value.getExtra().get(0).getExtra(), empty());
+        assertThat(value.getExtra().get(1), hasColorAndText("blue", "inner2"));
+        assertThat(value.getExtra().get(1).getExtra(), empty());
+    }
+
     @Test
     public void shouldSerializeProperly() throws IOException {
         // given
-        File file = TestUtils.getJarFile("/beanmapper/nested_chat_component.yml");
+        File file = TestUtils.copyFileFromResources(NESTED_CHAT_COMPONENT_YML, temporaryFolder);
         SettingsManager settingsManager = SettingsManagerBuilder.withYamlFile(file)
             .configurationData(PropertyHolder.class).create();
 
         // when
+        settingsManager.save();
+
+        // then
+        List<String> lines = Files.readAllLines(file.toPath());
+        assertThat(lines, contains(
+            "",
+            "message-key:",
+            "    color: blue",
+            "    extra: ",
+            "    - color: green",
+            "      extra: []",
+            "      text: inner1",
+            "    - color: blue",
+            "      extra: []",
+            "      text: inner2",
+            "    text: outside"
+        ));
+    }
+
+    @Test
+    public void shouldSerializeComplexObject() throws IOException {
+        // given
+        File file = TestUtils.copyFileFromResources(NESTED_CHAT_COMPONENT_YML, temporaryFolder);
+        SettingsManager settingsManager = SettingsManagerBuilder.withYamlFile(file)
+            .configurationData(PropertyHolder.class).create();
         settingsManager.setProperty(PropertyHolder.TEST, createComplexComponent());
+
+        // when
         settingsManager.save();
 
         // then
-        fail(String.join("\n", Files.readAllLines(file.toPath())));
+        List<String> lines = Files.readAllLines(file.toPath());
+        List<String> expectedLines = Files.readAllLines(TestUtils.getJarPath("/beanmapper/nested_chat_component_complex_expected.yml"));
+        assertThat(lines, equalTo(expectedLines));
+
+        // Some checks to ensure that we can read the file again
+        settingsManager.reload();
+        ChatComponent result = settingsManager.getProperty(PropertyHolder.TEST);
+        assertThat(result, hasColorAndText("green", "outside"));
+        assertThat(result.getConditionalElem().isPresent(), equalTo(true));
+        ExtendedChatComponent extendedComp = result.getConditionalElem().get();
+        assertThat(extendedComp.getConditionals().keySet(), contains("low", "med", "high"));
+        assertThat(extendedComp.getConditionals().get("med").getExtra().get(0), hasColorAndText("green", "med child"));
+        assertThat(extendedComp.getBold().isPresent(), equalTo(false));
+        assertThat(extendedComp.getItalic().isPresent(), equalTo(false));
+        assertThat(extendedComp.getConditionals().get("high").getConditionalElem().isPresent(), equalTo(true));
+        ExtendedChatComponent highExtendedComp = extendedComp.getConditionals().get("high").getConditionalElem().get();
+        assertThat(highExtendedComp.getConditionals(), anEmptyMap());
+        assertThat(highExtendedComp.getBold(), equalTo(Optional.of(true)));
+        assertThat(highExtendedComp.getItalic(), equalTo(Optional.of(false)));
+    }
+
+    private static CombinableMatcher<? super ChatComponent> hasColorAndText(String color, String text) {
+        return Matchers.both(hasProperty("color", equalTo(color)))
+            .and(hasProperty("text", equalTo(text)));
     }
 
     private static ChatComponent createComplexComponent() {
         ChatComponent comp = new ChatComponent("green", "outside");
-        ChatComponent extra = new ChatComponent("yellow", "inner");
-        comp.getExtra().add(extra);
+        ChatComponent greenExtra = new ChatComponent("yellow", "inner1");
+        comp.getExtra().add(greenExtra);
+        ChatComponent blueExtra = new ChatComponent("blue", "inner2");
+        comp.getExtra().add(blueExtra);
+        ChatComponent nestedExtra = new ChatComponent("red", "level2 text");
+        blueExtra.getExtra().add(nestedExtra);
+        ExtendedChatComponent extendedOrange = new ExtendedChatComponent("orange", "orange extension");
+        comp.setConditionalElem(Optional.of(extendedOrange));
+        extendedOrange.getConditionals().put("low", new ExtendedChatComponent("white", "low text"));
+        extendedOrange.getConditionals().put("med", new ExtendedChatComponent("gray", "med text"));
+        extendedOrange.getConditionals().get("med").getExtra().add(new ChatComponent("green", "med child"));
+        extendedOrange.getConditionals().put("high", new ExtendedChatComponent("black", "high text"));
+        ExtendedChatComponent extendedHighChild = new ExtendedChatComponent("teal", "teal addition");
+        extendedHighChild.setBold(Optional.of(true));
+        extendedHighChild.setItalic(Optional.of(false));
+        extendedOrange.getConditionals().get("high").setConditionalElem(Optional.of(extendedHighChild));
         return comp;
     }
 
@@ -58,6 +157,7 @@ public static class ChatComponent {
         private String color;
         private String text;
         private List<ChatComponent> extra = new ArrayList<>();
+        private Optional<ExtendedChatComponent> conditionalElem = Optional.empty();
 
         public ChatComponent() {
         }
@@ -90,5 +190,57 @@ public List<ChatComponent> getExtra() {
         public void setExtra(List<ChatComponent> extra) {
             this.extra = extra;
         }
+
+        @Override
+        public String toString() {
+            return "[color=" + color + ";text=" + text + ";extra="
+                + TestUtils.transform(extra, Object::toString) + "]";
+        }
+
+        public Optional<ExtendedChatComponent> getConditionalElem() {
+            return conditionalElem;
+        }
+
+        public void setConditionalElem(Optional<ExtendedChatComponent> conditionalElem) {
+            this.conditionalElem = conditionalElem;
+        }
+    }
+
+    public static class ExtendedChatComponent extends ChatComponent {
+
+        private Map<String, ChatComponent> conditionals = new LinkedHashMap<>();
+        private Optional<Boolean> italic = Optional.empty();
+        private Optional<Boolean> bold = Optional.empty();
+
+        public ExtendedChatComponent() {
+        }
+
+        public ExtendedChatComponent(String color, String text) {
+            super(color, text);
+        }
+
+        public Map<String, ChatComponent> getConditionals() {
+            return conditionals;
+        }
+
+        public void setConditionals(Map<String, ChatComponent> conditionals) {
+            this.conditionals = conditionals;
+        }
+
+        public Optional<Boolean> getItalic() {
+            return italic;
+        }
+
+        public void setItalic(Optional<Boolean> italic) {
+            this.italic = italic;
+        }
+
+        public Optional<Boolean> getBold() {
+            return bold;
+        }
+
+        public void setBold(Optional<Boolean> bold) {
+            this.bold = bold;
+        }
     }
 }

src/test/java/ch/jalu/configme/beanmapper/MapperExportValueTest.java

@@ -19,7 +19,6 @@
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertThat;
 
 /**
@@ -83,7 +82,7 @@ public void shouldAddEmptyMapAsLeafProperty() {
     }
 
     @Test
-    public void shouldHandleNullValue() {
+    public void shouldSkipNullValue() {
         // given
         Command command = new Command();
         command.setCommand("ping");
@@ -97,10 +96,9 @@ public void shouldHandleNullValue() {
         // then
         assertThat(exportValue, instanceOf(Map.class));
         Map<?, ?> values = (Map) exportValue;
-        assertThat(values.keySet(), containsInAnyOrder("command", "arguments", "execution"));
+        assertThat(values.keySet(), containsInAnyOrder("command", "arguments"));
         assertThat(values.get("command"), equalTo(command.getCommand()));
         assertThat(values.get("arguments"), equalTo(command.getArguments()));
-        assertThat(values.get("execution"), nullValue());
     }
 
     @SuppressWarnings("unchecked")

src/test/resources/beanmapper/nested_chat_component_complex_expected.yml

@@ -0,0 +1,44 @@
+
+message-key:
+    color: green
+    conditionalElem:
+        color: orange
+        conditionals:
+            low:
+                color: white
+                conditionals: {}
+                extra: []
+                text: low text
+            med:
+                color: gray
+                conditionals: {}
+                extra: 
+                - color: green
+                  extra: []
+                  text: med child
+                text: med text
+            high:
+                color: black
+                conditionalElem:
+                    bold: true
+                    color: teal
+                    conditionals: {}
+                    extra: []
+                    italic: false
+                    text: teal addition
+                conditionals: {}
+                extra: []
+                text: high text
+        extra: []
+        text: orange extension
+    extra: 
+    - color: yellow
+      extra: []
+      text: inner1
+    - color: blue
+      extra:
+      - color: red
+        extra: []
+        text: level2 text
+      text: inner2
+    text: outside