diff --git a/codecs/xml-codec/model/test.smithy b/codecs/xml-codec/model/test.smithy index c5ee46ea8..4a9509aaa 100644 --- a/codecs/xml-codec/model/test.smithy +++ b/codecs/xml-codec/model/test.smithy @@ -272,3 +272,35 @@ structure AllListsStruct { blobs: BlobList timestamps: TimestampList } + +structure DenseListStruct { + structs: StructItemList + strings: PlainStringList +} + +structure SparseListStruct { + structs: SparseStructItemList + strings: SparsePlainStringList +} + +structure StructItem { + id: String +} + +list StructItemList { + member: StructItem +} + +@sparse +list SparseStructItemList { + member: StructItem +} + +list PlainStringList { + member: String +} + +@sparse +list SparsePlainStringList { + member: String +} diff --git a/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlDeserializer.java b/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlDeserializer.java index 868a11861..cb7704cd2 100644 --- a/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlDeserializer.java +++ b/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlDeserializer.java @@ -575,7 +575,12 @@ public Instant readTimestamp(Schema schema) { @Override public boolean isNull() { try { - return reader.getText().isEmpty(); + // An element is null only when it is genuinely empty: no text content AND no child + // elements. getText() returns "" both for a truly empty element ( or ) + // and for an element whose content is entirely child elements, e.g. a structure + // (..). To tell them apart, require that consuming the text left + // us on this element's own end tag rather than on a nested child node. + return reader.getText().isEmpty() && reader.atEndElement(); } catch (XMLStreamException e) { throw error("Failed to determine if value is null", e); } diff --git a/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlReader.java b/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlReader.java index 0d5cea08a..74a75eaa0 100644 --- a/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlReader.java +++ b/codecs/xml-codec/src/main/java/software/amazon/smithy/java/xml/XmlReader.java @@ -77,6 +77,11 @@ final String getText() throws XMLStreamException { return textReader.toString(); } + final boolean atEndElement() throws XMLStreamException { + nextIfNeeded(); + return getEventType() == XMLStreamConstants.END_ELEMENT; + } + private static boolean readNextString(int event) { return switch (event) { case XMLStreamReader.CHARACTERS, XMLStreamConstants.CDATA -> true; diff --git a/codecs/xml-codec/src/test/java/software/amazon/smithy/java/xml/GeneratedModelSerdeTest.java b/codecs/xml-codec/src/test/java/software/amazon/smithy/java/xml/GeneratedModelSerdeTest.java index ba218b594..5b22cc309 100644 --- a/codecs/xml-codec/src/test/java/software/amazon/smithy/java/xml/GeneratedModelSerdeTest.java +++ b/codecs/xml-codec/src/test/java/software/amazon/smithy/java/xml/GeneratedModelSerdeTest.java @@ -24,6 +24,7 @@ import smithy.java.xml.test.model.BlobStruct; import smithy.java.xml.test.model.Color; import smithy.java.xml.test.model.ComplexStruct; +import smithy.java.xml.test.model.DenseListStruct; import smithy.java.xml.test.model.FlattenedListStruct; import smithy.java.xml.test.model.FlattenedMapStruct; import smithy.java.xml.test.model.InnerStruct; @@ -32,7 +33,9 @@ import smithy.java.xml.test.model.NumericStruct; import smithy.java.xml.test.model.RecursiveStruct; import smithy.java.xml.test.model.SimpleStruct; +import smithy.java.xml.test.model.SparseListStruct; import smithy.java.xml.test.model.StringStruct; +import smithy.java.xml.test.model.StructItem; import smithy.java.xml.test.model.TimestampStruct; import smithy.java.xml.test.model.XmlAttributeStruct; import smithy.java.xml.test.model.XmlNameStruct; @@ -739,6 +742,69 @@ void selfClosingElementsInListSkippedAsNull() { assertThat(nativeResult.getTags()).containsExactly(null, "hello", null); } + @PerProvider + void denseStructListDeserializes(boolean useNative) { + String xml = "abcd"; + var result = deserialize(useNative, xml, DenseListStruct.builder()); + assertThat(result.getStructs()).hasSize(1); + assertThat(result.getStructs().get(0).getId()).isEqualTo("abcd"); + } + + @PerProvider + void prettyPrintedStructListDeserializes(boolean useNative) { + String xml = """ + + + + abcd + + + + """; + var result = deserialize(useNative, xml, DenseListStruct.builder()); + assertThat(result.getStructs()).hasSize(1); + assertThat(result.getStructs().get(0).getId()).isEqualTo("abcd"); + } + + // Both empty forms, and , are null and rejected by a dense list. + @PerProvider + void denseStructListNullMemberThrows(boolean useNative) { + String singleTag = ""; + assertThatThrownBy(() -> deserialize(useNative, singleTag, DenseListStruct.builder())) + .isInstanceOf(SerializationException.class); + String separateTag = ""; + assertThatThrownBy(() -> deserialize(useNative, separateTag, DenseListStruct.builder())) + .isInstanceOf(SerializationException.class); + } + + @PerProvider + void sparseStructListNullMemberIsNull(boolean useNative) { + String xml = ""; + var result = deserialize(useNative, xml, SparseListStruct.builder()); + assertThat(result.getStructs()).containsExactly((StructItem) null); + } + + @PerProvider + void densePopulatedStringListDeserializes(boolean useNative) { + String xml = "ax"; + var result = deserialize(useNative, xml, DenseListStruct.builder()); + assertThat(result.getStrings()).containsExactly("a", "x"); + } + + @PerProvider + void denseStringListEmptyMemberThrows(boolean useNative) { + String xml = "x"; + assertThatThrownBy(() -> deserialize(useNative, xml, DenseListStruct.builder())) + .isInstanceOf(SerializationException.class); + } + + @PerProvider + void sparseStringListEmptyMemberIsNull(boolean useNative) { + String xml = "x"; + var result = deserialize(useNative, xml, SparseListStruct.builder()); + assertThat(result.getStrings()).containsExactly(null, "x"); + } + @PerProvider void trailingContentAfterRootIsRejected(boolean useNative) { String xml = "hi1extra";