RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.76k stars 1.29k forks source link

Nswag, c# Required = Newtonsoft.Json.Required.DisallowNull, how to handle #1991

Open Unders0n opened 5 years ago

Unders0n commented 5 years ago

Copy of https://stackoverflow.com/questions/54656603/swagger-nswagstudio-c-required-newtonsoft-json-required-disallownull-how , which got no attention, please help.

Given: api that i have limited influence to in terms of changing, built on net core 2.2. Standart netCore swagger used. Some classes of DTO have fields in it marked with [System.ComponentModel.DataAnnotations.Required] But for some reasons (which are also discussable) some methods return objects of this classes with nulls in this fields. Annotation resulted in

"required": [ "given", - this field for example "family", "email", "postCode" ], "type": "object", ...

in swagger spec which then results in

[Newtonsoft.Json.JsonProperty("given", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]

in c# generated code (i'm using nswag studio and c# client with pretty standart settings). And then when i'm trying to get list of such objects from api using generated c# client, if some of such properties have null it obviosly throws newtonsoft deserialization exception. so how we can handle that? I thought of both client side and server side solutions:

1)on server we can configure not to expose info about required to swagger spec.

2) on client we can possibly configure behavior of translating that Required block to Required = Newtonsoft.Json.Required.Default

3) forget about all that and insist so api will not return object with null vaues which properties market Required.

RicoSuter commented 5 years ago

Is this swagger 2.0 or openapi 3.0?

Unders0n commented 5 years ago

@RSuter as from swagger spec: "swagger": "2.0",

RicoSuter commented 5 years ago

To clarify the generation of this:

image

image

base.IsNullable:

image

The biggest problem is that Swagger 2.0 does not have a way to express nullability - so we need to use required for nullability which is technically wrong - in JSON Schema this just specifies whether the the value must be set or not...

RicoSuter commented 5 years ago

Another option is to transform the swagger spec before generating the clients, sample here: https://github.com/Picturepark/Picturepark.SDK.Playground/blob/master/src/AutoRestTransformer/Program.cs

VitalickS commented 5 years ago

As alternative, you can manually override behavior for JsonSerializerSettings in partial class method (Client). Just add this to partial class

partial void UpdateJsonSerializerSettings(JsonSerializerSettings settings)
{
    settings.ContractResolver = new SafeContractResolver();
}

class SafeContractResolver : DefaultContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        var jsonProp = base.CreateProperty(member, memberSerialization);
        jsonProp.Required = Required.Default;
        return jsonProp;
    }
}
kjkrum commented 4 years ago

@RicoSuter Given that this is a limitation of Swagger 2.0, is there an official position on where the fault lies if a 2.0 API describes a property as required but then sends null for that property? Is it unequivocally a bug in the API, or is it a matter of there being no good general solution for NSwag and therefore up to the client to implement a workaround?

kjkrum commented 4 years ago

To shed some light on my own question, this SO Q&A suggests that an API sending a null value for a string property is wrong, not because the property is required, but because null isn't a string. It sounds like Swagger 2.0 having no means to express nullability strictly means that nothing is nullable. Any way of representing nullability is just a hack, whether it's an API sending nulls, or NSwag's treatment of required. Is that correct?

kyleb4020 commented 1 year ago

As alternative, you can manually override behavior for JsonSerializerSettings in partial class method (Client). Just add this to partial class

partial void UpdateJsonSerializerSettings(JsonSerializerSettings settings)
{
    settings.ContractResolver = new SafeContractResolver();
}

class SafeContractResolver : DefaultContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        var jsonProp = base.CreateProperty(member, memberSerialization);
        jsonProp.Required = Required.Default;
        return jsonProp;
    }
}

This answer helped me to create a more nuanced approach to intelligently adjust the Required property of the JsonProperty. Here is my code:

internal class SafeContractResolver : DefaultContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        var jsonProp = base.CreateProperty(member, memberSerialization);
        jsonProp.Required = member.MemberType switch
        {
            MemberTypes.Property when member is PropertyInfo info && IsNullable(info) => Required.Default,
            MemberTypes.Field when member is FieldInfo fieldInfo && IsNullable(fieldInfo) => Required.Default,
            _ => jsonProp.Required
        };

        return jsonProp;
    }
}

private static bool IsNullable(PropertyInfo property) => IsNullableHelper(property.PropertyType, property.DeclaringType, property.CustomAttributes);

private static bool IsNullable(FieldInfo field) => IsNullableHelper(field.FieldType, field.DeclaringType, field.CustomAttributes);

private static bool IsNullableHelper(Type memberType, MemberInfo? declaringType, IEnumerable<CustomAttributeData> customAttributes)
{
    if (memberType.IsValueType)
        return Nullable.GetUnderlyingType(memberType) != null;

    var nullable = customAttributes
        .FirstOrDefault(x => x.AttributeType.FullName == "System.Runtime.CompilerServices.NullableAttribute");
    if (nullable is { ConstructorArguments.Count: 1 })
    {
        var attributeArgument = nullable.ConstructorArguments[0];
        if (attributeArgument.ArgumentType == typeof(byte[]))
        {
            var args = (ReadOnlyCollection<CustomAttributeTypedArgument>)attributeArgument.Value!;
            if (args.Count > 0 && args[0].ArgumentType == typeof(byte))
            {
                return (byte)args[0].Value! == 2;
            }
        }
        else if (attributeArgument.ArgumentType == typeof(byte))
        {
            return (byte)attributeArgument.Value! == 2;
        }
    }

    for (var type = declaringType; type != null; type = type.DeclaringType)
    {
        var context = type.CustomAttributes
            .FirstOrDefault(x => x.AttributeType.FullName == "System.Runtime.CompilerServices.NullableContextAttribute");
        if (context is { ConstructorArguments.Count: 1 } &&
            context.ConstructorArguments[0].ArgumentType == typeof(byte))
        {
            return (byte)context.ConstructorArguments[0].Value! == 2;
        }
    }

    // Couldn't find a suitable attribute
    return false;
}

With the introduction of C# 8 we also need to account for nullable reference types. This handy IsNullableHelper() method (found here: https://stackoverflow.com/a/58454489/7546999) uses reflection to determine if the property is nullable and should therefore have the Required property of the JsonProperty set to Required.Default.

I hope this helps the next person who comes here looking for a good workaround.

rfcdejong commented 1 year ago

For someone who isn't using the JsonConvert himself. You can set the default DataContractSerializer to the Safe one:

JsonConvert.DefaultSettings = () => new JsonSerializerSettings { ContractResolver = new SafeContractResolver() };