OF-1156: Be consistent with the contents of DefaultCache with the

Hazelcast clustered cache implemented by MapProxyImpl by preventing null
keys and values being added. Also document this behaviour in the Cache
interface.

src/java/org/jivesoftware/util/cache/Cache.java

@@ -30,7 +30,7 @@ package org.jivesoftware.util.cache;
  *
  * If the cache does grow too large, objects will be removed such that those
  * that are accessed least frequently are removed first. Because expiration
- * happens automatically, the cache makes <b>no</b> gaurantee as to how long
+ * happens automatically, the cache makes <b>no</b> guarantee as to how long
  * an object will remain in cache after it is put in.<p>
  *
  * Optionally, a maximum lifetime for all objects can be specified. In that
@@ -41,6 +41,9 @@ package org.jivesoftware.util.cache;
  *
  * All cache operations are thread safe.<p>
  *
+ * Note that neither keys or values can be null; A {@link NullPointerException}
+ * will be thrown attempting to place or retrieve null values in to the cache.
+ *
  * @see Cacheable
  */
 public interface Cache<K,V> extends java.util.Map<K,V> {

src/java/org/jivesoftware/util/cache/DefaultCache.java

@@ -59,6 +59,9 @@ import org.slf4j.LoggerFactory;
  */
 public class DefaultCache<K, V> implements Cache<K, V> {
 
+    private static final String NULL_KEY_IS_NOT_ALLOWED = "Null key is not allowed!";
+    private static final String NULL_VALUE_IS_NOT_ALLOWED = "Null value is not allowed!";
+
     private static final Logger Log = LoggerFactory.getLogger(DefaultCache.class);
 
     /**
@@ -133,6 +136,8 @@ public class DefaultCache<K, V> implements Cache<K, V> {
 
     @Override
     public synchronized V put(K key, V value) {
+        checkNotNull(key, NULL_KEY_IS_NOT_ALLOWED);
+        checkNotNull(value, NULL_VALUE_IS_NOT_ALLOWED);
         // Delete an old entry if it exists.
         V answer = remove(key);
 
@@ -174,6 +179,7 @@ public class DefaultCache<K, V> implements Cache<K, V> {
 
     @Override
     public synchronized V get(Object key) {
+        checkNotNull(key, NULL_KEY_IS_NOT_ALLOWED);
         // First, clear all entries that have been in cache longer than the
         // maximum defined age.
         deleteExpiredEntries();
@@ -200,6 +206,7 @@ public class DefaultCache<K, V> implements Cache<K, V> {
 
     @Override
     public synchronized V remove(Object key) {
+        checkNotNull(key, NULL_KEY_IS_NOT_ALLOWED);
         DefaultCache.CacheObject<V> cacheObject = map.get(key);
         // If the object is not in cache, stop trying to remove it.
         if (cacheObject == null) {
@@ -285,6 +292,7 @@ public class DefaultCache<K, V> implements Cache<K, V> {
 
         @Override
         public boolean contains(Object o) {
+            checkNotNull(o, NULL_KEY_IS_NOT_ALLOWED);
             Iterator<V> it = iterator();
             while (it.hasNext()) {
                 if (it.next().equals(o)) {
@@ -391,6 +399,7 @@ public class DefaultCache<K, V> implements Cache<K, V> {
 
     @Override
     public boolean containsKey(Object key) {
+        checkNotNull(key, NULL_KEY_IS_NOT_ALLOWED);
         // First, clear all entries that have been in cache longer than the
         // maximum defined age.
         deleteExpiredEntries();
@@ -409,14 +418,11 @@ public class DefaultCache<K, V> implements Cache<K, V> {
 
     @Override
     public boolean containsValue(Object value) {
+        checkNotNull(value, NULL_VALUE_IS_NOT_ALLOWED);
         // First, clear all entries that have been in cache longer than the
         // maximum defined age.
         deleteExpiredEntries();
 
-        if(value == null) {
-            return containsNullValue();
-        }
-
         Iterator it = values().iterator();
         while(it.hasNext()) {
             if(value.equals(it.next())) {
@@ -426,16 +432,6 @@ public class DefaultCache<K, V> implements Cache<K, V> {
         return false;
     }
 
-    private boolean containsNullValue() {
-        Iterator it = values().iterator();
-        while(it.hasNext()) {
-            if(it.next() == null) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     @Override
     public Set<Entry<K, V>> entrySet() {
         // First, clear all entries that have been in cache longer than the
@@ -703,4 +699,10 @@ public class DefaultCache<K, V> implements Cache<K, V> {
             this.size = size;
         }
     }
+
+    private void checkNotNull(final Object argument, final String message) {
+        if (argument == null) {
+            throw new NullPointerException(message);
+        }
+    }
 }