Improve the item meta deserialization code-style

Fixed the ItemMetaFireworkTest

Add set power unit tests for FireworkMeta

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaBook.java

@@ -259,9 +259,4 @@ class CraftMetaBook extends CraftMetaItem implements BookMeta {
 
         return builder;
     }
-
-    @Override
-    SerializableMeta.Deserializers deserializer() {
-        return SerializableMeta.Deserializers.BOOK;
-    }
 }

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaCharge.java

@@ -8,7 +8,6 @@ import org.bukkit.FireworkEffect;
 import org.bukkit.Material;
 import org.bukkit.configuration.serialization.DelegateDeserialization;
 import org.bukkit.craftbukkit.inventory.CraftMetaItem.SerializableMeta;
-import org.bukkit.craftbukkit.inventory.CraftMetaItem.SerializableMeta.Deserializers;
 import org.bukkit.inventory.meta.FireworkEffectMeta;
 
 import com.google.common.collect.ImmutableMap.Builder;
@@ -126,9 +125,4 @@ class CraftMetaCharge extends CraftMetaItem implements FireworkEffectMeta {
 
         return builder;
     }
-
-    @Override
-    Deserializers deserializer() {
-        return Deserializers.FIREWORK_EFFECT;
-    }
 }

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaEnchantedBook.java

@@ -8,7 +8,6 @@ import net.minecraft.server.NBTTagCompound;
 import org.bukkit.Material;
 import org.bukkit.configuration.serialization.DelegateDeserialization;
 import org.bukkit.craftbukkit.inventory.CraftMetaItem.SerializableMeta;
-import org.bukkit.craftbukkit.inventory.CraftMetaItem.SerializableMeta.Deserializers;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.inventory.meta.EnchantmentStorageMeta;
 
@@ -123,11 +122,6 @@ class CraftMetaEnchantedBook extends CraftMetaItem implements EnchantmentStorage
         return builder;
     }
 
-    @Override
-    Deserializers deserializer() {
-        return Deserializers.ENCHANTED;
-    }
-
     boolean isEnchantedEmpty() {
         return !hasStoredEnchants();
     }

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaFirework.java

@@ -17,7 +17,6 @@ import org.bukkit.configuration.serialization.DelegateDeserialization;
 import org.bukkit.craftbukkit.inventory.CraftMetaItem.ItemMetaKey.Specific;
 import org.bukkit.craftbukkit.inventory.CraftMetaItem.ItemMetaKey.Specific.To;
 import org.bukkit.craftbukkit.inventory.CraftMetaItem.SerializableMeta;
-import org.bukkit.craftbukkit.inventory.CraftMetaItem.SerializableMeta.Deserializers;
 import org.bukkit.inventory.meta.FireworkMeta;
 
 import com.google.common.collect.ImmutableList;
@@ -314,11 +313,6 @@ class CraftMetaFirework extends CraftMetaItem implements FireworkMeta {
         return builder;
     }
 
-    @Override
-    Deserializers deserializer() {
-        return Deserializers.FIREWORK;
-    }
-
     @Override
     public CraftMetaFirework clone() {
         CraftMetaFirework meta = (CraftMetaFirework) super.clone();

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaItem.java

@@ -4,6 +4,8 @@ import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -86,79 +88,55 @@ class CraftMetaItem implements ItemMeta, Repairable {
     public static class SerializableMeta implements ConfigurationSerializable {
         static final String TYPE_FIELD = "meta-type";
 
-        enum Deserializers {
+        static final ImmutableMap<Class<? extends CraftMetaItem>, String> classMap;
-            BOOK {
+        static final ImmutableMap<String, Constructor<? extends CraftMetaItem>> constructorMap;
-                @Override
-                CraftMetaBook deserialize(Map<String, Object> map) {
-                    return new CraftMetaBook(map);
-                }
-            },
-            SKULL {
-                @Override
-                CraftMetaSkull deserialize(Map<String, Object> map) {
-                    return new CraftMetaSkull(map);
-                }
-            },
-            LEATHER_ARMOR {
-                @Override
-                CraftMetaLeatherArmor deserialize(Map<String, Object> map) {
-                    return new CraftMetaLeatherArmor(map);
-                }
-            },
-            MAP {
-                @Override
-                CraftMetaMap deserialize(Map<String, Object> map) {
-                    return new CraftMetaMap(map);
-                }
-            },
-            POTION {
-                @Override
-                CraftMetaPotion deserialize(Map<String, Object> map) {
-                    return new CraftMetaPotion(map);
-                }
-            },
-            ENCHANTED {
-                @Override
-                CraftMetaEnchantedBook deserialize(Map<String, Object> map) {
-                    return new CraftMetaEnchantedBook(map);
-                }
-            },
-            FIREWORK {
-                @Override
-                CraftMetaFirework deserialize(Map<String, Object> map) {
-                    return new CraftMetaFirework(map);
-                }
-            },
-            FIREWORK_EFFECT {
-                @Override
-                CraftMetaCharge deserialize(Map<String, Object> map) {
-                    return new CraftMetaCharge(map);
-                }
-            },
-            UNSPECIFIC {
-                @Override
-                CraftMetaItem deserialize(Map<String, Object> map) {
-                    return new CraftMetaItem(map);
-                }
-            };
 
-            abstract CraftMetaItem deserialize(Map<String, Object> map);
+        static {
+            classMap = ImmutableMap.<Class<? extends CraftMetaItem>, String>builder()
+                    .put(CraftMetaBook.class, "BOOK")
+                    .put(CraftMetaSkull.class, "SKULL")
+                    .put(CraftMetaLeatherArmor.class, "LEATHER_ARMOR")
+                    .put(CraftMetaMap.class, "MAP")
+                    .put(CraftMetaPotion.class, "POTION")
+                    .put(CraftMetaEnchantedBook.class, "ENCHANTED")
+                    .put(CraftMetaFirework.class, "FIREWORK")
+                    .put(CraftMetaCharge.class, "FIREWORK_EFFECT")
+                    .put(CraftMetaItem.class, "UNSPECIFIC")
+                    .build();
+
+            final ImmutableMap.Builder<String, Constructor<? extends CraftMetaItem>> classConstructorBuilder = ImmutableMap.builder();
+            for (Map.Entry<Class<? extends CraftMetaItem>, String> mapping : classMap.entrySet()) {
+                try {
+                    classConstructorBuilder.put(mapping.getValue(), mapping.getKey().getDeclaredConstructor(Map.class));
+                } catch (NoSuchMethodException e) {
+                    throw new AssertionError(e);
+                }
+            }
+            constructorMap = classConstructorBuilder.build();
         }
 
         private SerializableMeta() {
         }
 
-        public static ItemMeta deserialize(Map<String, Object> map) {
+        public static ItemMeta deserialize(Map<String, Object> map) throws Throwable {
             Validate.notNull(map, "Cannot deserialize null map");
 
             String type = getString(map, TYPE_FIELD, false);
-            Deserializers deserializer = Deserializers.valueOf(type);
+            Constructor<? extends CraftMetaItem> constructor = constructorMap.get(type);
 
-            if (deserializer == null) {
+            if (constructor == null) {
                 throw new IllegalArgumentException(type + " is not a valid " + TYPE_FIELD);
             }
 
-            return deserializer.deserialize(map);
+            try {
+                return constructor.newInstance(map);
+            } catch (final InstantiationException e) {
+                throw new AssertionError(e);
+            } catch (final IllegalAccessException e) {
+                throw new AssertionError(e);
+            } catch (final InvocationTargetException e) {
+                throw e.getCause();
+            }
         }
 
         public Map<String, Object> serialize() {
@@ -516,7 +494,7 @@ class CraftMetaItem implements ItemMeta, Repairable {
 
     public final Map<String, Object> serialize() {
         ImmutableMap.Builder<String, Object> map = ImmutableMap.builder();
-        map.put(SerializableMeta.TYPE_FIELD, deserializer().name());
+        map.put(SerializableMeta.TYPE_FIELD, SerializableMeta.classMap.get(getClass()));
         serialize(map);
         return map.build();
     }
@@ -553,11 +531,6 @@ class CraftMetaItem implements ItemMeta, Repairable {
         builder.put(key.BUKKIT, enchants.build());
     }
 
-    @Overridden
-    SerializableMeta.Deserializers deserializer() {
-        return SerializableMeta.Deserializers.UNSPECIFIC;
-    }
-
     static void safelyAdd(Iterable<?> addFrom, Collection<String> addTo, int maxItemLength) {
         if (addFrom == null) {
             return;
@@ -584,6 +557,6 @@ class CraftMetaItem implements ItemMeta, Repairable {
 
     @Override
     public final String toString() {
-        return deserializer().toString() + "_META:" + serialize(); // TODO: cry
+        return SerializableMeta.classMap.get(getClass()) + "_META:" + serialize(); // TODO: cry
     }
 }

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaLeatherArmor.java

@@ -46,6 +46,7 @@ class CraftMetaLeatherArmor extends CraftMetaItem implements LeatherArmorMeta {
         setColor(SerializableMeta.getObject(Color.class, map, COLOR.BUKKIT, true));
     }
 
+    @Override
     void applyToItem(NBTTagCompound itemTag) {
         super.applyToItem(itemTag);
 
@@ -63,6 +64,7 @@ class CraftMetaLeatherArmor extends CraftMetaItem implements LeatherArmorMeta {
         return !(hasColor());
     }
 
+    @Override
     boolean applicableTo(Material type) {
         switch(type) {
             case LEATHER_HELMET:
@@ -103,11 +105,6 @@ class CraftMetaLeatherArmor extends CraftMetaItem implements LeatherArmorMeta {
         return builder;
     }
 
-    @Override
-    SerializableMeta.Deserializers deserializer() {
-        return SerializableMeta.Deserializers.LEATHER_ARMOR;
-    }
-
     @Override
     boolean equalsCommon(CraftMetaItem meta) {
         if (!super.equalsCommon(meta)) {
@@ -126,6 +123,7 @@ class CraftMetaLeatherArmor extends CraftMetaItem implements LeatherArmorMeta {
         return super.notUncommon(meta) && (meta instanceof CraftMetaLeatherArmor || isLeatherArmorEmpty());
     }
 
+    @Override
     int applyHash() {
         final int original;
         int hash = original = super.applyHash();

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaMap.java

@@ -132,9 +132,4 @@ class CraftMetaMap extends CraftMetaItem implements MapMeta {
 
         return builder;
     }
-
-    @Override
-    SerializableMeta.Deserializers deserializer() {
-        return SerializableMeta.Deserializers.MAP;
-    }
 }

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaPotion.java

@@ -252,9 +252,4 @@ class CraftMetaPotion extends CraftMetaItem implements PotionMeta {
 
         return builder;
     }
-
-    @Override
-    SerializableMeta.Deserializers deserializer() {
-        return SerializableMeta.Deserializers.POTION;
-    }
 }

src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaSkull.java

@@ -126,9 +126,4 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
         }
         return builder;
     }
-
-    @Override
-    SerializableMeta.Deserializers deserializer() {
-        return SerializableMeta.Deserializers.SKULL;
-    }
 }

src/test/java/org/bukkit/craftbukkit/inventory/ItemMetaTest.java

@@ -6,6 +6,7 @@ import static org.hamcrest.Matchers.*;
 import java.util.Arrays;
 import java.util.List;
 
+import org.bukkit.Bukkit;
 import org.bukkit.Color;
 import org.bukkit.FireworkEffect;
 import org.bukkit.Material;
@@ -30,6 +31,41 @@ import org.junit.Test;
 
 public class ItemMetaTest extends AbstractTestingBase {
 
+    static final int MAX_FIREWORK_POWER = 127; // Please update ItemStackFireworkTest if/when this gets changed.
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testPowerLimitExact() {
+        newFireworkMeta().setPower(MAX_FIREWORK_POWER + 1);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testPowerLimitMax() {
+        newFireworkMeta().setPower(Integer.MAX_VALUE);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testPowerLimitMin() {
+        newFireworkMeta().setPower(Integer.MIN_VALUE);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testPowerLimitNegative() {
+        newFireworkMeta().setPower(-1);
+    }
+
+    @Test
+    public void testPowers() {
+        for (int i = 0; i <= MAX_FIREWORK_POWER; i++) {
+            FireworkMeta firework = newFireworkMeta();
+            firework.setPower(i);
+            assertThat(String.valueOf(i), firework.getPower(), is(i));
+        }
+    }
+
+    private static FireworkMeta newFireworkMeta() {
+        return ((FireworkMeta) Bukkit.getItemFactory().getItemMeta(Material.FIREWORK));
+    }
+
     @Test
     public void testCrazyEquality() {
         CraftItemStack craft = CraftItemStack.asCraftCopy(new ItemStack(1));

src/test/java/org/bukkit/craftbukkit/inventory/ItemStackFireworkTest.java

@@ -129,7 +129,7 @@ public class ItemStackFireworkTest extends ItemStackTest {
                     new Operator() {
                         public ItemStack operate(ItemStack cleanStack) {
                             FireworkMeta meta = (FireworkMeta) cleanStack.getItemMeta();
-                            meta.setPower(150);
+                            meta.setPower(127);
                             cleanStack.setItemMeta(meta);
                             return cleanStack;
                         }
@@ -148,7 +148,7 @@ public class ItemStackFireworkTest extends ItemStackTest {
                     new Operator() {
                         public ItemStack operate(ItemStack cleanStack) {
                             FireworkMeta meta = (FireworkMeta) cleanStack.getItemMeta();
-                            meta.setPower(200);
+                            meta.setPower(42);
                             cleanStack.setItemMeta(meta);
                             return cleanStack;
                         }