Fix #293: improve error message for ObjectId collision fail

release-notes/VERSION-2.x

@@ -16,6 +16,8 @@ NOTE: Jackson 3.x components rely on 2.x annotations; there are no separate
 
 2.20 (not yet released)
 
+#293: Improve duplicate Id with different associated object error message
+ (requested by @moutyque)
 #294: Drop patch number from version for 2.20 and later (no more 2.20.0)
 #296: Drop Java 6 compatibility for 2.20 (Java 8 baseline)
 

src/main/java/com/fasterxml/jackson/annotation/ObjectIdGenerator.java

@@ -1,5 +1,7 @@
 package com.fasterxml.jackson.annotation;
 
+import java.util.Objects;
+
 /**
  * Definition of API used for constructing Object Identifiers
  * (as annotated using {@link JsonIdentityInfo}).
@@ -148,18 +150,13 @@ public final static class IdKey
         private final int hashCode;
 
         public IdKey(Class<?> type, Class<?> scope, Object key) {
-            if (key == null) {
-                throw new IllegalArgumentException("Can not construct IdKey for null key");
-            }
-            this.type = type;
+            this.type = Objects.requireNonNull(type, "Type must not be null");
+            // Scope can be null
             this.scope = scope;
-            this.key = key;
+            this.key = Objects.requireNonNull(key, "Key must not be null");
 
-            int h = key.hashCode() + type.getName().hashCode();
-            if (scope != null) {
-                h ^= scope.getName().hashCode();
-            }
-            hashCode = h;
+            hashCode = Objects.hashCode(key) + Objects.hashCode(type.getName())
+                ^ Objects.hashCode(scope);
         }
 
         @Override
@@ -178,7 +175,7 @@ public boolean equals(Object o)
         @Override
         public String toString() {
             return String.format("[ObjectId: key=%s, type=%s, scope=%s]", key,
-                    (type == null) ? "NONE" : type.getName(),
+                    type.getName(),
                     (scope == null) ? "NONE" : scope.getName());
         }
     }

src/main/java/com/fasterxml/jackson/annotation/SimpleObjectIdResolver.java

@@ -11,29 +11,46 @@
  * @author Pascal Gélinas
  */
 public class SimpleObjectIdResolver implements ObjectIdResolver {
-    protected Map<IdKey,Object> _items;
+    protected Map<IdKey, Object> _items;
 
     public SimpleObjectIdResolver() { }
 
     @Override
     public void bindItem(IdKey id, Object ob)
     {
         if (_items == null) {
-            _items = new HashMap<ObjectIdGenerator.IdKey,Object>();
+            _items = new HashMap<>();
         } else {
             Object old = _items.get(id);
             if (old != null) {
                 // 11-Nov-2020, tatu: As per [annotations#180] allow duplicate calls:
                 if (old == ob) {
                     return;
                 }
-                throw new IllegalStateException("Already had POJO for id (" + id.key.getClass().getName() + ") [" + id
-                        + "]");
+                throw new IllegalStateException(String.format(
+"Object Id conflict: Id %s already bound to an Object %s: attempt to re-bind to a different Object %s",
+                id.toString(), _desc(old), _desc(ob)));
             }
         }
         _items.put(id, ob);
     }
 
+    private String _desc(Object ob) {
+        if (ob == null) { 
+            return "(null)";
+        }
+        String desc;
+        if (ob instanceof String) {
+            desc = "\""+ob+"\"";
+        } else {
+            desc = ob.toString();
+            if (desc.length() > 100) {
+                desc = desc.substring(0, 100) + "[... truncated]";
+            }
+        }
+        return  ("(type: `"+ob.getClass().getName()+"`, value: "+desc+")");
+    }
+
     @Override
     public Object resolveId(IdKey id) {
         return (_items == null) ? null : _items.get(id);

src/test/java/com/fasterxml/jackson/annotation/SimpleObjectIdResolverTest.java

@@ -0,0 +1,44 @@
+package com.fasterxml.jackson.annotation;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+
+public class SimpleObjectIdResolverTest
+{
+    @Test
+    public void testSimpleHandling() 
+    {
+        // Let's create a simple ObjectIdResolver with 2 non-duplicate items
+        SimpleObjectIdResolver resolver = new SimpleObjectIdResolver();
+        ObjectIdGenerator.IdKey key1 = new ObjectIdGenerator.IdKey(String.class, null, "key1");
+        ObjectIdGenerator.IdKey key2 = new ObjectIdGenerator.IdKey(String.class, null, "key2");
+
+        // Bind 2 non-duplicate items, check they can be resolved
+        resolver.bindItem(key1, "value1");
+        resolver.bindItem(key2, "value2");
+
+        assertEquals("value1", resolver.resolveId(key1));
+        assertEquals("value2", resolver.resolveId(key2));
+        assertEquals(2, resolver._items.size());
+
+        // And then verify that multiple bindings of the same key/bound value is ok
+        resolver.bindItem(key1, "value1");
+        resolver.bindItem(key2, "value2");
+        assertEquals(2, resolver._items.size());
+
+        // But that overriding is not
+        try {
+            resolver.bindItem(key1, "value3");
+            fail("Should have thrown an exception for re-binding");
+        } catch (IllegalStateException e) {
+            assertEquals(
+"Object Id conflict: Id [ObjectId: key=key1, type=java.lang.String, scope=NONE]"
++" already bound to an Object (type: `java.lang.String`, value: \"value1\"):"
++" attempt to re-bind to a different Object (type: `java.lang.String`, value: \"value3\")",
+                    e.getMessage());
+        }
+    }
+
+}