Fix #318

FasterXML · May 1, 2020 · dbd1fe5 · dbd1fe5
1 parent 3eadc65
commit dbd1fe5
Show file tree

Hide file tree

Showing 10 changed files with 99 additions and 52 deletions.
diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
   <parent>
     <groupId>com.fasterxml.jackson</groupId>
     <artifactId>jackson-base</artifactId>
-    <version>2.11.1-SNAPSHOT</version>
+    <version>2.12.0-SNAPSHOT</version>
   </parent>
   <groupId>com.fasterxml.jackson.dataformat</groupId>
   <artifactId>jackson-dataformat-xml</artifactId>

diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -60,3 +60,7 @@ Luke Korth (lkorth@github.com)
 
 * Reported #366: XML containing xsi:nil is improperly parsed
  (2.10.2)
+
+Jochen Schalanda (joschi@github)
+* Repoerted #318: XMLMapper fails to deserialize null (POJO reference) from blank tag
+ (2.12.0)
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -4,6 +4,11 @@ Project: jackson-dataformat-xml
 = Releases
 ------------------------------------------------------------------------
 
+2.12.0 (not yet released)
+
+#318: XMLMapper fails to deserialize null (POJO reference) from blank tag
+ (reported by Jochen S)
+
 2.11.1 (not yet released)
 
 -

diff --git a/src/main/java/com/fasterxml/jackson/dataformat/xml/JacksonXmlModule.java b/src/main/java/com/fasterxml/jackson/dataformat/xml/JacksonXmlModule.java
@@ -4,6 +4,7 @@
 import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.fasterxml.jackson.dataformat.xml.deser.FromXmlParser;
 import com.fasterxml.jackson.dataformat.xml.deser.XmlBeanDeserializerModifier;
+import com.fasterxml.jackson.dataformat.xml.deser.XmlBeanInstantiator;
 import com.fasterxml.jackson.dataformat.xml.deser.XmlStringDeserializer;
 import com.fasterxml.jackson.dataformat.xml.ser.XmlBeanSerializerModifier;
 
@@ -65,6 +66,9 @@ public void setupModule(SetupContext context)
         // as well as AnnotationIntrospector
         context.insertAnnotationIntrospector(_constructIntrospector());
 
+        // 2.11 adds ValueInstantiator, too
+        context.addValueInstantiators(new XmlBeanInstantiator.Provider());
+
         // and finally inform XmlFactory about overrides, if need be:
         if (_cfgNameForTextElement != FromXmlParser.DEFAULT_UNNAMED_TEXT_PROPERTY) {
             XmlMapper m = (XmlMapper) context.getOwner();

diff --git a/src/main/java/com/fasterxml/jackson/dataformat/xml/deser/XmlBeanDeserializerModifier.java b/src/main/java/com/fasterxml/jackson/dataformat/xml/deser/XmlBeanDeserializerModifier.java
@@ -96,9 +96,11 @@ public JsonDeserializer<?> modifyDeserializer(DeserializationConfig config,
         ValueInstantiator inst = deser.getValueInstantiator();
         // 03-Aug-2017, tatu: [dataformat-xml#254] suggests we also should
         //    allow passing `int`/`Integer`/`long`/`Long` cases, BUT
-        //    unfortunately we can not simple use default handling. Would need
+        //    unfortunately we can not simply use default handling. Would need
         //    coercion.
-        if (!inst.canCreateFromString()) {
+        // 30-Apr-2020, tatu: Complication from [dataformat-xml#318] as we now
+        //    have a delegate too...
+        if ((inst instanceof XmlBeanInstantiator) || !inst.canCreateFromString()) {
             SettableBeanProperty textProp = _findSoleTextProp(config, deser.properties());
             if (textProp != null) {
                 return new XmlTextDeserializer(deser, textProp);

diff --git a/src/main/java/com/fasterxml/jackson/dataformat/xml/deser/XmlBeanInstantiator.java b/src/main/java/com/fasterxml/jackson/dataformat/xml/deser/XmlBeanInstantiator.java
@@ -0,0 +1,71 @@
+package com.fasterxml.jackson.dataformat.xml.deser;
+
+import java.io.IOException;
+
+import com.fasterxml.jackson.databind.*;
+import com.fasterxml.jackson.databind.deser.ValueInstantiator;
+import com.fasterxml.jackson.databind.deser.ValueInstantiators;
+import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator;
+
+/**
+ * {@link ValueInstantiator} implementation needed mostly to support
+ * creating "empty" instance (or {@code null}) of POJOs from String
+ * value consisting of all whitespace -- something that happens with
+ * indentation.
+ *
+ * @since 2.12
+ */
+public class XmlBeanInstantiator
+    extends ValueInstantiator.Delegating
+    implements java.io.Serializable
+{
+    private static final long serialVersionUID = 1L;
+
+    protected final JavaType _type;
+
+    protected final boolean _canCreateDefault;
+
+    public XmlBeanInstantiator(JavaType type, ValueInstantiator base) {
+        super(base);
+        _type = type;
+        _canCreateDefault = base.canCreateUsingDefault();
+    }
+
+    @Override
+    public boolean canInstantiate() { return true; }
+
+    @Override
+    public boolean canCreateFromString() {
+        return true;
+    }
+
+    @Override
+    public Object createFromString(DeserializationContext ctxt, String value)
+            throws IOException
+    {
+        if (_canCreateDefault) {
+            if (value.isEmpty() || value.trim().isEmpty()) {
+                return delegate().createUsingDefault(ctxt);
+            }
+            // Should we try to give more meaningful error otherwise?
+        }
+        return delegate().createFromString(ctxt, value);
+    }
+
+    public static class Provider extends ValueInstantiators.Base
+        implements java.io.Serializable
+    {
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        public ValueInstantiator findValueInstantiator(DeserializationConfig config,
+                BeanDescription beanDesc, ValueInstantiator defaultInstantiator)
+        {
+            // let's avoid custom ones?
+            if (defaultInstantiator instanceof StdValueInstantiator) {
+                return new XmlBeanInstantiator(beanDesc.getType(), defaultInstantiator);
+            }
+            return defaultInstantiator;
+        }
+    }
+}
diff --git a/...at/xml/failing/EmptyBeanDeser318Test.java → ...rmat/xml/deser/EmptyBeanDeser318Test.java b/...at/xml/failing/EmptyBeanDeser318Test.java → ...rmat/xml/deser/EmptyBeanDeser318Test.java
@@ -1,6 +1,5 @@
-package com.fasterxml.jackson.dataformat.xml.failing;
+package com.fasterxml.jackson.dataformat.xml.deser;
 
-import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.dataformat.xml.XmlMapper;
 import com.fasterxml.jackson.dataformat.xml.XmlTestBase;
 import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
@@ -43,7 +42,8 @@ public void testEmptyString() throws Exception {
 
         Wrapper value = MAPPER.readValue(s, Wrapper.class);
         assertEquals("id", value.id);
-        assertNull(value.nested);
+        assertNotNull(value.nested);
+        assertNull(value.nested.nested2);
     }
 
     public void testBlankString() throws Exception {
@@ -57,7 +57,8 @@ public void testBlankString() throws Exception {
         // Cannot construct instance of `JacksonXMLTest$Nested` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('    ')
         Wrapper value = MAPPER.readValue(s, Wrapper.class);
         assertEquals("id", value.id);
-        assertNull(value.nested);
+        assertNotNull(value.nested);
+        assertNull(value.nested.nested2);
     }
 
     public void testBlankString2() throws Exception {
@@ -66,14 +67,11 @@ public void testBlankString2() throws Exception {
                 + "  <nested>    </nested>"
                 + "</wrapper>";
 
-        // This fails with the following exception:
-        // com.fasterxml.jackson.databind.exc.MismatchedInputException:
-        // Cannot construct instance of `JacksonXMLTest$Nested` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('    ')
         Wrapper value = MAPPER.readerFor(Wrapper.class)
-                .with(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT)
                 .readValue(s);
         assertEquals("id", value.id);
-        assertNull(value.nested);
+        assertNotNull(value.nested);
+        assertNull(value.nested.nested2);
     }
 
     public void testMissing() throws Exception {

diff --git a/src/test/java/com/fasterxml/jackson/dataformat/xml/failing/EmptyListDeser124Test.java b/src/test/java/com/fasterxml/jackson/dataformat/xml/failing/EmptyListDeser124Test.java
@@ -4,8 +4,6 @@
 import java.util.List;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonSetter;
-import com.fasterxml.jackson.annotation.Nulls;
 import com.fasterxml.jackson.dataformat.xml.*;
 
 // for [dataformat-xml#124]

diff --git a/.../java/com/fasterxml/jackson/dataformat/xml/lists/Issue101UnwrappedListAttributesTest.java b/.../java/com/fasterxml/jackson/dataformat/xml/lists/Issue101UnwrappedListAttributesTest.java
@@ -10,25 +10,7 @@
 // Failing unit test(s) wrt [Issue#64]
 public class Issue101UnwrappedListAttributesTest extends XmlTestBase
 {
-    static class Optional {
-        @JacksonXmlText
-        public String number = "NOT SET";
-
-        @JacksonXmlProperty(isAttribute=true)
-        public String type = "NOT SET";
-
-        public Optional() { }
-
-        // uncommenting this ALSO works:
-//        public Optional(String n) { number = n; }
-    }
-
-    static class Optionals {
-        @JacksonXmlElementWrapper(useWrapping = false)
-        public List<Optional> optional;
-    } 
-
-    // For [Issue#101]
+    // For [dataformat-xml#101]
     @JacksonXmlRootElement(localName = "root")    
     @JsonPropertyOrder({ "unwrapped", "name" })
     static class Root {
@@ -62,24 +44,7 @@ public UnwrappedElement (String id, String type) {
 
     private final XmlMapper MAPPER = new XmlMapper();
 
-    // // [Issue#64]
-    public void testOptionalsWithMissingType() throws Exception
-    {
-//        Optionals ob = MAPPER.readValue("<MultiOptional><optional type='work'>123-456-7890</optional></MultiOptional>",
-        Optionals ob = MAPPER.readValue("<MultiOptional><optional>123-456-7890</optional></MultiOptional>",
-                Optionals.class);
-        assertNotNull(ob);
-        assertNotNull(ob.optional);
-        assertEquals(1, ob.optional.size());
-
-//            System.err.println("ob: " + ob); // works fine
-
-        Optional opt = ob.optional.get(0);
-        assertEquals("123-456-7890", opt.number);
-        assertEquals("NOT SET", opt.type);
-    }
-
-    // [Issue#101]
+    // [dataformat-xml#101]
     public void testWithTwoAttributes() throws Exception
     {
         final String EXP = "<root>"

diff --git a/src/test/java/com/fasterxml/jackson/dataformat/xml/lists/UnwrappedListsTest.java b/src/test/java/com/fasterxml/jackson/dataformat/xml/lists/UnwrappedListsTest.java
@@ -38,7 +38,7 @@ static class DefaultList {
         public Value[] value;
     }
 
-    // [Issue#64]
+    // [dataformat-xml#64]
     static class Optionals {
         @JacksonXmlElementWrapper(useWrapping = false)
         public List<Optional> optional;