Fix #4364: add PropertyName.merge(), use by AnnotationIntrospectorPair (#4365)

Author: cowtowncoder

release-notes/VERSION-2.x

@@ -41,6 +41,7 @@ Project: jackson-databind
  (reported by @k-wall)
  (fix contributed by Joo-Hyuk K)
 #4337: `AtomicReference` serializer does not support `@JsonSerialize(contentConverter=...)`
+#4364: `@JsonProperty` and equivalents should merge with `AnnotationIntrospectorPair`
 - JUnit5 upgraded to 5.10.1
 
 2.16.2 (not yet released)

src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java

@@ -412,35 +412,6 @@ public JsonIncludeProperties.Value findPropertyInclusionByName(MapperConfig<?> c
      */
     public String findClassDescription(AnnotatedClass ac) { return null; }
 
-    /**
-     * @param forSerialization True if requesting properties to ignore for serialization;
-     *   false if for deserialization
-     * @param ac Annotated class to introspect
-     *
-     * @return Array of names of properties to ignore
-     *
-     * @since 2.6
-     *
-     * @deprecated Since 2.8, use {@link #findPropertyIgnoralByName} instead
-     */
-    @Deprecated // since 2.8
-    public String[] findPropertiesToIgnore(Annotated ac, boolean forSerialization) {
-        return null;
-    }
-
-    /**
-     * Method for checking whether an annotation indicates that all unknown properties
-     * should be ignored.
-     *
-     * @param ac Annotated class to introspect
-     *
-     * @return True if class has something indicating "ignore [all] unknown properties"
-     *
-     * @deprecated Since 2.8, use {@link #findPropertyIgnoralByName} instead
-     */
-    @Deprecated // since 2.8
-    public Boolean findIgnoreUnknownProperties(AnnotatedClass ac) { return null; }
-
     /**
      * @param ac Annotated class to introspect
      *

src/main/java/com/fasterxml/jackson/databind/PropertyName.java

@@ -108,6 +108,51 @@ public static PropertyName construct(String simpleName, String ns)
         return new PropertyName(InternCache.instance.intern(simpleName), ns);
     }
 
+    /**
+     * Method that will combine information from two {@link PropertyName}
+     * instances
+     *
+     * @param name1 Name with higher precedence; may be {@code null}
+     * @param name2 Name with lower precedence; may be {@code null}
+     *
+     * @return Merged information; only {@code null} if both arguments
+     *   are {@code null}s.
+     *
+     * @since 2.17
+     */
+    public static PropertyName merge(PropertyName name1, PropertyName name2) {
+        if (name1 == null) {
+            return name2;
+        }
+        if (name2 == null) {
+            return name1;
+        }
+        String ns = _nonEmpty(name1._namespace, name2._namespace);
+        String simple = _nonEmpty(name1._simpleName, name2._simpleName);
+
+        // But see if we can just return one of arguments as-is:
+        if (ns == name1._namespace && simple == name1._simpleName) {
+            return name1;
+        }
+        if (ns == name2._namespace && simple == name2._simpleName) {
+            return name2;
+        }
+        return construct(simple, ns);
+    }
+
+    private static String _nonEmpty(String str1, String str2) {
+        if (str1 == null) {
+            return str2;
+        }
+        if (str2 == null) {
+            return str1;
+        }
+        if (str1.isEmpty()) {
+            return str2;
+        }
+        return str1;
+    }
+
     public PropertyName internSimpleName()
     {
         if (_simpleName.isEmpty()) { // empty String is canonical already
@@ -222,9 +267,8 @@ public boolean equals(Object o)
     {
         if (o == this) return true;
         if (o == null) return false;
-        /* 13-Nov-2012, tatu: by default, require strict type equality.
-         *   Re-evaluate if this becomes an issue.
-         */
+        // 13-Nov-2012, tatu: by default, require strict type equality.
+        //   Re-evaluate if this becomes an issue.
         if (o.getClass() != getClass()) return false;
         // 13-Nov-2012, tatu: Should we have specific rules on matching USE_DEFAULT?
         //   (like, it only ever matching exact instance)
@@ -244,7 +288,8 @@ public boolean equals(Object o)
 
     @Override
     public int hashCode() {
-        return Objects.hash(_namespace, _simpleName);
+        return Objects.hashCode(_simpleName) * 31
+                + Objects.hashCode(_namespace);
     }
 
     @Override

src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java

@@ -97,16 +97,8 @@ public boolean isAnnotationBundle(Annotation ann) {
     @Override
     public PropertyName findRootName(AnnotatedClass ac)
     {
-        PropertyName name1 = _primary.findRootName(ac);
-        if (name1 == null) {
-            return _secondary.findRootName(ac);
-        }
-        if (name1.hasSimpleName()) {
-            return name1;
-        }
-        // name1 is empty; how about secondary?
-        PropertyName name2 = _secondary.findRootName(ac);
-        return (name2 == null) ? name1 : name2;
+        return PropertyName.merge(_primary.findRootName(ac),
+                _secondary.findRootName(ac));
     }
 
     // since 2.12
@@ -177,27 +169,6 @@ public String findClassDescription(AnnotatedClass ac) {
         return str;
     }
 
-    @Override
-    @Deprecated // since 2.8
-    public String[] findPropertiesToIgnore(Annotated ac, boolean forSerialization) {
-        String[] result = _primary.findPropertiesToIgnore(ac, forSerialization);
-        if (result == null) {
-            result = _secondary.findPropertiesToIgnore(ac, forSerialization);
-        }
-        return result;
-    }
-
-    @Override
-    @Deprecated // since 2.8
-    public Boolean findIgnoreUnknownProperties(AnnotatedClass ac)
-    {
-        Boolean result = _primary.findIgnoreUnknownProperties(ac);
-        if (result == null) {
-            result = _secondary.findIgnoreUnknownProperties(ac);
-        }
-        return result;
-    }
-
     @Override
     @Deprecated // since 2.12
     public JsonIgnoreProperties.Value findPropertyIgnorals(Annotated a)
@@ -464,17 +435,8 @@ public JsonFormat.Value findFormat(Annotated ann) {
 
     @Override
     public PropertyName findWrapperName(Annotated ann) {
-        PropertyName name = _primary.findWrapperName(ann);
-        if (name == null) {
-            name = _secondary.findWrapperName(ann);
-        } else if (name == PropertyName.USE_DEFAULT) {
-            // does the other introspector have a better idea?
-            PropertyName name2 = _secondary.findWrapperName(ann);
-            if (name2 != null) {
-                name = name2;
-            }
-        }
-        return name;
+        return PropertyName.merge(_primary.findWrapperName(ann),
+                _secondary.findWrapperName(ann));
     }
 
     @Override
@@ -534,11 +496,8 @@ public AnnotatedMethod resolveSetterConflict(MapperConfig<?> config,
     @Override // since 2.11
     public PropertyName findRenameByField(MapperConfig<?> config,
             AnnotatedField f, PropertyName implName) {
-        PropertyName n = _secondary.findRenameByField(config, f, implName);
-        if (n == null) {
-            n = _primary.findRenameByField(config, f, implName);
-        }
-        return n;
+        return PropertyName.merge(_secondary.findRenameByField(config, f, implName),
+                    _primary.findRenameByField(config, f, implName));
     }
 
     // // // Serialization: type refinements
@@ -577,17 +536,8 @@ public void findAndAddVirtualProperties(MapperConfig<?> config, AnnotatedClass a
 
     @Override
     public PropertyName findNameForSerialization(Annotated a) {
-        PropertyName n = _primary.findNameForSerialization(a);
-        // note: "use default" should not block explicit answer, so:
-        if (n == null) {
-            n = _secondary.findNameForSerialization(a);
-        } else if (n == PropertyName.USE_DEFAULT) {
-            PropertyName n2 = _secondary.findNameForSerialization(a);
-            if (n2 != null) {
-                n = n2;
-            }
-        }
-        return n;
+        return PropertyName.merge(_primary.findNameForSerialization(a),
+                _secondary.findNameForSerialization(a));
     }
 
     @Override
@@ -764,17 +714,9 @@ public JsonPOJOBuilder.Value findPOJOBuilderConfig(AnnotatedClass ac) {
     @Override
     public PropertyName findNameForDeserialization(Annotated a)
     {
-        // note: "use default" should not block explicit answer, so:
-        PropertyName n = _primary.findNameForDeserialization(a);
-        if (n == null) {
-            n = _secondary.findNameForDeserialization(a);
-        } else if (n == PropertyName.USE_DEFAULT) {
-            PropertyName n2 = _secondary.findNameForDeserialization(a);
-            if (n2 != null) {
-                n = n2;
-            }
-        }
-        return n;
+        return PropertyName.merge(
+                _primary.findNameForDeserialization(a),
+                _secondary.findNameForDeserialization(a));
     }
 
     @Override

src/test/java/com/fasterxml/jackson/databind/PropertyNameTest.java

@@ -0,0 +1,26 @@
+package com.fasterxml.jackson.databind;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+public class PropertyNameTest
+{
+    @Test
+    public void testMerging() {
+        PropertyName name1 = PropertyName.construct("name1", "ns1");
+        PropertyName name2 = PropertyName.construct("name2", "ns2");
+        PropertyName empty = PropertyName.construct("", null);
+        PropertyName nsX = PropertyName.construct("", "nsX");
+
+        assertSame(name1, PropertyName.merge(name1, name2));
+        assertSame(name2, PropertyName.merge(name2, name1));
+
+        assertSame(name1, PropertyName.merge(name1, empty));
+        assertSame(name1, PropertyName.merge(empty, name1));
+
+        assertEquals(PropertyName.construct("name1", "nsX"),
+                PropertyName.merge(nsX, name1));
+    }
+}