From 76cbbc19ff154218b1d289690f62c27650c2624b Mon Sep 17 00:00:00 2001
From: Rob Austin <rob.austin@boundedbuffer.co.uk>
Date: Mon, 15 Dec 2014 12:05:19 +0000
Subject: [PATCH] HCOLL-250 providing better error message for failure and
 attempts to create using reflection

---
 pom.xml                                       |  8 ++--
 .../chronicle/map/CloseablesManager.java      |  2 +-
 .../chronicle/map/VanillaChronicleMap.java    | 29 ++++++++++++--
 .../chronicle/map/CHMUseCasesTest.java        | 40 ++++++++++++++++++-
 4 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/pom.xml b/pom.xml
index 8d221b978..99e605ffe 100644
--- a/pom.xml
+++ b/pom.xml
@@ -17,13 +17,15 @@
   ~ limitations under the License.
   -->
 
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
 
     <parent>
         <groupId>net.openhft</groupId>
         <artifactId>java-parent-pom</artifactId>
         <version>1.1.2</version>
-        <relativePath />
+        <relativePath/>
     </parent>
 
     <modelVersion>4.0.0</modelVersion>
@@ -46,7 +48,7 @@
             <dependency>
                 <groupId>net.openhft</groupId>
                 <artifactId>lang</artifactId>
-                <version>6.4.17</version>
+                <version>6.4.18-SNAPSHOT</version>
             </dependency>
 
             <dependency>
diff --git a/src/main/java/net/openhft/chronicle/map/CloseablesManager.java b/src/main/java/net/openhft/chronicle/map/CloseablesManager.java
index 8022e0ab2..fda6bc718 100755
--- a/src/main/java/net/openhft/chronicle/map/CloseablesManager.java
+++ b/src/main/java/net/openhft/chronicle/map/CloseablesManager.java
@@ -59,7 +59,7 @@ final class CloseablesManager implements Closeable {
             try {
                 closeable.close();
             } catch (IOException e) {
-                LOG.error("", e);
+                LOG.debug("", e);
                 ex = e;
             }
         }
diff --git a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java
index a2adbbecd..6cffea0d6 100755
--- a/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java
+++ b/src/main/java/net/openhft/chronicle/map/VanillaChronicleMap.java
@@ -663,9 +663,32 @@ class VanillaChronicleMap<K, KI, MKI extends MetaBytesInterop<K, ? super KI>,
     }
 
 
-    static <T> T newInstance(Class<T> interfaceClass, boolean isKey) {
-        return isKey ? DataValueClasses.newInstance(interfaceClass) :
-                DataValueClasses.newDirectInstance(interfaceClass);
+    static <T> T newInstance(Class<T> aClass, boolean isKey) {
+        try {
+            return isKey ? DataValueClasses.newInstance(aClass) :
+                    DataValueClasses.newDirectInstance(aClass);
+        } catch (Exception e) {
+            if (aClass.isInterface())
+                throw new IllegalStateException("It not possible to create a instance from " +
+                        "interface=" + aClass.getSimpleName() + " we recommend you create an " +
+                        "instance in the usual way.");
+
+            try {
+                return (T) aClass.newInstance();
+            } catch (Exception e1) {
+                throw new IllegalStateException("It has not been possible to create a instance " +
+                        "of class=" + aClass.getSimpleName() +
+                        ", Note : its more efficient if your chronicle map is configured with " +
+                        "interface key " +
+                        "and value types rather than classes, as this method is able to use " +
+                        "interfaces to generate off heap proxies that point straight at your data. " +
+                        "In this case you have used a class and chronicle is unable to create an " +
+                        "instance of this class has it does not have a default constructor. " +
+                        "If your class is mutable, we " +
+                        "recommend you create and instance of your class=" + aClass.getSimpleName() +
+                        " in the usual way, rather than using this method.");
+            }
+        }
     }
 
     private XStreamConverter xStreamConverter;
diff --git a/src/test/java/net/openhft/chronicle/map/CHMUseCasesTest.java b/src/test/java/net/openhft/chronicle/map/CHMUseCasesTest.java
index 4f8569770..b96c79ac6 100644
--- a/src/test/java/net/openhft/chronicle/map/CHMUseCasesTest.java
+++ b/src/test/java/net/openhft/chronicle/map/CHMUseCasesTest.java
@@ -332,8 +332,8 @@ public class CHMUseCasesTest {
         }
     }
 
-    @Ignore("HCOLL-250 map.newKeyInstance() throws NPE for simple types like Long")
-    @Test
+
+    @Test(expected = IllegalStateException.class)
     public void testLongNewKeyValueInstance() {
         try (ChronicleMap map = ChronicleMapBuilder
                 .of(Long.class, Long.class)
@@ -344,6 +344,42 @@ public class CHMUseCasesTest {
         }
     }
 
+
+    @Test(expected = IllegalStateException.class)
+    public void testNewKeyValueInstanceWithMapType() {
+        try (ChronicleMap map = ChronicleMapBuilder
+                .of(Map.class, Map.class)
+                .create()) {
+
+            map.newKeyInstance();
+            map.newValueInstance();
+        }
+    }
+
+    @Test
+    public void testNewKeyValueInstanceWithHahMapType() {
+        try (ChronicleMap map = ChronicleMapBuilder
+                .of(HashMap.class, HashMap.class)
+                .create()) {
+
+            map.newKeyInstance();
+            map.newValueInstance();
+        }
+    }
+
+
+    @Test(expected = IllegalStateException.class)
+    public void testNewKeyValueInstanceWithByteArray() {
+        try (ChronicleMap map = ChronicleMapBuilder
+                .of(byte[].class, byte[].class)
+                .create()) {
+
+            map.newKeyInstance();
+            map.newValueInstance();
+        }
+    }
+
+
     /**
      * StringValue represents any bean which contains a String Value
      */