pdvrieze / xmlutil

XML Serialization library for Kotlin
https://pdvrieze.github.io/xmlutil/
Apache License 2.0
363 stars 30 forks source link

Partial regression on checking root tag name checking #187

Closed pdvrieze closed 6 months ago

pdvrieze commented 8 months ago

Discussed in https://github.com/pdvrieze/xmlutil/discussions/186

Originally posted by **jpd236** October 22, 2023 In 0.86.0, the following code: ``` @Serializable @SerialName("test-b") data class TestB(val data: Boolean) @Test fun parse() { val xml = "" val testB = XML.decodeFromString(TestB.serializer(), xml) println(testB) } ``` will result in an error: ``` nl.adaptivity.xmlutil.XmlException: local name test-a does not match expected "test-b" at nl.adaptivity.xmlutil.XmlReader$DefaultImpls.require(XmlReader.kt:78) at nl.adaptivity.xmlutil.XmlBufferedReaderBase.require(XmlBufferedReaderBase.kt:27) at nl.adaptivity.xmlutil.XmlReader$DefaultImpls.require(XmlReader.kt:84) at nl.adaptivity.xmlutil.XmlBufferedReaderBase.require(XmlBufferedReaderBase.kt:27) at nl.adaptivity.xmlutil.serialization.XmlDecoderBase$TagDecoder.endStructure(XMLDecoder.kt:849) at com.jeffpdavidson.kotwords.model.CrosswordTest$TestB$$serializer.deserialize(CrosswordTest.kt:18) at com.jeffpdavidson.kotwords.model.CrosswordTest$TestB$$serializer.deserialize(CrosswordTest.kt:18) at nl.adaptivity.xmlutil.serialization.XmlDecoderBase$XmlDecoder.decodeSerializableValue(XMLDecoder.kt:236) at nl.adaptivity.xmlutil.serialization.XML.decodeFromReader(XML.kt:414) at nl.adaptivity.xmlutil.serialization.XML.decodeFromReader$default(XML.kt:385) at nl.adaptivity.xmlutil.serialization.XML.decodeFromString(XML.kt:351) at nl.adaptivity.xmlutil.serialization.XML$Companion.decodeFromString(XML.kt:783) ``` This makes sense to me, and is long-standing behavior - the tag specified in `@SerialName` doesn't match the tag in the XML. However, in 0.86.1 (or 0.86.2), parsing succeeds as `TestB(data=true)`. Is it expected that parsing should work even if the root tag is wrong? I looked at the 0.86.1 release notes and didn't see anything that obviously corresponded to such a behavior change.
pdvrieze commented 8 months ago

@jpd236 This particular example was intended to work as the new version does, but it highlights a general issue of limited checking of root tag names. I've pushed an update that fixes this testing. What will now happen is that if the policy sets a tag qname (this requires XmlSerialName) it will use and enforce that (or if the tag name is explicitly given), but if the name is not explicit it will use the name in the reader.

Note that serialization distinguishes type and usage name, and the SerialName annotation just changes that from property / type names. In XML it is generally practice to reuse tag definitions, thus the policy needs to support that. The default policy does this based on whether explicit naming can be detected (SerialName annotations are invisible). In such case it is considered that the type also represents a reusable tag name unless again explicitly overridden at use space.

jpd236 commented 8 months ago

Thanks. I used @SerialName to try and simplify the toy example here as much as possible, but my actual code is indeed using @XmlSerialName so your fix should impact it and restore the prior behavior. I'm parsing XML that can use one of two potential root elements without knowing which one up front, and so I try one parse, and if it fails, try the other. This only works if the parser fails when the root tag mismatches.

pdvrieze commented 8 months ago

@jpd236 For your use case you may want to consider that the xml format does support polymorphic root tags, so you could deal with your use case that way (there is a test on polymorphic root tags) - you can use (sealed) interfaces or just use any as base type with a serializersModule for the subtypes.

pdvrieze commented 6 months ago

Just released.