feat: Swagger improvements (Retargeted)

[nikcio]

Retarget of #15346

Prerequisites

• I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes

Description

This PR improves the DocInclusionPredicate for Swagger and adds needed fixes to support minimal APIs in the Swagger schemas.

Notice: Because this needs to test the minimal APIs it's targeted to v13/dev v14/dev

This PR includes the following

Updated the DocInclusionPredicate

The current solution to the DocInclusionPredicate for Swagger dictates that the endpoints are created with the conventional controller class setup and not the minimal APIs approach. Furthermore, it forces you to use the custom MapToApiAttribute to map an endpoint to anything other than the default schema.

TLDR the old logic looked something like this:

1. Is the group name null = never found in any schema
2. Is the endpoint a controller. If not = Never include in a schema
3. Does the apiDescription have an MapToApiAttribue if not = Default schema
4. Else map only to that schema

I've tried to update the logic so that it's a bit more intuitive:
New logic:

1. If the controller has a MapToApiAttribute this takes precedence and will determine if it's included
2. If there's not group name we include the API in the default schema
3. If the group name is the same as the document name we include the API
4. If the group name matches any other document name then we don't include the API
5. Else we include it in the default schema.

This way no APIs "go missing" from the Swagger interface*

• That would be true if the API Explorer supported conventional routing like the UmbracoApiController uses but it doesn't so these APIs cannot be part of the Swagger interface. See this Issue

This also ensures that the MapToApiAttribute is opt-in instead of the single source of truth. Therefore, you can set up your endpoints like in any other Asp Net Core application (By group names).

Added builder.Services.AddEndpointsApiExplorer();

To add minimal APIs to the API Explorer you need this call.

Refactored two of the methods in the src/Umbraco.Cms.Api.Common/Extensions/MethodInfoApiCommonExtensions.cs

Here you had two extension methods that did some casting and hidden logic that wasn't described by the extension name. To simplify this I've added GetMapToApiAttribute which is a helper for getting the MapToApiAttribute if there is one.

Unexpected errors using Minimal APIs

When I added the minimal APIs I noticed that a tag of null was added when no group name was added to the API. Therefore I've expanded the logic of TagActionsBy to check if the value is null.

In the ActionOrderBy method I noticed that the method threw an exception due to missing data so I've added checks that all values that should be added is only added if the value isn't null.

To test

To test this you can create normal endpoints with controllers like this:

using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Options;
using Microsoft.OpenApi.Models;
using Swashbuckle.AspNetCore.SwaggerGen;
using Umbraco.Cms.Api.Common.Attributes;
using Umbraco.Cms.Web.Common.Controllers;

namespace Umbraco.Cms.Web.UI;

[ApiController]
[Route("api/[controller]")]
public class TestController : ControllerBase
{
    [HttpGet]
    public string Get()
    {
        return "Hello World";
    }
}

/// <summary>
/// Not part of Swagger schema as the Api Expolorer doesn't support conventional routing that the <see cref="UmbracoApiController"/> is based on: https://stackoverflow.com/a/62810131/7153801
/// </summary>
public class TestUmbracoApiController : UmbracoApiController
{
    [HttpGet]
    public string Get()
    {
        return "Hello World";
    }
}

[ApiController]
[Route("api/[controller]")]
[ApiExplorerSettings(GroupName = "My Group")]
public class TestGroupController : ControllerBase
{
    [HttpGet]
    public string Get()
    {
        return "Hello World";
    }
}

/// <summary>
/// This can't be done because UmbrcoApiControllerBase used convential routing and not attribute routing.
/// </summary>
//[ApiExplorerSettings(GroupName = "My Group")]
//public class TestGroupUmbracoApiController : UmbracoApiController
//{
//    [HttpGet]
//    public string Get()
//    {
//        return "Hello World";
//    }
//}

[ApiController]
[Route("api/[controller]")]
[MapToApi("My Group")]
public class TestMapToController : ControllerBase
{
    [HttpGet]
    public string Get()
    {
        return "Hello World";
    }
}

/// <summary>
/// Not part of Swagger schema as the Api Expolorer doesn't support conventional routing that the <see cref="UmbracoApiController"/> is based on: https://stackoverflow.com/a/62810131/7153801
/// </summary>
[MapToApi("My Group")]
public class TestMapToUmbracoApiController : UmbracoApiController
{
    [HttpGet]
    public string Get()
    {
        return "Hello World";
    }
}

[ApiController]
[Route("api/[controller]")]
[ApiExplorerSettings(GroupName = "Not found My Group")]
public class TestNotFoundGroupController : ControllerBase
{
    [HttpGet]
    public string Get()
    {
        return "Hello World";
    }
}

public class MyCustomSchemaOptions : IConfigureOptions<SwaggerGenOptions>
{
    public void Configure(SwaggerGenOptions swaggerGenOptions)
    {
        swaggerGenOptions.SwaggerDoc(
            "My Group",
            new OpenApiInfo
            {
                Title = "My Group API",
                Version = "Latest",
                Description = "All endpoints not defined under specific APIs"
            });
    }
}

Add the Swagger options in the Program.cs like so:

builder.Services.ConfigureOptions<MyCustomSchemaOptions>();

Add minimal APIs in the Program.cs like so:

app.MapGet("api/MyMinimal/get", () => "Hello World!");

app.MapGet("api/grouped/MyMinimal/get", () => "Hello World!")
    .WithGroupName("My Group");

app.MapGet("api/notgrouped/MyMinimal/get", () => "Hello World!")
    .WithGroupName("Not My Group");

Final thoughts for HQ

When looking at this I came to wonder why the ActionOrderBy customization is even needed and why it includes the template for an endpoint apiDescription.ActionDescriptor.AttributeRouteInfo?.Template. It would be nice if HQ added a comment in the code about what problem it solves because it seems like a weird customization. (And also thought about whether the value should allow spaces as it does now from the group name for example).

Also, I have a hard time seeing the benefit of using the custom MapToApiAttribute instead of advocating for using the built-in [ApiExplorerSettings(GroupName = "My Group")] attribute or .WithGroupName("My Group") extension.

[github-actions]

Hi there @nikcio, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[georgebid]

Thanks for the PR @nikcio, someone on the core collabs team will review this soon! 😸

[nikcio]

Hey @georgebid and @JasonElkin I think this PR fell through the cracks. Do you want me to resync the PR or change the target again.

[georgebid]

Ah yes, sorry about that @nikcio, if you could re-target this at /contrib - that would be very handy! Thanks 😸

[nikcio]

@georgebid The PR is now updated again :)

[nikcio]

Hey @georgebid anything else you want be to do with this PR?

[kjac]

Hi @nikcio 👋

Good stuff! I'll give it a spin next week 👍

[kjac]

Hi @nikcio,

Thanks for all this 😄 you're spot on - Minimal APIs clearly don't play nice with OpenAPI in Umbraco right now.

Just for clarification: [MapToApi] is supposed to be an easy way to map API controllers to specific OpenAPI documents. This is not to be confused with group names, which is a logical sub-division of individual OpenAPI documents.

I think there might be a slight confusion here as to how this is all supposed to work. I've created an alternative PR in #17622 to support minimal APIs. Could you have a look at that?

[kjac]

Closed in favour of #17622