#450 Add integration test for potentially difficult values

- Ensure that edge cases such as backslashes, apostrophes and new lines are handled properly by the YAML writer & reader

src/main/java/fr/xephi/authme/settings/NewSetting.java

@@ -76,16 +76,10 @@ public class NewSetting {
 
     public void save(PropertyMap propertyMap) {
         try (FileWriter writer = new FileWriter(file)) {
+            Yaml simpleYaml = newYaml(false);
+            Yaml singleQuoteYaml = newYaml(true);
+
             writer.write("");
-
-            DumperOptions simpleOptions = new DumperOptions();
-            simpleOptions.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);
-            Yaml simpleYaml = new Yaml(simpleOptions);
-            DumperOptions singleQuoteOptions = new DumperOptions();
-            singleQuoteOptions.setDefaultScalarStyle(DumperOptions.ScalarStyle.SINGLE_QUOTED);
-            singleQuoteOptions.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);
-            Yaml singleQuoteYaml = new Yaml(singleQuoteOptions);
-
             // Contains all but the last node of the setting, e.g. [DataSource, mysql] for "DataSource.mysql.username"
             List<String> currentPath = new ArrayList<>();
             for (Map.Entry<Property<?>, String[]> entry : propertyMap.entrySet()) {
@@ -150,6 +144,16 @@ public class NewSetting {
         return join("\n" + indent(indent), representation.split("\\n"));
     }
 
+    private static Yaml newYaml(boolean useSingleQuotes) {
+        DumperOptions options = new DumperOptions();
+        options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);
+        options.setAllowUnicode(true);
+        if (useSingleQuotes) {
+            options.setDefaultScalarStyle(DumperOptions.ScalarStyle.SINGLE_QUOTED);
+        }
+        return new Yaml(options);
+    }
+
     private static String join(String delimiter, String[] items) {
         StringBuilder sb = new StringBuilder();
         String delim = "";

src/test/java/fr/xephi/authme/settings/NewSettingIntegrationTest.java

@@ -7,7 +7,6 @@ import fr.xephi.authme.settings.properties.TestConfiguration;
 import fr.xephi.authme.settings.propertymap.PropertyMap;
 import fr.xephi.authme.util.WrapperMock;
 import org.bukkit.configuration.file.YamlConfiguration;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.io.File;
@@ -15,8 +14,10 @@ import java.lang.reflect.Field;
 import java.net.URL;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
+import static fr.xephi.authme.settings.domain.Property.newProperty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assume.assumeThat;
@@ -30,22 +31,10 @@ public class NewSettingIntegrationTest {
     private static final String COMPLETE_FILE = "config-sample-values.yml";
     /** File name of the sample config missing certain {@link TestConfiguration} values. */
     private static final String INCOMPLETE_FILE = "config-incomplete-sample.yml";
+    /** File name for testing difficult values. */
+    private static final String DIFFICULT_FILE = "config-difficult-values.yml";
 
-    private static PropertyMap propertyMap;
+    private static PropertyMap propertyMap = generatePropertyMap();
-
-    @BeforeClass
-    public static void generatePropertyMap() {
-        WrapperMock.createInstance();
-        propertyMap = new PropertyMap();
-        for (Field field : TestConfiguration.class.getDeclaredFields()) {
-            Object fieldValue = ReflectionTestUtils.getFieldValue(TestConfiguration.class, null, field.getName());
-            if (fieldValue instanceof Property<?>) {
-                Property<?> property = (Property<?>) fieldValue;
-                String[] comments = new String[]{"Comment for '" + property.getPath() + "'"};
-                propertyMap.put(property, comments);
-            }
-        }
-    }
 
     @Test
     public void shouldLoadAndReadAllProperties() {
@@ -107,6 +96,62 @@ public class NewSettingIntegrationTest {
         }
     }
 
+    /** Verify that "difficult cases" such as apostrophes in strings etc. are handled properly. */
+    @Test
+    public void shouldProperlyExportAnyValues() {
+        // given
+        File file = getConfigFile(DIFFICULT_FILE);
+        YamlConfiguration configuration = YamlConfiguration.loadConfiguration(file);
+        assumeThat(configuration.contains(TestConfiguration.DUST_LEVEL.getPath()), equalTo(false));
+
+        // Additional string properties
+        List<Property<String>> additionalProperties = Arrays.asList(
+            newProperty("more.string1", "it's a text with some \\'apostrophes'"),
+            newProperty("more.string2", "\tthis one\nhas some\nnew '' lines-test")
+        );
+        PropertyMap propertyMap = generatePropertyMap();
+        for (Property<?> property : additionalProperties) {
+            propertyMap.put(property, new String[0]);
+        }
+
+        // when
+        new NewSetting(configuration, file, propertyMap);
+        // reload the file as settings should hav been rewritten
+        configuration = YamlConfiguration.loadConfiguration(file);
+
+        // then
+        // assert that we won't rewrite the settings again! One rewrite should produce a valid, complete configuration
+        File unusedFile = new File("config-difficult-values.unused.yml");
+        NewSetting settings = new NewSetting(configuration, unusedFile, propertyMap);
+        assertThat(unusedFile.exists(), equalTo(false));
+        assertThat(configuration.contains(TestConfiguration.DUST_LEVEL.getPath()), equalTo(true));
+
+        Map<Property<?>, Object> expectedValues = ImmutableMap.<Property<?>, Object>builder()
+            .put(TestConfiguration.DURATION_IN_SECONDS, 20)
+            .put(TestConfiguration.SYSTEM_NAME, "A 'test' name")
+            .put(TestConfiguration.RATIO_LIMIT, -41.8)
+            .put(TestConfiguration.RATIO_FIELDS, Arrays.asList("Australia\\", "\tBurundi'", "Colombia?\n''"))
+            .put(TestConfiguration.VERSION_NUMBER, -1337)
+            .put(TestConfiguration.SKIP_BORING_FEATURES, false)
+            .put(TestConfiguration.BORING_COLORS, Arrays.asList("it's a difficult string!", "gray\nwith new lines\n"))
+            .put(TestConfiguration.DUST_LEVEL, 0.2)
+            .put(TestConfiguration.USE_COOL_FEATURES, true)
+            .put(TestConfiguration.COOL_OPTIONS, Collections.EMPTY_LIST)
+            .put(additionalProperties.get(0), additionalProperties.get(0).getDefaultValue())
+            .put(additionalProperties.get(1), additionalProperties.get(1).getDefaultValue())
+            .build();
+        for (Map.Entry<Property<?>, Object> entry : expectedValues.entrySet()) {
+            assertThat("Property '" + entry.getKey().getPath() + "' has expected value"
+                + entry.getValue() + " but found " + settings.getProperty(entry.getKey()),
+                settings.getProperty(entry.getKey()), equalTo(entry.getValue()));
+        }
+    }
+
+    /**
+     * Return a {@link File} instance to an existing file in the target/test-classes folder.
+     *
+     * @return The generated File
+     */
     private File getConfigFile(String file) {
         URL url = getClass().getClassLoader().getResource(file);
         if (url == null) {
@@ -115,4 +160,23 @@ public class NewSettingIntegrationTest {
         return new File(url.getFile());
     }
 
+    /**
+     * Generate a property map with all properties in {@link TestConfiguration}.
+     *
+     * @return The generated property map
+     */
+    private static PropertyMap generatePropertyMap() {
+        WrapperMock.createInstance();
+        PropertyMap propertyMap = new PropertyMap();
+        for (Field field : TestConfiguration.class.getDeclaredFields()) {
+            Object fieldValue = ReflectionTestUtils.getFieldValue(TestConfiguration.class, null, field.getName());
+            if (fieldValue instanceof Property<?>) {
+                Property<?> property = (Property<?>) fieldValue;
+                String[] comments = new String[]{"Comment for '" + property.getPath() + "'"};
+                propertyMap.put(property, comments);
+            }
+        }
+        return propertyMap;
+    }
+
 }

src/test/resources/config-difficult-values.yml

@@ -0,0 +1,29 @@
+# Test config file with some "difficult" values
+
+test:
+    duration: 20.102
+    systemName: 'A ''test'' name'
+sample:
+    ratio:
+        limit: -41.8
+        fields:
+            - Australia\
+            - '	Burundi'''
+            - 'Colombia?
+
+            '''''
+        # The last element above represents "Colombia?\n''"
+version: -1337
+features:
+    boring:
+        # YAML allows both "yes"/"no" and "true"/"false" for expressing booleans
+        skip: no
+        colors:
+            - 'it''s a difficult string!'
+            - |
+                gray
+                with new lines
+        # dustLevel: 0.81 <-- missing property triggering rewrite
+    cool:
+        enabled: yes
+        options: []