#450 Use SnakeYAML for writing properties

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

@@ -8,6 +8,8 @@ import fr.xephi.authme.settings.propertymap.PropertyMap;
 import fr.xephi.authme.util.CollectionUtils;
 import fr.xephi.authme.util.StringUtils;
 import org.bukkit.configuration.file.FileConfiguration;
+import org.yaml.snakeyaml.DumperOptions;
+import org.yaml.snakeyaml.Yaml;
 
 import java.io.File;
 import java.io.FileWriter;
@@ -36,16 +38,12 @@ public class NewSetting {
         this.configuration = configuration;
         this.file = file;
 
-        // TODO ljacqu 20160109: Ensure that save() works as desired (i.e. that it always produces valid YAML)
+        PropertyMap propertyMap = SettingsFieldRetriever.getAllPropertyFields();
-        // and then uncomment the lines below. Once this is uncommented, the checks in the old Settings.java should
+        if (SettingsMigrationService.checkAndMigrate(configuration, propertyMap)) {
-        // be removed as we should check to rewrite the config.yml file only at one place
+            ConsoleLogger.info("Merged new config options");
-        // --------
+            ConsoleLogger.info("Please check your config.yml file for new settings!");
-        // PropertyMap propertyMap = SettingsFieldRetriever.getAllPropertyFields();
+            save(propertyMap);
-        // if (SettingsMigrationService.checkAndMigrate(configuration, propertyMap)) {
+        }
-        //     ConsoleLogger.info("Merged new config options");
-        //     ConsoleLogger.info("Please check your config.yml file for new settings!");
-        //     save(propertyMap);
-        // }
     }
 
     /**
@@ -76,14 +74,18 @@ public class NewSetting {
         return property.getFromFile(configuration);
     }
 
-    public void save() {
-        save(SettingsFieldRetriever.getAllPropertyFields());
-    }
-
     public void save(PropertyMap propertyMap) {
         try (FileWriter writer = new FileWriter(file)) {
             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()) {
@@ -118,14 +120,8 @@ public class NewSetting {
                 writer.append("\n")
                     .append(indent(indentationLevel))
                     .append(CollectionUtils.getRange(newPathParts, newPathParts.size() - 1).get(0))
-                    .append(": ");
+                    .append(": ")
-
+                    .append(toYaml(property, indentationLevel, simpleYaml, singleQuoteYaml));
-                List<String> yamlLines = property.formatValueAsYaml(configuration);
-                String delim = "";
-                for (String yamlLine : yamlLines) {
-                    writer.append(delim).append(yamlLine);
-                    delim = "\n" + indent(indentationLevel);
-                }
 
                 currentPath = propertyPath.subList(0, propertyPath.size() - 1);
             }
@@ -137,6 +133,33 @@ public class NewSetting {
         }
     }
 
+    private <T> String toYaml(Property<T> property, int indent, Yaml simpleYaml, Yaml singleQuoteYaml) {
+        T value = property.getFromFile(configuration);
+        String representation = property.hasSingleQuotes()
+            ? singleQuoteYaml.dump(value)
+            : simpleYaml.dump(value);
+
+        // If the property is a non-empty list we need to append a new line because it will be
+        // something like the following, which requires a new line:
+        // - 'item 1'
+        // - 'second item in list'
+        if (property.isList() && !((List) value).isEmpty()) {
+            representation = "\n" + representation;
+        }
+
+        return join("\n" + indent(indent), representation.split("\\n"));
+    }
+
+    private static String join(String delimiter, String[] items) {
+        StringBuilder sb = new StringBuilder();
+        String delim = "";
+        for (String item : items) {
+            sb.append(delim).append(item);
+            delim = delimiter;
+        }
+        return sb.toString();
+    }
+
     private static String indent(int level) {
         // YAML uses indentation of 4 spaces
         StringBuilder sb = new StringBuilder(level * 4);

src/main/java/fr/xephi/authme/settings/domain/EnumPropertyType.java

@@ -2,12 +2,9 @@ package fr.xephi.authme.settings.domain;
 
 import org.bukkit.configuration.file.FileConfiguration;
 
-import java.util.List;
-
-import static java.util.Arrays.asList;
-
 /**
  * Enum property type.
+ *
  * @param <E> The enum class
  */
 class EnumPropertyType<E extends Enum<E>> extends PropertyType<E> {
@@ -28,17 +25,17 @@ class EnumPropertyType<E extends Enum<E>> extends PropertyType<E> {
         return mappedValue != null ? mappedValue : property.getDefaultValue();
     }
 
-    @Override
-    protected List<String> asYaml(E value) {
-        return asList("'" + value + "'");
-    }
-
     @Override
     public boolean contains(Property<E> property, FileConfiguration configuration) {
         return super.contains(property, configuration)
             && mapToEnum(configuration.getString(property.getPath())) != null;
     }
 
+    @Override
+    public boolean hasSingleQuotes() {
+        return true;
+    }
+
     private E mapToEnum(String value) {
         for (E entry : clazz.getEnumConstants()) {
             if (entry.name().equalsIgnoreCase(value)) {

src/main/java/fr/xephi/authme/settings/domain/Property.java

@@ -7,7 +7,7 @@ import java.util.List;
 import java.util.Objects;
 
 /**
- * Properties (i.e. a <i>setting</i> that is read from the config.yml file).
+ * Property class, representing a <i>setting</i> that is read from the config.yml file.
  */
 public class Property<T> {
 
@@ -22,15 +22,43 @@ public class Property<T> {
         this.defaultValue = defaultValue;
     }
 
+    /**
+     * Create a new property. See also {@link #newProperty(PropertyType, String, Object[])} for lists and
+     * {@link #newProperty(Class, String, Enum)}.
+     *
+     * @param type The property type
+     * @param path The property's path
+     * @param defaultValue The default value
+     * @param <T> The type of the property
+     * @return The created property
+     */
     public static <T> Property<T> newProperty(PropertyType<T> type, String path, T defaultValue) {
         return new Property<>(type, path, defaultValue);
     }
 
+    /**
+     * Create a new list property.
+     *
+     * @param type The list type of the property
+     * @param path The property's path
+     * @param defaultValues The default value's items
+     * @param <U> The list type
+     * @return The created list property
+     */
     @SafeVarargs
     public static <U> Property<List<U>> newProperty(PropertyType<List<U>> type, String path, U... defaultValues) {
         return new Property<>(type, path, Arrays.asList(defaultValues));
     }
 
+    /**
+     * Create a new enum property.
+     *
+     * @param clazz The enum class
+     * @param path The property's path
+     * @param defaultValue The default value
+     * @param <E> The enum type
+     * @return The created enum property
+     */
     public static <E extends Enum<E>> Property<E> newProperty(Class<E> clazz, String path, E defaultValue) {
         return new Property<>(new EnumPropertyType<>(clazz), path, defaultValue);
     }
@@ -53,9 +81,8 @@ public class Property<T> {
     // -----
     // Hooks to the PropertyType methods
     // -----
-
     /**
-     * Get the property value from the given configuration.
+     * Get the property value from the given configuration &ndash; guaranteed to never return null.
      *
      * @param configuration The configuration to read the value from
      * @return The value, or default if not present
@@ -64,16 +91,6 @@ public class Property<T> {
         return type.getFromFile(this, configuration);
     }
 
-    /**
-     * Format the property value as YAML.
-     *
-     * @param configuration The configuration to read the value from
-     * @return The property value as YAML
-     */
-    public List<String> formatValueAsYaml(FileConfiguration configuration) {
-        return type.asYaml(this, configuration);
-    }
-
     /**
      * Return whether or not the given configuration file contains the property.
      *
@@ -84,10 +101,30 @@ public class Property<T> {
         return type.contains(this, configuration);
     }
 
+    /**
+     * Return whether the property should be represented wrapped in single quotes in YAML.
+     * The YAML format allows both using single quotes and not for all types but for certain
+     * types one representation makes more sense. Typically we wrap string-like types in
+     * single quotes and all other ones not.
+     *
+     * @return True if single quotes should be used, false otherwise
+     */
+    public boolean hasSingleQuotes() {
+        return type.hasSingleQuotes();
+    }
+
+    /**
+     * Return whether the property has a list type.
+     *
+     * @return True if the property type is a list, false otherwise
+     */
+    public boolean isList() {
+        return type.isList();
+    }
+
     // -----
     // Trivial getters
     // -----
-
     /**
      * Return the default value of the property.
      *

src/main/java/fr/xephi/authme/settings/domain/PropertyType.java

@@ -2,11 +2,8 @@ package fr.xephi.authme.settings.domain;
 
 import org.bukkit.configuration.file.FileConfiguration;
 
-import java.util.ArrayList;
 import java.util.List;
 
-import static java.util.Arrays.asList;
-
 /**
  * Handles a certain property type and provides type-specific functionality.
  *
@@ -30,17 +27,6 @@ public abstract class PropertyType<T> {
      */
     public abstract T getFromFile(Property<T> property, FileConfiguration configuration);
 
-    /**
-     * Return the property's value (or its default) as YAML.
-     *
-     * @param property The property to transform
-     * @param configuration The YAML configuration to read from
-     * @return The read value or its default in YAML format
-     */
-    public List<String> asYaml(Property<T> property, FileConfiguration configuration) {
-        return asYaml(getFromFile(property, configuration));
-    }
-
     /**
      * Return whether the property is present in the given configuration.
      *
@@ -53,12 +39,17 @@ public abstract class PropertyType<T> {
     }
 
     /**
-     * Transform the given value to YAML.
+     * Return whether the property type should be wrapped in single quotes in YAML.
      *
-     * @param value The value to transform
+     * @return True if single quotes should be used, false if not
-     * @return The value as YAML
      */
-    protected abstract List<String> asYaml(T value);
+    public boolean hasSingleQuotes() {
+        return false;
+    }
+
+    public boolean isList() {
+        return false;
+    }
 
 
     /**
@@ -69,11 +60,6 @@ public abstract class PropertyType<T> {
         public Boolean getFromFile(Property<Boolean> property, FileConfiguration configuration) {
             return configuration.getBoolean(property.getPath(), property.getDefaultValue());
         }
-
-        @Override
-        protected List<String> asYaml(Boolean value) {
-            return asList(value ? "true" : "false");
-        }
     }
 
     /**
@@ -84,11 +70,6 @@ public abstract class PropertyType<T> {
         public Double getFromFile(Property<Double> property, FileConfiguration configuration) {
             return configuration.getDouble(property.getPath(), property.getDefaultValue());
         }
-
-        @Override
-        protected List<String> asYaml(Double value) {
-            return asList(String.valueOf(value));
-        }
     }
 
     /**
@@ -99,11 +80,6 @@ public abstract class PropertyType<T> {
         public Integer getFromFile(Property<Integer> property, FileConfiguration configuration) {
             return configuration.getInt(property.getPath(), property.getDefaultValue());
         }
-
-        @Override
-        protected List<String> asYaml(Integer value) {
-            return asList(String.valueOf(value));
-        }
     }
 
     /**
@@ -114,15 +90,9 @@ public abstract class PropertyType<T> {
         public String getFromFile(Property<String> property, FileConfiguration configuration) {
             return configuration.getString(property.getPath(), property.getDefaultValue());
         }
-
         @Override
-        protected List<String> asYaml(String value) {
+        public boolean hasSingleQuotes() {
-            return asList(toYamlLiteral(value));
+            return true;
-        }
-
-        public static String toYamlLiteral(String str) {
-            // TODO: Need to handle new lines properly
-            return "'" + str.replace("'", "''") + "'";
         }
     }
 
@@ -139,23 +109,18 @@ public abstract class PropertyType<T> {
         }
 
         @Override
-        protected List<String> asYaml(List<String> value) {
+        public boolean contains(Property<List<String>> property, FileConfiguration configuration) {
-            if (value.isEmpty()) {
+            return configuration.contains(property.getPath()) && configuration.isList(property.getPath());
-                return asList("[]");
-            }
-
-            List<String> resultLines = new ArrayList<>();
-            resultLines.add(""); // add
-            for (String entry : value) {
-                // TODO: StringProperty#toYamlLiteral will return List<String>...
-                resultLines.add("    - " + StringProperty.toYamlLiteral(entry));
-            }
-            return resultLines;
         }
 
         @Override
-        public boolean contains(Property<List<String>> property, FileConfiguration configuration) {
+        public boolean hasSingleQuotes() {
-            return configuration.contains(property.getPath()) && configuration.isList(property.getPath());
+            return true;
+        }
+
+        @Override
+        public boolean isList() {
+            return true;
         }
     }