Code Generation: Non-Required Open Api 3.0 scalar properties are non nullable

[andi0b]

I defined an Open API 3.0 spec with optional parameters. Optional parameters mean in my understanding, that they are nullable in C# and omitted in JSON serialization if they are null in C#.

But when I generate C# code, now the generated properties are never nullable, and I have no idea how to specify now not to serialize them. Am I doing something wrong or is this a major issue?

If I set the same spec to Open API 2.0, it behaves like I expect. The integers get generated as int? as I expect.

See this spec:

openapi: "3.0.0"
servers:
  - url: //petstore.swagger.io/v2
    description: Default server

components:
  schemas:
  
    Pet:
      type: object
      required:
        - id

      properties:
        id:
          type: integer
          
        id2:
          type: integer

Generated code:

//----------------------
// <auto-generated>
//     Generated using the NSwag toolchain v13.0.2.0 (NJsonSchema v10.0.20.0 (Newtonsoft.Json v11.0.0.0)) (http://NSwag.org)
// </auto-generated>
//----------------------


    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.0.20.0 (Newtonsoft.Json v11.0.0.0)")]
    public partial class Pet 
    {
        [Newtonsoft.Json.JsonProperty("id", Required = Newtonsoft.Json.Required.Always)]
        public int Id { get; set; }
    
        [Newtonsoft.Json.JsonProperty("id2", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int Id2 { get; set; }
    
        private System.Collections.Generic.IDictionary<string, object> _additionalProperties = new System.Collections.Generic.Dictionary<string, object>();
    
        [Newtonsoft.Json.JsonExtensionData]
        public System.Collections.Generic.IDictionary<string, object> AdditionalProperties
        {
            get { return _additionalProperties; }
            set { _additionalProperties = value; }
        }
    
    
    }

I would like to send this Json as a response (set required id and leave out id2)

{
  "id" : 20
}

[timmvonputtkamer]

We currently face the same problem. Is there some setting we need to tweak?

[RicoSuter]

We’d need to set defaultvaluehandling to ignore, correct?

[andi0b]

We’d need to set defaultvaluehandling to ignore, correct?

The problem is that the generated property is not nullable (the type is int but I expect it to be int?)

If the generated property is an object or a string, it can be set to null. If it is a number (int) it can't be set to null. It would need to be generated as int?, then it can be set to null

[RicoSuter]

The reason it is not nullable is because it is not specified as nullable in the spec, you'd need:

    id2:
      type: integer
      nullable: true

[RicoSuter]

But yes you could argue that non-required properties also should be nullable and just ignored when null whereas nullable properties should serialize "null", right?

[andi0b]

But yes you could argue that non-required properties also should be nullable and just ignored when null whereas nullable properties should serialize "null", right?

That's what I mean. It's easy to deal with nullable and required properties (null is serialized as null) and for nun-nullable and non-required properties (null is left out/ignored).

This is already handled very well by the generated Json.NET attributes. But just not for value types. Just change int to int? and it works!

It only get's difficult for nullable and non-required properties; should null result in null or in a missing property. But those combination is anyway a bit useless ;)

[timmvonputtkamer]

I figured out where the problem is. Namely, the code in the NJsonSchema project needs to be updated, specifically the IsNullable method of the JsonSchemaProperty class, see https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema/JsonSchemaProperty.cs

The relevant snippet currently reads:

        public override bool IsNullable(SchemaType schemaType)
        {
            if (schemaType == SchemaType.Swagger2 && IsRequired == false)
            {
                return true;
            }

            return base.IsNullable(schemaType);
        }

It should read:

        public override bool IsNullable(SchemaType schemaType)
        {
            if ((schemaType == SchemaType.Swagger2 || schemaType == SchemaType.OpenApi3) && IsRequired == false)
            {
                return true;
            }

            return base.IsNullable(schemaType);
        }

[RicoSuter]

The problem here is that this is a huge breaking change for some...

Wouldnt it be possible to generate DefaultValueHandling

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

And then leaving it to the default (0) would ignore it in the JSON?

[RicoSuter]

My proposal:
RicoSuter/NJsonSchema#1044

We should not fix JsonSchemaProperty class (because the schema is not really nullable) but just enhance the C# generator with an additional setting (to avoid breaking changes).

[andi0b]

@RicoSuter: I think you‘re right. The property is optional and not nullable. That’s a difference C# can’t make.

Making it configurable makes sense (not breaking other people’s code), although I think the defaulting to enabled would be the correct behaviour.

[RicoSuter]

Making it configurable makes sense (not breaking other people’s code), although I think the defaulting to enabled would be the correct behaviour.

It makes sense to change this default for the C# generator - but only in the next major version (breaking change) => RicoSuter/NJsonSchema#1045

[RicoSuter]

Do you think GenerateNullableOptionalProperties is a good name? Any ideas? Can you review the PR?

[andi0b]

I'll try to review it until Friday 👍

[timmvonputtkamer]

Okay, that looks like a good solution. Maybe GenerateOptionalPropertiesAsNullable is a better name for the setting?