More work on #888; rewriting Map-include handling, adding temporarily test failures

Author: cowtowncoder

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

@@ -930,12 +930,24 @@ public abstract JsonSerializer<Object> serializerInstance(Annotated annotated,
      * filter instance,
      * given a {@link Class} to instantiate (with default constructor, by default).
      *
+     * @param forProperty (optional) If filter is created for a property, that property;
+     *    `null` if filter created via defaulting, global or per-type.
+     *
      * @since 2.9
      */
     public abstract Object includeFilterInstance(BeanPropertyDefinition forProperty,
             Class<?> filterClass)
         throws JsonMappingException;
 
+    /**
+     * Follow-up method that may be called after calling {@link #includeFilterInstance},
+     * to check handling of `null` values by the filter.
+     *
+     * @since 2.9
+     */
+    public abstract boolean includeFilterSuppressNulls(Object filter)
+        throws JsonMappingException;
+
     /*
     /**********************************************************
     /* Support for contextualization
@@ -1180,7 +1192,17 @@ public <T> T reportBadDefinition(JavaType type, String msg, Throwable cause)
         e.initCause(cause);
         throw e;
     }
-    
+
+    /**
+     * @since 2.9
+     */
+    public <T> T reportBadDefinition(Class<?> raw, String msg, Throwable cause)
+            throws JsonMappingException {
+        InvalidDefinitionException e = InvalidDefinitionException.from(getGenerator(), msg, constructType(raw));
+        e.initCause(cause);
+        throw e;
+    }
+
     /**
      * Helper method called to indicate problem; default behavior is to construct and
      * throw a {@link JsonMappingException}, but in future may collect more than one

src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java

@@ -774,12 +774,7 @@ protected JsonSerializer<?> buildMapSerializer(SerializerProvider prov,
                 MapSerializer mapSer = MapSerializer.construct(ignored,
                         type, staticTyping, elementTypeSerializer,
                         keySerializer, elementValueSerializer, filterId);
-                Object suppressableValue = findSuppressableContentValue(config,
-                        type.getContentType(), beanDesc);
-                if (suppressableValue != null) {
-                    mapSer = mapSer.withContentInclusion(suppressableValue);
-                }
-                ser = mapSer;
+                ser = _checkMapContentInclusion(prov, beanDesc, mapSer);
             }
         }
         // [databind#120]: Allow post-processing
@@ -792,42 +787,95 @@ protected JsonSerializer<?> buildMapSerializer(SerializerProvider prov,
     }
 
     /**
-     *<p>
-     * NOTE: although return type is left opaque, it really needs to be
-     * <code>JsonInclude.Include</code> for things to work as expected.
+     * Helper method that does figures out content inclusion value to use, if any,
+     * and construct re-configured {@link MapSerializer} appropriately.
      *
-     * @since 2.5
+     * @since 2.9
      */
-    protected Object findSuppressableContentValue(SerializationConfig config,
-            JavaType contentType, BeanDescription beanDesc)
+    protected MapSerializer _checkMapContentInclusion(SerializerProvider prov,
+            BeanDescription beanDesc, MapSerializer mapSer)
         throws JsonMappingException
     {
-        /* 16-Apr-2016, tatu: Should this consider possible property-config overrides?
-         *    Quite possibly yes, but would need to carefully check that content type being
-         *    used is appropriate.
-         */
-        JsonInclude.Value inclV = beanDesc.findPropertyInclusion(config.getDefaultPropertyInclusion());
+        final SerializationConfig config = prov.getConfig();
 
-        if (inclV == null) {
-            return null;
+        // 30-Sep-2016, tatu: Defaulting gets complicated because we might have two distinct
+        //   axis to consider: Map itself, and then value type.
+        //  Start with Map-defaults, then use more-specific value override, if any
+
+        // Start by getting global setting, overridden by Map-type-override
+        JsonInclude.Value inclV = config.getDefaultPropertyInclusion(Map.class,
+                config.getDefaultPropertyInclusion());
+        // and then merge content-type overrides, if any. But note that there's
+        // content-to-value inclusion shift we have to do
+        final JavaType contentType = mapSer.getContentType();
+        JsonInclude.Value valueIncl = config.getDefaultPropertyInclusion(contentType.getRawClass(), null);
+
+        if (valueIncl != null) {
+            switch (valueIncl.getValueInclusion()) {
+            case USE_DEFAULTS:
+                break;
+            case CUSTOM:
+                inclV = inclV.withContentFilter(valueIncl.getContentFilter());
+                break;
+            default:
+                inclV = inclV.withContentInclusion(valueIncl.getValueInclusion());
+            }
         }
-        JsonInclude.Include incl = inclV.getContentInclusion();
-        switch (incl) {
-        case USE_DEFAULTS: // means "dunno"
-            return null;
+        JsonInclude.Include incl = (inclV == null) ? JsonInclude.Include.USE_DEFAULTS : inclV.getContentInclusion();
+        if (incl == JsonInclude.Include.USE_DEFAULTS) {
+            if (!config.isEnabled(SerializationFeature.WRITE_NULL_MAP_VALUES)) {
+                return mapSer.withContentInclusion(null, true);
+            }
+            return mapSer;
+        }
+        // NOTE: mostly copied from `PropertyBuilder`; would be nice to refactor
+        // but code is not identical nor are these types related
+        Object valueToSuppress;
+        boolean suppressNulls;
+
+        switch (inclV.getContentInclusion()) {
         case NON_DEFAULT:
-            // 19-Oct-2014, tatu: Not sure what this'd mean; so take it to mean "NON_EMPTY"...
-            // 11-Nov-2015, tatu: With 2.6, we did indeed revert to "NON_EMPTY", but that did
-            //    not go well, so with 2.7, we'll do this instead...
-            //   But not 100% sure if we ought to call new `JsonSerializer.findDefaultValue()`;
-            //   to do that, would need to locate said serializer
-//            incl = JsonInclude.Include.NON_EMPTY;
+            valueToSuppress = BeanUtil.getDefaultValue(contentType);
+            suppressNulls = true;
+            if (valueToSuppress != null) {
+                if (valueToSuppress.getClass().isArray()) {
+                    valueToSuppress = ArrayBuilders.getArrayComparator(valueToSuppress);
+                }
+            }
             break;
+        case NON_ABSENT: // new with 2.6, to support Guava/JDK8 Optionals
+            // always suppress nulls
+            suppressNulls = true;
+            // and for referential types, also "empty", which in their case means "absent"
+            valueToSuppress = contentType.isReferenceType()
+                    ? MapSerializer.MARKER_FOR_EMPTY : null;
+            break;
+        case NON_EMPTY:
+            // always suppress nulls
+            suppressNulls = true;
+            // but possibly also 'empty' values:
+            valueToSuppress = MapSerializer.MARKER_FOR_EMPTY;
+            break;
+        case CUSTOM: // new with 2.9
+            valueToSuppress = prov.includeFilterInstance(null, inclV.getContentFilter());
+            if (valueToSuppress == null) { // is this legal?
+                suppressNulls = true;
+            } else {
+                suppressNulls = prov.includeFilterSuppressNulls(valueToSuppress);
+            }
+            break;
+        case NON_NULL:
+            valueToSuppress = null;
+            suppressNulls = true;
+            // fall through
+        case ALWAYS: // default
         default:
-            // all other modes actually good as is, unless we'll find better ways
+            valueToSuppress = null;
+            suppressNulls = !prov.isEnabled(SerializationFeature.WRITE_NULL_MAP_VALUES);
             break;
         }
-        return incl;
+
+        return mapSer.withContentInclusion(valueToSuppress, suppressNulls);
     }
 
     /*

src/main/java/com/fasterxml/jackson/databind/ser/DefaultSerializerProvider.java

@@ -155,6 +155,25 @@ public Object includeFilterInstance(BeanPropertyDefinition forProperty,
         return filter;
     }
 
+    @Override
+    public boolean includeFilterSuppressNulls(Object filter) throws JsonMappingException
+    {
+        if (filter == null) {
+            return true;
+        }
+        // should let filter decide what to do with nulls:
+        // But just case, let's handle unexpected (from our perspective) problems explicitly
+        try {
+            return filter.equals(null);
+        } catch (Throwable t) {
+            String msg = String.format(
+"Problem determining whether filter of type '%s' should filter out `null` values: (%s) %s",
+filter.getClass().getName(), t.getClass().getName(), t.getMessage());
+            reportBadDefinition(filter.getClass(), msg, t);
+            return false; // never gets here
+        }
+    }
+
     /*
     /**********************************************************
     /* Object Id handling

src/main/java/com/fasterxml/jackson/databind/ser/PropertyBuilder.java

@@ -143,7 +143,6 @@ protected BeanPropertyWriter buildWriter(SerializerProvider prov,
         if (inclusion == JsonInclude.Include.USE_DEFAULTS) { // should not occur but...
             inclusion = JsonInclude.Include.ALWAYS;
         }
-
         switch (inclusion) {
         case NON_DEFAULT:
             // 11-Nov-2015, tatu: This is tricky because semantics differ between cases,
@@ -158,7 +157,7 @@ protected BeanPropertyWriter buildWriter(SerializerProvider prov,
                 }
                 valueToSuppress = getPropertyDefaultValue(propDef.getName(), am, actualType);
             } else {
-                valueToSuppress = getDefaultValue(actualType);
+                valueToSuppress = BeanUtil.getDefaultValue(actualType);
                 suppressNulls = true;
             }
             if (valueToSuppress == null) {
@@ -185,20 +184,10 @@ protected BeanPropertyWriter buildWriter(SerializerProvider prov,
             break;
         case CUSTOM: // new with 2.9
             valueToSuppress = prov.includeFilterInstance(propDef, inclV.getValueFilter());
-
             if (valueToSuppress == null) { // is this legal?
                 suppressNulls = true;
             } else {
-                // should let filter decide what to do with nulls:
-                // But just case, let's handle unexpected (from our perspective) problems explicitly
-                try {
-                    suppressNulls = valueToSuppress.equals(null);
-                } catch (Throwable t) {
-                    String msg = String.format(
-"Problem determining whether filter of type '%s' should filter out `null` values: (%s) %s",
-valueToSuppress.getClass().getName(), t.getClass().getName(), t.getMessage());
-                    prov.reportBadDefinition(_beanDesc.getType(), msg, t);
-                }
+                suppressNulls = prov.includeFilterSuppressNulls(valueToSuppress);
             }
             break;
         case NON_NULL:
@@ -342,37 +331,13 @@ protected Object getPropertyDefaultValue(String name, AnnotatedMember member,
     }
 
     /**
-     * Accessor used to find out "default value" to use for comparing values to
-     * serialize, to determine whether to exclude value from serialization with
-     * inclusion type of {@link com.fasterxml.jackson.annotation.JsonInclude.Include#NON_DEFAULT}.
-     *<p>
-     * Default logic is such that for primitives and wrapper types for primitives, expected
-     * defaults (0 for `int` and `java.lang.Integer`) are returned; for Strings, empty String,
-     * and for structured (Maps, Collections, arrays) and reference types, criteria
-     * {@link com.fasterxml.jackson.annotation.JsonInclude.Include#NON_DEFAULT}
-     * is used.
-     *
-     * @since 2.7
+     * @deprecated Since 2.9
      */
-    protected Object getDefaultValue(JavaType type)
-    {
-        // 06-Nov-2015, tatu: Returning null is fine for Object types; but need special
-        //   handling for primitives since they are never passed as nulls.
-        Class<?> cls = type.getRawClass();
-
-        Class<?> prim = ClassUtil.primitiveType(cls);
-        if (prim != null) {
-            return ClassUtil.defaultValue(prim);
-        }
-        if (type.isContainerType() || type.isReferenceType()) {
-            return JsonInclude.Include.NON_EMPTY;
-        }
-        if (cls == String.class) {
-            return "";
-        }
-        return null;
+    @Deprecated // since 2.9
+    protected Object getDefaultValue(JavaType type) {
+        return BeanUtil.getDefaultValue(type);
     }
-    
+
     /*
     /**********************************************************
     /* Helper methods for exception handling

src/main/java/com/fasterxml/jackson/databind/ser/std/BeanSerializerBase.java

@@ -494,7 +494,6 @@ public JsonSerializer<?> createContextual(SerializerProvider provider,
                             objectIdInfo.getAlwaysAsId());
                 }
             }
-            
             // Or change Filter Id in use?
             Object filterId = intr.findFilterId(accessor);
             if (filterId != null) {
@@ -523,6 +522,7 @@ public JsonSerializer<?> createContextual(SerializerProvider provider,
         if (shape == null) {
             shape = _serializationShape;
         }
+        // last but not least; may need to transmute into as-array serialization
         if (shape == JsonFormat.Shape.ARRAY) {
             return contextual.asArraySerializer();
         }