Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG][csharp] Nullable property does not have a set default value #20535

Open
5 of 6 tasks
balassit opened this issue Jan 23, 2025 · 1 comment
Open
5 of 6 tasks

[BUG][csharp] Nullable property does not have a set default value #20535

balassit opened this issue Jan 23, 2025 · 1 comment

Comments

@balassit
Copy link

balassit commented Jan 23, 2025

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

For types such as dictionary or list, there is no default value which then forces it to be required even though the EmitDefaultValue is set to true. What is needed is to set the property for these types like the below for collection types.

Issue

Collections (like Dictionary, List, etc.) do not have a natural default value in C#. Without initialization, they remain null. This causes an issue when you serialize or deserialize these types. Even if EmitDefaultValue = true is set, the property still remains null unless explicitly initialized in code.

openapi-generator version

7.11.0-SNAPSHOT

OpenAPI declaration file content or url
openapi: 3.0.0
info:
  title: Test Dictionary Nullability
  description: Test Dictionary Nullability
  version: '2025-01-31'
tags: []
paths: {}
components:
  schemas:
    JobDefinition:
      type: object
      properties:
        JobName:
          type: string
          description: Name of the Job
          readOnly: true
        SparkConf:
          type: object
          additionalProperties:
            type: string
          nullable: true
          description: The key value pairs for spark configuration.
Generation Details
$env:INPUT_PATH = "libraries/openapi.yaml"
$env:OUTPUT_PATH = "libraries/csharp_client"

docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate -i /local/${INPUT_PATH} -g csharp -o /local/${OUTPUT_PATH} --enable-post-process-file --additional-properties=targetFramework=net8.0,packageName=Test.Dictionary,packageVersion=0.0.1
Steps to reproduce

JobDefinition.cs

/*
 * Test Dictionary Nullability
 *
 * Test Dictionary Nullability
 *
 * The version of the OpenAPI document: 2025-01-31
 * Generated by: https://github.com/openapitools/openapi-generator.git
 */


using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.IO;
using System.Runtime.Serialization;
using System.Text;
using System.Text.RegularExpressions;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using Newtonsoft.Json.Linq;
using System.ComponentModel.DataAnnotations;
using OpenAPIDateConverter = Microsoft.SentinelGraph.JobService.Client.OpenAPIDateConverter;

namespace Microsoft.SentinelGraph.JobService.Model
{
    /// <summary>
    /// JobDefinition
    /// </summary>
    [DataContract(Name = "JobDefinition")]
    public partial class JobDefinition : IValidatableObject
    {
        /// <summary>
        /// Initializes a new instance of the <see cref="JobDefinition" /> class.
        /// </summary>
        /// <param name="sparkConf">The key value pairs for spark configuration..</param>
        public JobDefinition(Dictionary<string, string> sparkConf = default(Dictionary<string, string>))
        {
            this.SparkConf = sparkConf;
        }

        /// <summary>
        /// Name of the Job
        /// </summary>
        /// <value>Name of the Job</value>
        [DataMember(Name = "JobName", EmitDefaultValue = false)]
        public string JobName { get; private set; }

        /// <summary>
        /// Returns false as JobName should not be serialized given that it's read-only.
        /// </summary>
        /// <returns>false (boolean)</returns>
        public bool ShouldSerializeJobName()
        {
            return false;
        }
        /// <summary>
        /// The key value pairs for spark configuration.
        /// </summary>
        /// <value>The key value pairs for spark configuration.</value>
        [DataMember(Name = "SparkConf", EmitDefaultValue = true)]
        public Dictionary<string, string> SparkConf { get; set; }

        /// <summary>
        /// Returns the string presentation of the object
        /// </summary>
        /// <returns>String presentation of the object</returns>
        public override string ToString()
        {
            StringBuilder sb = new StringBuilder();
            sb.Append("class JobDefinition {\n");
            sb.Append("  JobName: ").Append(JobName).Append("\n");
            sb.Append("  SparkConf: ").Append(SparkConf).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 Newtonsoft.Json.JsonConvert.SerializeObject(this, Newtonsoft.Json.Formatting.Indented);
        }

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

}

Request Body

{
	"JobName": "demo"
}

Error Response

{
	"errors": {
		"sparkConf": [
			"The SparkConf field is required."
		]
	},
	"type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
	"title": "One or more validation errors occurred.",
	"status": 400,
	"traceId": "00-89d8bac983fa49cfaa3b35d2ac8fb577-99b716e53e94e3f7-00"
}
Related issues/PRs
Suggest a fix

To ensure that collections are never null, you must initialize the property (either in the field, in the constructor, or via a custom setter). This ensures that property will always be an empty dictionary (new Dictionary<string, string>()) instead of null, even if it's not included in the request payload. This initialization avoids the issue where the property is "missing" and treated as required, triggering a 400 Bad Request.

public Dictionary<string, string> SparkConf { get; set; } = [];

If using [] I think it can be applied like the below. Unsure how to specify for language >= net5, otherwise specify the same way type was included in the method return type. (it would be nice to use language supported features where possible)

{{#isContainer}}{{#vendorExtensions.x-emit-default-value}} = []{{/vendorExtensions.x-emit-default-value}}{{^vendorExtensions.x-emit-default-value}}{{^required}} = []{{/required}}{{/vendorExtensions.x-emit-default-value}}{{/isContainer}}

modelGeneric.mustache#L54

  1. If it is a container type, proceed
  2. If vendorExtensions.x-emit-default-value is true, we emit = [].
  3. If vendorExtensions.x-emit-default-value is not true and required is true, we emit = [].
  4. If neither condition is true, nothing will be emitted.

Note since .net5, the empty collection format has been supported. A check can be made for any collection types that are known to be auto generated (List, Dictionary, HashSet) and applied.

@balassit
Copy link
Author

@wing328 does this sound ok as a proposal? I included a sample of needed coded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant