Update Config Adapters to run as expected, added tests.

Author: dualspiral

src/main/java/uk/co/drnaylor/quickstart/ModuleContainer.java

@@ -142,12 +142,11 @@ private void discoverModules() throws IOException {
 
         // Attaches config adapter and loads in the defaults.
         config.attachModulesConfig(m);
+        config.saveAdapterDefaults();
 
         // Load what we have in config into our discovered modules.
         try {
-            config.getConfigAdapter().getNode().forEach((k, v) -> {
-                discoveredModules.get(k).setStatus(v);
-            });
+            config.getConfigAdapter().getNode().forEach((k, v) -> discoveredModules.get(k).setStatus(v));
         } catch (ObjectMappingException e) {
             Logger.getLogger("QuickStart").warning("Could not load modules config, falling back to defaults.");
             e.printStackTrace();
@@ -293,6 +292,12 @@ public void loadModules(boolean failOnOneError) throws QuickStartModuleLoaderExc
             throw new QuickStartModuleLoaderException.Enabling(null, "No modules were enabled.", null);
         }
 
+        try {
+            config.saveAdapterDefaults();
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+
         currentPhase = ConstructionPhase.ENABLED;
     }
 

src/main/java/uk/co/drnaylor/quickstart/config/AbstractAdaptableConfig.java

@@ -91,9 +91,13 @@ public final void attachConfigAdapter(String module, AbstractConfigAdapter<?> co
             throw new IllegalArgumentException();
         }
 
-        configAdapter.attachConfig(module.toLowerCase(), this, () -> node.getNode(module.toLowerCase()), n -> node.setValue(n), nodeCreator);
+        configAdapter.attachConfig(
+                module.toLowerCase(),
+                this,
+                () -> nodeCreator.get().setValue(node.getNode(module.toLowerCase())),
+                n -> node.getNode(module.toLowerCase()).setValue(n),
+                nodeCreator);
         moduleConfigAdapters.put(module.toLowerCase(), configAdapter);
-        saveAdapterDefaults();
     }
 
     /**
@@ -105,9 +109,19 @@ public void save() throws IOException {
         loader.save(node);
     }
 
-    protected void saveAdapterDefaults() throws IOException {
+    public void saveAdapterDefaults() throws IOException {
         CommentedConfigurationNode n = SimpleCommentedConfigurationNode.root();
-        moduleConfigAdapters.forEach((k, v) -> n.getNode(k.toLowerCase()).setValue(v.getDefaults()));
+        moduleConfigAdapters.forEach((k, v) -> {
+            // Configurate does something I wasn't expecting. If we set a single value with a key on a node, it seems
+            // to be set as the root - which causes havoc! So, we get the parent if it exists, because that's the
+            // actual null node we're interested in.
+            ConfigurationNode cn = v.getDefaults();
+            if (cn.getParent() != null) {
+                cn = cn.getParent();
+            }
+
+            n.getNode(k.toLowerCase()).setValue(cn);
+        });
 
         node.mergeValuesFrom(n);
         save();

src/test/java/uk/co/drnaylor/quickstart/tests/scaffolding/FakeLoaderTests.java

@@ -8,19 +8,26 @@
 import ninja.leaping.configurate.SimpleConfigurationNode;
 import ninja.leaping.configurate.loader.ConfigurationLoader;
 import org.junit.Before;
+import uk.co.drnaylor.quickstart.ModuleContainer;
+import uk.co.drnaylor.quickstart.exceptions.QuickStartModuleDiscoveryException;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class FakeLoaderTests {
 
     protected ConfigurationLoader<ConfigurationNode> loader;
+    protected ConfigurationNode n = SimpleConfigurationNode.root();
 
     @Before
     @SuppressWarnings("unchecked")
     public void beforeTests() throws Exception {
         loader = (ConfigurationLoader<ConfigurationNode>)mock(ConfigurationLoader.class);
         when(loader.createEmptyNode()).thenReturn(SimpleConfigurationNode.root());
-        when(loader.load()).thenReturn(SimpleConfigurationNode.root());
+        when(loader.load()).thenReturn(n);
+    }
+
+    protected ModuleContainer getContainer(String p) throws QuickStartModuleDiscoveryException {
+        return ModuleContainer.builder().setConfigurationLoader(loader).setPackageToScan(p).build();
     }
 }

src/test/java/uk/co/drnaylor/quickstart/tests/tests/ModuleConfigurationTests.java

@@ -0,0 +1,45 @@
+/*
+ * This file is part of QuickStart Module Loader, licensed under the MIT License (MIT). See the LICENSE.txt file
+ * at the root of this project for more details.
+ */
+package uk.co.drnaylor.quickstart.tests.tests;
+
+import ninja.leaping.configurate.ConfigurationNode;
+import org.junit.Assert;
+import org.junit.Test;
+import uk.co.drnaylor.quickstart.ModuleContainer;
+import uk.co.drnaylor.quickstart.tests.config.adapters.SimpleWithDefault;
+import uk.co.drnaylor.quickstart.tests.scaffolding.FakeLoaderTests;
+
+import static org.junit.Assert.assertEquals;
+
+public class ModuleConfigurationTests extends FakeLoaderTests {
+
+    @Test
+    public void testThatMergedDefaultsFromConfigAdapterArePresent() throws Exception {
+        // When we load these modules...
+        ModuleContainer mc = getContainer("uk.co.drnaylor.quickstart.tests.modules.adapterstest");
+        mc.loadModules(true);
+
+        SimpleWithDefault s2 = mc.getConfigAdapterForModule("moduletwo", SimpleWithDefault.class);
+        assertEquals(s2.getNode().getNode("test").getString(), "test");
+    }
+
+    @Test
+    public void testThatUpdatingConfigAdapterPutsValueInCorrectPlace() throws Exception {
+        // When we load these modules...
+        ModuleContainer mc = getContainer("uk.co.drnaylor.quickstart.tests.modules.adapterstest");
+        mc.loadModules(true);
+
+        SimpleWithDefault s2 = mc.getConfigAdapterForModule("moduletwo", SimpleWithDefault.class);
+        ConfigurationNode newNode = s2.getNode();
+        newNode.getNode("newnode").setValue("result");
+
+        // This should not update the current node yet
+        Assert.assertNotEquals("result", n.getNode("moduletwo", "newnode").getString());
+        s2.setNode(newNode);
+
+        // Now it should have done.
+        Assert.assertEquals("result", n.getNode("moduletwo", "newnode").getString());
+    }
+}

src/test/java/uk/co/drnaylor/quickstart/tests/tests/ModuleContainerConstructionTests.java

@@ -13,15 +13,10 @@
 import uk.co.drnaylor.quickstart.tests.config.adapters.SimpleWithDefault;
 import uk.co.drnaylor.quickstart.tests.scaffolding.FakeLoaderTests;
 
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
 public class ModuleContainerConstructionTests extends FakeLoaderTests {
 
-    private ModuleContainer getContainer(String p) throws QuickStartModuleDiscoveryException {
-        return ModuleContainer.builder().setConfigurationLoader(loader).setPackageToScan(p).build();
-    }
-
     @Test(expected = QuickStartModuleLoaderException.Construction.class)
     public void testThatUnconstructableModulesThrow() throws QuickStartModuleDiscoveryException, QuickStartModuleLoaderException.Enabling, QuickStartModuleLoaderException.Construction {
         getContainer("uk.co.drnaylor.quickstart.tests.modules.exceptions.construction").loadModules(true);
@@ -57,14 +52,4 @@ public void testThatConfigAdaptersGetRegisteredOnConstruction() throws Exception
         assertNotNull(s);
         assertNotNull(s2);
     }
-
-    @Test
-    public void testThatMergedDefaultsFromConfigAdapterArePresent() throws Exception {
-        // When we load these modules...
-        ModuleContainer mc = getContainer("uk.co.drnaylor.quickstart.tests.modules.adapterstest");
-        mc.loadModules(true);
-
-        SimpleWithDefault s2 = mc.getConfigAdapterForModule("moduletwo", SimpleWithDefault.class);
-        assertEquals(s2.getNode().getNode("test").getString(), "test");
-    }
 }