Swashbuckle.AspNetCore icon indicating copy to clipboard operation
Swashbuckle.AspNetCore copied to clipboard

Configure non-nullable types as required

Open coder925 opened this issue 4 years ago • 23 comments

In my project we do not distinguish between required and nullable: false. If a type is non-nullable, it is also required. This is regardless if it is a value type or reference type.

It would be great if one could optionally configure Swashbuckle to set all non-nullable properties to required. E.g. like: setupAction.MarkNonNullableTypesAsRequired(); This way, I could remove all my [Required] annotations on the non-nullable C# properties.

This is a subsequent request following #1686. As originally posted by @felschr in https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/1686#issuecomment-674074694 , treating non-nullable as required is the default for nullable contexts in .NET validation.

coder925 avatar Feb 26 '21 12:02 coder925

Temporary solution:

serviceCollection.AddSwaggerGen(builder =>
{
    builder.SupportNonNullableReferenceTypes();
    builder.SchemaFilter<RequiredNotNullableSchemaFilter>();

    ...
});
class RequiredNotNullableSchemaFilter : ISchemaFilter {
    public void Apply(OpenApiSchema schema, SchemaFilterContext context) {
        if (schema.Properties == null) {
            return;
        }

        var notNullableProperties = schema
            .Properties
            .Where(x => !x.Value.Nullable  && !schema.Required.Contains(x.Key))
            .ToList();

        foreach (var property in notNullableProperties)
        {
            schema.Required.Add(property.Key);
        }
    }
}

UPD: This solution is not completely correct, see example:

public class User
{
    public Guid Id { get; set; }
    public Guid? OrganizationId { get; set; }
    public string FirstName { get; set; } = default!;
    public string? MiddleName { get; set; }
    public string LastName { get; set; } = default!;

    public UserEmail Email { get; set; } = default!;
    public UserPhone? Phone { get; set; }
}

public class UserEmail
{
    public string Address { get; set; } = default!;
}

public class UserPhone
{
    public string Number { get; set; } = default!;
}
{
      "User": {
        "required": [
          "email",
          "firstName",
          "id",
          "lastName",
          "phone" // Incorrect
        ],
        "type": "object",
        "properties": {
          "id": {
            "type": "string",
            "format": "uuid"
          },
          "organizationId": {
            "type": "string",
            "format": "uuid",
            "nullable": true
          },
          "firstName": {
            "type": "string"
          },
          "middleName": {
            "type": "string",
            "nullable": true
          },
          "lastName": {
            "type": "string"
          },
          "email": {
            "$ref": "#/components/schemas/UserEmail"
          },
          "phone": {
            "$ref": "#/components/schemas/UserPhone"
          }
        },
        "additionalProperties": false
      }

flibustier7seas avatar Mar 23 '21 05:03 flibustier7seas

@flibustier7seas you mention that your solution isn't completely correct, and I see you mention the phone field being marked as required, even though it's not. Do you know what in particular causes this case, or if there's any workarounds?

JosNun avatar Jul 22 '21 12:07 JosNun

@JosNun I'm using the following workaround:

class RequiredNotNullableSchemaFilter : ISchemaFilter
{
    public void Apply(OpenApiSchema schema, SchemaFilterContext context)
    {
        if (schema.Properties == null)
        {
            return;
        }

        FixNullableProperties(schema, context);

        var notNullableProperties = schema
            .Properties
            .Where(x => !x.Value.Nullable && !schema.Required.Contains(x.Key))
            .ToList();

        foreach (var property in notNullableProperties)
        {
            schema.Required.Add(property.Key);
        }
    }

    /// <summary>
    /// Option "SupportNonNullableReferenceTypes" not working with complex types ({ "type": "object" }), 
    /// so they always have "Nullable = false",
    /// see method "SchemaGenerator.GenerateSchemaForMember"
    /// </summary>
    private static void FixNullableProperties(OpenApiSchema schema, SchemaFilterContext context)
    {
        foreach (var property in schema.Properties)
        {
            if (property.Value.Reference != null)
            {
                var field = context.Type
                    .GetMembers(BindingFlags.Public | BindingFlags.Instance)
                    .FirstOrDefault(x =>
                        string.Equals(x.Name, property.Key, StringComparison.InvariantCultureIgnoreCase));

                if (field != null)
                {
                    var fieldType = field switch
                    {
                        FieldInfo fieldInfo => fieldInfo.FieldType,
                        PropertyInfo propertyInfo => propertyInfo.PropertyType,
                        _ => throw new NotSupportedException(),
                    };

                    property.Value.Nullable = fieldType.IsValueType
                        ? Nullable.GetUnderlyingType(fieldType) != null
                        : !field.IsNonNullableReferenceType();
                }
            }
        }
    }    
}

Result:

{
  "User": {
    "required": ["email", "firstName", "id", "lastName"],
    "type": "object",
    "properties": {
      "id": {
        "type": "string",
        "format": "uuid"
      },
      "organizationId": {
        "type": "string",
        "format": "uuid",
        "nullable": true
      },
      "firstName": {
        "type": "string"
      },
      "middleName": {
        "type": "string",
        "nullable": true
      },
      "lastName": {
        "type": "string"
      },
      "email": {
        "$ref": "#/components/schemas/UserEmail"
      },
      "phone": {
        "$ref": "#/components/schemas/UserPhone"
      }
    },
    "additionalProperties": false
  }
}

flibustier7seas avatar Aug 06 '21 05:08 flibustier7seas

small modification on @flibustier7seas 's answer

var notNullableProperties = schema
      .Properties
      .Where(x => !x.Value.Nullable && x.Value.Default == default && !schema.Required.Contains(x.Key))
      .ToList();

ahmednfwela avatar Jan 11 '22 14:01 ahmednfwela

If you use the SwaggerGenOptions.UseAllOfToExtendReferenceSchemas() extension method, the FixNullableProperties workaround of @flibustier7seas is not necessary. With that, references are wrapped in an allOf and SupportNonNullableReferenceTypes also applies to them.

brather1ng avatar Jan 28 '22 13:01 brather1ng

I would love to see this work out of the box

niemyjski avatar Feb 24 '22 19:02 niemyjski

+1 For this feature

sebasptsch avatar Jun 21 '22 04:06 sebasptsch

Thx @flibustier7seas, that works for me! But I had to remove if (property.Value.Reference != null).

mladenkn avatar Jun 22 '22 20:06 mladenkn

If you use the SwaggerGenOptions.UseAllOfToExtendReferenceSchemas() extension method, the FixNullableProperties workaround of @flibustier7seas is not necessary. With that, references are wrapped in an allOf and SupportNonNullableReferenceTypes also applies to them.

This won't work for string type or e.g. DateTimeOffset.

Rookian avatar Sep 19 '22 17:09 Rookian

With C# 11.0/.NET 7.0, the required keyword is being introduced for properties. Properties with this keyword needs to be initialized through the object initializer when the class/struct/record is initialized from code.

But required properties also affects the behavior of System.Text.Json 7.x deserialization and thereby aspnetcore model validation.

The default behavior for System.Text.Json deserialization / aspnetcore model validation in .NET 7.0 is as follows.

Nullable/not required properties:

  • Property CAN be null, and property CAN be omitted from JSON payload.

Nullable/required properties:

  • Property CAN be null, but property CAN NOT be omitted from JSON payload.

Not nullable/not required properties:

  • Property CAN NOT be null, but property CAN be omitted from JSON payload (since it implies null).

Not nullable/required properties:

  • Property CAN NOT be null, and property CAN NOT be omitted from JSON payload.

Since nullablility and required/not are different things where one does not necessarily imply the other, I think SwasBuckle should use the C# property nullability for swagger property nullable, and C# property required/not for the swagger property required.

If the C# property is annotated with RequiredAttribute, I think this should override the above, and set both swagger nullable/required to true.

References:

josundt avatar Oct 27 '22 10:10 josundt

With C# 11.0/.NET 7.0, the required keyword is being introduced for properties. Properties with this keyword needs to be initialized through the object initializer when the class/struct/record is initialized from code.

But required properties also affects the behavior of System.Text.Json 7.x deserialization and thereby aspnetcore model validation.

The default behavior for System.Text.Json deserialization / aspnetcore model validation in .NET 7.0 is as follows.

Nullable/not required properties:

  • Property CAN be null, and property CAN be omitted from JSON payload.

Nullable/required properties:

  • Property CAN be null, but property CAN NOT be omitted from JSON payload.

Not nullable/not required properties:

  • Property CAN NOT be null, but property CAN NOT be omitted from JSON payload (since it implies null).

Not nullable/required properties:

  • Property CAN NOT be null, and property CAN NOT be omitted from JSON payload.

Since nullablility and required/not are different things where one does not necessarily imply the other, I think SwasBuckle should use the C# property nullability for swagger property nullable, and C# property required/not for the swagger property required.

If the C# property is annotated with RequiredAttribute, I think this should override the above, and set both swagger nullable/required to true.

References:

... And if C# 11's required modifier is specified it should also set required regardless of if it's a string or not.

JohnGalt1717 avatar Nov 15 '22 19:11 JohnGalt1717

One more case to consider: avoiding inferred required attribute when default value is defined.

public string Prop { get; init; } = "default value";

It’s non-nullable, yet it shoudn’t be required as it has a default value.

See how ASP validation is working as a result of this issue

piskov avatar Oct 08 '23 00:10 piskov

Keep in mind that the required keyword rules everything now.

JohnGalt1717 avatar Oct 08 '23 01:10 JohnGalt1717

@JohnGalt1717

Keep in mind that the required keyword rules everything now.

Absolutely not. When you use required, caller must provide the value. For example this code will not compile even with default value:

var p = new Person(); 
// ↑ compile error: Required member 'Person.Name' must be set in the object initializer



public class Person

{

    public required string Name { get; init; } = "Will not work";

}

There’re a lot of use-cases for non-nullable but also not required properties. For example, some options with default values. You don’t want them to be nullable in your code (as they will never be with default value), yet you don’t want to force consumers to supply them, hence the default value:



public class Project
{

    public SomeSettings Prop { get; init; } = new();
}

↑ // non-nullable, yet not required. This is exactly how ASP checker works

tldr; required non-nullable means you must provide the value; non-nullable with default value means, it’s ok to ommit as some sensible value will be used.

piskov avatar Oct 08 '23 01:10 piskov

We’re in agreement. No required means same as before.

Required being there means always must be filled in.

JohnGalt1717 avatar Oct 08 '23 01:10 JohnGalt1717

Yet some PRs and this sentence ↓ seem to ignore the default logic.

Not nullable/not required properties: Property CAN NOT be null, but property CAN NOT be omitted from JSON payload (since it implies null).

It obviosly CAN be ommited from json IF default value is set (this way it DOES not imply null).

piskov avatar Oct 08 '23 02:10 piskov

Yet some PRs and this sentence ↓ seem to ignore the default logic.

Not nullable/not required properties: Property CAN NOT be null, but property CAN NOT be omitted from JSON payload (since it implies null).

It obviosly CAN be ommited from json IF default value is set (this way it DOES not imply null).

You're right, for this bullet there was a unintentional NOT in my original comment. I updated it now.

josundt avatar Oct 11 '23 11:10 josundt

@josundt About your updated comment:

Not nullable/not required properties:

* Property CAN NOT be null, but property CAN be omitted from JSON payload (since it implies null).

Is this really true for System.Text.Json deserialization / aspnetcore model validation? This doesn't make any sense to me. If it is implicitly null when it is omitted, then why can it be omitted if it can't be null? It would make more sense if it could only be omitted if a non-null default value is set (like the last comment from @piskov mentions).

cremor avatar Oct 11 '23 14:10 cremor

@cremor I guess not nullable properties in most cases should also be required.

But I also see that it sometimes may be useful to make not nullable properties optional (a.k.a. not required) with default values, and allow them to be omitted in JSON payloads or in object initializers.

I agree that the combination not nullable/not required could be a bit illogical, and arguably in many cases a bad design decision. F.ex. in a WebAPI context when a JSON payload is submitted by a client, the client may not expect properties omitted in the payload to magically have default values assigned on the server-side. The client is unaware of this hidden logic.

josundt avatar Oct 16 '23 11:10 josundt

@josundtit it will be “bad design decision” only if the default values are surprising to the user. Suppose you have aProfileDto and a VisibilityOptionsDto Settings(which consists of 20 bool flags) inside it.

You have 3 options:

  1. Make VisibilityOptionsDto? Settings nullable in C# and thus be forced to always check it for null in code.
  2. Make it required non-nullable required VisibilityOptionsDto Settings and force both API client and your own c# code to provide default value even if it’s just an object with 20 false bools.
  3. Make it non-nullable with simple default value VisibilityOptionsDto Settings {get; set; } = new(). This is the best of all worlds: neither you, nor client is needed to supply an object with 20 false bools, and you are free of useless nullability checks in C# code.

So as long as default value is clear, it’s completely reasonable to support this. Imagine if you were forced to explicitly supply every property of the class every time you were to create a new object in C#.

piskov avatar Oct 16 '23 17:10 piskov

https://github.com/domaindrivendev/Swashbuckle.AspNetCore/pull/2710#issuecomment-2053572112

martincostello avatar Apr 14 '24 10:04 martincostello

i am no longer able to work on it.

blouflashdb avatar Apr 14 '24 12:04 blouflashdb

If anyone else would like to pick up the above fix and take it forward that would be welcome.

martincostello avatar Apr 14 '24 12:04 martincostello