OpenApi ApiVersionAttribute.Deprecated not reflected in schema

JTeeuwissen commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

When adding an ApiVersionAttribute with Deprecated = true to a controller, the operation schema is not marked as "deprecated": true

Expected Behavior

When adding an ApiVersionAttribute with Deprecated = true to a controller, the operation schema should be marked with "deprecated": true

Steps To Reproduce

Services

services.AddRouting();
services.AddControllers();

services.AddApiVersioning();
services.AddVersionedApiExplorer(options =>
{
  options.GroupNameFormat = "'v'VVV";
  options.SubstituteApiVersionInUrl = true;
});
services.AddOpenApi("v1");

Controller

[ApiController]
[Route("api/v{version:apiVersion}/[controller]")]
[ApiVersion("1", Deprecated = true)]
public class ExampleController : ControllerBase
{
  [HttpGet("{id}")]
  public int Get(int id)
  {
    return id;
  }
}

Result

{
  "openapi": "3.0.1",
  "info": {
    "title": "ExampleProject | v1",
    "version": "1.0.0"
  },
  "paths": {
    "/api/v1/Example/{id}": {
      "get": {
        "tags": [
          "Example"
        ],
        "parameters": [
          {
            "name": "id",
            "in": "path",
            "required": true,
            "schema": {
              "type": "integer",
              "format": "int32"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "text/plain": {
                "schema": {
                  "type": "integer",
                  "format": "int32"
                }
              },
              "application/json": {
                "schema": {
                  "type": "integer",
                  "format": "int32"
                }
              },
              "text/json": {
                "schema": {
                  "type": "integer",
                  "format": "int32"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": { },
  "tags": [
    {
      "name": "Example"
    }
  ]
}

Exceptions (if any)

No response

.NET Version

9.0.100-rc.1.24452.12

Anything else?

An IOpenApiOperationTransformer like below could serve as a workaround, but I would prefer this functionality to be built in.

class ApiVersionDeprecatedTransformer : IOpenApiOperationTransformer
{
  public Task TransformAsync(
    OpenApiOperation operation,
    OpenApiOperationTransformerContext context,
    CancellationToken cancellationToken)
  {
    if (context.Description.ActionDescriptor is ControllerActionDescriptor controllerActionDescriptor &&
        context.Description.Properties.TryGetValue(typeof(ApiVersion), out var apiVersionObject) &&
        apiVersionObject is ApiVersion apiVersion)
    {
      var versionAttributes = controllerActionDescriptor.ControllerTypeInfo.GetCustomAttributes<ApiVersionAttribute>();
      if (versionAttributes.Any(attribute => attribute.Deprecated && attribute.Versions.Single() == apiVersion))
      {
        operation.Deprecated = true;
      }
    }

    return Task.CompletedTask;
  }
}

martincostello commented 1 month ago

This looks like a feature request for dotnet/aspnet-api-versioning - [ApiVersion] isn't part of ASP.NET Core itself.

JTeeuwissen commented 1 month ago

martincostello commented 1 month ago

WithOpenApi() on endpoints behaves differently to the new AddOpenApi() functionality in .NET 9. WithOpenApi() is for existing OpenAPI libraries, like Swashbuckle, to use, whereas the latter is the new functionality that generates the entire document.

Transformers etc. are only used with AddOpenApi(), so API Versioning would still need to be updated to be able to use them to enrich the OpenAPI document with extra metadata related to its versioning attributes.

captainsafia commented 1 month ago

@JTeeuwissen Yes, as @martincostello mentioned, the [ApiVersion] attribute comes from the Asp.Versioning library and we don't support it by-default in ASP.NET Core. As a result, the user has to use Swashbuckle filters/OpenAPI transformers in order to see the deprecation status in the document. The strategy for doing this with Swashbuckle is documented in this sample in the aspnet-api-versioning repo. The strategy for doing this with M.A.OpenApi 9 is currently only documented in the eShop repo here.

As to why this behavior isn't built in at the moment, there is actually an issue about adding support for an IsDeprecated property to ApiDescription over in https://github.com/dotnet/aspnetcore/issues/43493.

In the past, I've been meh about adding a specific property for this in the ApiDescription in favor of a more metadata-based approach. We could imagine a future where we expose IDeprecatedMetadata as the source of truth for this state and the [ApiVersion] attribute could implement it. Then, OpenAPI implementations like Swashbuckle and M.A.OpenApi can query endpoint metadata for implementations of IDeprecatedMetadata and use it to influence the deprecation status of the given OpenAPI operation.

There admittedly hasn't been much traction on that issue since the defacto approach has long been "use workarounds in the Open API implementation" to add this support. I'll leave this issue to track interest in supporting this feature. We'll definitely want to spend some time reasoning through whether a metadata-based approach or a new property on ApiDescription is the better strategy.

JTeeuwissen commented 1 month ago

Thank you for the elaborate response. I'm trying to migrate from nswag to m.a.openapi in dotnet 9, and nswag (afaik) supports api deprecation. I'll have a look at the eShop example, but I'm somewhat surprised this relatively simple example does not work out of the box.

captainsafia commented 1 month ago

I'll have a look at the eShop example, but I'm somewhat surprised this relatively simple example does not work out of the box.

I feel you. 😕 Unfortunately, this is one of those things that appears simple on the surface but has some implications to consider if we were to figure out the best way to address this issue in the framework itself. NSwag models its support for this using an IOperationProcessor implementation that makes heavy use of private reflection to access types defined in Asp.Versioning. That means that NSwag doesn't have to solve any problems related to exposing a more stable public API for the information that is being accessed via private reflection or working through the limitation we have of not being able to reference external dependencies in the shared framework (where ApiExplorer lives).

dotnet / aspnetcore