RicoSuter / NSwag

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

Nullable response types are not marked nullable unless [return: CanBeNull] is added #3179

Open lafritay opened 3 years ago

lafritay commented 3 years ago

This is a bug to track the issue mentioned here:

https://github.com/RicoSuter/NSwag/issues/2435#issuecomment-576035741

It would be really nice if a method like Get below didn't need the [return: CanBeNull] line given that it is redundant:

namespace Test {

    public class Foo { }

    [Route("api/[controller]")]
    public class FooController {

        public FooController() { }

        // this one will generate '"nullable": true'
        [return: CanBeNull]
        [HttpGet("{id}")]
        public Foo? Get(long id) => null;

        // this one does not generate '"nullable": true'
        [HttpGet("alt/{id}")]
        public Foo? GetAlt(long id) => null;
    }
}
RicoSuter commented 3 years ago

Maybe the problem here is that we dont have the nullable information for the response type...

We need to check the code: https://github.com/RicoSuter/NSwag/blob/master/src/NSwag.Generation.AspNetCore/Processors/OperationResponseProcessor.cs#L83

As you can see, we only have the apiResponse.Type from ASP.NET Core (without NRT context) - so we'd need to get this information from the original reflected method and then return type - but only if it's not annotated with ProducesResponseType etc. (there you cannot use NRT at all as it's not carried over in .NET reflection).

My general recommendation is to never return "null" from an API but either 404 or a wrapper response object with a "status". Returning null is a bit too .NETty :-)

jeremyVignelles commented 3 years ago

Beside, you will face an infamous known issue where aspnetcore returns 204 when you return null, and it will throw an exception on the client side.

lafritay commented 3 years ago

@RicoSuter Thanks for the info.

@jeremyVignelles That's easy enough to workaround by copying Client.Class.liquid and changing line 286 to this:

if (status_ == {{ response.StatusCode }}{% if response.CheckChunkedStatusCode %} || status_ == 206{% endif %}{% if response.IsNullable == true %} || status_ == 204{% endif %})
jeremyVignelles commented 3 years ago

@lafritay : Could you add your workaround on this issue please? https://github.com/RicoSuter/NSwag/issues/2995 :)

Anderman commented 2 years ago

@RicoSuter

There is nullable information. But not on the returntype but on the methode return parameter. If you add this line

            var isNullable = new NullabilityInfoContext().Create(context.MethodInfo!.ReturnParameter).ReadState == NullabilityState.Nullable;

before line 60

You can replcae the processor with this

builder.Services.AddOpenApiDocument(document =>
{
    document.DocumentName = "openApi";
    document.OperationProcessors.Replace<OperationResponseProcessor>(new MyOperationResponseProcessor(new AspNetCoreOpenApiDocumentGeneratorSettings()));
});

and an the class

public class MyOperationResponseProcessor : OperationResponseProcessor, IOperationProcessor
{
    private readonly AspNetCoreOpenApiDocumentGeneratorSettings _settings;

    public MyOperationResponseProcessor(AspNetCoreOpenApiDocumentGeneratorSettings settings) : base(settings)
    {
        _settings = settings;
    }

    public new bool Process(OperationProcessorContext operationProcessorContext)
    {
        if (!(operationProcessorContext is AspNetCoreOperationProcessorContext context))
        {
            return false;
        }

        var responseTypeAttributes = context.MethodInfo?
            .GetCustomAttributes()
            .Where(a => a.GetType().IsAssignableToTypeName("ResponseTypeAttribute", TypeNameStyle.Name) ||
                        a.GetType().IsAssignableToTypeName("SwaggerResponseAttribute", TypeNameStyle.Name) ||
                        a.GetType().IsAssignableToTypeName("SwaggerDefaultResponseAttribute", TypeNameStyle.Name))
            .Concat(context.MethodInfo.DeclaringType!.GetTypeInfo().GetCustomAttributes()
                .Where(a => a.GetType().IsAssignableToTypeName("SwaggerResponseAttribute", TypeNameStyle.Name) ||
                            a.GetType().IsAssignableToTypeName("SwaggerDefaultResponseAttribute", TypeNameStyle.Name)))
            .ToArray() ?? Array.Empty<Attribute>();

        if (responseTypeAttributes.Length > 0)
        {
            // if SwaggerResponseAttribute \ ResponseTypeAttributes are present, we'll only use those.
            ProcessResponseTypeAttributes(context, responseTypeAttributes);
        }
        else
        {
            var isNullable = new NullabilityInfoContext().Create(context.MethodInfo!.ReturnParameter).ReadState == NullabilityState.Nullable;
            foreach (var apiResponse in context.ApiDescription.SupportedResponseTypes)
            {
                var returnType = apiResponse.Type;
                var response = new OpenApiResponse();
                string httpStatusCode;

                if (apiResponse.TryGetPropertyValue<bool>("IsDefaultResponse"))
                {
                    httpStatusCode = "default";
                }
                else if (apiResponse.StatusCode == 0 && IsVoidResponse(returnType))
                {
                    httpStatusCode = "200";
                }
                else
                {
                    httpStatusCode = apiResponse.StatusCode.ToString(CultureInfo.InvariantCulture);
                }

                if (IsVoidResponse(returnType) == false)
                {
                    var returnTypeAttributes = context.MethodInfo?.ReturnParameter?.GetCustomAttributes(false).OfType<Attribute>();
                    var contextualReturnType = returnType.ToContextualType(returnTypeAttributes);

                    response.IsNullableRaw = isNullable;
                    response.Schema = context.SchemaGenerator.GenerateWithReferenceAndNullability<JsonSchema>(
                        contextualReturnType, isNullable, context.SchemaResolver);
                }

                context.OperationDescription.Operation.Responses[httpStatusCode] = response;
            }
        }

        if (context.OperationDescription.Operation.Responses.Count == 0)
        {
            context.OperationDescription.Operation.Responses[GetVoidResponseStatusCode()] = new OpenApiResponse
            {
                IsNullableRaw = true,
                Schema = new JsonSchema
                {
                    Type = _settings.SchemaType == SchemaType.Swagger2 ? JsonObjectType.File : JsonObjectType.String,
                    Format = _settings.SchemaType == SchemaType.Swagger2 ? null : JsonFormatStrings.Binary,
                }
            };
        }

        UpdateResponseDescription(context);
        return true;
    }
    private bool IsVoidResponse(Type returnType)
    {
        return returnType == null || returnType.FullName == "System.Void";
    }
}