[CSharp] Swagger-gen converts required primitive-types to nullable types

MoienTajik commented 4 years ago

Description

I have a simple class called Test which contains only two primitive properties in my server-side application:

public class Test
{
    [Required]
    public int Number { get; set; }

    [Required]
    public bool IsEnabled { get; set; }
}

When I try to generate the client version of this class, swagger-gen generates a class like this for me:

using System;
using System.Linq;
using System.IO;
using System.Text;
using System.Text.RegularExpressions;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Runtime.Serialization;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using System.ComponentModel.DataAnnotations;
using SwaggerDateConverter = IO.Swagger.Client.SwaggerDateConverter;

namespace IO.Swagger.Model
{
    /// <summary>
    /// TestValueTypesModelsTest
    /// </summary>
    [DataContract]
    public partial class TestValueTypesModelsTest :  IEquatable<TestValueTypesModelsTest>, IValidatableObject
    {
        /// <summary>
        /// Initializes a new instance of the <see cref="TestValueTypesModelsTest" /> class.
        /// </summary>
        /// <param name="number">number (required).</param>
        /// <param name="isEnabled">isEnabled (required).</param>
        public TestValueTypesModelsTest(int? number = default(int?), bool? isEnabled = default(bool?))
        {
            // to ensure "number" is required (not null)
            if (number == null)
            {
                throw new InvalidDataException("number is a required property for TestValueTypesModelsTest and cannot be null");
            }
            else
            {
                this.Number = number;
            }
            // to ensure "isEnabled" is required (not null)
            if (isEnabled == null)
            {
                throw new InvalidDataException("isEnabled is a required property for TestValueTypesModelsTest and cannot be null");
            }
            else
            {
                this.IsEnabled = isEnabled;
            }
        }

        /// <summary>
        /// Gets or Sets Number
        /// </summary>
        [DataMember(Name="number", EmitDefaultValue=false)]
        public int? Number { get; set; }

        /// <summary>
        /// Gets or Sets IsEnabled
        /// </summary>
        [DataMember(Name="isEnabled", EmitDefaultValue=false)]
        public bool? IsEnabled { get; set; }

        /// <summary>
        /// Returns the string presentation of the object
        /// </summary>
        /// <returns>String presentation of the object</returns>
        public override string ToString()
        {
            var sb = new StringBuilder();
            sb.Append("class TestValueTypesModelsTest {\n");
            sb.Append("  Number: ").Append(Number).Append("\n");
            sb.Append("  IsEnabled: ").Append(IsEnabled).Append("\n");
            sb.Append("}\n");
            return sb.ToString();
        }

        /// <summary>
        /// Returns the JSON string presentation of the object
        /// </summary>
        /// <returns>JSON string presentation of the object</returns>
        public virtual string ToJson()
        {
            return JsonConvert.SerializeObject(this, Formatting.Indented);
        }

        /// <summary>
        /// Returns true if objects are equal
        /// </summary>
        /// <param name="input">Object to be compared</param>
        /// <returns>Boolean</returns>
        public override bool Equals(object input)
        {
            return this.Equals(input as TestValueTypesModelsTest);
        }

        /// <summary>
        /// Returns true if TestValueTypesModelsTest instances are equal
        /// </summary>
        /// <param name="input">Instance of TestValueTypesModelsTest to be compared</param>
        /// <returns>Boolean</returns>
        public bool Equals(TestValueTypesModelsTest input)
        {
            if (input == null)
                return false;

            return 
                (
                    this.Number == input.Number ||
                    (this.Number != null &&
                    this.Number.Equals(input.Number))
                ) && 
                (
                    this.IsEnabled == input.IsEnabled ||
                    (this.IsEnabled != null &&
                    this.IsEnabled.Equals(input.IsEnabled))
                );
        }

        /// <summary>
        /// Gets the hash code
        /// </summary>
        /// <returns>Hash code</returns>
        public override int GetHashCode()
        {
            unchecked // Overflow is fine, just wrap
            {
                int hashCode = 41;
                if (this.Number != null)
                    hashCode = hashCode * 59 + this.Number.GetHashCode();
                if (this.IsEnabled != null)
                    hashCode = hashCode * 59 + this.IsEnabled.GetHashCode();
                return hashCode;
            }
        }

        /// <summary>
        /// To validate all properties of the instance
        /// </summary>
        /// <param name="validationContext">Validation context</param>
        /// <returns>Validation Result</returns>
        IEnumerable<System.ComponentModel.DataAnnotations.ValidationResult> IValidatableObject.Validate(ValidationContext validationContext)
        {
            yield break;
        }
    }
}

As you can see, swagger-gen has converted the required primitive types like int and bool to nullable types automatically. That's not my desired result and I think it's not true or at least there should be a flag to turn this feature on or off!

Swagger-codegen version

3.0.18

Swagger declaration file content or url

openapi: 3.0.1
info:
  title: Test Value Types
  version: v1
paths:
  /api/Test:
    post:
      tags:
        - Test
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/TestValueTypes.Models.Test'
          text/json:
            schema:
              $ref: '#/components/schemas/TestValueTypes.Models.Test'
          application/*+json:
            schema:
              $ref: '#/components/schemas/TestValueTypes.Models.Test'
      responses:
        '200':
          description: Success
components:
  schemas:
    TestValueTypes.Models.Test:
      required:
        - isEnabled
        - number
      type: object
      properties:
        number:
          type: integer
          format: int32
        isEnabled:
          type: boolean
      additionalProperties: false

Command line used for generation

CMD: java -jar swagger-codegen-cli.jar generate -i ./swagger.json -l csharp

Related issues/PRs

https://github.com/swagger-api/swagger-codegen/issues/2885

Suggest a fix/enhancement

I've started investigation on swagger-gen source code and I've found out that model generation template is placed in modelGeneric.mustache file.

Property generation is happening on line 35 and the question mark(?) is placed near the type name when the required flag is set to \^ which means false in mustache language:

public {{#complexType}}{{{complexType}}}{{/complexType}}{{^complexType}}{{{datatypeWithEnum}}}{{/complexType}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}} {{name}} { get; set; }

I'm pretty sure that required is set to true in that place and ? should not be rendered here because this variable is used to determine and generate InvalidDataException in the constructor of the same class earlier and both of these statements loop over the same #vars variable:

{{#vars}}
{{^isInherited}}
{{^isReadOnly}}
{{#required}}
// to ensure "{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}}" is required (not null)
if ({{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} == null)
{
    throw new InvalidDataException("{{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}} is a required property for {{classname}} and cannot be null");
}
else
{
    this.{{name}} = {{#lambda.camelcase_param}}{{name}}{{/lambda.camelcase_param}};
}
{{/required}}
{{/isReadOnly}}
{{/isInherited}}
{{/vars}}

This section works correctly and exceptions exist in the constructor:

public TestValueTypesModelsTest(int? number = default(int?), bool? isEnabled = default(bool?))
{
    // to ensure "number" is required (not null)
    if (number == null)
    {
        throw new InvalidDataException("number is a required property for TestValueTypesModelsTest and cannot be null");
    }
    else
    {
        this.Number = number;
    }
    // to ensure "isEnabled" is required (not null)
    if (isEnabled == null)
    {
        throw new InvalidDataException("isEnabled is a required property for TestValueTypesModelsTest and cannot be null");
    }
    else
    {
        this.IsEnabled = isEnabled;
    }
}

But I couldn't find out why required is working for generating constructor exceptions and not working for property generations!

tylermichael commented 2 years ago

Also running into this and I think @MoienTajik comments are spot on.

cixonline commented 2 years ago

We are also running into the exact same issue. The linked issue suggests it was fixed but that's not the case.

piersb commented 1 year ago

I believe this can be fixed by

removing the nullable annotation from the mapping at https://github.com/swagger-api/swagger-codegen/blob/712f77d454fef86bf6edec9da2725c50f382f042/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractCSharpCodegen.java#L112
re-inserting the annotation for nullable items only in modelGeneric.mustache by changing the line

public {{{datatype}}} {{name}} { get; {{#isReadOnly}}private {{/isReadOnly}}set; }

to

public {{{datatype}}}{{^isContainer}}{{^required}}?{{/required}}{{/isContainer}} {{name}} { get; {{#isReadOnly}}private {{/isReadOnly}}set; }

swagger-api / swagger-codegen