RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.77k stars 1.29k forks source link

API Version deprecated tag not affected document #2012

Open WiseJoeyD opened 5 years ago

WiseJoeyD commented 5 years ago

[Related to #1355 ]

Hi,

Been using Nswag instead of other options as I like how it automatically pulls in XML comments without additional setup or configuration.

As part of the project that I'm using NSwag with we are creating different versions of a Web API. The original v1 endpoints are deprecated and I've added the necessary ApiVersion attribute

[ApiController]
[ApiVersion("1.0", Deprecated = true)]
[Route("api/v{version:apiVersion}/[controller]")]
public class ValuesController : ControllerBase
{...`

Using the example project from AspNetCore.Tests.Web, and reading the reponses to a few other Api Versioning issues here, I got the swagger document to display the different versions of the endpoints through the dropdown on the generated Swagger page

While working to get ApiVersioning working with NSwag I noticed some examples using Swashbuckle that would render the endpoints and method with a visual strikethrough if they were marked as deprecated. However with NSwag there is nothing shown.

You can add a Swagger Tag

[SwaggerTag("VersionedValues", Description = "Old operations")]

but it doesn't have the same effect, and the value of the ApiVersion attribute doesn't seem to be shown anywhere on the generated page.

Is there any option or code I'm missing, or is that something that could be added as an enhancement? Perhaps as a tag beside the document info at the top?

Many thanks Joey

RicoSuter commented 5 years ago

[Obsolete] attribute? Otherwise a custom operation processor...

RicoSuter commented 5 years ago

Why would [SwaggerTag("VersionedValues", Description = "Old operations")] mark an operation as deprecated?

WiseJoeyD commented 5 years ago

Hi,

Thanks for the reply

Why would [SwaggerTag("VersionedValues", Description = "Old operations")] mark an operation as deprecated?

In answer to your question - I wasn't thinking it would mark the operation as deprecated.

Just noting that in the example NSwag solution code (ASpNetCore.Tests.Web) there doesn't seem to be any effect if a version is marked [...Deprecated = true] In the example code that SwaggerTag seemed to the only way to mark an operation as old/deprecated

[Obsolete] attribute? Otherwise a custom operation processor...

Thanks, hadn't noticed that was an attribute I could use (new to NET Core Api).

Tried that and it provides the behaviour I was looking for - strikethrough text on the Swagger page. It at least shows me how to achieve a similar effect to Swashbuckle.

The only thing is [Obsolete] marks the operation regardless of whether it's mapped to a deprecated version or a live version, which sadly won't fit my needs.

(If I had v1.0 and v1.1 api versions setup in a single controller and wanted to mark operations mapped to v1.0 as deprecated/obsolete(strikethrough), but show operations mapped to version 1.1 normally (no strikethrough) I'd have to split them into separate methods/operations, because the [Obsolete] attribute doesn't have a way to limit itself to a single version)

It looks like if I want that functionality I'll have to switch, which is a shame because apart from the Api Versioning setup/handling it has really nice clean setup compared to Swashbuckle

Thanks for your help, Joey

RicoSuter commented 5 years ago

Can you post a screenshot of how this looks like in the Swagger UI? How do you mark a version as deprecated in ASP.NET Core? I think this must be just a simple config which has to be set in the Swagger spec or in the Swagger UI config...

ben-wallis commented 5 years ago

You mark a version as Deprecated by using the optional Deprecated parameter in the ApiVersion attribute on the controller:

[ApiVersion("1.0", Deprecated = true)]

https://github.com/microsoft/aspnet-api-versioning/wiki/Deprecating-a-Service-Version

Note that it appears that this is controller-specific - you can deprecate v1.0 of one controller, while not deprecating it for another.

I've just come across the same issue where deprecating an API version for a controller doesn't mark the methods mapped to that version as deprecated in SwaggerUI.

The UI for deprecated methods can be seen in the Swagger Petstore demo here (the /pet/findByTags method) https://petstore.swagger.io

ideoclickVanessa commented 5 years ago

Is this fix supposed to be in NSwag.AspNetCore version 13.1.3?

It currently has no effect for me, like the original poster.

I have [ApiVersion("4", Deprecated = true)] on all of the Sales controllers, but it has no effect on the OpenAPI doc: image

RicoSuter commented 5 years ago

Yes should be in the latest version

RasmusWesterlundh commented 4 years ago

Just chiming in with others, using API Versioning:

[ApiVersion("1.0", Deprecated = true)]

Deprecating controllers or actions does not work with NSwag. Tested with NSwag13.1.3 and 13.2.

aaronasmith commented 4 years ago

I'm doing this with NSwag 13.5 and still not seeing it deprecate them. I ended up doing this to fix it for now:

document.AddOperationFilter(genericContext => {
   var context = genericContext as AspNetCoreOperationProcessorContext;
   var apiDescription = context.ApiDescription;
   var operation = context.OperationDescription.Operation;
   operation.IsDeprecated |= apiDescription.IsDeprecated();
});
RicoSuter commented 4 years ago

operation.IsDeprecated |= apiDescription.IsDeprecated();

Not sure but maybe this line is not in nswag out of the box, can you check and maybe create a PR + test?

EngineerCoding commented 3 years ago

I created a simple operation processor which marks Obsolete methods or Controllers as deprecated:

/// <summary>
/// Marks actions (or controllers) with the <seealso cref="System.ObsoleteAttribute"/> as deprecated in the API
/// </summary>
public class DeprecatedOperationProcessor : IOperationProcessor
{
    /// <inheritdoc/>
    public bool Process(OperationProcessorContext context)
    {
        bool obsolete = IsObsolete(context.ControllerType) || IsObsolete(context.MethodInfo);
        if (obsolete)
        {
            context.OperationDescription.Operation.IsDeprecated = true;
        }
        return true;
    }

    /// <summary>
    /// Determines whether the specified custom attribute provider is obsolete.
    /// </summary>
    /// <param name="customAttributeProvider">The custom attribute provider.</param>
    /// <returns>
    ///   <c>true</c> if the specified custom attribute provider is obsolete; otherwise, <c>false</c>.
    /// </returns>
    private static bool IsObsolete(ICustomAttributeProvider customAttributeProvider)
    {
        return customAttributeProvider.GetCustomAttributes(typeof(ObsoleteAttribute), true).Any();
    }
}
hazzik commented 3 years ago

@RicoSuter I figured out that depreciation does not work if GroupNameFormat is used.

I was using this:

.AddVersionedApiExplorer(o => o.GroupNameFormat = "'v'VVV")

And so, here the condition in Any was not true:

https://github.com/RicoSuter/NSwag/blob/f7a396f6ea21aef95594b5f7d9b8ef455dba959b/src/NSwag.Generation.AspNetCore/AspNetCoreOpenApiDocumentGenerator.cs#L264-L266

Because v.ToString() is "1", and "apiDescription.GroupName" is "v1"

RicoSuter commented 3 years ago

@hazzik do you see a way to fix this in a "generic" way, i.e. that it just works as expected?

aunikitin commented 3 years ago

Hello guys, I face with same problem now. Any updates?

GMillerVarian commented 2 years ago

We are facing the same issue. We are using the ApiVersion annotation with Deprecated = true and it is not showing as deprecated in the Swagger UI. We have resorted to adding the Obsolete annotation as well, but this is not quite correct and has some disadvantages. We are using GroupNameFormat as mentioned above.

Can this issue be re-opened as it is not fixed yet?

commonsensesoftware commented 2 years ago

I guess I need to troll here more often 😛. I just happened be looking for something else and came across this issue.

API Versioning provides the built-in extension method ApiDescription.IsDeprecated that performs this logic for you, which was introduced in 4.0.0. If you adapt over that, you should always get the right answer without having to duplicate the logic. It will also protect you from any underlying changes in patches.

imichalarias commented 1 year ago

still encountering this issue... Any news/workarounds? I do not want to remove the GroupNameFormat definition...

BrunoBeraud commented 2 months ago

Here a quick dirty fix with an operation processor I made

internal class ApiDeprecatedOperationProcessor : IOperationProcessor
{
    public bool Process(OperationProcessorContext context)
    {
        var aspNetCoreContext = (AspNetCoreOperationProcessorContext)context;
        var apiDescription = aspNetCoreContext.ApiDescription;
        var operation = context.OperationDescription.Operation;

        var apiVersionModel = aspNetCoreContext
            .ApiDescription
            .ActionDescriptor
            .GetApiVersionMetadata()
            .Map(ApiVersionMapping.Explicit);

        var isDeprecated = apiVersionModel != null &&
            apiVersionModel.DeprecatedApiVersions.OfType<object>()
            .Any(v => $"v{v}" == aspNetCoreContext.ApiDescription.GroupName);

        context.OperationDescription.Operation.IsDeprecated = isDeprecated;

        return true;
    }

From https://github.com/dotnet/aspnet-api-versioning/issues/1026#issuecomment-1748319326, it looks like we have a breaking change to get ApiVersionModel that Nswag doesn't seem to have integrated yet.

Based on https://github.com/RicoSuter/NSwag/issues/2012#issuecomment-851743109, I prefixed the deprecated versions found in ApiVersionModel with the letter "v" to match GroupName, I don't know if there's a cleaner way to do it.