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

TypedResults metadata are not inferred for API Controllers #44988

Open
1 task done
brunolins16 opened this issue Nov 9, 2022 · 23 comments · May be fixed by #60096
Open
1 task done

TypedResults metadata are not inferred for API Controllers #44988

brunolins16 opened this issue Nov 9, 2022 · 23 comments · May be fixed by #60096
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. feature-openapi Needs: Design This issue requires design work before implementating.
Milestone

Comments

@brunolins16
Copy link
Member

brunolins16 commented Nov 9, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

As described in HttpResults type, IResult types are supported in API Controller, however, after #43543 the metadata from IEndpointMetadataProvider are not populated to API Controller Actions anymore.

It is caused because the ActionDescriptor is already created when the EndpointMetadataPopulator is called and not getting updated.

EndpointMetadataPopulator.PopulateMetadata(controllerActionDescriptor.MethodInfo, builder);

swagger.json

{
  "openapi": "3.0.1",
  "info": {
    "title": "WebApplication22",
    "version": "1.0"
  },
  "paths": {
    "/api/Test": {
      "get": {
        "tags": [
          "Test"
        ],
        "responses": {
          "200": {
            "description": "Success"
          }
        }
      }
    }
  },
  "components": { }
}

Expected Behavior

swagger.json

{
  "openapi": "3.0.1",
  "info": {
    "title": "WebApplication22",
    "version": "1.0"
  },
  "paths": {
    "/api/Test": {
      "get": {
        "tags": [
          "Test"
        ],
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/FooBar"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "FooBar": {
        "type": "object",
        "properties": {
          "requiredParam": {
            "type": "string",
            "nullable": true
          }
        },
        "additionalProperties": false
      }
    }
  }
}

Steps To Reproduce

using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddControllers();
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();

var app = builder.Build();

if (app.Environment.IsDevelopment())
{
    app.UseSwagger();
    app.UseSwaggerUI();
}


app.MapControllers();


app.Run();


[ApiController]
[Route("api/[controller]")]
public class TestController : ControllerBase
{
    [HttpGet]
    public Ok<FooBar> GEt()
     => TypedResults.Ok(new FooBar() { RequiredParam = ""});
}

public class FooBar
{
    public required string RequiredParam { get; set; }
}

Exceptions (if any)

No response

.NET Version

8.0.100-alpha.1.22472.9

Anything else?

No response

@brunolins16 brunolins16 added feature-openapi area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Nov 9, 2022
@brunolins16 brunolins16 self-assigned this Nov 9, 2022
@rafikiassumani-msft rafikiassumani-msft changed the title TypeResults metadata are not inferred for API Controllers TypedResults metadata are not inferred for API Controllers Nov 10, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the 7.0.x milestone Nov 10, 2022
@andresmoschini
Copy link

+1

@honzapatCZ
Copy link

bump

@brunolins16 brunolins16 modified the milestones: 7.0.x, .NET 8 Planning Jan 31, 2023
@brunolins16 brunolins16 added the bug This issue describes a behavior which is not expected - a bug. label Feb 7, 2023
@captainsafia captainsafia moved this to Committed in [.NET 8] Web Frameworks Feb 7, 2023
@captainsafia captainsafia added the Needs: Design This issue requires design work before implementating. label Feb 7, 2023
@honzapatCZ
Copy link

Has anyone found a workaround/polyfill? This is an unsolvable thing for me.

@brunolins16 brunolins16 removed their assignment Feb 24, 2023
@NMillard
Copy link

NMillard commented Apr 5, 2023

👀 Bump

@honzapatCZ
Copy link

I managed to get around this issue with a special openapi filter provider. But that's just a super big hack rather than a solution ....

@vernou
Copy link

vernou commented Apr 6, 2023

@honzapatCZ , well play. Can you share the filter?

@honzapatCZ
Copy link

    public class TypedResultsMetadataProvider : IOperationFilter
    {
        public void Apply(OpenApiOperation operation, OperationFilterContext context)
        {
            var responseType = context.MethodInfo.ReturnType;
            //Console.WriteLine(context.MethodInfo.DeclaringType.Name);
            //Console.WriteLine(context.MethodInfo.Name);
            //Console.WriteLine(responseType);
            var t = IsSubclassOfRawGeneric(typeof(Microsoft.AspNetCore.Http.HttpResults.Results<,>), responseType);
            if (t == null)
            {
                return;
            }

            var parArg = t.GetGenericArguments();
            if (operation.Responses.ContainsKey("200"))
                operation.Responses.Remove("200");

            foreach (var arg in parArg)
            {
                if (arg == typeof(NotFound))
                {
                    operation.Responses.Add("404", new OpenApiResponse { Description = "Not found" });
                }
                else if (arg == typeof(Ok))
                {
                    operation.Responses.Add("200", new OpenApiResponse { Description = "Success" });
                }
                else if (IsSubclassOfRawGeneric(typeof(Ok<>), arg) != null)
                {

                    var okArg = IsSubclassOfRawGeneric(typeof(Ok<>), arg).GetGenericArguments()[0];
                    Console.WriteLine("Adding: " + okArg);

                    //get or generate the schema
                    var schema = context.SchemaGenerator.GenerateSchema(okArg, context.SchemaRepository);
                    operation.Responses.Add("200", new OpenApiResponse { Description = "Success", Content = { { "application/json", new OpenApiMediaType { Schema = schema } } } });

                }
                else if (arg == typeof(CreatedAtRoute))
                {
                    operation.Responses.Add("201", new OpenApiResponse { Description = "Success" });
                }
                else if (IsSubclassOfRawGeneric(typeof(CreatedAtRoute<>), arg) != null)
                {
                    if (operation.Responses.ContainsKey("201"))
                        operation.Responses.Remove("201");

                    var okArg = IsSubclassOfRawGeneric(typeof(CreatedAtRoute<>), arg).GetGenericArguments()[0];
                    Console.WriteLine("Adding: " + okArg);

                    //get or generate the schema
                    var schema = context.SchemaGenerator.GenerateSchema(okArg, context.SchemaRepository);
                    operation.Responses.Add("201", new OpenApiResponse { Description = "Success", Content = { { "application/json", new OpenApiMediaType { Schema = schema } } } });
                }
                else if (arg == typeof(BadRequest))
                {
                    operation.Responses.Add("400", new OpenApiResponse { Description = "There was an error" });
                }
                else if (IsSubclassOfRawGeneric(typeof(BadRequest<>), arg) != null)
                {
                    if (operation.Responses.ContainsKey("400"))
                        operation.Responses.Remove("400");

                    var okArg = IsSubclassOfRawGeneric(typeof(BadRequest<>), arg).GetGenericArguments()[0];
                    Console.WriteLine("Adding: " + okArg);

                    //get or generate the schema
                    var schema = context.SchemaGenerator.GenerateSchema(okArg, context.SchemaRepository);
                    operation.Responses.Add("400", new OpenApiResponse { Description = "There was an error", Content = { { "application/json", new OpenApiMediaType { Schema = schema } } } });
                }
                else
                {
                    Console.WriteLine("Unknown type: " + arg);
                }
            }
        }

        static Type? IsSubclassOfRawGeneric(Type generic, Type toCheck)
        {
            while (toCheck != null && toCheck != typeof(object))
            {
                //if Task is used, we need to check the underlying type
                var realTypeNoTask = toCheck.IsGenericType && toCheck.GetGenericTypeDefinition() == typeof(Task<>) ? toCheck.GetGenericArguments()[0] : toCheck;
                var cur = realTypeNoTask.IsGenericType ? realTypeNoTask.GetGenericTypeDefinition() : realTypeNoTask;
                //Console.WriteLine(cur);
                if (generic == cur)
                {
                    return realTypeNoTask;
                }
                toCheck = toCheck.BaseType;
            }
            return null;
        }
    }

@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 20, 2023
@opflucker
Copy link

Any news?

@vernou
Copy link

vernou commented Oct 4, 2023

While waiting for a solution, I do a operation filter to generate OpenApi response from HttpResults type and I wrapped it in a NuGet package :
Vernou.Swashbuckle.HttpResultsAdapter

You can add the package :

dotnet add package Vernou.Swashbuckle.HttpResultsAdapter

So you can register the filter :

var builder = WebApplication.CreateBuilder(args);
...
builder.Services.AddSwaggerGen(options =>
{
    ...
    options.OperationFilter<Vernou.Swashbuckle.HttpResultsAdapter.HttpResultsOperationFilter>();
});

If you prefer, you can just copy/paste the filter file from repository :
HttpResultsOperationFilter.cs

@RobinMeow
Copy link

+1

1 similar comment
@flashgordon2016
Copy link

+1

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@miguelEsteban
Copy link

+1

@Dejavu333
Copy link

still waiting for a solution...

@opflucker
Copy link

+1

@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
@grafsnikers
Copy link

+1

@Krusen
Copy link
Contributor

Krusen commented Mar 3, 2024

Here's my solution, using NSwag, in case someone else wants to use it.
It should support most cases, if not all.

BaseOperationProcessor.cs

using System.Net;
using System.Reflection;
using Microsoft.AspNetCore.Http.Metadata;
using Namotion.Reflection;
using NJsonSchema;
using NSwag;
using NSwag.Generation.Processors;
using NSwag.Generation.Processors.Contexts;

namespace YourProject.NSwag.OperationProcessors;

public abstract class BaseOperationProcessor : IOperationProcessor
{
    protected static readonly MethodInfo PopulateMetadataInterfaceMethod =
        typeof(IEndpointMetadataProvider).GetMethod(nameof(IEndpointMetadataProvider.PopulateMetadata))!;

    public bool Process(OperationProcessorContext context)
    {
        var providerTypes = GetEndpointMetadataProviderTypes(context.MethodInfo);

        foreach (var type in providerTypes)
        {
            var populateMetadataMethod = GetInterfaceMethodImplementation(type, PopulateMetadataInterfaceMethod);
            var endpointBuilder = new FakeEndpointBuilder();
            populateMetadataMethod.Invoke(null, new object[] { context.MethodInfo, endpointBuilder });

            var responseTypeMetadata = endpointBuilder.Metadata.OfType<IProducesResponseTypeMetadata>();

            ProcessMetadata(context, responseTypeMetadata);
        }

        return true;
    }

    /// <summary>
    /// Gets the matching implemented method on <paramref name="implementationType"/>
    /// matching the given <paramref name="interfaceMethod"/> from an implemented interface.
    /// See <a href="https://stackoverflow.com/a/52743438/5358985">this answer</a> on Stack Overflow for more details.
    /// </summary>
    /// <param name="implementationType">The type implementing the <paramref name="interfaceMethod"/>.</param>
    /// <param name="interfaceMethod"></param>
    protected static MethodInfo GetInterfaceMethodImplementation(Type implementationType, MethodInfo interfaceMethod)
    {
        var map = implementationType.GetInterfaceMap(interfaceMethod.DeclaringType!);
        for (var i = 0; i < map.InterfaceMethods.Length; i++)
        {
            if (map.InterfaceMethods[i].Equals(interfaceMethod))
            {
                return map.TargetMethods[i];
            }
        }

        throw new InvalidOperationException(
            $"Interface method ({interfaceMethod.Name}) not found on implementation type ({implementationType.FullName})");
    }

    protected void ProcessMetadata(OperationProcessorContext context, IEnumerable<IProducesResponseTypeMetadata> responseTypeMetadata)
    {
        var operation = context.OperationDescription.Operation;

        foreach (var metadata in responseTypeMetadata)
        {
            var response = new OpenApiResponse
            {
                Description = ((HttpStatusCode)metadata.StatusCode).ToString(),
            };

            if (metadata.Type is not null)
            {
                var schema = context.SchemaGenerator.GenerateWithReference<JsonSchema>(metadata.Type.ToContextualType(), context.SchemaResolver);
                foreach (var contentType in metadata.ContentTypes)
                {
                    response.Content[contentType] = new OpenApiMediaType
                    {
                        Schema = schema,
                    };
                }
            }

            operation.Responses[metadata.StatusCode.ToString()] = response;
            operation.Produces.AddRange(metadata.ContentTypes);
        }

        operation.Produces = operation.Produces.Distinct().ToList();
    }

    /// <summary>
    /// A simple implementation of <see cref="EndpointBuilder"/>
    /// so we can capture the populated metadata.
    /// </summary>
    private class FakeEndpointBuilder : EndpointBuilder
    {
        public override Endpoint Build()
        {
            throw new NotImplementedException();
        }
    }

    protected abstract IEnumerable<Type> GetEndpointMetadataProviderTypes(MethodInfo method);
}

TypedResultsOperationProcessor.cs

using System.Reflection;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Http.Metadata;
using IEndpointMetadataProvider = Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider;

namespace YourProject.NSwag.OperationProcessors;

/// <summary>
/// Operation processor that reads <see cref="IProducesResponseTypeMetadata"/>
/// from the return types implementing <see cref="IResult"/>, i.e. when using <see cref="TypedResults"/>
/// like <see cref="NotFound"/>, <see cref="Ok"/>, <see cref="Ok{TValue}"/> etc.
/// </summary>
/// <remarks>
/// See <a href="https://learn.microsoft.com/en-us/aspnet/core/web-api/action-return-types?view=aspnetcore-7.0#httpresults-type">here</a> for details about HttpResults types.
/// </remarks>
public class TypedResultsOperationProcessor : BaseOperationProcessor
{
    /// <summary>
    /// Finds the actual return type implementing <see cref="IEndpointMetadataProvider"/>
    /// (i.e. in case of <see cref="Task"/>) or null if the return type does not implement <see cref="IEndpointMetadataProvider"/>.
    /// </summary>
    protected override IEnumerable<Type> GetEndpointMetadataProviderTypes(MethodInfo method)
    {
        if (method.ReturnType.IsAssignableTo(typeof(IEndpointMetadataProvider)))
            return new List<Type>(1) { method.ReturnType };

        var returnType = method.ReturnType;

        if (!returnType.IsGenericType)
            return new List<Type>();

        if (!returnType.IsAssignableTo(typeof(Task)))
            return new List<Type>();

        var type = returnType.GenericTypeArguments.First();
        if (type.IsAssignableTo(typeof(IEndpointMetadataProvider)))
            return new List<Type>(1) { type };

        return new List<Type>();
    }
}

And here's how to use it:

services.AddOpenApiDocument(x =>
{
    x.OperationProcessors.Add(new TypedResultsOperationProcessor());
});

@opflucker
Copy link

still waiting for a solution...

@jgarciadelanoceda
Copy link
Contributor

@captainsafia, are you planning to handle this issue?
It's an issue in Swashbuckle and also in Microsoft.AspNetcore.OpenApi

@captainsafia
Copy link
Member

@jgarciadelanoceda I'm not planning on handling this issue at the moment. There's some subtleties to untangle with this one, especially with regard to how we handle metadata in the ActionEndpointFactory component mentioned above.

If you'd like to take a look at it, I'd be happy to to review a PR. As a heads up, from what I recall the last time I looked at it it requires a bit more legwork to fix in a backwards-compatible way. You might have more luck figuring out a nice solution here...

@jgarciadelanoceda
Copy link
Contributor

jgarciadelanoceda commented Aug 21, 2024

I have just did a change that seems to be working, but the problem is that I think that could be a Breaking change:

if (action.FilterDescriptors != null && action.FilterDescriptors.Count > 0)
{
foreach (var filter in action.FilterDescriptors.OrderBy(f => f, FilterDescriptorOrderComparer.Comparer).Select(f => f.Filter))
{
builder.Metadata.Add(filter);
}
}

Replace it with this code:

if (action.FilterDescriptors != null && action.FilterDescriptors.Count > 0)
{
    foreach (var filter in action.FilterDescriptors.OrderBy(f => f, FilterDescriptorOrderComparer.Comparer).Select(f => f.Filter))
    {
        builder.Metadata.Add(filter);
    }
}
action.FilterDescriptors ??= [];
foreach (var metadata in builder.Metadata.OfType<IProducesResponseTypeMetadata>())
{
    if (metadata?.Type is not null)
    {
        action.FilterDescriptors.Add(new(new ProducesResponseTypeAttribute(metadata.Type, metadata.StatusCode), 0));
    }
}

Can you guide my in the process of the elements involved and so on to continue thinking about a right solution?. I think that this solution is only valid if there is no ProducesResponseTypeAttribute on the endpoint

@captainsafia
Copy link
Member

@jgarciadelanoceda Yeah, as you mentioned the proposed change will only impact IProducesResponseTypeMetadata and doesn't handle any metadata produced by an IEndpointMetadataProvider implementation.

We have a set of test cases that check for EndpointMetadataProvider behavior here although I suspect they don't capture the real E2E given they pass.

@jgarciadelanoceda
Copy link
Contributor

jgarciadelanoceda commented Aug 21, 2024

I just tested with Results<Ok,NotFound>, and what is getting indeed is the IEndpointMetadataProvider implementation here https://github.com/dotnet/aspnetcore/blob/main/src%2FHttp%2FHttp.Results%2Fsrc%2FResultsOfT.Generated.cs#L70-L71
I find the solution quite clean, because in the builder.Metadata aspNetcore is storing the responseTypes for Results types :)

@DavidBal
Copy link

DavidBal commented Oct 2, 2024

Describe the bug

I want to generate a OpenApi json for my Controller based Api.

I have the following controller Code:

[Route("api/[controller]")]
[ApiController]
public class TestController : ControllerBase
{
    [HttpPost("test")]
    public async Task<IActionResult> PostTest([FromBody] XmlPlaceholder? xmlPlaceholder = null)
    {
        return Ok();
    }
}

The XmlPlaceholder class looks like this:

public class XmlPlaceholder : IEndpointParameterMetadataProvider
{
    public static void PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder)
    {
        builder.Metadata.Add(new AcceptsMetadata(["application/xml", "text/xml"], typeof(XmlPlaceholder)));
    }
}

The generated OpenAPI json looks like this:

 "/api/Test/test": {
      "post": {
        "tags": [
          "Test"
        ],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/XmlPlaceholder"
              }
            },
            "text/json": {
              "schema": {
                "$ref": "#/components/schemas/XmlPlaceholder"
              }
            },
            "application/*+json": {
              "schema": {
                "$ref": "#/components/schemas/XmlPlaceholder"
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "OK"
          }
        }
      }
    }

Expected Behavior

I would expect the same behavior as for minimal apis.

There the metadata is used and produces the following output:

    "/minimalTest": {
      "post": {
        "tags": [
          "WebApplication2"
        ],
        "operationId": "Minimal Test",
        "requestBody": {
          "content": {
            "application/xml": {
              "schema": {
                "$ref": "#/components/schemas/XmlPlaceholder"
              }
            },
            "text/xml": {
              "schema": {
                "$ref": "#/components/schemas/XmlPlaceholder"
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "OK"
          }
        }
      }
    },

Steps To Reproduce

https://github.com/DavidBal/-Demo_IEndpointParameterMetadataProvider_for_Controller/tree/master

Exceptions (if any)

No response

.NET Version

9.0.100-rc.1.24452.12

Anything else?

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>net9.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="9.0.0-rc.1.24452.1" />
  </ItemGroup>

</Project>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. feature-openapi Needs: Design This issue requires design work before implementating.
Projects
No open projects
Status: No status
Development

Successfully merging a pull request may close this issue.