Fix #1575

Author: cowtowncoder

release-notes/VERSION

@@ -9,6 +9,8 @@ Project: jackson-databind
 #1533: `AsPropertyTypeDeserializer` ignores `DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT`
 #1543: JsonFormat.Shape.NUMBER_INT does not work when defined on enum type in 2.8
  (reported by Alex P)
+#1575: Problem with `@JsonIgnoreProperties` on recursive property (regression in 2.8)
+ (reported by anujkumar04@github)
 - Minor fix to creation of `PropertyMetadata`, had one path that could lead to NPE
 
 2.8.7 (21-Feb-2017)

src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java

@@ -356,7 +356,6 @@ public BeanDeserializerBase(BeanDeserializerBase src, ObjectIdReader oir)
     public BeanDeserializerBase(BeanDeserializerBase src, Set<String> ignorableProps)
     {
         super(src._beanType);
-        
         _classAnnotations = src._classAnnotations;
         _beanType = src._beanType;
 
@@ -461,55 +460,35 @@ public void resolve(DeserializationContext ctxt)
 
         if (_valueInstantiator.canCreateFromObjectWith()) {
             creatorProps = _valueInstantiator.getFromObjectArguments(ctxt.getConfig());
-
-            // 21-Jun-2015, tatu: This resolution was duplicated later on and seems like
-            //    it really should be only done at a later point. So commented out in 2.8.
-            //   However, just in case there was a reason for it, leaving commented out
-            //   here instead of immediately removing.
-
-            /*
-            // also: need to try to resolve 'external' type ids...
-            for (SettableBeanProperty prop : creatorProps) {
-                if (prop.hasValueTypeDeserializer()) {
-                    TypeDeserializer typeDeser = prop.getValueTypeDeserializer();
-                    if (typeDeser.getTypeInclusion() == JsonTypeInfo.As.EXTERNAL_PROPERTY) {
-                        if (extTypes == null) {
-                            extTypes = new ExternalTypeHandler.Builder();
-                        }
-                        extTypes.addExternal(prop, typeDeser);
-                    }
-                }
-            }
-            */
         } else {
             creatorProps = null;
         }
-
         UnwrappedPropertyHandler unwrapped = null;
 
-        for (SettableBeanProperty origProp : _beanProperties) {
-            SettableBeanProperty prop = origProp;
+        // 24-Mar-2017, tatu: Looks like we may have to iterate over
+        //   properties twice, to handle potential issues with recursive
+        //   types (see [databind#1575] f.ex).
 
-            // May already have deserializer from annotations, if so, skip:
+        // First loop: find deserializer if not yet known, but do not yet
+        // contextualize (since that can lead to problems with self-references)
+        for (SettableBeanProperty prop : _beanProperties) {
             if (!prop.hasValueDeserializer()) {
                 // [databind#125]: allow use of converters
                 JsonDeserializer<?> deser = findConvertingDeserializer(ctxt, prop);
                 if (deser == null) {
-                    deser = findDeserializer(ctxt, prop.getType(), prop);
-                }
-                prop = prop.withValueDeserializer(deser);
-            } else { // may need contextual version
-                JsonDeserializer<Object> deser = prop.getValueDeserializer();
-                /* Important! This is the only place where actually handle "primary"
-                 * property deserializers -- call is different from other places.
-                 */
-                JsonDeserializer<?> cd = ctxt.handlePrimaryContextualization(deser, prop,
-                        prop.getType());
-                if (cd != deser) {
-                    prop = prop.withValueDeserializer(cd);
+                    deser = ctxt.findNonContextualValueDeserializer(prop.getType());
                 }
+                SettableBeanProperty newProp = prop.withValueDeserializer(deser);
+                _replaceProperty(_beanProperties, creatorProps, prop, newProp);
             }
+        }
 
+        // Second loop: contextualize, find other pieces
+        for (SettableBeanProperty origProp : _beanProperties) {
+            SettableBeanProperty prop = origProp;
+            JsonDeserializer<?> deser = prop.getValueDeserializer();
+            deser = ctxt.handlePrimaryContextualization(deser, prop, prop.getType());
+            prop = prop.withValueDeserializer(deser);
             // Need to link managed references with matching back references
             prop = _resolveManagedReferenceProperty(ctxt, prop);
 
@@ -525,36 +504,18 @@ public void resolve(DeserializationContext ctxt)
                     unwrapped = new UnwrappedPropertyHandler();
                 }
                 unwrapped.addProperty(prop);
-                /* 12-Dec-2014, tatu: As per [databind#647], we will have problems if
-                 *    the original property is left in place. So let's remove it now.
-                 */
+                // 12-Dec-2014, tatu: As per [databind#647], we will have problems if
+                //    the original property is left in place. So let's remove it now.
+                // 25-Mar-2017, tatu: Wonder if this could be problematic wrt creators?
                 _beanProperties.remove(prop);
                 continue;
             }
             // non-static inner classes too:
             prop = _resolveInnerClassValuedProperty(ctxt, prop);
             if (prop != origProp) {
-                _beanProperties.replace(prop);
-                // [databind#795]: Make sure PropertyBasedCreator's properties stay in sync
-                if (creatorProps != null) {
-                    // 18-May-2015, tatu: _Should_ start with consistent set. But can we really
-                    //   fully count on this? May need to revisit in future; seems to hold for now.
-                    for (int i = 0, len = creatorProps.length; i < len; ++i) {
-                        if (creatorProps[i] == origProp) {
-                            creatorProps[i] = prop;
-                            break;
-                        }
-                        // ... as per above, it is possible we'd need to add this as fallback
-                        // if (but only if) identity check fails?
-                        /*
-                        if (creatorProps[i].getName().equals(prop.getName())) {
-                            creatorProps[i] = prop;
-                            break;
-                        }
-                        */
-                    }
-                }
+                _replaceProperty(_beanProperties, creatorProps, origProp, prop);
             }
+
             // one more thing: if this property uses "external property" type inclusion,
             // it needs different handling altogether
             if (prop.hasValueTypeDeserializer()) {
@@ -616,11 +577,38 @@ public void resolve(DeserializationContext ctxt)
         if (unwrapped != null) { // we consider this non-standard, to offline handling
             _nonStandardCreation = true;
         }
-
         // may need to disable vanilla processing, if unwrapped handling was enabled...
         _vanillaProcessing = _vanillaProcessing && !_nonStandardCreation;
     }
 
+    /**
+     * @since 2.8.8
+     */
+    protected void _replaceProperty(BeanPropertyMap props, SettableBeanProperty[] creatorProps,
+            SettableBeanProperty origProp, SettableBeanProperty newProp)
+    {
+        props.replace(newProp);
+        // [databind#795]: Make sure PropertyBasedCreator's properties stay in sync
+        if (creatorProps != null) {
+            // 18-May-2015, tatu: _Should_ start with consistent set. But can we really
+            //   fully count on this? May need to revisit in future; seems to hold for now.
+            for (int i = 0, len = creatorProps.length; i < len; ++i) {
+                if (creatorProps[i] == origProp) {
+                    creatorProps[i] = newProp;
+                    return;
+                }
+            }
+            // ... as per above, it is possible we'd need to add this as fallback
+            // if (but only if) identity check fails?
+            /*
+            if (creatorProps[i].getName().equals(prop.getName())) {
+                creatorProps[i] = prop;
+                break;
+            }
+            */
+        }
+    }
+
     private JsonDeserializer<Object> _findDelegateDeserializer(DeserializationContext ctxt, JavaType delegateType,
             AnnotatedWithParams delegateCreator) throws JsonMappingException {
         // Need to create a temporary property to allow contextual deserializers:
@@ -645,6 +633,9 @@ private JsonDeserializer<Object> _findDelegateDeserializer(DeserializationContex
      * Helper method that can be used to see if specified property is annotated
      * to indicate use of a converter for property value (in case of container types,
      * it is container type itself, not key or content type).
+     *<p>
+     * NOTE: returned deserializer is NOT yet contextualized, caller needs to take
+     * care to do that.
      * 
      * @since 2.2
      */
@@ -658,8 +649,10 @@ protected JsonDeserializer<Object> findConvertingDeserializer(DeserializationCon
             if (convDef != null) {
                 Converter<Object,Object> conv = ctxt.converterInstance(prop.getMember(), convDef);
                 JavaType delegateType = conv.getInputType(ctxt.getTypeFactory());
-                JsonDeserializer<?> ser = ctxt.findContextualValueDeserializer(delegateType, prop);
-                return new StdDelegatingDeserializer<Object>(conv, delegateType, ser);
+                // 25-Mar-2017, tatu: should not yet contextualize
+//                JsonDeserializer<?> deser = ctxt.findContextualValueDeserializer(delegateType, prop);
+                JsonDeserializer<?> deser = ctxt.findNonContextualValueDeserializer(delegateType);
+                return new StdDelegatingDeserializer<Object>(conv, delegateType, deser);
             }
         }
         return null;
@@ -677,7 +670,7 @@ public JsonDeserializer<?> createContextual(DeserializationContext ctxt,
             BeanProperty property) throws JsonMappingException
     {
         ObjectIdReader oir = _objectIdReader;
-        
+
         // First: may have an override for Object Id:
         final AnnotationIntrospector intr = ctxt.getAnnotationIntrospector();
         final AnnotatedMember accessor = (property == null || intr == null)

src/main/java/com/fasterxml/jackson/databind/deser/std/StdDelegatingDeserializer.java

@@ -104,6 +104,8 @@ protected StdDelegatingDeserializer<T> withDelegate(Converter<Object,T> converte
     /**********************************************************
      */
 
+    // Note: unlikely to get called since most likely instances explicitly constructed;
+    // if so, caller must ensure delegating deserializer is properly resolve()d.
     @Override
     public void resolve(DeserializationContext ctxt)
         throws JsonMappingException

src/test/java/com/fasterxml/jackson/databind/convert/ConvertingAbstractSerializer795Test.java

@@ -34,9 +34,9 @@ public static class AbstractCustomTypeUser {
         @JsonDeserialize(converter = AbstractCustomTypeDeserializationConverter.class)
         private final AbstractCustomType customField;
 
-        @JsonCreator AbstractCustomTypeUser(@JsonProperty("customField")
-        AbstractCustomType customField) {
-            this.customField = customField;
+        @JsonCreator
+        AbstractCustomTypeUser(@JsonProperty("customField") AbstractCustomType cf) {
+            this.customField = cf;
         }
     }