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

Unintuitive/incorrect behaviour from use of WithOpenApi() #41623

Closed
1 task done
martincostello opened this issue May 11, 2022 · 5 comments
Closed
1 task done

Unintuitive/incorrect behaviour from use of WithOpenApi() #41623

martincostello opened this issue May 11, 2022 · 5 comments
Labels

Comments

@martincostello
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Using the new WithOpenApi() method with Minimal APIs in .NET 7 preview 4 in conjunction with Swashbuckle.AspNetCore produces unintuitive results in the generated swagger.json file.

If either WithOpenApi() overload is used, the metadata that would be present without it is lost, some information is incorrectly added, and information that doesn't work at all without it (see #40753) does render.

Below is the swagger.json generated by Swashbuckle.AspNetCore using the repro application included later in this issue.

For resource /a:

  1. The response is correctly described as HTTP 200 returning MyModel and has a description
  2. There is no request body
  3. The summary and description are missing

For resources /b and /c:

  1. The response is described as an HTTP 200, but is just described as an object with no description
  2. The request has gained an empty request body
  3. The summary and description are present

Neither resource is 100% correct in its description.

swagger.json
{
  "openapi": "3.0.1",
  "info": {
    "title": "API",
    "version": "v1"
  },
  "paths": {
    "/a": {
      "get": {
        "tags": [
          "NoProblemDetailsOpenApi"
        ],
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/MyModel"
                }
              }
            }
          }
        }
      }
    },
    "/b": {
      "get": {
        "tags": [
          "NoProblemDetailsOpenApi"
        ],
        "summary": "Say hi",
        "description": "Says hi to you",
        "requestBody": {
          "content": { }
        },
        "responses": {
          "200": {
            "description": null,
            "content": {
              "application/json": {
                "schema": {
                  "type": "object"
                }
              }
            }
          }
        }
      }
    },
    "/c": {
      "get": {
        "tags": [
          "NoProblemDetailsOpenApi"
        ],
        "summary": "Say bonjour",
        "description": "Says bonjour to you",
        "requestBody": {
          "content": { }
        },
        "responses": {
          "200": {
            "description": null,
            "content": {
              "application/json": {
                "schema": {
                  "type": "object"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "MyModel": {
        "type": "object",
        "properties": {
          "text": {
            "type": "string",
            "nullable": true
          }
        },
        "additionalProperties": false
      }
    }
  }
}

Expected Behavior

WithOpenApi() provides the ability for the user to extend the operations generated by default (or allows them to be completely replaced) without losing any of those conventions.

I would expect that the object passed to WithOpenApi() would contain the same detail as if it had not been used on the endpoint at all.

In the example given, this would mean that:

  1. The response is described as MyModel with a description
  2. There is no request body
  3. The summary and description for the operation are present

Steps To Reproduce

Program.cs

using Microsoft.AspNetCore.OpenApi;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen(options =>
{
    options.SwaggerDoc("v1", new() { Title = "API", Version = "v1" });
});

var app = builder.Build();

// Endpoint has response, but no summary or description
app.MapGet("/a", () => TypedResults.Ok(new MyModel("Hello world!")))
   .WithSummary("Say hello")
   .WithDescription("Says hello to you");

// Endpoint has summary and description, but a non-specific response and gains a body
app.MapGet("/b", () => TypedResults.Ok(new MyModel("Hi world!")))
   .WithSummary("Say hi")
   .WithDescription("Says hi to you")
   .WithOpenApi();

// Endpoint has summary and description, but a non-specific response and gains a body
app.MapGet("/c", () => TypedResults.Ok(new MyModel("Bonjour world!")))
   .WithOpenApi(operation =>
   {
       operation.Summary = "Say bonjour";
       operation.Description = "Says bonjour to you";
       return operation;
   });

app.UseSwagger();

app.Run();

public record MyModel(string Text);

Project.csproj

<Project Sdk="Microsoft.NET.Sdk.Web">
  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="7.0.0-preview.4.22251.1" /> 
    <PackageReference Include="Swashbuckle.AspNetCore" Version="6.3.1" />
  </ItemGroup>
</Project>

Exceptions (if any)

No response

.NET Version

7.0.100-preview.4.22252.9

Anything else?

No response

@martincostello
Copy link
Member Author

Also in the case where the user wants to adjust the operation (rather than replace it), maybe an overload that takes an Action would be useful?

public static partial class OpenApiRouteHandlerBuilderExtensions
{
+    public static RouteHandlerBuilder WithOpenApi(this RouteHandlerBuilder builder, Action<OpenApiOperation> configureOperation);
}

Doing something like the below to tweak a single field is a bit cumbersome, in my opinion:

.WithOpenApi(operation =>
{
    operation.Summary = "whatever";
    return operation;
});

Where it could alternatively be:

.WithOpenApi(operation => operation.Summary = "whatever");

@captainsafia
Copy link
Member

WithOpenApi() provides the ability for the user to extend the operations generated by default (or allows them to be completely replaced) without losing any of those conventions.

Yep, this is the goal. However, there are some challenges here particularly when it comes to schema generation for complex models like MyModel above. We're doing some thinking about integrating JSON schema generation support into the OpenApi library itself via microsoft/OpenAPI.NET#836. There's an analogous issue in the repo to address this in #41246.

For resource a: The summary and description are missing

As you mentioned, this is an artifact of this bug #40753 and not the new WithOpenApi extension method.

For resources /b and /c:

The response is described as an HTTP 200, but is just described as an object with no description
The request has gained an empty request body

Yep, this is an artifact of the schema generation issue above.

So, yes, this issue is particularly annoying and getting the schema generation working right and at the right point in the stack will be challenging. For the moment, users have the workaround of setting the object themselves via the WithOpenApi method.

I've also been noodling about making the work I did in domaindrivendev/Swashbuckle.AspNetCore#2404 to support "merging" with the schema produced in Swashbuckle instead of overriding it which should alleviate the issue.

Also in the case where the user wants to adjust the operation (rather than replace it), maybe an overload that takes an Action would be useful?

I actually started out with this API in the first draft of the PR but we moved to the override model to:

So in the future, you'd be able to do

.WithOpenApi(operation => new(operation) {
    Summary = "whatever";
});

Which lends itself very nicely to the object initialization pattern used extensively in the M.OpenApi library.

@martincostello
Copy link
Member Author

Thanks for the links to the other repos' issues Safia, I'll keep an eye on those 👀

Your comment here microsoft/OpenAPI.NET#836 (comment) also touches on something else I'd noticed while playing with this that I didn't mention, which was getting access to things from the IServiceProvider (the JsonSerializerOptions) to control the serialization of things. My use case for this was being able to generate example payloads from models with the right casing etc.

Otherwise that all makes sense, it was just a bit of a challenge to try out at this current stage in its development - I appreciate it's a complex area 😄

@BrennanConroy
Copy link
Member

Triage: Marking as a dupe of #41246
Feel free to continue the conversation there 😃

@BrennanConroy BrennanConroy added the ✔️ Resolution: Duplicate Resolved as a duplicate of another issue label May 12, 2022
@ghost ghost added the Status: Resolved label May 12, 2022
@ghost
Copy link

ghost commented May 13, 2022

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed May 13, 2022
@dotnet dotnet locked as resolved and limited conversation to collaborators Jun 12, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants