microsoft / typespec

https://typespec.io/
MIT License
4.5k stars 215 forks source link

[Bug]: A model containing an @header property doesn't keep that property when the object is generated #4244

Open trangevi opened 2 months ago

trangevi commented 2 months ago

Describe the bug

When I specify a model in typespec which contains a property with the @header decorator, the generated code produces an object for the model which doesn't contain the header property. This is true regardless of whether the model is spread in an operation or not. I've repro'ed with a basic solution, below.

Reproduction

I put the typespec I used in a playground here, but I'm not exactly sure what pieces need to be included for it to compile, and felt like the C# code which was produced is relevant as well, so I've included that here.

So the typespec, with a model containing a header and a property:

model HeaderBugReproModel {
  @header("x-foo")
  foo: string;

  bar: int32;
}

And an operation asking for the model to be spread:

@actionSeparator("/")
@route("header/test")
op bugTest(...HeaderBugReproModel): void;

Produces (I removed most of the comments to make it smaller)

An anonymous model without the header property:

namespace Azure.AI.Inference
{
    /// <summary> The BugTestRequest. </summary>
    internal partial class BugTestRequest
    {
        private IDictionary<string, BinaryData> _serializedAdditionalRawData;

        internal BugTestRequest(int bar)
        {
            Bar = bar;
        }

        internal BugTestRequest(int bar, IDictionary<string, BinaryData> serializedAdditionalRawData)
        {
            Bar = bar;
            _serializedAdditionalRawData = serializedAdditionalRawData;
        }

        internal BugTestRequest()
        {
        }

        /// <summary> Gets the bar. </summary>
        public int Bar { get; }
    }
}

And a request method with the parameters spread, including the header

public virtual async Task<Response> BugTestAsync(string foo, int bar, CancellationToken cancellationToken = default)

If I change the operation to not spread the model

op bugTest(body: HeaderBugReproModel): void;

It produces

The model from the typespec, but without the header property

namespace Azure.AI.Inference
{
    /// <summary> The HeaderBugReproModel. </summary>
    public partial class HeaderBugReproModel
    {
        private IDictionary<string, BinaryData> _serializedAdditionalRawData;

        public HeaderBugReproModel(int bar)
        {
            Bar = bar;
        }

        internal HeaderBugReproModel(int bar, IDictionary<string, BinaryData> serializedAdditionalRawData)
        {
            Bar = bar;
            _serializedAdditionalRawData = serializedAdditionalRawData;
        }

        internal HeaderBugReproModel()
        {
        }

        public int Bar { get; }
    }
}

The anonymous model, now taking in the defined model instead of just the property

namespace Azure.AI.Inference
{
    /// <summary> The BugTestRequest. </summary>
    internal partial class BugTestRequest
    {
        private IDictionary<string, BinaryData> _serializedAdditionalRawData;

        internal BugTestRequest(HeaderBugReproModel body)
        {
            Argument.AssertNotNull(body, nameof(body));

            Body = body;
        }

        internal BugTestRequest(HeaderBugReproModel body, IDictionary<string, BinaryData> serializedAdditionalRawData)
        {
            Body = body;
            _serializedAdditionalRawData = serializedAdditionalRawData;
        }

        internal BugTestRequest()
        {
        }

        public HeaderBugReproModel Body { get; }
    }
}

And a request method which includes both the header and the model object

public virtual async Task<Response> BugTestAsync(string foo, HeaderBugReproModel body, CancellationToken cancellationToken = default)

Checklist

iscai-msft commented 2 months ago
  1. With spread, tcgc is giving an anonymous model for the body with 1 prop: bar. Header foo is on the command line. This is correct behavior
  2. If we group the model, tcgc returns the model with the header property on it as well (see existing test here). In this case, for c#, it appears the header is now missing because it's not on the method signature, and it's not in the generated model. Python still moves everything onto the method signature and this is by design. So @m-nash it appears that c# is dropping the header at some point
JoshLove-msft commented 2 weeks ago

Decide whether we want to generate the property and skip serialization, or if we want to collapse this into the method signature.