[Bug]: nullable: true is not generated for nullable model properties

AnReZa commented 6 days ago

Describe the bug

We're having the issue, that the typespec compiler doesn't set the nullable: true property inside the openapi.yaml file for nullable model properties. If we try to add the openapi.yaml as a web service reference in a c# projects, it doesn't generate any c# property as nullable. Therefore, we get an exeption whenever our API rightfully returns null for nullable values.

Our TypeSpec model

See that motorSizeCode is marked as nullable using the ? decorator.

model TestStandSetupParameter {
  unloadPosition: int32;
  loadPosition: int32;
  motorSizeCode?: int32;
}

The generated YAML file

See, that the motorSizeCode is not decorated with nullable: true.

TestStandSetupParameter:
  type: object
  required:
    - unloadPosition
    - loadPosition
  properties:
    unloadPosition:
      type: integer
      format: int32
    loadPosition:
      type: integer
      format: int32
    motorSizeCode:
      type: integer
      format: int32

NSwag-generated c# code

Using the built-in web service reference wizard in Visual Studio 2022, version 17.11.0.

public partial class TestStandSetupParameter
{
    [Newtonsoft.Json.JsonProperty("unloadPosition", Required = Newtonsoft.Json.Required.Always)]
    public int UnloadPosition { get; set; }

    [Newtonsoft.Json.JsonProperty("loadPosition", Required = Newtonsoft.Json.Required.Always)]
    public int LoadPosition { get; set; }

    [Newtonsoft.Json.JsonProperty("motorSizeCode", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public int MotorSizeCode { get; set; }

    private System.Collections.Generic.IDictionary<string, object> _additionalProperties;

    [Newtonsoft.Json.JsonExtensionData]
    public System.Collections.Generic.IDictionary<string, object> AdditionalProperties
    {
        get { return _additionalProperties ?? (_additionalProperties = new System.Collections.Generic.Dictionary<string, object>()); }
        set { _additionalProperties = value; }
    }
}

As you can see, the MotorSizeCode property is not recognized as nullable in the generated c# class. It even has the Required = Newtonsoft.Json.Required.DisallowNull attribute.

Weirdly, we're using the OpenAPI generator to generate the server-side implementation and there, everything works fine and the properties are marked as nullable when not set to b required.

What do we need to change in order to make that work as intended?

Reproduction

Simply generate the YAML file from the given tsp model or copy it into the online tool.

Checklist

[x] Follow our Code of Conduct
[x] Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
[x] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
[x] The provided reproduction is a minimal reproducible example of the bug.

timotheeguerin commented 6 days ago

? does not mean nullable it means optional. For nullable you should use | null

model TestStandSetupParameter {
  unloadPosition: int32;
  loadPosition: int32;
  motorSizeCode?: int32 | null;
}

however this more sounds an issue with nswag as you usually do not want null over the wire and the openapi is more what you'd want to represent

AnReZa commented 6 days ago

So we simply need to add the | null to any nullable property and it'll work?

timotheeguerin commented 6 days ago

if you do that it will indeed make the property nullable, you are saying by that that the property can accept the null type on top of string. But as said above this is often not waht you want except for json merge patch.(Json payloads over http are not filled with null they are just not providing the properties)

See playground

AnReZa commented 6 days ago

@timotheeguerin You are right. For some reason, the API explicitly returns null right now, while in reality it could simply drop the property. Do you know the setting for the OpenAPI c# server-side compiler by coincidence?

timotheeguerin commented 6 days ago

If you are using dotnet I think this one https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializeroptions.ignorenullvalues?view=net-8.0

timotheeguerin commented 6 days ago

I'll close thisd issue @AnReZa , please reopen or create a new one if there is something else

microsoft / typespec