Fix #8: handle List wrapping even prop declared as Object (#809)

Author: cowtowncoder

release-notes/VERSION

@@ -7,6 +7,8 @@ Version: 3.x (for earlier see VERSION-2.x)
 
 3.2.0 (not yet released)
 
+#8: Serialization of `List` incorrect if property declared as plain `java.lang.Object`
+ (fix by @cowtowncoder, w/ Claude code)
 #802: Serialization with Polymorphisme and `EXTERNAL_PROPERTY` = duplicate property
  (reported by @ Adrien-dev25 )
  (fix by @cowtowncoder, w/ Claude code)

src/main/java/tools/jackson/dataformat/xml/ser/XmlBeanPropertyWriter.java

@@ -9,13 +9,11 @@
 
 /**
  * Property writer sub-class used for handling element wrapping needed for serializing
- * collection (array, Collection; possibly Map) types.
+ * collection (array, Collection; not Map) types.
  */
 public class XmlBeanPropertyWriter
     extends BeanPropertyWriter
 {
-    private static final long serialVersionUID = 1L;
-
     /*
     /**********************************************************************
     /* Config settings
@@ -32,24 +30,60 @@ public class XmlBeanPropertyWriter
      */
     protected final QName _wrappedQName;
 
+    /**
+     * Whether wrapping should be checked dynamically based on the runtime value type.
+     * When {@code true}, wrapping is only applied if the runtime value is actually
+     * a Collection, Iterable, or array. Used for properties declared as {@code Object}
+     * that may or may not contain a collection at runtime.
+     *
+     * @since 3.2
+     */
+    protected final boolean _dynamicWrapping;
+
     /*
     /**********************************************************************
     /* Life-cycle: construction, configuration
     /**********************************************************************
      */
 
+    /**
+     * @deprecated Since 3.2
+     */
+    @Deprecated
     public XmlBeanPropertyWriter(BeanPropertyWriter wrapped,
             PropertyName wrapperName, PropertyName wrappedName) {
-        this(wrapped, wrapperName, wrappedName, null);
+        this(wrapped, wrapperName, wrappedName, null, false);
     }
 
+    /**
+     * @deprecated Since 3.2
+     */
+    @Deprecated
     public XmlBeanPropertyWriter(BeanPropertyWriter wrapped,
             PropertyName wrapperName, PropertyName wrappedName,
             ValueSerializer<Object> serializer)
+    {
+        this(wrapped, wrapperName, wrappedName, serializer, false);
+    }
+
+    /**
+     * @since 3.2
+     */
+    public XmlBeanPropertyWriter(BeanPropertyWriter wrapped,
+            PropertyName wrapperName, PropertyName wrappedName,
+            boolean dynamicWrapping)
+    {
+        this(wrapped, wrapperName, wrappedName, null, dynamicWrapping);
+    }
+
+    private XmlBeanPropertyWriter(BeanPropertyWriter wrapped,
+            PropertyName wrapperName, PropertyName wrappedName,
+            ValueSerializer<Object> serializer, boolean dynamicWrapping)
     {
         super(wrapped);
         _wrapperQName = _qname(wrapperName);
         _wrappedQName = _qname(wrappedName);
+        _dynamicWrapping = dynamicWrapping;
 
         if (serializer != null) {
             assignSerializer(serializer);
@@ -64,7 +98,7 @@ private QName _qname(PropertyName n)
         }
         return new QName(ns, n.getSimpleName());
     }
-    
+
     /*
     /**********************************************************************
     /* Overridden methods
@@ -81,10 +115,17 @@ public void serializeAsProperty(Object bean, JsonGenerator g, SerializationConte
     {
         Object value = get(bean);
 
+        // [dataformat-xml#8]: For dynamic wrapping (Object-typed properties),
+        // check runtime type and delegate to standard handling if not a collection
+        if (_dynamicWrapping && (value == null || !_isIndexedValue(value))) {
+            super.serializeAsProperty(bean, g, ctxt);
+            return;
+        }
+
         /* 13-Feb-2014, tatu: As per [#103], default handling does not really
          *   work here. Rather, we need just a wrapping and should NOT call
          *   null handler, as it does not know what to do...
-         *   
+         *
          *   Question, however, is what should it be serialized as. We have two main
          *   choices; equivalent empty List, and "nothing" (missing). Let's start with
          *   empty List? But producing missing entry is non-trivial...
@@ -102,7 +143,7 @@ public void serializeAsProperty(Object bean, JsonGenerator g, SerializationConte
             }
             */
             // but for missing thing, well, just output nothing
-            
+
             return;
         }
 
@@ -150,4 +191,14 @@ public void serializeAsProperty(Object bean, JsonGenerator g, SerializationConte
             xmlGen.finishWrappedValue(_wrapperQName, _wrappedQName);
         }
     }
+
+    /**
+     * Check if the runtime value is a Collection, array, or Iterable
+     * (i.e. something that should get wrapper element handling).
+     */
+    private static boolean _isIndexedValue(Object value) {
+        return (value instanceof java.util.Collection<?>)
+                || (value instanceof Iterable<?>)
+                || value.getClass().isArray();
+    }
 }

src/main/java/tools/jackson/dataformat/xml/ser/XmlBeanSerializerModifier.java

@@ -48,10 +48,22 @@ public List<BeanPropertyWriter> changeProperties(SerializationConfig config,
             bpw.setInternalSetting(XmlBeanSerializerBase.KEY_XML_INFO,
             		new XmlInfo(isAttribute, ns, isText, isCData));
 
-            // Actually: if we have a Collection type, easiest place to add wrapping would be here...
-            //  or: let's also allow wrapping of "untyped" (Object): assuming it is a dynamically
-            //   typed Collection...
-            if (!TypeUtil.isIndexedType(bpw.getType())) {
+            // If we have a Collection/array type, the easiest place to add wrapping is here.
+            // [dataformat-xml#8]: also allow wrapping of "untyped" (Object): assuming it may
+            // be a dynamically typed Collection at runtime. Use dynamic wrapping so that
+            // wrapping is only applied when runtime value is actually a Collection.
+            final JavaType propType = bpw.getType();
+            final boolean dynamicWrapping;
+
+            if (TypeUtil.isIndexedType(propType)) {
+                dynamicWrapping = false;
+            } else if (propType.isJavaLangObject()
+                    // [dataformat-xml#8]: for Object-typed properties, only wrap standard
+                    // BeanPropertyWriters; skip virtual properties (e.g. @JsonAppend)
+                    // and other custom subclasses whose get() won't survive wrapping
+                    && bpw.getClass() == BeanPropertyWriter.class) {
+                dynamicWrapping = true;
+            } else {
                 continue;
             }
             PropertyName wrappedName = PropertyName.construct(bpw.getName(), ns);
@@ -66,7 +78,10 @@ public List<BeanPropertyWriter> changeProperties(SerializationConfig config,
             if (localName == null || localName.length() == 0) {
                 wrapperName = wrappedName;
             }
-            beanProperties.set(i, new XmlBeanPropertyWriter(bpw, wrapperName, wrappedName));
+            // [dataformat-xml#8]: for Object-typed properties, use dynamic wrapping
+            // that checks at runtime if the value is actually a Collection
+            beanProperties.set(i,
+                    new XmlBeanPropertyWriter(bpw, wrapperName, wrappedName, dynamicWrapping));
         }
         return beanProperties;
     }

src/main/java/tools/jackson/dataformat/xml/util/TypeUtil.java

@@ -10,19 +10,21 @@ public class TypeUtil
      */
     public static boolean isIndexedType(JavaType type)
     {
-        Class<?> cls = type.getRawClass();
         // 25-Mar-2024, tatu [dataformat-xml#646]: Need to support Iterable too
-        if (type.isContainerType() || type.isIterationType() || cls == Iterable.class) {
-            // One special case; byte[] will be serialized as base64-encoded String, not real array, so:
-            // (actually, ditto for char[]; thought to be a String)
-            if (cls == byte[].class || cls == char[].class) {
-                return false;
-            }
+        if (type.isContainerType() || type.isIterationType() || type.hasRawClass(Iterable.class)) {
             // Also, should not add wrapping for Maps
             // [dataformat-xml#220]: nor map-like (Scala Map) types
             if (type.isMapLikeType()) {
                 return false;
             }
+            if (type.isArrayType()) {
+                // Other special cases; byte[] will be serialized as base64-encoded String,
+                // not real array, so...
+                // (actually, ditto for char[]; thought to be a String)
+                if (type.hasRawClass(byte[].class) || type.hasRawClass(char[].class)) {
+                    return false;
+                }
+            }
             return true;
         }
         return false;

…tofix/UntypedListSerialization8Test.java …l/ser/UntypedListSerialization8Test.javasrc/test/java/tools/jackson/dataformat/xml/tofix/UntypedListSerialization8Test.java renamed to src/test/java/tools/jackson/dataformat/xml/ser/UntypedListSerialization8Test.java src/test/java/tools/jackson/dataformat/xml/tofix/UntypedListSerialization8Test.java renamed to src/test/java/tools/jackson/dataformat/xml/ser/UntypedListSerialization8Test.java

@@ -1,4 +1,4 @@
-package tools.jackson.dataformat.xml.tofix;
+package tools.jackson.dataformat.xml.ser;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -10,17 +10,17 @@
 
 import tools.jackson.dataformat.xml.XmlMapper;
 import tools.jackson.dataformat.xml.XmlTestUtil;
-import tools.jackson.dataformat.xml.testutil.failure.JacksonTestFailureExpected;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+// [dataformat-xml#8]: Serialization of List incorrect if property declared as Object
 public class UntypedListSerialization8Test extends XmlTestUtil
 {
     @JsonRootName("L")
     static class UntypedListBean
     {
         public final Object list;
-    	
+
         public UntypedListBean() {
             ArrayList<String> l= new ArrayList<String>();
             l.add("first");
@@ -33,7 +33,7 @@ public UntypedListBean() {
     static class TypedListBean
     {
         public final List<String> list;
-    	
+
         public TypedListBean() {
             ArrayList<String> l= new ArrayList<String>();
             l.add("first");
@@ -50,11 +50,7 @@ public TypedListBean() {
 
     private final XmlMapper MAPPER = newMapper();
 
-    /*
-     *  For [dataformat-xml#8] -- Will not use wrapping, if type is not statically known
-     *  to be a Collection
-     */
-    @JacksonTestFailureExpected
+    // [dataformat-xml#8]
     @Test
     public void testListAsObject() throws IOException
     {