Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing empty tags without default no-arguments constructor fails in 2.14 #547

Closed
henrik242 opened this issue Sep 27, 2022 · 14 comments
Closed
Labels
2.15 For issues planned for 2.15
Milestone

Comments

@henrik242
Copy link

henrik242 commented Sep 27, 2022

As in #491, I've found an additional issue where parsing empty tags without default constructor fails. This worked in 2.11.3, but has failed since.

Check should_parse_empty_tag_without_default_constructor() in https://github.com/henrik242/jackson-xml-problem/blob/no-string-argument/src/test/java/jackson/xml/NoStringArgumentTest.java#L37

Test (java):

    @Test
    void  should_parse_empty_tag_without_default_constructor() {
        String xml = "<outer><inner></inner></outer>";
        Outer res = XmlTool.parseOuter(xml);
        assertEquals(res, new Outer(null)); // SUCCESS in Jackson 2.11.x, but FAIL in 2.14.x
    }

Code (kotlin):

object XmlTool {
    val xmlIn = XMLInputFactory.newInstance()
    val factory = XmlFactory(xmlIn)
    val xmlModule = JacksonXmlModule()
    val mapper = XmlMapper(factory, xmlModule).registerKotlinModule()

    @JvmStatic
    fun parseOuter(xml: String?): Outer = mapper.readValue(xml, Outer::class.java)

    data class Inner(val code: String?)
    data class Outer(val inner: Inner?)
}

Fails with:

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `jackson.xml.XmlTool$Inner` (although at least one Creator exists): no default no-arguments constructor found
 at [Source: (StringReader); line: 1, column: 15] (through reference chain: jackson.xml.XmlTool$Outer["inner"])
	at app//com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at app//com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1733)
	at app//com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1358)
	at app//com.fasterxml.jackson.databind.deser.ValueInstantiator.createUsingDefault(ValueInstantiator.java:248)
	at app//com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createUsingDefault(StdValueInstantiator.java:275)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializerBase.getEmptyValue(BeanDeserializerBase.java:1042)
	at app//com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromEmptyString(StdDeserializer.java:322)
	at app//com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromString(StdDeserializer.java:270)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromString(BeanDeserializerBase.java:1500)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:197)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:187)
	at app//com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:542)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:564)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:439)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:352)
	at app//com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
	at app//com.fasterxml.jackson.dataformat.xml.deser.XmlDeserializationContext.readRootValue(XmlDeserializationContext.java:91)
	at app//com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4697)
	at app//com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3652)
	at app//com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3620)
	at app//jackson.xml.XmlTool.parseOuter(XmlTool.kt:22)
	at app//jackson.xml.NoStringArgumentTest.should_parse_empty_tag_without_default_constructor(NoStringArgumentTest.java:39)
	at java.base@11.0.16.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@11.0.16.1/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base@11.0.16.1/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@11.0.16.1/java.lang.reflect.Method.invoke(Method.java:566)
	at app//org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
	at app//org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at ...

This is the exact same issue as mentioned in FasterXML/jackson-module-kotlin#396, and is already covered a couple of places with existing failing tests:

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 27, 2022

@henrik242 Since we cannot use Kotlin in tests here (not allowed to add dependency), would it be possible to have Java version? It also rules out possibility the issue with Kotlin module, which is quite often the case.

@henrik242
Copy link
Author

henrik242 commented Sep 27, 2022

I'm not sure if I'm able to reproduce it with plain java. Maybe move this issue to jackson-kotlin-module and rename Github396.kt to match? Or reopen issue 396 there?

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 27, 2022

It has to be in Kotlin if it uses Kotlin code. Or to avoid Kotlin module (test) dependency to XML, could also go into jackson-integration-tests. But we need to get into habit of running those tests somehow... maybe scheduled on daily basis. Plus update to refer to latest publish versions etc etc. Still, that test module is intended for cross-Module tests.

Looking at stack trace this does seem to call the wrong constructor via getEmptyValue(). I wonder if Kotlin module overrides something to prevent things from working as expected.

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 27, 2022

Oh. Or this might occur with Java too. The problem, I think, is that there is ambiguity on empty String value being found. There are 2 ways to consider it:

  1. Ignorable white space (-> empty content for Object/Element)
  2. String with white space (-> Needs to have @JacksonXmlText)

so perhaps this would occur with similar POJO. But I think Jackson 2.13 added "coerce Scalar from Object [in some cases]" which might solve this... unless it only works for not-all-white-space case (seems plausible).

@henrik242
Copy link
Author

Or to avoid Kotlin module (test) dependency to XML, could also go into jackson-integration-tests.

Yeah, it can easily go there too since it also has a failing test that cover this (as mentioned above).

  1. Ignorable white space ...
  2. String with white space ...

But in this case there's no white space at all, just no definition of the arg in the default constructor.

@cowtowncoder
Copy link
Member

But in this case there's no white space at all, just no definition of the arg in the default constructor.

Ah. Yes, indeed.

@magicaljellybeans
Copy link

I'm getting a couple of test regressions going from 2.11.3 to 2.14.0 writing scala which I think are related to this.

One failing test I have seems to be a carbon copy of @henrik242's posted situation.

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `uk.gov.hmrc.wco.dec.ResponseDateTimeElement` (although at least one Creator exists): no default no-arguments constructor found
 at [Source: (StringReader); line: 1, column: 240] (through reference chain: uk.gov.hmrc.wco.dec.MetaData["Response"]->com.fasterxml.jackson.module.scala.deser.GenericFactoryDeserializerResolver$BuilderWrapper[0]->uk.gov.hmrc.wco.dec.Response["IssueDateTime"])
	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1733)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1358)
	at com.fasterxml.jackson.databind.deser.ValueInstantiator.createUsingDefault(ValueInstantiator.java:248)
	at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createUsingDefault(StdValueInstantiator.java:275)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.getEmptyValue(BeanDeserializerBase.java:1042)
	at com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromEmptyString(StdDeserializer.java:322)
	at com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromString(StdDeserializer.java:270)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromString(BeanDeserializerBase.java:1500)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:197)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:187)
	at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.deserialize(OptionDeserializerModule.scala:62)
	at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.deserialize(OptionDeserializerModule.scala:11)
	at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:542)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:564)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:439)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:352)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
	at com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer.deserialize(WrapperHandlingDeserializer.java:122)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer._deserializeFromArray(CollectionDeserializer.java:359)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:244)
	at com.fasterxml.jackson.module.scala.deser.GenericFactoryDeserializerResolver$Deserializer.deserialize(GenericFactoryDeserializerResolver.scala:125)
	at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:542)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:564)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:439)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:352)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:185)
	at com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer.deserialize(WrapperHandlingDeserializer.java:122)
	at com.fasterxml.jackson.dataformat.xml.deser.XmlDeserializationContext.readRootValue(XmlDeserializationContext.java:91)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4730)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3677)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3645)

On another test an empty tag returns Some("") instead of the expected None.

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 16, 2022

At this point what I would really need is a plain Java test. I'd love to have a look but I'd need to translate Data classes into their equivalent (from JVM perspective) counterparts.

The specific distinction has to do with EXACTLY how constructor for Outer and Inner is declared: problem being they both take 1 argument and may use mode of either Mode.DELEGATING of Mode.PROPERTIES -- and there are basically 4 combinations all of which may behave slightly differently.

I am guessing tho that Outer must have Mode.PROPERTIES so question really is about constructor setting for Inner. I will try it out a bit on Java side but definitive answer here would be helpful.

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 21, 2022

@henrik242 Ok, I think that what you can do is to add @JsonCreator(mode = JsonCreator.Mode.DELEGATING) for constructor of Inner. In this case String "" will be passed (not null).

I will try to see if the alternative (Inner declared to take PROPERTIES) could possibly be made to also work but thought I'll add a note of work-around; and you'd probably want to indicate mode in either since behavior does generally differ between two modes.

EDIT: there is actually another work-around that does work with PROPERTIES approach -- just add no-argument constructor. That will make deserializer happy.

cowtowncoder added a commit that referenced this issue Nov 21, 2022
@cowtowncoder cowtowncoder changed the title Regression: parsing empty tags without default no-arguments constructor fails in 2.14.0-rc1 Parsing empty tags without default no-arguments constructor fails in 2.14 Nov 21, 2022
@cowtowncoder cowtowncoder added 2.15 For issues planned for 2.15 and removed test-needed labels Nov 21, 2022
@cowtowncoder cowtowncoder added this to the 2.15.0 milestone Nov 21, 2022
@cowtowncoder
Copy link
Member

Fixed in 2.15 branch (for 2.15.0) -- actual fix is in jackson-databind but I'll modify test to verify it. Fix cannot be backported in 2.14 due to requiring a small API change for ValueInstantiator. In the meantime 2 work-arounds I mentioned hopefully help.

@henrik242
Copy link
Author

@cowtowncoder Thanks! Is it a bit too early to ask for an 2.15 release date estimate? 😄

@cowtowncoder
Copy link
Member

Yes it is. :)

It will take a while, most likely, looking at historic trends.

@henrik242
Copy link
Author

@cowtowncoder Okay, I kinda assumed as much :) We have a lot of data classes affected by this, so a per-class workaround will require a lot of work. There aren't any workarounds that can be put on the ObjectMapper, right?

@cowtowncoder
Copy link
Member

I can't think of any based on specific fix, but that does not mean someone could not figure out a way around it.

However... I suspect that you could use AnnotationIntrospector to essentially force mode = Mode.DELEGATING for specific inner classes; detect cases where it is needed, return.
I don't remember off-hand which method in AnnotationIntrospector but shouldn't be too hard to find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.15 For issues planned for 2.15
Projects
None yet
Development

No branches or pull requests

3 participants