Skip to content

Fix XML serialization for model collections#9826

Merged
jorgerangel-msft merged 11 commits intomainfrom
copilot/fix-xml-serialization-collections
Feb 27, 2026
Merged

Fix XML serialization for model collections#9826
jorgerangel-msft merged 11 commits intomainfrom
copilot/fix-xml-serialization-collections

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 26, 2026

  • 1. Add nameHint parameter to WriteObjectValue for XmlWriter in ModelSerializationExtensionsDefinition.Xml.cs
  • 2. Update ModelSerializationExtensionsSnippets.Xml.cs to support nameHint parameter
  • 3. Add new XML FromEnumerable overload in BinaryContentHelperDefinition.cs with rootNameHint and childNameHint
  • 4. Update BinaryContentHelperSnippets.cs with new FromEnumerable overload for XML
  • 5. Update ScmMethodProviderCollection.cs serialization path to use XML FromEnumerable when content type is XML and body is list
  • 6. Update ScmMethodProviderCollection.cs deserialization path to use XDocument-based XML deserialization when response content type is XML
  • 7. Address review feedback (round 1)
  • 8. Address review feedback (round 2)
  • 9. Make BinaryContentHelperDefinition class public
  • 10. Use AddRange in ModelSerializationExtensionsDefinition.Xml.cs
  • 11. Fix variable naming in FromEnumerable XML method (stream0/writer0/stream1 → stream/writer) and add test
  • 12. Regenerate generated libraries with eng/scripts/Generate.ps1
  • 13. Run tests and validate changes
  • 14. Update test to validate entire FromEnumerable method including signature using TypeProviderWriter
  • 15. Address review feedback (round 3): use AddRange in BinaryContentHelperDefinition.cs, revert Generate.ps1, simplify test to validate only FromEnumerable method (signature + body)
  • 16. Run code review and security checks
Original prompt

This section details on the original issue you should resolve

<issue_title>Fix XML Serialization For Model Collections</issue_title>
<issue_description>Consider this operation:

@Xml.name("SignedIdentifiers")
model SignedIdentifiers is SignedIdentifier[];

/** The signed identifier. */
@Xml.name("SignedIdentifier")
model SignedIdentifier {
  /** The unique ID for the signed identifier. */
  @Xml.name("Id") id: string;
}

@post
op Foo(@body body: SignedIdentifiers, @header contentType: "application/xml"): void;

@get
@route("/foo")
op GetFoo(): {
  @header contentType: "application/xml";
  @body body: SignedIdentifiers;
};

Since the request and response payload bodies are model collections, we cannot use ModelReaderWriter directly to serialize. We need to update our BinaryContentHelper and ModelSerializationExtensions to correctly handle this case.

For BinaryContentHelper, we need to generate this new helper method:

public static BinaryContent FromEnumerable<T>(IEnumerable<T> enumerable, string rootNameHint, string childNameHint)
            where T : notnull
        {
            using (MemoryStream stream = new MemoryStream(256))
            {
                using (XmlWriter writer = XmlWriter.Create(stream, ModelSerializationExtensions.XmlWriterSettings))
                {
                    writer.WriteStartElement(rootNameHint);
                    foreach (var item in enumerable)
                    {
                        writer.WriteObjectValue(item, ModelSerializationExtensions.WireOptions, childNameHint);
                    }
                    writer.WriteEndElement();
                }

                if (stream.Position > int.MaxValue)
                {
                    return BinaryContent.Create(BinaryData.FromStream(stream));
                }
                else
                {
                    return BinaryContent.Create(new BinaryData(stream.GetBuffer().AsMemory(0, (int)stream.Position)));
                }
            }
        }

In the generated conv method, we then need to call this helper and pass in the both the serialized name of the collection and the serialized name of it's model elements:

 public virtual ClientResult Foo(IEnumerable<SignedIdentifier> body, CancellationToken cancellationToken = default)
        {
            Argument.AssertNotNull(body, nameof(body));

            using BinaryContent content = BinaryContentHelper.FromEnumerable(body, "SignedIdentifiers", "SignedIdentifier");
            return Foo(content, cancellationToken.ToRequestOptions());
        }

The ModelSerializationExtension's WriteObjectValue method for XML writer will need to be updated to this:

public static void WriteObjectValue<T>(this XmlWriter writer, T value, ModelReaderWriterOptions options = null, string nameHint = null)
{
    switch (value)
    {
        case IPersistableModel<T> persistableModel:
            BinaryData data = ModelReaderWriter.Write(persistableModel, options ?? WireOptions, SampleTypeSpecContext.Default);
            using (Stream stream = data.ToStream())
            {
                using (XmlReader reader = XmlReader.Create(stream, XmlReaderSettings))
                {
                    reader.MoveToContent();
                    if (nameHint != null)
                    {
                        writer.WriteStartElement(nameHint);
                        reader.ReadStartElement();
                        while (reader.NodeType != XmlNodeType.EndElement)
                        {
                            writer.WriteNode(reader, true);
                        }
                        writer.WriteEndElement();
                    }
                    else
                    {
                        reader.ReadStartElement();
                        while (reader.NodeType != XmlNodeType.EndElement)
                        {
                            writer.WriteNode(reader, true);
                        }
                    }
                }
            }
            return;
    }
}

For deserialization, we need to update the generated conv method to handle deserializing the payload directly:

public virtual ClientResult<IReadOnlyList<SignedIdentifier>> GetFoo(CancellationToken cancellationToken = default)
        {
            ClientResult result = GetFoo(cancellationToken.ToRequestOptions());
            IReadOnlyList<SignedIdentifier> value = default;
            BinaryData data = result.GetRawResponse().Content;
            using Stream stream = data.ToStream();
            XDocument document = XDocument.Load(stream, LoadOptions.PreserveWhitespace);
            if (document.Element("SignedIdentifiers") is XElement signedIdentifiersElement)
            {
                List<SignedIdentifier> array = new List<SignedIdentifier>();
                foreach (XElement item in signedIdentifiersElement.Elements("SignedIdentifier"))
                {
                 ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes microsoft/typespec#9824

<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)

- Add nameHint parameter to WriteObjectValue for XmlWriter
- Add XML FromEnumerable overload in BinaryContentHelper with rootNameHint/childNameHint
- Update ScmMethodProviderCollection for XML list serialization/deserialization
- Add helper methods to detect XML collection operations

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Feb 26, 2026
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@jorgerangel-msft
Copy link
Copy Markdown
Contributor

@copilot try fixing the feedback again

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 26, 2026

@copilot try fixing the feedback again

All review feedback addressed in c149c5f:

  • Removed unused createBinaryContent variable in BinaryContentHelperDefinition.cs
  • Switched to RequestContentApiSnippets.Create() for BinaryContent creation
  • Changed root name calculation to operationBodyParam.Type.Name (serialization) and arrayType.Name (deserialization) with TODO referencing [tcgc] Missing info on model that is a collection/array Azure/typespec-azure#3978
  • Now returns false if element doesn't have xml serialization options
  • Removed GetXmlRootNameFromArrayType method, inlined the logic
  • Fixed reader variable reference by sharing a single CodeWriterDeclaration between the UsingScopeStatement and the statements that use it

@jorgerangel-msft
Copy link
Copy Markdown
Contributor

@copilot lets also make BinaryContentHelperDefinition public

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 26, 2026

@copilot lets also make BinaryContentHelperDefinition public

Done in 322d29a. Changed BinaryContentHelperDefinition from internal class to public class.

…tionExtensionsDefinition.Xml

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 26, 2026

@copilot we need to also add a test for validating the FromEnumerable method. It is not being generated correctly:

Fixed in 33c80de. The issue was that standalone VariableExpression objects had different CodeWriterDeclaration instances than the ones created by UsingScopeStatement, causing numbered suffixes. Now using the out var pattern from UsingScopeStatement to capture the variables directly. Also added the FromEnumerableXmlMethodIsCorrectlyDefined test that validates the correct generated output.

…TypeProviderWriter

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
…s1, simplify test to validate only FromEnumerable method

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@jorgerangel-msft jorgerangel-msft changed the title [WIP] Fix XML serialization for model collections Fix XML serialization for model collections Feb 26, 2026
@jorgerangel-msft jorgerangel-msft marked this pull request as ready for review February 26, 2026 23:32
@jorgerangel-msft jorgerangel-msft added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit e46bb79 Feb 27, 2026
23 checks passed
@jorgerangel-msft jorgerangel-msft deleted the copilot/fix-xml-serialization-collections branch February 27, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants