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

Support Deprecation Metadata in the API Explorer #43493

Open
commonsensesoftware opened this issue Aug 23, 2022 · 6 comments
Open

Support Deprecation Metadata in the API Explorer #43493

commonsensesoftware opened this issue Aug 23, 2022 · 6 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Milestone

Comments

@commonsensesoftware
Copy link

Background and Motivation

The API Explorer has been the pinnacle of sharing API metadata across different ASP.NET Core libraries. The concept of a deprecated API exists in a number of different libraries such as ASP.NET API Versioning, Swashbuckle, NSwag, and OpenAPI .NET to name a few. Each library has its own method of describing a deprecated API, without any shared metadata, developers are required to create their own bridges to connect the two.

While not strictly required, it is reasonable to have the existing API Explorer metadata discovery indicate an API is deprecated when the ObsoleteAttribute is applied.

Proposed API

namespace Microsoft.AspNetCore.Mvc.ApiExplorer;

public class ApiDescription
{
+    /// <summary>
+    /// Gets or sets a value indicating whether the API is deprecated.
+    /// </summary>
+    /// <value>True if the API deprecated; otherwise, false. The default value is <c>false</c>.</value>
+    public bool IsDeprecated { get; set; }
}

Usage Examples

Default Behavior

The presence of ObsoleteAttribute on a valid action would set ApiDescription.IsDeprecated to true.

Default Action (Not Deprecated)

[HttpGet]
public IActionResult Get() => Ok();

Deprecated Action

[Obsolete, HttpGet]
public IActionResult Get() => Ok();

Non-Action (Ignored)

[Obsolete, NonAction, HttpGet]
public override IActionResult Get() => NotImplemented();

Deprecated Controller

[Obsolete]
[Route("[controller]")]
public class ValuesController : ControllerBase
{
    [HttpGet]
    public IActionResult Get() => Ok();
}

Non-Action on Deprecated Controller (Ignored)

[Obsolete]
[Route("[controller]")]
public class Values2Controller : ValuesController
{
    [NonAction]
    [HttpGet]
    public override IActionResult Get() => NotImplemented();
}

API Versioning Behavior

The API Versioning extensions for the API Explorer will set ApiDescription.IsDeprecated to true when
a known, deprecated API version is encountered.

var builder = WebApplication.CreateBuilder( args );
var app = builder.Build();
var orders = app.NewApiVersionSet( "Orders" ).Build();

app.MapGet( "/orders/{id:int}", ( int id ) => new Order() { Id = id, Customer = "John Doe" } )
   .Produces<Order>()
   .Produces( 404 )
   .WithApiVersionSet( orders )
   .HasDeprecatedApiVersion( 0.9 )
   .HasApiVersion( 1.0 );

Alternative Designs

Minimal APIs have exposed an extension method to set the OpenAPI metadata, but this approach is no different than using an OpenAPI extension in Swashbuckle, NSwag, and so on or simply using OpenAPI .NET directly.

Risks

There are no tangible risks. The default behavior will continue to indicate that an API is not deprecated by default. Library will authors will be required to update this information when appropriate and consumers will be obliged to honor the value when set.

Related Links

@commonsensesoftware commonsensesoftware added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 23, 2022
@captainsafia
Copy link
Member

Each library has its own method of describing a deprecated API, without any shared metadata, developers are required to create their own bridges to connect the two.

Is there a reason that the OpenApiOperation metadata with the Deprecated property is not a sufficient "shared" metadata to use here consider that NSwag/Swashbuckle/Microsoft.AspNetCore.OpenApi leverage it for minimal APIs?

For MVC controllers, is it possible to check for the Obsolete attribute on the action descriptor directly?

apiDescription.CustomAttributes().OfType<ObsoleteAttribute>().Any();

@commonsensesoftware
Copy link
Author

Yes - there is a reason. Not every library needs or uses OpenAPI. API Versioning does not directly reference or otherwise specifically care about OpenAPI. OpenAPI document generators like Swashbuckle and NSwag use the metadata provided by the API Explorer. API Versioning exposes this metadata to them (and anyone else) without any direct knowledge or dependency on OpenAPI.

Is an attribute is possible - yes; however, you're missing the point. Using the ObsoleteAttribute is one method of how IsDeprecated could be set. API Versioning doesn't use or honor the ObsoleteAttribute because it is typically a code-only concept. Instead, API Versioning exposes an explicit, intention-revealing method of providing this metadata; for example:

[ApiVersion(0.9, Deprecated = true)]
[Route("[controller]")]
public class LegacyController : ControllerBase { }

This is just one example of how it can be done. API Versioning also doesn't care about specific attributes. Using attributes are just one way that the versioning metadata can be expressed. The problem with using an attribute - any attribute, such as ObsoleteAttribute, is that this has to be agreed upon across libraries. The entire point of the API Explorer is to provide what common set of metadata is available and separate that from how it is realized (e.g. populated).

There are potential use cases outside of OpenAPI. Looking back at how the API Explorer was brought into the ASP.NET Core world, there are definitely some things that probably should have been done differently. This is how I managed to get IsRequired and DefaultValue support added to explored parameters a few years back. API Explorer shouldn't have to rely on MVC to function, but that is now a 🐀 🧶 to unwind. The purpose of API Explorer was not just to service OpenAPI, that is just one way it can be used. Developers and library authors have been getting along without this very simple piece of metadata, but it is incredibly low-hanging fruit that would make everyone's life on the consuming side just a little bit better. 😉

@commonsensesoftware
Copy link
Author

Honestly, I thought this would be easy win.

Since we're on the topic of deprecation, there are other, related pieces of metadata that are not exposed either. Deprecation typically implies that an API will sunset (e.g. go away) at some point, but not necessarily. IsDeprecated doesn't indicate if the API will sunset or when. At best, it simply says "You shouldn't use this anymore."

API Versioning now supports Sunset Policies as defined by RFC 8594. This provides information as to when or if an API will be sunset as well as any related links; for example, an API might have a web page with a publicly stated policy.

These not concepts are not mutually exclusive. A deprecated API should have a stated sunset policy, but an API can also have a sunset policy without being deprecated. While it might be possible via an extension, OpenAPI nor any document generator that I know of can provide this type of information today. This type of information, however, is useful for API clients when the onboard.

This information can also change on a dime. It's possible to onboard to a supported API and a week later it's deprecated. API Versioning now provides DI extensions for HttpClient that can detect when it's using a deprecated API and/or it is approaching sunset and forward that to the configured instrumentation. This assumes the API reports that information, presumably a la API Versioning on the server side. This informs API clients that they need to update as well as how much time they have left to do so.

I figured adding numerous metadata bits to the API Explorer would have resistance so I initially didn't think it worth mentioning. Since this appears to be turning into a longer debate and justification, I'll just throw all my cards out on the table. A sunset policy would consist of nothing more than an ISO 8601 date (e.g. DateTimeOffset) and an optional collection of links. Links are supposed to conform to RFC 8288, which is what API Versioning does, but something simpler could work for API Explorer. I think would still be more than URLs, since are other useful pieces of metadata such as relationship type, language, title, [media] type, and title. You can peek at what SunsetPolicy and Link might look like. I'm open to as much or as little is worth being added. I can see these additions driving formal support in OpenAPI at some point in the future.

@captainsafia
Copy link
Member

I figured adding numerous metadata bits to the API Explorer would have resistance so I initially didn't think it worth mentioning.

Yep, the metadata pattern is what motivated us to pursue the more OpenAPI-based approach in the first place. As we examined some of the feedback on features that were missing in ApiExplorer, we realized a lot of them were addressed by more complete OpenApi support. Essentially, switching from a model where an adhoc-ish collection of metadata that we maintain is the contract, to more formally using OpenAPI as the contract.

I recognize that this means that these tools now have to take a dependency on OpenAPI, but given that OpenAPI is meant to provide a standard for annotating these RESTful APIs, it makes sense to lean on that instead of continuing to re-invent in our own metadata.

API Versioning now supports Sunset Policies as defined by RFC 8594. This provides information as to when or if an API will be sunset as well as any related links; for example, an API might have a web page with a publicly stated policy.

Interesting. Is this information that could be communicated in the OpenApi.Extensions property for an endpoint's annotation? How does API versioning currently derive the information needed to construct the sunset policy? I assume users have to input it manually using API versioning's APIs....

@commonsensesoftware
Copy link
Author

I understand the motivation, but I'll have to agree to disagree. The foundation of all OpenAPI support in ASP.NET Core, even with Minimal APIs, still relies on API Explorer and I don't see that changing anytime soon. For things that are truly OpenAPI-specific, I think the extension paths that have been added make complete sense. There are several other areas, however, that will feel are disjointed because it requires a combination of some API Explorer and some OpenAPI extensions.

The real issue I see with this approach is that is forcing everyone using OpenAPI in the .NET ecosystem to use Microsoft.OpenAPI which feels highly opinionated. As far as I tell, NSwag is does not directly use this library, but is still wildly popular. If there is some larger, longer roadmap which sees API Explorer and OpenAPI reimagined as near one in the same, so be it, but I not aware of that being communicated. API Versioning has stayed neutral on Swagger/OpenAPI for that very reason. There is no preference on library, implementation, or whether you even use OpenAPI. There have been plenty of scenarios, such as automated tests, that run suites by version without any concern for documentation. If there is some longer-term strategy at play, it'd be nice to advertise that roadmap with the most popular platform extenders.

Side bar: OpenAPI is an Interface Definition Language (IDL) that addresses the impedance mismatch between HTTP (the API) and client/server implementation code. "I am getting frustrated by the number of people calling any HTTP-based interface a REST API" - @fieldinghimself 😉

Yes - it could most definitely be added as an general extension. That's certainly where it would start, but given that it's based on a now ratified RFC, I think there is a possibility for it to evolve into a standard part of OpenAPI at some point. If using Microsoft.OpenAPI is now what's required to play in the ecosystem, I'll consider how things might be able to hook up (in yet one more library). The Swagger/OpenAPI landscape has changed a lot over the years and, as a one-person team, it's hard for me to keep up with moving targets.

To your point, API Versioning does not derive policy; it can't. Only an API author knows what the policy is. This has long been a gap for APIs. Deprecation is one thing, but how is policy conveyed? When will a deprecated API sunset? Maybe a supported API has a known sunset policy in the future. Maybe an API has none of the above and just wants to advertise to clients here's where you find out what an API's stated policy is. Even knowing where the OpenAPI document is located is not standard. Being able to send OPTIONS /api and retrieve links to discover versions, policy, and OpenAPI information is useful; especially for tooling.

API Versioning allows the sunset policy to be defined by API name, API version, or a combination of both. It is emitted any time an API author opts into reporting API version information to clients. This could be for an entire application or just for one or more OPTIONS endpoints. The configuration setup looks like this:

options.Policies.Sunset( 0.9 )
                .Effective( DateTimeOffset.Now.AddDays( 60 ) )
                .Link( "policy.html" )
                    .Title( "Versioning Policy" )
                    .Type( "text/html" );

I'm still working on putting together comprehensive documentation, but you can get a sense of how it can be consumed by looking at these tests. It's important to know that the capabilities transcend OpenAPI. Documentation is great, but there is no substitution for real-time data. APIs need a way to advertise to their clients when new versions are available or when APIs will sunset without having to rely solely human interaction. If a client onboards to a supported API via an OpenAPI document which later becomes deprecated and will sunset, how would the client know? If API clients configure alerts from their telemetry, they can be notified when these events occur and take appropriate action, if any.

@captainsafia captainsafia added this to the .NET 8 Planning milestone Aug 30, 2022
@commonsensesoftware
Copy link
Author

@captainsafia,

An interesting issue came up in dotnet/aspnet-api-versioning#920 that is relevant to this issue and I would be relevant to share.

Since deprecation isn't directly supported, there needs to be a bridge in Swashbuckle (or somewhere):

public class SwaggerDefaultValues : IOperationFilter
{
    public void Apply( OpenApiOperation operation, OperationFilterContext context ) =>
        operation.Deprecated |= context.ApiDescription.IsDeprecated(); // ← API Versioning extension method
}

Now that we have grouping, consider:

var builder = WebApplication.CreateBuilder( args );
var services = builder.Services;

services.AddProblemDetails();
services.AddEndpointsApiExplorer();
services.AddApiVersioning()
        .AddApiExplorer(options => options.GroupNameFormat = "'v'VVV");

services.AddSwaggerGen( options => options.OperationFilter<SwaggerDefaultValues>() );

var app = builder.Build();
var orders = app.MapApiGroup( "Orders" );

var v1 = orders.MapGroup( "/orders" )
               .HasDeprecatedApiVersion( 0.9 )
               .HasApiVersion( 1.0 );

v1.MapGet( "/{id:int}", ( int id ) => new Order() { Id = id, Customer = "John Doe" } )
  .Produces<Order>()
  .Produces( 404 );

No surprise, if you run this in the Swagger UI, things work as expected, which is to say:

  1. /orders/{id:int} is listed in 0.9 and is shown as deprecated
  2. /orders/{id:int} is listed in 1.0

If we then, bring in Microsoft.AspNetCore.OpenApi and add WithOpenApi (in any form) like this:

v1.MapGet( "/{id:int}", ( int id ) => new Order() { Id = id, Customer = "John Doe" } )
  .WithOpenApi()
  .Produces<Order>()
  .Produces( 404 );

things fall down; specifically:

/orders/{id:int} is listed in 0.9 and is shown as deprecated
/orders/{id:int} is listed in 1.0 and is shown as deprecated

This happens because WithOpenApi presumes that there is a 1:1 mapping between Endpoint and OpenApiOperation, which isn't always true. In this scenario, the same OpenApiOperation instance passed through the filter. Typically, you'd honor any explicit setting on OpenApiOperation.Deprecated by using |=. Here, that works on the first pass, but fails on the second. It's easily remedied by changing things to a direct assignment (e.g. =).

It's also worth noting that a IOperationFilter is required for this to work. There's no method (I can think of), where you could make WithOpenApi work because the value of Deprecated can be conditional or otherwise fanned.

API Versioning ensures things work through the API Explorer by fanning out ApiDescription instances for each API version (e.g. clone it). That avoids this type of scenario. This isn't the only case where this can happen. If you're versioning by media type, the you might only configure [Produces("application/json")], but the API Explorer will add application/json; v=1.0, and so on, according the API version and endpoint combination being visited. I don't see how WithOpenApi can support that type of configuration.

If the direction is an entirely new API and overhaul of what the API Explorer did that is purely OpenAPI focused, I can get onboard with that, but there are significant barriers to achieving parity with how things work today.

As an aside, there were previous gaps that were addressed long ago. I believe this is the last hurdle to achieve parity, even if Swashbuckle still doesn't honor them all (which is a separate issue).

@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-web-frameworks labels Jun 20, 2023
@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
No open projects
Status: No status
Development

No branches or pull requests

3 participants