swagger-api / swagger-api/swagger-codegen
[CSharp] Swagger-gen converts required primitive-types to nullable types
Nobody has claimed this yet.
- Dominant language
- Mustache
- Stars
- 17.8k
- Forks
- 6k
- PR merge metrics
- No merged PRs in 30d
Description
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
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!
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with modules/swagger-codegen/src/main/resources/csharp/modelGeneric.mustache, especially the property and constructor sections identified in the report. Run the stated swagger-codegen CLI command with the supplied OpenAPI schema and compare the generated C# model; done means required primitive properties have the intended generated types without breaking constructor validation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, openapi
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100