Make IDocumentFilter async compatible

[Davilink]

Hi, in our application we want to support feature flag (https://docs.microsoft.com/en-us/azure/azure-app-configuration/use-feature-flags-dotnet-core) and some endpoint will be activate/deactivate based on the maturity of the feature or the target environment (Dev, SIT, UAT, Prod, ...), we want to remove the endpoint from the Swagger UI when the feature endpoint is not active. I use the IDocumentFilter to do so, but feature flag is Task-based, so i had to use GetAwaiter().GetResult() to make it work, but it would be better to have a async version of the IDocumentFilter like IActionFilter/IAsyncFilter of aspnetcore (https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/filters?view=aspnetcore-3.1#implementation). Here the code that i use as an example :

public class FeatureGateDocumentFilter : IDocumentFilter
{
    private readonly IFeatureManager _featureManager;
    public FeatureGateDocumentFilter(IFeatureManager featureManager)
    {
        _featureManager = featureManager;
    }

    public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context)
    {
        foreach(var apiDescription in context.ApiDescriptions)
        {
            if (apiDescription.ActionDescriptor is ControllerActionDescriptor action)
            {
                var controllerFeatureNames = action.MethodInfo
                    .DeclaringType
                    ?.GetCustomAttributes<FeatureGateAttribute>()
                    .SelectMany(att => att.Features)
                    .ToList() ?? new List<string>();

                var actionFeatureNames = action.MethodInfo
                    .GetCustomAttributes<FeatureGateAttribute>()
                    .SelectMany(att => att.Features)
                    .ToList();

                bool showOperation = true;
                if (actionFeatureNames.Any())
                {
                    showOperation = actionFeatureNames.Any(featureName => _featureManager.IsEnabledAsync(featureName).GetAwaiter().GetResult());
                }
                else if(controllerFeatureNames.Any())
                {
                    showOperation = controllerFeatureNames.Any(featureName => _featureManager.IsEnabledAsync(featureName).GetAwaiter().GetResult());
                }

                if (!showOperation)
                {
                    if(Enum.TryParse<OperationType>(apiDescription.HttpMethod, true, out var httpVerb))
                    {
                        swaggerDoc.Paths[$"/{apiDescription.RelativePath}"].Operations.Remove(httpVerb);
                    }
                    else
                    {
                        swaggerDoc.Paths[$"/{apiDescription.RelativePath}"].Operations.Clear();
                    }
                }
            }
        }
    }
}

[aersam]

I've had a look at the code and it seems implementing must be done wisely to not break anything. I'd propose to implement this in the following way:

• Add an interface IAsyncDocumentFilter that inherits IDocumentFilter and adds a ApplyAsync Method with the same signature but returning a Task. This makes it possible to still use DocumentFilters.Add with an implementation. The trade off is that you have to implement Apply as well, usually just throwing an NotImplementedException
• In the SwaggerGenerator.cs class, the GetSwagger whould need be become async, which requires changing ISwaggerProvider which is not really a good option as this might break people. Therefore one had to add two implementations: One that just executes synchronously and one that executes asynchronously (like it's done with many .Net Framework methods). This would mean to add an IAsyncSwaggerProvider as well

Would you accept a PR implementing this? I think this will get important as in .Net Core 3.0 a lot of synchronous code is considered bad practice and this could hurt some more complex IDocumentFilter implementations

[domaindrivendev]

@aersamkull - sounds good, do you want to get an initial PR submitted and we can iterate on the specifics? I'll keep an eye out for it as this does feel like a high priority.

[aersam]

I've added PR #1817 which should implement it in a backward-compatible way. The only thing that might is critical is that DocumentProvider now expects IAsyncSwaggerProvider, I hope that is ok?

[iskandersierra]

Is this issue resolved? I want to create a SwaggerMerger (using YARP) to get swagger documents from multiple microservices (async operation even with caching) and produce a unique swagger document. Because ISwaggerProvider is synchronous, I had to create my own copy of this interface:

public interface IAsyncSwaggerProvider
{
    Task<OpenApiDocument> GetSwaggerAsync(
        string documentName,
        string? host = null,
        string? basePath = null);
}

and also of AsyncSwaggerMiddleware, which is basically a copy of SwaggerMiddleware, but instead of calling synchronously to GetSwagger inside middleware Invoke :

Before:

var swagger = swaggerProvider.GetSwagger(
    documentName,
    host: null,
    basePath);

After:

var swagger = await swaggerProvider.GetSwaggerAsync(
    documentName,
    host: null,
    basePath);

I think it is possible to create a backward compatible code to introduce such a change into the library.

SwaggerUI does not get affected by this changes app.UseSwaggerUI(); but I had to change:

• builder.Services.AddSwaggerGen(); by my own builder.Services.AddSwaggerMerger(builder.Configuration)
• app.UseSwagger(); by my own app.UseAsyncSwagger();

[schehlmj]

Hi @domaindrivendev, I've added PR #2635. It is slightly different approach from @aersam 's. It adds a separate IDocumentAsyncFilter that is its own type and does not extend IDocumentFilter. In this way, it is still backward compatible, but those using IDocumentAsyncFilter do not have to wonder what to implement for the unused "void Apply()" method. I also saw a lot of discussion about naming, and thankfully, in the last couple years, some other naming precedent and changes have already occurred in the project, and I tried to follow in that same vein. Of note is that to help developers be clear about the intent of this new filter, I named it very similar to the old one, and only added Async to the name.

[silkfire]

Any status on this issue? I see no new commits since over a year on Swashbuckle, has the library been abandoned?

[martincostello]

Any status on this issue? I see no new commits since over a year on Swashbuckle, has the library been abandoned?

#2778