Configurations now properly support lists of serializable objects, and ItemStack is properly serializable. Big thanks to GICodeWarrior for the PR. This fixes BUKKIT-425

7 files changed, 143 insertions, 103 deletions

diff --git a/src/main/java/org/bukkit/configuration/MemorySection.java b/src/main/java/org/bukkit/configuration/MemorySection.java
index 8e16eb7e..036b2258 100644
--- a/src/main/java/org/bukkit/configuration/MemorySection.java
+++ b/src/main/java/org/bukkit/configuration/MemorySection.java
@@ -189,7 +189,7 @@ public class MemorySection implements ConfigurationSection {
             if (value == null) {
                 map.remove(key);
             } else {
-                map.put(key, prepForStorage(value));
+                map.put(key, value);
             }
         } else {
             section.set(key, value);
@@ -905,20 +905,6 @@ public class MemorySection implements ConfigurationSection {
         return val instanceof ConfigurationSection;
     }
 
-    protected Object prepForStorage(Object input) {
-        if (input == null) {
-            throw new IllegalArgumentException("Cannot store null");
-        }
-
-        if (isPrimitiveWrapper(input) || isNaturallyStorable(input)) {
-            return input;
-        } else if (input instanceof ConfigurationSerializable) {
-            return input;
-        }
-
-        throw new IllegalArgumentException("Cannot store " + input + " into " + this + ", unsupported class");
-    }
-
     protected boolean isPrimitiveWrapper(Object input) {
         return input instanceof Integer || input instanceof Boolean ||
                 input instanceof Character || input instanceof Byte ||
@@ -926,12 +912,6 @@ public class MemorySection implements ConfigurationSection {
                 input instanceof Long || input instanceof Float;
     }
 
-    protected boolean isNaturallyStorable(Object input) {
-        return input instanceof List || input instanceof Iterable ||
-                input instanceof String || input instanceof File ||
-                input instanceof Enum;
-    }
-
     protected Object getDefault(String path) {
         if (path == null) {
             throw new IllegalArgumentException("Path cannot be null");
diff --git a/src/main/java/org/bukkit/configuration/file/YamlConfiguration.java b/src/main/java/org/bukkit/configuration/file/YamlConfiguration.java
index a15f69b7..6349dc9a 100644
--- a/src/main/java/org/bukkit/configuration/file/YamlConfiguration.java
+++ b/src/main/java/org/bukkit/configuration/file/YamlConfiguration.java
@@ -27,8 +27,8 @@ public class YamlConfiguration extends FileConfiguration {
     protected static final String COMMENT_PREFIX = "# ";
     protected static final String BLANK_CONFIG = "{}\n";
     private final DumperOptions yamlOptions = new DumperOptions();
-    private final Representer yamlRepresenter = new Representer();
-    private final Yaml yaml = new Yaml(new SafeConstructor(), yamlRepresenter, yamlOptions);
+    private final Representer yamlRepresenter = new YamlRepresenter();
+    private final Yaml yaml = new Yaml(new YamlConstructor(), yamlRepresenter, yamlOptions);
 
     @Override
     public String saveToString() {
@@ -38,10 +38,8 @@ public class YamlConfiguration extends FileConfiguration {
         yamlOptions.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);
         yamlRepresenter.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK);
 
-        serializeValues(output, getValues(false));
-
         String header = buildHeader();
-        String dump = yaml.dump(output);
+        String dump = yaml.dump(getValues(false));
 
         if (dump.equals(BLANK_CONFIG)) {
             dump = "";
@@ -56,90 +54,34 @@ public class YamlConfiguration extends FileConfiguration {
             throw new IllegalArgumentException("Contents cannot be null");
         }
 
-        @SuppressWarnings("unchecked")
-        Map<Object, Object> input = (Map<Object, Object>) yaml.load(contents);
-        int size = (input == null) ? 0 : input.size();
-        Map<String, Object> result = new LinkedHashMap<String, Object>(size);
-
-        if (size > 0) {
-            for (Map.Entry<Object, Object> entry : input.entrySet()) {
-                result.put(entry.getKey().toString(), entry.getValue());
-            }
+        Map<Object, Object> input;
+        try {
+            input = (Map<Object, Object>) yaml.load(contents);
+        } catch (YAMLException e) {
+            throw new InvalidConfigurationException(e);
+        } catch (ClassCastException e) {
+            throw new InvalidConfigurationException("Top level is not a Map.");
         }
 
         String header = parseHeader(contents);
-
         if (header.length() > 0) {
             options().header(header);
         }
 
-        deserializeValues(result, this);
-    }
-
-    protected void deserializeValues(Map<String, Object> input, ConfigurationSection section) throws InvalidConfigurationException {
-        if (input == null) {
-            return;
-        }
-
-        for (Map.Entry<String, Object> entry : input.entrySet()) {
-            Object value = entry.getValue();
-
-            if (value instanceof Map) {
-                @SuppressWarnings("unchecked")
-                Map<Object, Object> subinput = (Map<Object, Object>) value;
-                int size = (subinput == null) ? 0 : subinput.size();
-                Map<String, Object> subvalues = new LinkedHashMap<String, Object>(size);
-
-                if (size > 0) {
-                    for (Map.Entry<Object, Object> subentry : subinput.entrySet()) {
-                        subvalues.put(subentry.getKey().toString(), subentry.getValue());
-                    }
-                }
-
-                if (subvalues.containsKey(ConfigurationSerialization.SERIALIZED_TYPE_KEY)) {
-                    try {
-                        ConfigurationSerializable serializable = ConfigurationSerialization.deserializeObject(subvalues);
-                        section.set(entry.getKey(), serializable);
-                    } catch (IllegalArgumentException ex) {
-                        throw new InvalidConfigurationException("Could not deserialize object", ex);
-                    }
-                } else {
-                    ConfigurationSection subsection = section.createSection(entry.getKey());
-                    deserializeValues(subvalues, subsection);
-                }
-            } else {
-                section.set(entry.getKey(), entry.getValue());
-            }
+        if (input != null) {
+            convertMapsToSections(input, this);
         }
     }
 
-    protected void serializeValues(Map<String, Object> output, Map<String, Object> input) {
-        if (input == null) {
-            return;
-        }
-
-        for (Map.Entry<String, Object> entry : input.entrySet()) {
+    protected void convertMapsToSections(Map<Object, Object> input, ConfigurationSection section) {
+        for (Map.Entry<Object, Object> entry : input.entrySet()) {
+            String key = entry.getKey().toString();
             Object value = entry.getValue();
 
-            if (value instanceof ConfigurationSection) {
-                ConfigurationSection subsection = (ConfigurationSection) entry.getValue();
-                Map<String, Object> subvalues = new LinkedHashMap<String, Object>();
-
-                serializeValues(subvalues, subsection.getValues(false));
-                value = subvalues;
-            } else if (value instanceof ConfigurationSerializable) {
-                ConfigurationSerializable serializable = (ConfigurationSerializable) value;
-                Map<String, Object> subvalues = new LinkedHashMap<String, Object>();
-                subvalues.put(ConfigurationSerialization.SERIALIZED_TYPE_KEY, ConfigurationSerialization.getAlias(serializable.getClass()));
-
-                serializeValues(subvalues, serializable.serialize());
-                value = subvalues;
-            } else if ((!isPrimitiveWrapper(value)) && (!isNaturallyStorable(value))) {
-                throw new IllegalStateException("Configuration contains non-serializable values, cannot process");
-            }
-
-            if (value != null) {
-                output.put(entry.getKey(), value);
+            if (value instanceof Map<?, ?>) {
+                convertMapsToSections((Map<Object, Object>) value, section.createSection(key));
+            } else {
+                section.set(key, value);
             }
         }
     }
diff --git a/src/main/java/org/bukkit/configuration/file/YamlConstructor.java b/src/main/java/org/bukkit/configuration/file/YamlConstructor.java
new file mode 100644
index 00000000..678730eb
--- /dev/null
+++ b/src/main/java/org/bukkit/configuration/file/YamlConstructor.java
@@ -0,0 +1,47 @@
+package org.bukkit.configuration.file;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.yaml.snakeyaml.nodes.Node;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
+import org.yaml.snakeyaml.error.YAMLException;
+import org.yaml.snakeyaml.nodes.Tag;
+
+import org.bukkit.configuration.serialization.ConfigurationSerialization;
+
+public class YamlConstructor extends SafeConstructor {
+
+    public YamlConstructor() {
+        this.yamlConstructors.put(Tag.MAP, new ConstructCustomObject());
+    }
+
+    private class ConstructCustomObject extends ConstructYamlMap {
+        public Object construct(Node node) {
+            if (node.isTwoStepsConstruction()) {
+                throw new YAMLException("Unexpected referential mapping structure. Node: " + node);
+            }
+
+            Map<Object, Object> raw = (Map<Object, Object>) super.construct(node);
+
+            if (raw.containsKey(ConfigurationSerialization.SERIALIZED_TYPE_KEY)) {
+                Map<String, Object> typed = new LinkedHashMap<String, Object>(raw.size());
+                for (Map.Entry<Object, Object> entry : raw.entrySet()) {
+                    typed.put(entry.getKey().toString(), entry.getValue());
+                }
+
+                try {
+                    return ConfigurationSerialization.deserializeObject(typed);
+                } catch (IllegalArgumentException ex) {
+                    throw new YAMLException("Could not deserialize object", ex);
+                }
+            }
+
+            return raw;
+        }
+
+        public void construct2ndStep(Node node, Object object) {
+            throw new YAMLException("Unexpected referential mapping structure. Node: " + node);
+        }
+    }
+}
diff --git a/src/main/java/org/bukkit/configuration/file/YamlRepresenter.java b/src/main/java/org/bukkit/configuration/file/YamlRepresenter.java
new file mode 100644
index 00000000..fbdc181c
--- /dev/null
+++ b/src/main/java/org/bukkit/configuration/file/YamlRepresenter.java
@@ -0,0 +1,40 @@
+
+package org.bukkit.configuration.file;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.bukkit.configuration.ConfigurationSection;
+import org.bukkit.configuration.serialization.ConfigurationSerializable;
+import org.bukkit.configuration.serialization.ConfigurationSerialization;
+
+import org.yaml.snakeyaml.nodes.Node;
+import org.yaml.snakeyaml.representer.Represent;
+import org.yaml.snakeyaml.representer.Representer;
+
+public class YamlRepresenter extends Representer {
+
+    public YamlRepresenter() {
+        this.multiRepresenters.put(ConfigurationSection.class, new RepresentConfigurationSection());
+        this.multiRepresenters.put(ConfigurationSerializable.class, new RepresentConfigurationSerializable());
+    }
+
+    private class RepresentConfigurationSection extends RepresentMap {
+        @SuppressWarnings("unchecked")
+        public Node representData(Object data) {
+            return super.representData(((ConfigurationSection) data).getValues(false));
+        }
+    }
+
+    private class RepresentConfigurationSerializable extends RepresentMap {
+        @SuppressWarnings("unchecked")
+        public Node representData(Object data) {
+            ConfigurationSerializable serializable = (ConfigurationSerializable) data;
+            Map<String, Object> values = new LinkedHashMap<String, Object>();
+            values.put(ConfigurationSerialization.SERIALIZED_TYPE_KEY, ConfigurationSerialization.getAlias(serializable.getClass()));
+            values.putAll(serializable.serialize());
+
+            return super.representData(values);
+        }
+    }
+}
diff --git a/src/main/java/org/bukkit/configuration/serialization/ConfigurationSerialization.java b/src/main/java/org/bukkit/configuration/serialization/ConfigurationSerialization.java
index 8bf50b95..3b2b67b9 100644
--- a/src/main/java/org/bukkit/configuration/serialization/ConfigurationSerialization.java
+++ b/src/main/java/org/bukkit/configuration/serialization/ConfigurationSerialization.java
@@ -7,6 +7,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import org.bukkit.inventory.ItemStack;
 import org.bukkit.util.BlockVector;
 import org.bukkit.util.Vector;
 
@@ -21,6 +22,7 @@ public class ConfigurationSerialization {
     static {
         registerClass(Vector.class);
         registerClass(BlockVector.class);
+        registerClass(ItemStack.class);
     }
 
     protected ConfigurationSerialization(Class<? extends ConfigurationSerializable> clazz) {
@@ -250,4 +252,4 @@ public class ConfigurationSerialization {
 
         return clazz.getName();
     }
-}
\ No newline at end of file
+}
diff --git a/src/main/java/org/bukkit/inventory/ItemStack.java b/src/main/java/org/bukkit/inventory/ItemStack.java
index c856b3ae..7a424c79 100644
--- a/src/main/java/org/bukkit/inventory/ItemStack.java
+++ b/src/main/java/org/bukkit/inventory/ItemStack.java
@@ -329,7 +329,7 @@ public class ItemStack implements ConfigurationSerializable {
     public Map<String, Object> serialize() {
         Map<String, Object> result = new LinkedHashMap<String, Object>();
 
-        result.put("type", getType());
+        result.put("type", getType().name());
 
         if (durability != 0) {
             result.put("damage", durability);
@@ -356,18 +356,18 @@ public class ItemStack implements ConfigurationSerializable {
 
     public static ItemStack deserialize(Map<String, Object> args) {
         Material type = Material.getMaterial((String) args.get("type"));
-        short damage = 0;
+        int damage = 0;
         int amount = 1;
 
         if (args.containsKey("damage")) {
-            damage = (Short) args.get("damage");
+            damage = (Integer) args.get("damage");
         }
 
         if (args.containsKey("amount")) {
             amount = (Integer) args.get("amount");
         }
 
-        ItemStack result = new ItemStack(type, amount, damage);
+        ItemStack result = new ItemStack(type, amount, (short) damage);
 
         if (args.containsKey("enchantments")) {
             Object raw = args.get("enchantments");
diff --git a/src/test/java/org/bukkit/configuration/file/YamlConfigurationTest.java b/src/test/java/org/bukkit/configuration/file/YamlConfigurationTest.java
index e6b90756..edb04486 100644
--- a/src/test/java/org/bukkit/configuration/file/YamlConfigurationTest.java
+++ b/src/test/java/org/bukkit/configuration/file/YamlConfigurationTest.java
@@ -1,5 +1,12 @@
 package org.bukkit.configuration.file;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import org.bukkit.configuration.InvalidConfigurationException;
+import org.bukkit.configuration.serialization.ConfigurationSerialization;
+import org.bukkit.inventory.ItemStack;
+
 import org.junit.Test;
 import static org.junit.Assert.*;
 
@@ -52,4 +59,26 @@ public  class YamlConfigurationTest extends FileConfigurationTest {
         
         assertEquals(expected, result);
     }
+
+    @Test
+    public void testSaveRestoreCompositeList() throws InvalidConfigurationException {
+        YamlConfiguration out = getConfig();
+
+        List<ItemStack> stacks = new ArrayList<ItemStack>();
+        stacks.add(new ItemStack(1));
+        stacks.add(new ItemStack(2));
+        stacks.add(new ItemStack(3));
+
+        out.set("composite-list.abc.def", stacks);
+        String yaml = out.saveToString();
+
+        YamlConfiguration in = new YamlConfiguration();
+        in.loadFromString(yaml);
+        List<Object> raw = in.getList("composite-list.abc.def");
+
+        assertEquals(stacks.size(), raw.size());
+        assertEquals(stacks.get(0), raw.get(0));
+        assertEquals(stacks.get(1), raw.get(1));
+        assertEquals(stacks.get(2), raw.get(2));
+    }
 }