[Bleeding] MetadataBase now properly takes the metadata key into account when computing hasMetadata(). Addresses BUKKIT-1211

src/main/java/org/bukkit/metadata/MetadataStoreBase.java

@@ -6,22 +6,22 @@ import java.util.*;
 
 public abstract class MetadataStoreBase<T> {
     private Map<String, List<MetadataValue>> metadataMap = new HashMap<String, List<MetadataValue>>();
-    private WeakHashMap<T, String> disambiguationCache = new WeakHashMap<T, String>();
+    private WeakHashMap<T, Map<String, String>> disambiguationCache = new WeakHashMap<T, Map<String, String>>();
 
     /**
      * Adds a metadata value to an object. Each metadata value is owned by a specific{@link Plugin}.
      * If a plugin has already added a metadata value to an object, that value
      * will be replaced with the value of {@code newMetadataValue}. Multiple plugins can set independent values for
      * the same {@code metadataKey} without conflict.
-     *
+     * <p/>
      * Implementation note: I considered using a {@link java.util.concurrent.locks.ReadWriteLock} for controlling
      * access to {@code metadataMap}, but decided that the added overhead wasn't worth the finer grained access control.
      * Bukkit is almost entirely single threaded so locking overhead shouldn't pose a problem.
      *
-     * @see MetadataStore#setMetadata(Object, String, MetadataValue)
-     * @param subject The object receiving the metadata.
-     * @param metadataKey A unique key to identify this metadata.
+     * @param subject          The object receiving the metadata.
+     * @param metadataKey      A unique key to identify this metadata.
      * @param newMetadataValue The metadata value to apply.
+     * @see MetadataStore#setMetadata(Object, String, MetadataValue)
      */
     public synchronized void setMetadata(T subject, String metadataKey, MetadataValue newMetadataValue) {
         String key = cachedDisambiguate(subject, metadataKey);
@@ -45,10 +45,10 @@ public abstract class MetadataStoreBase<T> {
      * Returns all metadata values attached to an object. If multiple plugins have attached metadata, each will value
      * will be included.
      *
-     * @see MetadataStore#getMetadata(Object, String)
-     * @param subject the object being interrogated.
+     * @param subject     the object being interrogated.
      * @param metadataKey the unique metadata key being sought.
      * @return A list of values, one for each plugin that has set the requested value.
+     * @see MetadataStore#getMetadata(Object, String)
      */
     public synchronized List<MetadataValue> getMetadata(T subject, String metadataKey) {
         String key = cachedDisambiguate(subject, metadataKey);
@@ -62,7 +62,7 @@ public abstract class MetadataStoreBase<T> {
     /**
      * Tests to see if a metadata attribute has been set on an object.
      *
-     * @param subject the object upon which the has-metadata test is performed.
+     * @param subject     the object upon which the has-metadata test is performed.
      * @param metadataKey the unique metadata key being queried.
      * @return the existence of the metadataKey within subject.
      */
@@ -74,10 +74,10 @@ public abstract class MetadataStoreBase<T> {
     /**
      * Removes a metadata item owned by a plugin from a subject.
      *
-     * @see MetadataStore#removeMetadata(Object, String, org.bukkit.plugin.Plugin)
-     * @param subject the object to remove the metadata from.
-     * @param metadataKey the unique metadata key identifying the metadata to remove.
+     * @param subject      the object to remove the metadata from.
+     * @param metadataKey  the unique metadata key identifying the metadata to remove.
      * @param owningPlugin the plugin attempting to remove a metadata item.
+     * @see MetadataStore#removeMetadata(Object, String, org.bukkit.plugin.Plugin)
      */
     public synchronized void removeMetadata(T subject, String metadataKey, Plugin owningPlugin) {
         String key = cachedDisambiguate(subject, metadataKey);
@@ -94,11 +94,11 @@ public abstract class MetadataStoreBase<T> {
      * Invalidates all metadata in the metadata store that originates from the given plugin. Doing this will force
      * each invalidated metadata item to be recalculated the next time it is accessed.
      *
-     * @see MetadataStore#invalidateAll(org.bukkit.plugin.Plugin)
      * @param owningPlugin the plugin requesting the invalidation.
+     * @see MetadataStore#invalidateAll(org.bukkit.plugin.Plugin)
      */
     public synchronized void invalidateAll(Plugin owningPlugin) {
-        if(owningPlugin == null) {
+        if (owningPlugin == null) {
             throw new IllegalArgumentException("owningPlugin cannot be null");
         }
 
@@ -116,16 +116,20 @@ public abstract class MetadataStoreBase<T> {
      * <a href="http://www.codeinstructions.com/2008/09/weakhashmap-is-not-cache-understanding.html">canonical list</a>
      * of disambiguation strings for objects in memory. When those objects are garbage collected, the disambiguation string
      * in the list is aggressively garbage collected as well.
-     * @param subject The object for which this key is being generated.
+     *
+     * @param subject     The object for which this key is being generated.
      * @param metadataKey The name identifying the metadata value.
      * @return a unique metadata key for the given subject.
      */
     private String cachedDisambiguate(T subject, String metadataKey) {
-        if (disambiguationCache.containsKey(subject)) {
-            return disambiguationCache.get(subject);
+        if (disambiguationCache.containsKey(subject) && disambiguationCache.get(subject).containsKey(metadataKey)) {
+            return disambiguationCache.get(subject).get(metadataKey);
         } else {
+            if (!disambiguationCache.containsKey(subject)) {
+                disambiguationCache.put(subject, new HashMap<String, String>());
+            }
             String disambiguation = disambiguate(subject, metadataKey);
-            disambiguationCache.put(subject, disambiguation);
+            disambiguationCache.get(subject).put(metadataKey, disambiguation);
             return disambiguation;
         }
     }
@@ -136,7 +140,7 @@ public abstract class MetadataStoreBase<T> {
      * same unique name. For example, two Player objects must generate the same string if they represent the same player,
      * even if the objects would fail a reference equality test.
      *
-     * @param subject The object for which this key is being generated.
+     * @param subject     The object for which this key is being generated.
      * @param metadataKey The name identifying the metadata value.
      * @return a unique metadata key for the given subject.
      */

src/test/java/org/bukkit/metadata/MetadataStoreTest.java

@@ -105,6 +105,13 @@ public class MetadataStoreTest {
         assertEquals(1, subject.getMetadata("subject", "key").size());
         assertEquals(10, subject.getMetadata("subject", "key").get(0).value());
     }
+    
+    @Test
+    public void testHasMetadata() {
+        subject.setMetadata("subject", "key", new FixedMetadataValue(pluginX, 10));
+        assertTrue(subject.hasMetadata("subject", "key"));
+        assertFalse(subject.hasMetadata("subject", "otherKey"));
+    }
 
     private class StringMetadataStore extends MetadataStoreBase<String> implements MetadataStore<String> {
         @Override