Fix #2909

cowtowncoder · cowtowncoder · commit 170c6d40710d · 2020-10-31T09:27:59.000-07:00
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -24,6 +24,8 @@ Project: jackson-databind
   values could be converted to integers losslessly
  (requested by Oguzhan U; implementation contributed by Siavash S)
 #2903: Allow preventing "Enum from integer" coercion using new `CoercionConfig` system
+#2909: `@JsonValue` not considered when evaluating inclusion
+ (reported by chrylis@github)
 
 2.12.0-rc1 (12-Oct-2020)
 
diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/std/JsonValueSerializer.java b/src/main/java/com/fasterxml/jackson/databind/ser/std/JsonValueSerializer.java
@@ -7,8 +7,10 @@
 import java.util.Set;
 
 import com.fasterxml.jackson.annotation.JsonTypeInfo.As;
+
 import com.fasterxml.jackson.core.*;
 import com.fasterxml.jackson.core.type.WritableTypeId;
+
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.annotation.JacksonStdImpl;
 import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
@@ -20,6 +22,7 @@
 import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
 import com.fasterxml.jackson.databind.ser.BeanSerializer;
 import com.fasterxml.jackson.databind.ser.ContextualSerializer;
+import com.fasterxml.jackson.databind.ser.impl.PropertySerializerMap;
 import com.fasterxml.jackson.databind.util.ClassUtil;
 
 /**
@@ -49,14 +52,29 @@ public class JsonValueSerializer
 
     protected final BeanProperty _property;
 
+    /**
+     * Declared type of the value accessed, as declared by accessor.
+     *
+     * @since 2.12
+     */
+    protected final JavaType _valueType;
+
     /**
      * This is a flag that is set in rare (?) cases where this serializer
      * is used for "natural" types (boolean, int, String, double); and where
      * we actually must force type information wrapping, even though
      * one would not normally be added.
      */
     protected final boolean _forceTypeInformation;
-    
+
+    /**
+     * If value type cannot be statically determined, mapping from
+     * runtime value types to serializers are cached in this object.
+     *
+     * @since 2.12
+     */
+    protected transient PropertySerializerMap _dynamicSerializers;
+
     /*
     /**********************************************************
     /* Life-cycle
@@ -77,9 +95,11 @@ public JsonValueSerializer(AnnotatedMember accessor, JsonSerializer<?> ser)
     {
         super(accessor.getType());
         _accessor = accessor;
+        _valueType = accessor.getType();
         _valueSerializer = (JsonSerializer<Object>) ser;
         _property = null;
         _forceTypeInformation = true; // gets reconsidered when we are contextualized
+        _dynamicSerializers = PropertySerializerMap.emptyForProperties();
     }
 
     @SuppressWarnings("unchecked")
@@ -88,9 +108,11 @@ public JsonValueSerializer(JsonValueSerializer src, BeanProperty property,
     {
         super(_notNullClass(src.handledType()));
         _accessor = src._accessor;
+        _valueType = src._valueType;
         _valueSerializer = (JsonSerializer<Object>) ser;
         _property = property;
         _forceTypeInformation = forceTypeInfo;
+        _dynamicSerializers = PropertySerializerMap.emptyForProperties();
     }
 
     @SuppressWarnings("unchecked")
@@ -107,7 +129,41 @@ public JsonValueSerializer withResolved(BeanProperty property,
         }
         return new JsonValueSerializer(this, property, ser, forceTypeInfo);
     }
-    
+
+    /*
+    /**********************************************************
+    /* Overrides
+    /**********************************************************
+     */
+
+    @Override // since 2.12
+    public boolean isEmpty(SerializerProvider ctxt, Object value0)
+    {
+        Object referenced = _accessor.getValue(value0);
+
+        if (referenced == null) {
+            return true;
+        }
+        JsonSerializer<Object> ser = _valueSerializer;
+        if (ser == null) {
+            try {
+                Class<?> cc = referenced.getClass();
+                ser = _dynamicSerializers.serializerFor(cc);
+                if (ser == null) {
+                    if (_valueType.hasGenericTypes()) {
+                        ser = _findAndAddDynamic(_dynamicSerializers,
+                                ctxt.constructSpecializedType(_valueType, cc), ctxt);
+                    } else {
+                        ser = _findAndAddDynamic(_dynamicSerializers, cc, ctxt);
+                    }
+                }
+            } catch (JsonMappingException e) {
+                throw new RuntimeJsonMappingException(e);
+            }
+        }
+        return ser.isEmpty(ctxt, referenced);
+    }
+
     /*
     /**********************************************************
     /* Post-processing
@@ -129,20 +185,19 @@ public JsonSerializer<?> createContextual(SerializerProvider provider,
             // if not, we don't really know the actual type until we get the instance.
 
             // 10-Mar-2010, tatu: Except if static typing is to be used
-            JavaType t = _accessor.getType();
-            if (provider.isEnabled(MapperFeature.USE_STATIC_TYPING) || t.isFinal()) {
+            if (provider.isEnabled(MapperFeature.USE_STATIC_TYPING) || _valueType.isFinal()) {
                 // false -> no need to cache
                 /* 10-Mar-2010, tatu: Ideally we would actually separate out type
                  *   serializer from value serializer; but, alas, there's no access
                  *   to serializer factory at this point... 
                  */
                 // 05-Sep-2013, tatu: I _think_ this can be considered a primary property...
-                ser = provider.findPrimaryPropertySerializer(t, property);
+                ser = provider.findPrimaryPropertySerializer(_valueType, property);
                 /* 09-Dec-2010, tatu: Turns out we must add special handling for
                  *   cases where "native" (aka "natural") type is being serialized,
                  *   using standard serializer
                  */
-                boolean forceTypeInformation = isNaturalTypeWithStdHandling(t.getRawClass(), ser);
+                boolean forceTypeInformation = isNaturalTypeWithStdHandling(_valueType.getRawClass(), ser);
                 return withResolved(property, ser, forceTypeInformation);
             }
             // [databind#2822]: better hold on to "property", regardless
@@ -164,27 +219,30 @@ public JsonSerializer<?> createContextual(SerializerProvider provider,
      */
     
     @Override
-    public void serialize(Object bean, JsonGenerator gen, SerializerProvider prov) throws IOException
+    public void serialize(Object bean, JsonGenerator gen, SerializerProvider provider) throws IOException
     {
         try {
             Object value = _accessor.getValue(bean);
             if (value == null) {
-                prov.defaultSerializeNull(gen);
+                provider.defaultSerializeNull(gen);
                 return;
             }
             JsonSerializer<Object> ser = _valueSerializer;
             if (ser == null) {
-                Class<?> c = value.getClass();
-                // 10-Mar-2010, tatu: Ideally we would actually separate out type
-                //   serializer from value serializer; but, alas, there's no access
-                //   to serializer factory at this point...
-
-                // let's cache it, may be needed soon again
-                ser = prov.findTypedValueSerializer(c, true, _property);
+                Class<?> cc = value.getClass();
+                ser = _dynamicSerializers.serializerFor(cc);
+                if (ser == null) {
+                    if (_valueType.hasGenericTypes()) {
+                        ser = _findAndAddDynamic(_dynamicSerializers,
+                                provider.constructSpecializedType(_valueType, cc), provider);
+                    } else {
+                        ser = _findAndAddDynamic(_dynamicSerializers, cc, provider);
+                    }
+                }
             }
-            ser.serialize(value, gen, prov);
+            ser.serialize(value, gen, provider);
         } catch (Exception e) {
-            wrapAndThrow(prov, e, bean, _accessor.getName() + "()");
+            wrapAndThrow(provider, e, bean, _accessor.getName() + "()");
         }
     }
 
@@ -203,7 +261,16 @@ public void serializeWithType(Object bean, JsonGenerator gen, SerializerProvider
             }
             JsonSerializer<Object> ser = _valueSerializer;
             if (ser == null) { // no serializer yet? Need to fetch
-                ser = provider.findValueSerializer(value.getClass(), _property);
+                Class<?> cc = value.getClass();
+                ser = _dynamicSerializers.serializerFor(cc);
+                if (ser == null) {
+                    if (_valueType.hasGenericTypes()) {
+                        ser = _findAndAddDynamic(_dynamicSerializers,
+                                provider.constructSpecializedType(_valueType, cc), provider);
+                    } else {
+                        ser = _findAndAddDynamic(_dynamicSerializers, cc, provider);
+                    }
+                }
             } else {
                 // 09-Dec-2010, tatu: To work around natural type's refusal to add type info, we do
                 //    this (note: type is for the wrapper type, not enclosed value!)
@@ -219,7 +286,7 @@ public void serializeWithType(Object bean, JsonGenerator gen, SerializerProvider
             }
             // 28-Sep-2016, tatu: As per [databind#1385], we do need to do some juggling
             //    to use different Object for type id (logical type) and actual serialization
-            //    (delegat type).
+            //    (delegate type).
             TypeSerializerRerouter rr = new TypeSerializerRerouter(typeSer0, bean);
             ser.serializeWithType(value, gen, provider, rr);
         } catch (Exception e) {
@@ -251,7 +318,6 @@ public void acceptJsonFormatVisitor(JsonFormatVisitorWrapper visitor, JavaType t
          *    
          *    Note that meaning of JsonValue, then, is very different for Enums. Sigh.
          */
-        final JavaType type = _accessor.getType();
         Class<?> declaring = _accessor.getDeclaringClass();
         if ((declaring != null) && ClassUtil.isEnumType(declaring)) {
             if (_acceptJsonFormatVisitorForEnum(visitor, typeHint, declaring)) {
@@ -260,13 +326,13 @@ public void acceptJsonFormatVisitor(JsonFormatVisitorWrapper visitor, JavaType t
         }
         JsonSerializer<Object> ser = _valueSerializer;
         if (ser == null) {
-            ser = visitor.getProvider().findTypedValueSerializer(type, false, _property);
+            ser = visitor.getProvider().findTypedValueSerializer(_valueType, false, _property);
             if (ser == null) { // can this ever occur?
                 visitor.expectAnyFormat(typeHint);
                 return;
             }
         }
-        ser.acceptJsonFormatVisitor(visitor, type);
+        ser.acceptJsonFormatVisitor(visitor, _valueType);
     }
 
     /**
@@ -322,6 +388,34 @@ protected boolean isNaturalTypeWithStdHandling(Class<?> rawType, JsonSerializer<
         return isDefaultSerializer(ser);
     }
 
+    // @since 2.12
+    protected final JsonSerializer<Object> _findAndAddDynamic(PropertySerializerMap map,
+            Class<?> type, SerializerProvider provider) throws JsonMappingException
+    {
+        // 31-Oct-2020, tatu: Should not get typed/root serializer, but for now has to do:
+        JsonSerializer<Object> serializer = provider.findTypedValueSerializer(type, false, _property);
+        PropertySerializerMap.SerializerAndMapResult result = _dynamicSerializers.addSerializer(type, serializer);
+        // did we get a new map of serializers? If so, start using it
+        if (map != result.map) {
+            _dynamicSerializers = result.map;
+        }
+        return serializer;
+    }
+
+    // @since 2.12
+    protected final JsonSerializer<Object> _findAndAddDynamic(PropertySerializerMap map,
+            JavaType type, SerializerProvider provider) throws JsonMappingException
+    {
+        // 31-Oct-2020, tatu: Should not get typed/root serializer, but for now has to do:
+        JsonSerializer<Object> serializer = provider.findTypedValueSerializer(type, false, _property);
+        PropertySerializerMap.SerializerAndMapResult result = _dynamicSerializers.addSerializer(type, serializer);
+        // did we get a new map of serializers? If so, start using it
+        if (map != result.map) {
+            _dynamicSerializers = result.map;
+        }
+        return serializer;
+    }
+
     /*
     /**********************************************************
     /* Other methods
diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/creators/TestPolymorphicDelegating.java b/src/test/java/com/fasterxml/jackson/databind/deser/creators/TestPolymorphicDelegating.java
@@ -43,7 +43,6 @@ public void testAbstractDelegateWithCreator() throws Exception
         Issue580Bean input = new Issue580Bean(new Issue580Impl(13));
         ObjectMapper mapper = new ObjectMapper();
         String json = mapper.writeValueAsString(input);
-
         Issue580Bean result = mapper.readValue(json, Issue580Bean.class);
         assertNotNull(result);
         assertNotNull(result.value);
diff --git a/src/test/java/com/fasterxml/jackson/databind/ser/filter/MapInclusionTest.java b/src/test/java/com/fasterxml/jackson/databind/ser/filter/MapInclusionTest.java
@@ -4,6 +4,7 @@
 import java.util.*;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonValue;
 import com.fasterxml.jackson.databind.*;
 
 public class MapInclusionTest extends BaseMapTest
@@ -41,6 +42,17 @@ public NoNullsNotEmptyMapContainer add(String key, String value) {
         }
     }
 
+    // [databind#2909]
+    static class Wrapper2909 {
+        @JsonValue
+        public Map<String, String> values = new HashMap<>();
+    }
+
+    static class TopLevel2099 {
+        @JsonInclude(JsonInclude.Include.NON_EMPTY)
+        public Wrapper2909 nested = new Wrapper2909();
+    }
+
     /*
     /**********************************************************
     /* Test methods
@@ -80,4 +92,10 @@ public void testNonEmptyNoNullsMap() throws IOException
                 .add("b", null));
         assertEquals(aposToQuotes("{}"), json);
     }
+
+    // [databind#2909]
+    public void testMapViaJsonValue() throws Exception
+    {
+        assertEquals(a2q("{}"), MAPPER.writeValueAsString(new TopLevel2099()));
+    }
 }
