Merge branch '2.x' into 3.x

Author: cowtowncoder

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/JsonFormat.java

@@ -17,7 +17,7 @@
  * or String (such as ISO-8601 compatible time value) -- as well as configuring
  * exact details with {@link #pattern} property.
  *<p>
- * As of Jackson 3.0, known special handling includes:
+ * As of Jackson 2.20, known special handling includes:
  *<ul>
  * <li>{@link java.util.Date} or {@link java.util.Calendar}: Shape can  be {@link Shape#STRING} or {@link Shape#NUMBER};
  *    pattern may contain {@link java.text.SimpleDateFormat}-compatible pattern definition.
@@ -149,6 +149,8 @@ public enum Shape
         /**
          * Value that indicates that Binary type (native, if format supports it;
          * encoding using Base64 if only textual types supported) should be used.
+         *
+         * @since 2.10
          */
         BINARY,
 
@@ -183,7 +185,7 @@ public enum Shape
 
         /**
          * Value that indicates shape should not be structural (that is, not
-         * {@link #ARRAY} or {@link #OBJECT}, but can be any other shape.
+         * {@link #ARRAY} or {@link #OBJECT}), but can be any other shape.
          */
         SCALAR,
 
@@ -232,7 +234,7 @@ public boolean isNumeric() {
             return (this == NUMBER) || (this == NUMBER_INT) || (this == NUMBER_FLOAT);
         }
 
-        /** @since 3.0 */
+        /** @since 2.20 */
         public static boolean isNumeric(Shape shapeOrNull) {
             return (shapeOrNull != null) && shapeOrNull.isNumeric();
         }
@@ -241,7 +243,7 @@ public boolean isStructured() {
             return (this == OBJECT) || (this == ARRAY) || (this == POJO);
         }
 
-        /** @since 3.0 */
+        /** @since 2.20 */
         public static boolean isStructured(Shape shapeOrNull) {
             return (shapeOrNull != null) && shapeOrNull.isStructured();
         }
@@ -531,6 +533,9 @@ public Value(String p, Shape sh, Locale l, String tzStr, TimeZone tz, Features f
             _lenient = lenient;
         }
 
+        /**
+         * @since 2.7
+         */
         public final static Value empty() {
             return EMPTY;
         }

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}).
@@ -144,18 +146,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
@@ -174,7 +171,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());
+        }
+    }
+
+}