Implement support for XmlAttribute in WriteAttribute method #129489
Open
StephenMolloy wants to merge 1 commit into
Open
Implement support for XmlAttribute in WriteAttribute method #129489StephenMolloy wants to merge 1 commit into
StephenMolloy wants to merge 1 commit into
Conversation
…orresponding test
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the reflection-based XmlSerializer reader to handle attributes whose mapping type can accept an attribute node (via TypeDesc.CanBeAttributeValue), and promotes the related regression test into the main XmlSerializerTests suite.
Changes:
- Updated
ReflectionXmlSerializationReader.WriteAttributeto acceptXmlAttributeinstances forSpecialMappingtypes whereTypeDesc.CanBeAttributeValueis true, instead of throwingNotImplementedException. - Moved
Xml_CustomDocumentWithXmlAttributesAsNodesfromXmlSerializerTests.Internal.csintoXmlSerializerTests.cs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Private.Xml/src/System/Xml/Serialization/ReflectionXmlSerializationReader.cs | Implements the missing CanBeAttributeValue attribute-node handling by consuming XmlAttribute values when provided. |
| src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.Internal.cs | Removes the previously “internal-only” placement of the regression test. |
| src/libraries/System.Private.Xml/tests/XmlSerializer/XmlSerializerTests.cs | Adds the promoted test to the main test suite. |
Comment on lines
+643
to
+646
| var actual = SerializeAndDeserialize(customDoc, | ||
| WithXmlHeader(@"<customElement name=""testElement"" regularAttribute=""regularValue"" customAttribute=""customValue""/>"), skipStringCompare: true); | ||
| Assert.NotNull(actual); | ||
| } |
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request addresses support for serializing XML attributes as nodes when
TypeDesc.CanBeAttributeValueis true, and moves the corresponding test to the main test suite. The changes ensure correct handling of XML attributes in the reflection-based serializer and improve test coverage.Serializer logic improvements:
WriteAttributeinReflectionXmlSerializationReader.csto handle cases whereTypeDesc.CanBeAttributeValueis true by checking if the attribute is anXmlAttributeand returning early if not, instead of throwing aNotImplementedException. This resolves a previously unimplemented scenario.Test suite updates:
Xml_CustomDocumentWithXmlAttributesAsNodestest fromXmlSerializerTests.Internal.csto the mainXmlSerializerTests.csfile, reflecting that the serializer now supports this scenario. [1] [2]Fixes #1398