Swashbuckle.aspnetcore: There is a bug about Nullable enum with UseReferencedDefinitionsForEnums.

[PMExtra]

C# class Dto1 { public SomeEnum Some {get; set;} } class Dto2 { public SomeEnum? Some {get; set;} }

It will generate redundant enums.

[PMExtra]

src/Swashbuckle.AspNetCore.SwaggerGen/Generator/SchemaRegistry.cs#L70

- (type.GetTypeInfo().IsEnum && _options.UseReferencedDefinitionsForEnums));
+ (_options.UseReferencedDefinitionsForEnums && (type.GetTypeInfo().IsEnum || Nullable.GetUnderlyingType(type)?.GetTypeInfo().IsEnum == true)));

https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/2a9efcfa64aa3c5fc5c665113b351c62906701bc/src/Swashbuckle.AspNetCore.SwaggerGen/Generator/SchemaRegistry.cs#L55-L75

[PMExtra]

@domaindrivendev
I sincerely hope that this problem can be fixed and released as soon as possible, which has troubled my front-end development. (with NSwag)

[PMExtra]

It's not fix completely.
Just make another Nullable reference.
I'm wondering if we can unpack Nullable generic at any time such as:
type = Nullable.GetUnderlyingType(type) ?? type;.

[PMExtra]

@onionhammer Could you help me to review that? #864

[kwinkel]

@PMExtra, any news on this (and #888)? Currently, I am unable to generate nullable enum models when I include referenced definitions with SwaggerGenOptions.UseReferencedDefinitionsForEnums.

[PMExtra]

@kwinkel I think I've fixed it in the PR #888 . But still no one to accepts it.

[domaindrivendev]

Could you add a test please?

[Rassilion]

Any news on this?

[PMExtra]

@Rassilion My patched version works fine. But I'm too busy to make an unittest.

[Rassilion]

I updated to 5.0.0-rc2 and looks like bug fixed, I was on 5.0.0-beta.

[rsibanez89]

@Rassilion that version has a different issue

    [Flags]
    public enum TargetGender : byte
    {
        None = 0,
        Male = 1,
        Female = 2,
        Both = Male | Female
    }

will get translated as:

"TargetGender": {
        "enum": [
          null,
          null,
          null,
          null
        ],
        "type": "integer",
        "format": "int32"
      },

[spanevin]

There is no release version of the package with this fix yet. When do we expect v5 release to be issued?
Can we backport the fix to v4?
Is there a workaround which I can use in swagger initialization code to "fix" the issue with 4.0.1 ?

[domaindrivendev]

@Rassilion if you've discovered a separate issue, please create a separate issue for it - it's too difficult to track issues within issues.

@PMExtra - after further review, I'm not convinced that the generation of two separate enums should be considered a bug ...

In OpenApi 3 (OA3), the schema definition has a Nullable property to indiciate if a null value can be submitted for a given property. In your example, this is true for one of the properties but not the other. So, using OA3, there _has to be_ two separate schema definitions to represent the API contract accurately.

[domaindrivendev]

Due to this unavoidable constraint, I don't believe this is an issue with SB. If it's causing issues in downstream tooling (e.g. Nswag) then I think the issue actually lies there.

In closing, I can offer an alternative approach that may or may not work for you. I assume your goal with the non-nullable vs non-nullable properties is to make the value "required" in one case and not in the other. In OA3, this should be represented using the Schema.Required property rather than the Nullable property (for a JSON API, there's actually a subtle different between optional and nullable). So to do this, you could try the following:

class Dto1 {
    [Required]
    public SomeEnum Some {get; set;}
}
class Dto2 {
    public SomeEnum Some {get; set;}
}

Of course the downside here is that if no value is submitted for the optional (non-required) property, then server side the value will default to the first value in your enum definition. So, you would need to introduce a value of Unspecified and have your code handle that in the same way you would have handled a null value. Hope that makese sense.

[TimothyByrd]

@domaindrivendev I would like UseReferencedDefinitionsForEnums to work with nullable enums.

Maybe I'm missing something, but I thought the semantics of C# worked like this:

1. Reference types (e.g., class instances) are nullable.
2. Value types (e.g. integers and enums) are not nullable unless explicitly made so using the "?" qualifier.

So having to put a [Required] on an enum seems backward to the language.

class MyClass
{
    public MyEnum RequiredEnum  { get; set; }
    public MyEnum? OptionalEnum  { get; set; }
}

It seems to me that the above should generate two references to MyEnum, one required one not.

    "MyClass": {
      "required": [
        "requiredEnum"
      ],
      "type": "object",
      "properties": {
        "requiredEnum": {
          "$ref": "#/definitions/MyEnum",
        },
        "optionalEnum": {
          "$ref": "#/definitions/MyEnum",
        }
      }
    }

[domaindrivendev]

In Swagger/OpenAPI, required and nullable are mutually exclusive. @TimothyByrd - I think you may be conflating the two. The former means that the property MUST be provided in the request, whereas the latter indicates the property MAY have a value of null. Given, your example:

class MyClass
{
    public MyEnum RequiredEnum  { get; set; }
    public MyEnum? OptionalEnum  { get; set; }
}

There's nothing in the code here that definitively indicates that OptionalEnum MUST be provided with the request. If JSON was submitted _without_ the OptionalEnum property then the property value would be defaulted to null but the request wouldn't fail. To definitively state that a value MUST be provided, Swashbuckle needs more information - i.e. the RequiredAttribute.

[spanevin]

I installed 5.0.0 release and haven't found UseReferencedDefinitionsForEnums in it, what did I miss?

5.0.0 still shows optional (nullable, not marked as [Required]) enums as required.

[TimothyByrd]

@domaindrivendev the issue I have is that OptionalEnum isn't a 'MyEnum'. Given this code:

[Newtonsoft.Json.JsonConverter(typeof(Newtonsoft.Json.Converters.StringEnumConverter))]
public enum MyEnum
{
    v1,
    v2
};

public class MyClass1
{
    public MyEnum RequiredEnum { get; set; }
    public MyEnum? OptionalEnum { get; set; }
}

The swagger I get is for it is:

"MyClass1": {
  "required": [
    "requiredEnum"
  ],
  "type": "object",
  "properties": {
    "requiredEnum": {
      "$ref": "#/definitions/MyEnum"
    },
    "optionalEnum": {
      "enum": [
        "v1",
        "v2"
      ],
      "type": "string"
    }
  }
},
"MyEnum": {
  "enum": [
    "v1",
    "v2"
  ],
  "type": "string"
},

Since OptionalEnum is not a reference to "#/definitions/MyEnum", when I use Nswag to generate code, they come out as two different types.

So parts of my interface use my "MyEnum" enum and parts use things with names like "MyClass1OptionalEnum". And then consumers of my SDK have to do weird casts.

[domaindrivendev]

Which version of SB are you using? Using v5.1.0, I see the following Swagger being generated for your example class MyClass:

"MyClass1": {
  "type": "object",
  "properties": {
    "requiredEnum": {
      "$ref": "#/components/schemas/MyEnum"
    },
    "optionalEnum": {
      "$ref": "#/components/schemas/MyEnum"
    }
  }
}

[TimothyByrd]

I'm using 4.0.1 which was the latest stable release when I wrote that comment last September.

We are in the process of moving from .Net core 2.1 to 3.1.

It looks like the problem I see got fixed.
I'll try updating Swashbuckle independently.

[TimothyByrd]

I'm still slogging through upgrading 4.0.1 => 5.1.0.
So now I'm asking why optionalEnum isn't marked as nullable like this?

"MyClass1": {
  "type": "object",
  "properties": {
    "requiredEnum": {
      "$ref": "#/components/schemas/MyEnum"
    },
    "optionalEnum": {
      "$ref": "#/components/schemas/MyEnum",
      "nullable": true
    }
  }
}

[TimothyByrd]

I've worked through most of the issues moving to 5.1.0, but this is a blocker for me. I have a shared code project to test against both my original code and the SDK and this causes the generated SDK code to be incompatible.

As it is, to build my C# SDK, I have to hand-edit the generated swagger, adding the "nullable": true line before calling NSwag to generate the SDK.

I added a schema filter to set Nullable to true but it doesn't affect the generated swagger. Am I doing something wrong here?

(Note: This and some other filters are here: https://github.com/TimothyByrd/swashbuckle_stuff )

        public void Apply(OpenApiSchema model, SchemaFilterContext context)
        {
            if (model == null) throw new ArgumentNullException(nameof(model));
            if (context == null) throw new ArgumentNullException(nameof(context));

            if (model.Properties == null) return;

            foreach (var property in context.Type.GetProperties())
            {
                if (property.PropertyType.Name.StartsWith("Nullable`", StringComparison.OrdinalIgnoreCase) && property.PropertyType.GenericTypeArguments[0].IsEnum)
                {
                    string schemaPropertyName = PropertyName(property); // camel case if necessary
                    if (model.Properties.TryGetValue(schemaPropertyName, out var modelProperty))
                    {
                        modelProperty.Nullable = true;
                    }
                }
            }
        }

[domaindrivendev]

@TimothyByrd the JSON that you're expecting is actually invalid Swagger/OpenAPI because a "reference" schema MUST NOT have any properties present other than the $ref property. This presents a bit of a dilemna. On the one hand you probably want enum schema's to be referenced as opposed to being redefined inline with every usage. But, if they're referenced then you can't really apply something "contextual" like nullable to the definition schema because it can be shared across different contexts. In fact, there's an issue in the Swagger/OpenAPI repo addressing this exact problem.

Swashbuckle gives you two options here which may or may not suit your needs. You can apply the suggested workaround from that issue with the UseAllOfToExtendReferenceSchemas() setting: This would generate the following JSON for your example:

      "MyClass": {
        "type": "object",
        "properties": {
          "requiredEnum": {
            "allOf": [
              {
                "$ref": "#/components/schemas/MyEnum"
              }
            ]
          },
          "optionalEnum": {
            "allOf": [
              {
                "$ref": "#/components/schemas/MyEnum"
              }
            ],
            "nullable": true
          }
        }
      }

Or, you can force all enum schema's to be defined inline by setting UseInlineDefinitionsForEnums(). This would generate the following JSON for your example:

      "MyClass": {
        "type": "object",
        "properties": {
          "requiredEnum": {
            "enum": [
              "Foo",
              "Bar"
            ],
            "type": "string"
          },
          "optionalEnum": {
            "enum": [
              "Foo",
              "Bar"
            ],
            "type": "string",
            "nullable": true
          }
        }
      }

[TimothyByrd]

@domaindrivendev Thanks - after trying them out, I think UseAllOfToExtendReferenceSchemas will work. (As you'd expect UseInlineDefinitionsForEnums, gives me all individual types.)

Now to do regression testing...

One thing I noticed - using UseAllOfToExtendReferenceSchemas causes my references to also include the XML documentation in the generated swagger. So that's better.

I suppose that's because a $ref doesn't allow any other properties, as you said above.

class MyClass
{
    /// <summary>This description is lost on a plain $ref</summary>
    public MyEnum SomeEnum  { get; set; }
}

[domaindrivendev]

Yes exactly - the AllOf approach allows any "contextual" metadata (including property comments) be assigned to the schema so it has a ton of advantages. In fact, I initially made it the default behavior, but this caused a lot of issues for downstream code generators (technically their issue not SB's but we're all in this together 😉) and so I ended up making it opt-in for now. In the next major version, I may switch to opt-out.