Fix #1044

Author: cowtowncoder

release-notes/VERSION

@@ -52,6 +52,8 @@ Project: jackson-databind
  (suggested by David B)
 #1043: @JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY) does not work on fields
  (reported by fabiolaa@github)
+#1044: Add `AnnotationIntrospector.resolveSetterConflict(...)` to allow custom setter conflict resolution
+ (suggested by clydebarrow@github)
 - Make `JsonValueFormat` (self-)serializable, deserializable, to/from valid external
   value (as per JSON Schema spec)
 

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

@@ -30,6 +30,11 @@
  * so that different sets of annotations can be supported, and support
  * plugged-in dynamically.
  *<p>
+ * Although default implementations are based on using annotations as the only
+ * (or at least main) information source, custom implementations are not limited
+ * in such a way, and in fact there is no expectation they should be. So the name
+ * is bit of misnomer; this is a general configuration introspection facility.
+ *<p>
  * NOTE: due to rapid addition of new methods (and changes to existing methods),
  * it is <b>strongly</b> recommended that custom implementations should not directly
  * extend this class, but rather extend {@link NopAnnotationIntrospector}.
@@ -559,6 +564,19 @@ public TypeResolverBuilder<?> findPropertyContentTypeResolver(MapperConfig<?> co
      */
     public JsonProperty.Access findPropertyAccess(Annotated ann) { return null; }
 
+    /**
+     * Method called in cases where a class has two methods eligible to be used
+     * for the same logical property, and default logic is not enough to figure
+     * out clear precedence. Introspector may try to choose one to use; or, if
+     * unable, return `null` to indicate it can not resolve the problem.
+     *
+     * @since 2.7
+     */
+    public AnnotatedMethod resolveSetterConflict(MapperConfig<?> config,
+            AnnotatedMethod setter1, AnnotatedMethod setter2) {
+        return null;
+    }
+
     /*
     /**********************************************************
     /* Serialization: general annotations

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

@@ -10,7 +10,10 @@
 
 /**
  * Checked exception used to signal fatal problems with mapping of
- * content.
+ * content, distinct from low-level I/O problems (signaled using
+ * simple {@link java.io.IOException}s) or data encoding/decoding
+ * problems (signaled with {@link com.fasterxml.jackson.core.JsonParseException},
+ * {@link com.fasterxml.jackson.core.JsonGenerationException}).
  *<p>
  * One additional feature is the ability to denote relevant path
  * of references (during serialization/deserialization) to help in

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

@@ -467,10 +467,20 @@ public JsonProperty.Access findPropertyAccess(Annotated ann) {
         return JsonProperty.Access.AUTO;
     }
 
+    @Override // since 2.7
+    public AnnotatedMethod resolveSetterConflict(MapperConfig<?> config,
+            AnnotatedMethod setter1, AnnotatedMethod setter2)
+    {
+        AnnotatedMethod res = _primary.resolveSetterConflict(config, setter1, setter2);
+        if (res == null) {
+            res = _secondary.resolveSetterConflict(config, setter1, setter2);
+        }
+        return res;
+    }
+
     // // // Serialization: type refinements
 
-    // since 2.7
-    @Override
+    @Override // since 2.7
     public JavaType refineSerializationType(MapperConfig<?> config,
             Annotated a, JavaType baseType) throws JsonMappingException
     {

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

@@ -414,6 +414,34 @@ public Class<?>[] findViews(Annotated a)
         return (ann == null) ? null : ann.value();
     }
 
+    @Override // since 2.7
+    public AnnotatedMethod resolveSetterConflict(MapperConfig<?> config,
+            AnnotatedMethod setter1, AnnotatedMethod setter2)
+    {
+        Class<?> cls1 = setter1.getRawParameterType(0);
+        Class<?> cls2 = setter2.getRawParameterType(0);
+        
+        // First: prefer primitives over non-primitives
+        // 11-Dec-2015, tatu: TODO, perhaps consider wrappers for primitives too?
+        if (cls1.isPrimitive()) {
+            if (!cls2.isPrimitive()) {
+                return setter1;
+            }
+        } else if (cls2.isPrimitive()) {
+            return setter2;
+        }
+        
+        if (cls1 == String.class) {
+            if (cls2 != String.class) {
+                return setter1;
+            }
+        } else if (cls2 == String.class) {
+            return setter2;
+        }
+
+        return null;
+    }
+
     /*
     /**********************************************************
     /* Annotations for Polymorphic Type handling

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

@@ -784,9 +784,8 @@ protected void _renameUsing(Map<String, POJOPropertyBuilder> propMap,
         for (POJOPropertyBuilder prop : props) {
             PropertyName fullName = prop.getFullName();
             String rename = null;
-            // As per [#428](https://github.com/FasterXML/jackson-databind/issues/428) need
-            // to skip renaming if property has explicitly defined name, unless feature
-            // is enabled
+            // As per [databind#428] need to skip renaming if property has
+            // explicitly defined name, unless feature  is enabled
             if (!prop.isExplicitlyNamed() || _config.isEnabled(MapperFeature.ALLOW_EXPLICIT_PROPERTY_RENAMING)) {
                 if (_forSerialization) {
                     if (prop.hasGetter()) {
@@ -983,8 +982,8 @@ protected POJOPropertyBuilder _property(Map<String, POJOPropertyBuilder> props,
     {
         POJOPropertyBuilder prop = props.get(implName);
         if (prop == null) {
-            prop = new POJOPropertyBuilder(PropertyName.construct(implName),
-                    _annotationIntrospector, _forSerialization);
+            prop = new POJOPropertyBuilder(_config, _annotationIntrospector, _forSerialization,
+                    PropertyName.construct(implName));
             props.put(implName, prop);
         }
         return prop;

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

@@ -4,8 +4,8 @@
 
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
-
 import com.fasterxml.jackson.databind.*;
+import com.fasterxml.jackson.databind.cfg.MapperConfig;
 import com.fasterxml.jackson.databind.util.ClassUtil;
 
 /**
@@ -22,6 +22,8 @@ public class POJOPropertyBuilder
      */
     protected final boolean _forSerialization;
 
+    protected final MapperConfig<?> _config;
+
     protected final AnnotationIntrospector _annotationIntrospector;
 
     /**
@@ -45,25 +47,27 @@ public class POJOPropertyBuilder
 
     protected Linked<AnnotatedMethod> _setters;
 
-    public POJOPropertyBuilder(PropertyName internalName, AnnotationIntrospector ai,
-            boolean forSerialization) {
-        this(internalName, internalName, ai, forSerialization);
+    public POJOPropertyBuilder(MapperConfig<?> config, AnnotationIntrospector ai,
+            boolean forSerialization, PropertyName internalName) {
+        this(config, ai, forSerialization, internalName, internalName);
     }
 
-    protected POJOPropertyBuilder(PropertyName internalName, PropertyName name,
-            AnnotationIntrospector annotationIntrospector, boolean forSerialization)
+    protected POJOPropertyBuilder(MapperConfig<?> config, AnnotationIntrospector ai,
+            boolean forSerialization, PropertyName internalName, PropertyName name)
     {
+        _config = config;
+        _annotationIntrospector = ai;
         _internalName = internalName;
         _name = name;
-        _annotationIntrospector = annotationIntrospector;
         _forSerialization = forSerialization;
     }
 
     public POJOPropertyBuilder(POJOPropertyBuilder src, PropertyName newName)
     {
+        _config = src._config;
+        _annotationIntrospector = src._annotationIntrospector;
         _internalName = src._internalName;
         _name = newName;
-        _annotationIntrospector = src._annotationIntrospector;
         _fields = src._fields;
         _ctorParameters = src._ctorParameters;
         _getters = src._getters;
@@ -269,9 +273,7 @@ public AnnotatedMethod getSetter()
         }
         // But if multiple, verify that they do not conflict...
         for (; next != null; next = next.next) {
-            /* [JACKSON-255] Allow masking, i.e. do not report exception if one
-             *   is in super-class from the other
-             */
+            // Allow masking, i.e. do not fail if one is in super-class from the other
             Class<?> currClass = curr.value.getDeclaringClass();
             Class<?> nextClass = next.value.getDeclaringClass();
             if (currClass != nextClass) {
@@ -283,20 +285,38 @@ public AnnotatedMethod getSetter()
                     continue;
                 }
             }
+            AnnotatedMethod nextM = next.value;
+            AnnotatedMethod currM = curr.value;
+
             /* 30-May-2014, tatu: Two levels of precedence:
              * 
              * 1. Regular setters ("setX(...)")
              * 2. Implicit, possible setters ("x(...)")
              */
-            int priNext = _setterPriority(next.value);
-            int priCurr = _setterPriority(curr.value);
+            int priNext = _setterPriority(nextM);
+            int priCurr = _setterPriority(currM);
 
             if (priNext != priCurr) {
                 if (priNext < priCurr) {
                     curr = next;
                 }
                 continue;
             }
+            // 11-Dec-2015, tatu: As per [databind#1033] allow pluggable conflict resolution
+            if (_annotationIntrospector != null) {
+                AnnotatedMethod pref = _annotationIntrospector.resolveSetterConflict(_config,
+                        currM, nextM);
+                
+                // note: should be one of nextM/currM; but no need to check
+                if (pref == currM) {
+                    continue;
+                }
+                if (pref == nextM) {
+                    curr = next;
+                    continue;
+                }
+            }
+            
             throw new IllegalArgumentException("Conflicting setter definitions for property \""+getName()+"\": "
                     +curr.value.getFullName()+" vs "+next.value.getFullName());
         }
@@ -408,7 +428,7 @@ public AnnotatedMember getPrimaryMember() {
     protected int _getterPriority(AnnotatedMethod m)
     {
         final String name = m.getName();
-        // [#238]: Also, regular getters have precedence over "is-getters"
+        // [databind#238]: Also, regular getters have precedence over "is-getters"
         if (name.startsWith("get") && name.length() > 3) {
             // should we check capitalization?
             return 1;
@@ -428,7 +448,7 @@ protected int _setterPriority(AnnotatedMethod m)
         }
         return 2;
     }
-    
+
     /*
     /**********************************************************
     /* Implementations of refinement accessors
@@ -885,7 +905,7 @@ private void _explode(Collection<PropertyName> newNames,
         for (Linked<?> node = accessors; node != null; node = node.next) {
             PropertyName name = node.name;
             if (!node.isNameExplicit || name == null) { // no explicit name -- problem!
-                // [Issue#541] ... but only as long as it's visible
+                // [databind#541] ... but only as long as it's visible
                 if (!node.isVisible) {
                     continue;
                 }
@@ -896,7 +916,8 @@ private void _explode(Collection<PropertyName> newNames,
             }
             POJOPropertyBuilder prop = props.get(name);
             if (prop == null) {
-                prop = new POJOPropertyBuilder(_internalName, name, _annotationIntrospector, _forSerialization);
+                prop = new POJOPropertyBuilder(_config, _annotationIntrospector, _forSerialization,
+                        _internalName, name);
                 props.put(name, prop);
             }
             // ultra-clumsy, part 2 -- lambdas would be nice here

src/test/java/com/fasterxml/jackson/databind/introspect/SetterConflictTest.java

@@ -0,0 +1,32 @@
+package com.fasterxml.jackson.databind.introspect;
+
+import com.fasterxml.jackson.databind.*;
+
+// mostly for [databind#1033]
+public class SetterConflictTest extends BaseMapTest
+{
+    // Should prefer primitives over Strings, more complex types, by default
+    static class Issue1033Bean {
+        public int value;
+
+        public void setValue(int v) { value = v; }
+        public void setValue(Issue1033Bean foo) {
+            throw new Error("Should not get called");
+        }
+    }
+
+    /*
+    /**********************************************************
+    /* Test methods
+    /**********************************************************
+     */
+    
+    private final ObjectMapper MAPPER = objectMapper();
+
+    public void testSetterPriority() throws Exception
+    {
+        Issue1033Bean bean = MAPPER.readValue(aposToQuotes("{'value':42}"),
+                Issue1033Bean.class);
+        assertEquals(42, bean.value);
+    }
+}

src/test/java/com/fasterxml/jackson/databind/introspect/TestPropertyConflicts.java

@@ -12,14 +12,14 @@
  */
 public class TestPropertyConflicts extends BaseMapTest
 {
-    // For [JACKSON-694]: error message for conflicting getters sub-optimal
+    // error message for conflicting getters sub-optimal
     static class BeanWithConflict
     {
         public int getX() { return 3; }
         public boolean getx() { return false; }
     }
 
-    // [Issue#238]
+    // [databind#238]
     protected static class Getters1A
     {
         @JsonProperty
@@ -67,7 +67,7 @@ public void _stuff(String value) {
         }
     }
 
-    // For [Issue#541]
+    // For [databind#541]
     static class Bean541 {
         protected String str;
 
@@ -81,14 +81,13 @@ public String getStr() {
             return str;
         }
      }
-     
+
     /*
     /**********************************************************
     /* Test methods
     /**********************************************************
      */
-    
-    // for [JACKSON-694]
+
     public void testFailWithDupProps() throws Exception
     {
         BeanWithConflict bean = new BeanWithConflict();
@@ -100,7 +99,7 @@ public void testFailWithDupProps() throws Exception
         }
     }        
 
-    // [Issue#238]: ok to have getter, "isGetter"
+    // [databind#238]: ok to have getter, "isGetter"
     public void testRegularAndIsGetter() throws Exception
     {
         final ObjectWriter writer = objectWriter();

src/test/java/com/fasterxml/jackson/databind/ser/TestBeanSerializer.java

@@ -157,7 +157,7 @@ public List<BeanPropertyWriter> changeProperties(SerializationConfig config,
         {
             JavaType strType = config.constructType(String.class);
             // we need a valid BeanPropertyDefinition; this will do (just need name to match)
-            POJOPropertyBuilder prop = new POJOPropertyBuilder(new PropertyName("bogus"), null, true);
+            POJOPropertyBuilder prop = new POJOPropertyBuilder(config, null, true, new PropertyName("bogus"));
             try {
                 AnnotatedField f = new AnnotatedField(null, EmptyBean.class.getDeclaredField("name"), null);
                 beanProperties.add(new BeanPropertyWriter(prop, f, null,