-
Notifications
You must be signed in to change notification settings - Fork 190
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
Allow IDocument to be modified before rendering #344
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThomasBleijendaal Thanks for the PR! As the other PRs have been merged, yours have got some conflicts. Would you please resolve those conflicts? Then, we can start discussion.
844f1c9
to
44ef015
Compare
@justinyoo I've fixed the conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since v1.1.0 release, the sample apps have been significantly updated. Therefore, your changes have also been affected. Would you please check that out?
44ef015
to
30f7f71
Compare
@justinyoo I've rebased this PR on the latest changes on main. |
@ThomasBleijendaal It might take time for review because there are many files involved in this PR. Thanks for your patience! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThomasBleijendaal Thanks for the PR! It looks promising. As far as I understand, this approach mimics the OpenApiDocument
built from the Build()
method. I think both #367 and #368 can be updated with this approach.
Would you please add relevant test codes and integration testing codes to validate your approach?
...icrosoft.Azure.WebJobs.Extensions.OpenApi.FunctionApp.Filters/VersionPrefixDocumentFilter.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Abstractions/IDocumentFilter.cs
Outdated
Show resolved
Hide resolved
30f7f71
to
593e825
Compare
I've updated the PR and added unit tests and added one integration test to validate the working of the document filter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @ThomasBleijendaal ! This would be a fantastic addition! I was looking for something like this in #376
I've left a couple comments, but of course feel free to ignore as I am not involved in the project.
🤞 it can be released soon. Happy to help if needed!
...Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Abstractions/IOpenApiConfigurationOptions.cs
Outdated
Show resolved
Hide resolved
...t.Azure.WebJobs.Extensions.OpenApi.Core/Configurations/DefaultOpenApiConfigurationOptions.cs
Outdated
Show resolved
Hide resolved
Thanks. I've addressed your suggestions. I hope it can be released soon, this feature is the only thing that prevents us from fully migrating to this package. |
Very much want this PR to get merged. It's holding us up as well with our SwaggerUI documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThomasBleijendaal Thanks for waiting! It all LGTM.
...Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Abstractions/IOpenApiConfigurationOptions.cs
Show resolved
Hide resolved
This was a very useful addition. I was looking at using it to add fluent validation to the generated schema. Similar to MicroElements.Swashbuckle.FluentValidation. The only thing that seems to be blocking this is the absence of support for dependency injection in instanciating the |
Ah, I think i've answered my own question, just had to sit with the problem a little longer. I decided to create a class that inherits from That was a mouthful. Just in case that was confusing, here's a couple code snippets for anyone who comes across this with the same requirements as me. Program.cs public class Program
{
public static async Task Main()
{
var host = new HostBuilder()
.ConfigureFunctionsWorkerDefaults(worker =>
{
// ... other middleware and stuff ...
//Recommended serialisation settings for OpenAPI extensions, extending default settings from:
//https://github.com/Azure/azure-functions-openapi-extension/blob/main/src/Microsoft.Azure.Functions.Worker.Extensions.OpenApi/Extensions/FunctionsWorkerApplicationBuilderExtensions.cs
var settings = NewtonsoftJsonObjectSerializer.CreateJsonSerializerSettings();
settings.ContractResolver = new CamelCasePropertyNamesContractResolver();
settings.NullValueHandling = NullValueHandling.Ignore;
//with one additional preference of mine
settings.Converters.Add(new StringEnumConverter());
worker.UseNewtonsoftJson(settings);
})
.ConfigureOpenApi()
.ConfigureServices(services =>
{
//...other config and service stuff...
//Override OpenApi context to support DI
services.AddTransient<IValidatorFactory, ServiceProviderValidatorFactory>();
services.AddSingleton<IOpenApiHttpTriggerContext, OpenApi.HomesOpenApiHttpTriggerContext>();
})
.Build().Run();
}
} MyOpenApiConfig.cs public class MyOpenApiHttpTriggerContext : OpenApiHttpTriggerContext
{
private readonly ILogger<HomesOpenApiHttpTriggerContext> _logger;
private readonly IValidatorFactory _validatorFactory;
private IOpenApiConfigurationOptions _configOptions;
public MyOpenApiHttpTriggerContext(ILogger<MyOpenApiHttpTriggerContext> logger, IValidatorFactory validatorFactory) : base()
{
_logger = logger;
_validatorFactory = validatorFactory;
}
public override IOpenApiConfigurationOptions OpenApiConfigurationOptions
{
get
{
if (_configOptions.IsNullOrDefault())
{
_configOptions = new OpenApiConfig(_logger, _validatorFactory);
}
return _configOptions;
}
}
}
public class MyOpenApiConfig : DefaultOpenApiConfigurationOptions
{
public MyOpenApiConfig(ILogger logger, IValidatorFactory validatorFactory)
{
OpenApiVersion = OpenApiVersionType.V3;
Info.Title = "My API";
// etc...
DocumentFilters.Add(new MyOpenApiDocumentFilter(logger, validatorFactory));
}
}
internal class MyOpenApiDocumentFilter : IDocumentFilter
{
private readonly IValidatorFactory _validatorFactory;
private readonly ILogger _logger;
public MyOpenApiDocumentFilter(ILogger logger, IValidatorFactory validatorFactory)
{
_logger = logger;
_validatorFactory = validatorFactory;
}
public void Apply(IHttpRequestDataObject req, OpenApiDocument document)
{
using (_logger?.BeginScope(nameof(MyOpenApiDocumentFilter)))
{
var schemasWithValidaation = document.Components.Schemas.Where(kvp => (kvp.Key.StartsWith("create") || kvp.Key.StartsWith("update")) && kvp.Key.EndsWith("Command"));
foreach (var schema in schemasWithValidaation)
{
//apply fluent validation to openapi schemas...
}
}
}
} |
This PR allows for adding
IDocumentFilter
s for modifying the generatedIDocument
just before it is rendered. Sometimes modifying theIDocument
at runtime is the only way to implement certain functionality in systems that depend on the generated documentation.Unit tests and documentation are still missing and I will add those once I know this approach is a good one.
After adding the unit tests I'll probably remove the new
VersionPrefixDocumentFilter
, I used that filter to test the functionality, but I think including this in the samples is distracting.--
I'm aware that #280 exists but I wrote most code before I discovered that PR. My approach is similar, but is applicable for all use cases (in-process & out-of-process and static & IoC). I can also merge this PR with that one, let me know.