From 4af4fce33cd5a3c4339574e64e48bd6215830996 Mon Sep 17 00:00:00 2001
From: leventov <leventov@ya.ru>
Date: Mon, 19 Jan 2015 07:06:21 +0700
Subject: [PATCH] Added eager nullability and type checks of values and
 functions passed to ChronicleMap, if is probable that they won't be passed
 (and checked) inside contexts, if, say the key is absent in the map

---
 .../openhft/chronicle/map/AbstractChronicleMap.java    | 10 ++++++++++
 .../net/openhft/chronicle/map/BytesChronicleMap.java   |  5 +++++
 src/main/java/net/openhft/chronicle/map/Objects.java   |  5 +++++
 .../net/openhft/chronicle/map/VanillaChronicleMap.java |  1 +
 4 files changed, 21 insertions(+)

diff --git a/src/main/java/net/openhft/chronicle/map/AbstractChronicleMap.java b/src/main/java/net/openhft/chronicle/map/AbstractChronicleMap.java
index 7755c75ed..c4665bea3 100644
--- a/src/main/java/net/openhft/chronicle/map/AbstractChronicleMap.java
+++ b/src/main/java/net/openhft/chronicle/map/AbstractChronicleMap.java
@@ -25,6 +25,7 @@ import java.io.Serializable;
 import java.util.*;
 
 import static java.util.Collections.emptyList;
+import static net.openhft.chronicle.map.Objects.requireNonNull;
 
 abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
         implements ChronicleMap<K, V>, Serializable {
@@ -63,6 +64,7 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
 
     @Override
     public final V putIfAbsent(K key, V value) {
+        checkValue(value);
         try (MapKeyContext<V> c = context(key)) {
             // putIfAbsent() shouldn't actually put most of the time,
             // so check if the key is present under read lock first:
@@ -87,6 +89,8 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
         }
     }
 
+    abstract void checkValue(V value);
+
     @Override
     @SuppressWarnings("unchecked")
     public final V get(Object key) {
@@ -137,6 +141,7 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
 
     @Override
     public <R> R getMapped(K key, @NotNull Function<? super V, R> function) {
+        requireNonNull(function);
         try (MapKeyContext<V> c = context(key)) {
             return c.containsKey() ? function.apply(c.get()) : null;
         }
@@ -144,6 +149,7 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
 
     @Override
     public V putMapped(K key, @NotNull UnaryOperator<V> unaryOperator) {
+        requireNonNull(unaryOperator);
         try (MapKeyContext<V> c = context(key)) {
             // putMapped() should find a value, update & put most of the time,
             // so don't try to check key presence under read lock first,
@@ -193,6 +199,7 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
     public final boolean remove(Object key, Object value) {
         if (value == null)
             return false; // CHM compatibility; General ChronicleMap policy is to throw NPE
+        checkValue((V) value);
         try (MapKeyContext<V> c = context((K) key)) {
             // remove(key, value) should find the entry & remove most of the time,
             // so don't try to check key presence and value equivalence under read lock first,
@@ -204,6 +211,7 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
 
     @Override
     public final V replace(K key, V value) {
+        checkValue(value);
         try (MapKeyContext<V> c = context(key)) {
             // replace(key, value) should find the key & put the value most of the time,
             // so don't try to check key presence under read lock first,
@@ -223,6 +231,8 @@ abstract class AbstractChronicleMap<K, V> extends AbstractMap<K, V>
 
     @Override
     public final boolean replace(K key, V oldValue, V newValue) {
+        checkValue(oldValue);
+        checkValue(newValue);
         try (MapKeyContext<V> c = context(key)) {
             // replace(key, old, new) should find the entry & put new value most of the time,
             // so don't try to check key presence and value equivalence under read lock first,
diff --git a/src/main/java/net/openhft/chronicle/map/BytesChronicleMap.java b/src/main/java/net/openhft/chronicle/map/BytesChronicleMap.java
index 13f066f05..acb5a5ea6 100644
--- a/src/main/java/net/openhft/chronicle/map/BytesChronicleMap.java
+++ b/src/main/java/net/openhft/chronicle/map/BytesChronicleMap.java
@@ -70,6 +70,11 @@ public class BytesChronicleMap extends AbstractChronicleMap<Bytes, Bytes> {
         return context;
     }
 
+    @Override
+    void checkValue(Bytes value) {
+        Objects.requireNonNull(value);
+    }
+
     @Override
     public Class<Bytes> keyClass() {
         return Bytes.class;
diff --git a/src/main/java/net/openhft/chronicle/map/Objects.java b/src/main/java/net/openhft/chronicle/map/Objects.java
index f3058681c..6994c31dc 100644
--- a/src/main/java/net/openhft/chronicle/map/Objects.java
+++ b/src/main/java/net/openhft/chronicle/map/Objects.java
@@ -40,4 +40,9 @@ final class Objects {
                 o != null && builder.getClass() == o.getClass() &&
                         builder.toString().equals(o.toString());
     }
+
+    static void requireNonNull(Object obj) {
+        if (obj == null)
+            throw new NullPointerException();
+    }
 }
diff --git a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java
index 112e86650..93bad7fa2 100755
--- a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java
+++ b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java
@@ -370,6 +370,7 @@ class VanillaChronicleMap<K, KI, MKI extends MetaBytesInterop<K, ? super KI>,
         }
     }
 
+    @Override
     final void checkValue(Object value) {
         if (vClass != Void.class && !vClass.isInstance(value)) {
             throw new ClassCastException("Value must be a " + vClass.getName() +