#450 Fix YAML export of enum values

- Move writing logic to PropertyType
- Remove unused double property type
- Add sample enum property type to tests

src/main/java/fr/xephi/authme/AuthMe.java

@@ -224,7 +224,7 @@ public class AuthMe extends JavaPlugin {
             return;
         }
 
-        messages = new Messages(Settings.messageFile);
+        messages = new Messages(newSettings.getMessagesFile());
 
         // Connect to the database and setup tables
         try {

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

@@ -171,19 +171,7 @@ 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;
-        }
-
+        String representation = property.toYaml(configuration, simpleYaml, singleQuoteYaml);
         return join("\n" + indent(indent), representation.split("\\n"));
     }
 
@@ -197,7 +185,7 @@ public class NewSetting {
     }
 
     private File buildMessagesFileFromCode(String language) {
-        return new File(pluginFolder.getName(),
+        return new File(pluginFolder,
             makePath("messages", "messages_" + language + ".yml"));
     }
 

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

@@ -1,6 +1,7 @@
 package fr.xephi.authme.settings.domain;
 
 import org.bukkit.configuration.file.FileConfiguration;
+import org.yaml.snakeyaml.Yaml;
 
 /**
  * Enum property type.
@@ -32,8 +33,8 @@ class EnumPropertyType<E extends Enum<E>> extends PropertyType<E> {
     }
 
     @Override
-    public boolean hasSingleQuotes() {
-        return true;
+    public String toYaml(E value, Yaml simpleYaml, Yaml singleQuoteYaml) {
+        return singleQuoteYaml.dump(value.name());
     }
 
     private E mapToEnum(String value) {

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

@@ -1,6 +1,7 @@
 package fr.xephi.authme.settings.domain;
 
 import org.bukkit.configuration.file.FileConfiguration;
+import org.yaml.snakeyaml.Yaml;
 
 import java.util.Arrays;
 import java.util.List;
@@ -102,24 +103,15 @@ public class Property<T> {
     }
 
     /**
-     * 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.
+     * Format the property's value as YAML.
      *
-     * @return True if single quotes should be used, false otherwise
+     * @param configuration The file configuration
+     * @param simpleYaml YAML object (default)
+     * @param singleQuoteYaml YAML object using single quotes
+     * @return The generated YAML
      */
-    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();
+    public String toYaml(FileConfiguration configuration, Yaml simpleYaml, Yaml singleQuoteYaml) {
+        return type.toYaml(getFromFile(configuration), simpleYaml, singleQuoteYaml);
     }
 
     // -----

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

@@ -1,6 +1,7 @@
 package fr.xephi.authme.settings.domain;
 
 import org.bukkit.configuration.file.FileConfiguration;
+import org.yaml.snakeyaml.Yaml;
 
 import java.util.List;
 
@@ -13,7 +14,6 @@ import java.util.List;
 public abstract class PropertyType<T> {
 
     public static final PropertyType<Boolean> BOOLEAN = new BooleanProperty();
-    public static final PropertyType<Double>  DOUBLE  = new DoubleProperty();
     public static final PropertyType<Integer> INTEGER = new IntegerProperty();
     public static final PropertyType<String>  STRING  = new StringProperty();
     public static final PropertyType<List<String>> STRING_LIST = new StringListProperty();
@@ -39,16 +39,15 @@ public abstract class PropertyType<T> {
     }
 
     /**
-     * Return whether the property type should be wrapped in single quotes in YAML.
+     * Format the value as YAML.
      *
-     * @return True if single quotes should be used, false if not
+     * @param value The value to export
+     * @param simpleYaml YAML object (default)
+     * @param singleQuoteYaml YAML object set to use single quotes
+     * @return The generated YAML
      */
-    public boolean hasSingleQuotes() {
-        return false;
-    }
-
-    public boolean isList() {
-        return false;
+    public String toYaml(T value, Yaml simpleYaml, Yaml singleQuoteYaml) {
+        return simpleYaml.dump(value);
     }
 
 
@@ -62,16 +61,6 @@ public abstract class PropertyType<T> {
         }
     }
 
-    /**
-     * Double property.
-     */
-    private static final class DoubleProperty extends PropertyType<Double> {
-        @Override
-        public Double getFromFile(Property<Double> property, FileConfiguration configuration) {
-            return configuration.getDouble(property.getPath(), property.getDefaultValue());
-        }
-    }
-
     /**
      * Integer property.
      */
@@ -91,8 +80,8 @@ public abstract class PropertyType<T> {
             return configuration.getString(property.getPath(), property.getDefaultValue());
         }
         @Override
-        public boolean hasSingleQuotes() {
-            return true;
+        public String toYaml(String value, Yaml simpleYaml, Yaml singleQuoteYaml) {
+            return singleQuoteYaml.dump(value);
         }
     }
 
@@ -114,13 +103,13 @@ public abstract class PropertyType<T> {
         }
 
         @Override
-        public boolean hasSingleQuotes() {
-            return true;
-        }
-
-        @Override
-        public boolean isList() {
-            return true;
+        public String toYaml(List<String> value, Yaml simpleYaml, Yaml singleQuoteYaml) {
+            String yaml = singleQuoteYaml.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'
+            return value.isEmpty() ? yaml : "\n" + yaml;
         }
     }
 

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

@@ -4,6 +4,7 @@ import com.google.common.collect.ImmutableMap;
 import fr.xephi.authme.ReflectionTestUtils;
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.TestConfiguration;
+import fr.xephi.authme.settings.properties.TestEnum;
 import fr.xephi.authme.settings.propertymap.PropertyMap;
 import fr.xephi.authme.util.WrapperMock;
 import org.bukkit.configuration.file.YamlConfiguration;
@@ -48,12 +49,12 @@ public class NewSettingIntegrationTest {
         Map<Property<?>, Object> expectedValues = ImmutableMap.<Property<?>, Object>builder()
             .put(TestConfiguration.DURATION_IN_SECONDS, 22)
             .put(TestConfiguration.SYSTEM_NAME, "Custom sys name")
-            .put(TestConfiguration.RATIO_LIMIT, -4.1)
+            .put(TestConfiguration.RATIO_ORDER, TestEnum.FIRST)
             .put(TestConfiguration.RATIO_FIELDS, Arrays.asList("Australia", "Burundi", "Colombia"))
             .put(TestConfiguration.VERSION_NUMBER, 2492)
             .put(TestConfiguration.SKIP_BORING_FEATURES, false)
             .put(TestConfiguration.BORING_COLORS, Arrays.asList("beige", "gray"))
-            .put(TestConfiguration.DUST_LEVEL, 0.81)
+            .put(TestConfiguration.DUST_LEVEL, 2)
             .put(TestConfiguration.USE_COOL_FEATURES, true)
             .put(TestConfiguration.COOL_OPTIONS, Arrays.asList("Dinosaurs", "Explosions", "Big trucks"))
             .build();
@@ -81,12 +82,12 @@ public class NewSettingIntegrationTest {
         Map<Property<?>, Object> expectedValues = ImmutableMap.<Property<?>, Object>builder()
             .put(TestConfiguration.DURATION_IN_SECONDS, 22)
             .put(TestConfiguration.SYSTEM_NAME, "[TestDefaultValue]")
-            .put(TestConfiguration.RATIO_LIMIT, 3.0)
+            .put(TestConfiguration.RATIO_ORDER, TestEnum.SECOND)
             .put(TestConfiguration.RATIO_FIELDS, Arrays.asList("Australia", "Burundi", "Colombia"))
             .put(TestConfiguration.VERSION_NUMBER, 32046)
             .put(TestConfiguration.SKIP_BORING_FEATURES, false)
             .put(TestConfiguration.BORING_COLORS, Collections.EMPTY_LIST)
-            .put(TestConfiguration.DUST_LEVEL, 0.2)
+            .put(TestConfiguration.DUST_LEVEL, -1)
             .put(TestConfiguration.USE_COOL_FEATURES, false)
             .put(TestConfiguration.COOL_OPTIONS, Arrays.asList("Dinosaurs", "Explosions", "Big trucks"))
             .build();
@@ -129,12 +130,12 @@ public class NewSettingIntegrationTest {
         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_ORDER, TestEnum.FOURTH)
             .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.DUST_LEVEL, -1)
             .put(TestConfiguration.USE_COOL_FEATURES, true)
             .put(TestConfiguration.COOL_OPTIONS, Collections.EMPTY_LIST)
             .put(additionalProperties.get(0), additionalProperties.get(0).getDefaultValue())

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

@@ -2,6 +2,7 @@ package fr.xephi.authme.settings;
 
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.TestConfiguration;
+import fr.xephi.authme.settings.properties.TestEnum;
 import fr.xephi.authme.settings.propertymap.PropertyMap;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.junit.Test;
@@ -37,7 +38,7 @@ public class NewSettingTest {
 
         setReturnValue(file, TestConfiguration.VERSION_NUMBER, 20);
         setReturnValue(file, TestConfiguration.SKIP_BORING_FEATURES, true);
-        setReturnValue(file, TestConfiguration.RATIO_LIMIT, 4.25);
+        setReturnValue(file, TestConfiguration.RATIO_ORDER, TestEnum.THIRD);
         setReturnValue(file, TestConfiguration.SYSTEM_NAME, "myTestSys");
 
         // when / then
@@ -45,7 +46,7 @@ public class NewSettingTest {
 
         assertThat(settings.getProperty(TestConfiguration.VERSION_NUMBER), equalTo(20));
         assertThat(settings.getProperty(TestConfiguration.SKIP_BORING_FEATURES), equalTo(true));
-        assertThat(settings.getProperty(TestConfiguration.RATIO_LIMIT), equalTo(4.25));
+        assertThat(settings.getProperty(TestConfiguration.RATIO_ORDER), equalTo(TestEnum.THIRD));
         assertThat(settings.getProperty(TestConfiguration.SYSTEM_NAME), equalTo("myTestSys"));
 
         assertDefaultValue(TestConfiguration.DURATION_IN_SECONDS, settings);
@@ -60,8 +61,8 @@ public class NewSettingTest {
             when(config.getInt(eq(property.getPath()), anyInt())).thenReturn((Integer) value);
         } else if (value instanceof Boolean) {
             when(config.getBoolean(eq(property.getPath()), anyBoolean())).thenReturn((Boolean) value);
-        } else if (value instanceof Double) {
-            when(config.getDouble(eq(property.getPath()), anyDouble())).thenReturn((Double) value);
+        } else if (value instanceof Enum<?>) {
+            when(config.getString(property.getPath())).thenReturn(((Enum<?>) value).name());
         } else {
             throw new UnsupportedOperationException("Value has unsupported type '"
                 + (value == null ? "null" : value.getClass().getSimpleName()) + "'");

src/test/java/fr/xephi/authme/settings/domain/PropertyTypeTest.java

@@ -13,7 +13,6 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Matchers.anyBoolean;
-import static org.mockito.Matchers.anyDouble;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
@@ -33,8 +32,6 @@ public class PropertyTypeTest {
 
         when(configuration.getBoolean(eq("bool.path.test"), anyBoolean())).thenReturn(true);
         when(configuration.getBoolean(eq("bool.path.wrong"), anyBoolean())).thenAnswer(secondParameter());
-        when(configuration.getDouble(eq("double.path.test"), anyDouble())).thenReturn(-6.4);
-        when(configuration.getDouble(eq("double.path.wrong"), anyDouble())).thenAnswer(secondParameter());
         when(configuration.getInt(eq("int.path.test"), anyInt())).thenReturn(27);
         when(configuration.getInt(eq("int.path.wrong"), anyInt())).thenAnswer(secondParameter());
         when(configuration.getString(eq("str.path.test"), anyString())).thenReturn("Test value");
@@ -69,31 +66,6 @@ public class PropertyTypeTest {
         assertThat(result, equalTo(true));
     }
 
-    /* Double */
-    @Test
-    public void shouldGetDoubleValue() {
-        // given
-        Property<Double> property = Property.newProperty(PropertyType.DOUBLE, "double.path.test", 3.8);
-
-        // when
-        double result = property.getFromFile(configuration);
-
-        // then
-        assertThat(result, equalTo(-6.4));
-    }
-
-    @Test
-    public void shouldGetDoubleDefault() {
-        // given
-        Property<Double> property = Property.newProperty(PropertyType.DOUBLE, "double.path.wrong", 12.0);
-
-        // when
-        double result = property.getFromFile(configuration);
-
-        // then
-        assertThat(result, equalTo(12.0));
-    }
-
     /* Integer */
     @Test
     public void shouldGetIntValue() {

src/test/java/fr/xephi/authme/settings/properties/TestConfiguration.java

@@ -19,8 +19,8 @@ public final class TestConfiguration implements SettingsClass {
     public static final Property<String> SYSTEM_NAME =
         newProperty("test.systemName", "[TestDefaultValue]");
 
-    public static final Property<Double> RATIO_LIMIT =
-        newProperty(PropertyType.DOUBLE, "sample.ratio.limit", 3.0);
+    public static final Property<TestEnum> RATIO_ORDER =
+        newProperty(TestEnum.class, "sample.ratio.order", TestEnum.SECOND);
 
     public static final Property<List<String>> RATIO_FIELDS =
         newProperty(PropertyType.STRING_LIST, "sample.ratio.fields", "a", "b", "c");
@@ -34,8 +34,8 @@ public final class TestConfiguration implements SettingsClass {
     public static final Property<List<String>> BORING_COLORS =
         newProperty(PropertyType.STRING_LIST, "features.boring.colors");
 
-    public static final Property<Double> DUST_LEVEL =
-        newProperty(PropertyType.DOUBLE, "features.boring.dustLevel", 0.2);
+    public static final Property<Integer> DUST_LEVEL =
+        newProperty(PropertyType.INTEGER, "features.boring.dustLevel", -1);
 
     public static final Property<Boolean> USE_COOL_FEATURES =
         newProperty("features.cool.enabled", false);

src/test/java/fr/xephi/authme/settings/properties/TestEnum.java

@@ -0,0 +1,16 @@
+package fr.xephi.authme.settings.properties;
+
+/**
+ * Test enum used in {@link TestConfiguration}.
+ */
+public enum TestEnum {
+
+    FIRST,
+
+    SECOND,
+
+    THIRD,
+
+    FOURTH
+
+}

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

@@ -5,7 +5,7 @@ test:
     systemName: 'A ''test'' name'
 sample:
     ratio:
-        limit: -41.8
+        order: Fourth
         fields:
             - Australia\
             - '	Burundi'''
@@ -23,7 +23,7 @@ features:
             - |
                 gray
                 with new lines
-        # dustLevel: 0.81 <-- missing property triggering rewrite
+        # dustLevel: 8 <-- missing property triggering rewrite
     cool:
         enabled: yes
         options: []

src/test/resources/config-incomplete-sample.yml

@@ -6,7 +6,7 @@ test:
 #    systemName: 'Custom sys name'
 sample:
     ratio:
-#        limit: 3.0
+#        order: 'THIRD'
         fields:
             - 'Australia'
             - 'Burundi'
@@ -18,7 +18,7 @@ features:
 #        colors:
 #            - 'beige'
 #            - 'gray'
-#        dustLevel: 0.81
+#        dustLevel: 1
     cool:
 #        enabled: true
         options:

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

@@ -6,7 +6,7 @@ test:
     systemName: 'Custom sys name'
 sample:
     ratio:
-        limit: -4.1
+        order: 'first'
         fields:
             - 'Australia'
             - 'Burundi'
@@ -18,7 +18,7 @@ features:
         colors:
             - 'beige'
             - 'gray'
-        dustLevel: 0.81
+        dustLevel: 2
     cool:
         enabled: true
         options: