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 @@
  *
  * 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 @@
  *
  * 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,7 +59,10 @@
  */
 public class DefaultCache<K, V> implements Cache<K, V> {
 
-	private static final Logger Log = LoggerFactory.getLogger(DefaultCache.class);
+    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);
 
     /**
      * The map the keys and values are stored in.
@@ -133,6 +136,8 @@ public DefaultCache(String name, long maxSize, long maxLifetime) {
 
     @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 synchronized V put(K key, V value) {
 
     @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 synchronized V get(Object key) {
 
     @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 boolean isEmpty() {
 
         @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 void clear() {
 
     @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 void putAll(Map<? extends K, ? extends V> map) {
 
     @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 boolean containsValue(Object value) {
         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 CacheObject(V object, int size) {
             this.size = size;
         }
     }
+
+    private void checkNotNull(final Object argument, final String message) {
+        if (argument == null) {
+            throw new NullPointerException(message);
+        }
+    }
 }