-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Support returning OpenAPI document in YAML from MapOpenApi #58616
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,8 +49,16 @@ public static IEndpointConventionBuilder MapOpenApi(this IEndpointRouteBuilder e | |
using var writer = Utf8BufferTextWriter.Get(output); | ||
try | ||
{ | ||
document.Serialize(new OpenApiJsonWriter(writer), documentOptions.OpenApiVersion); | ||
context.Response.ContentType = "application/json;charset=utf-8"; | ||
if (UseYaml(pattern)) | ||
{ | ||
document.Serialize(new OpenApiYamlWriter(writer), documentOptions.OpenApiVersion); | ||
context.Response.ContentType = "text/plain+yaml;charset=utf-8"; | ||
} | ||
else | ||
{ | ||
document.Serialize(new OpenApiJsonWriter(writer), documentOptions.OpenApiVersion); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are people going to be surprised when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe give more control for the user, passing all values in params, and give some magic extension .json or .yaml in the end of path if exists or not or wrong will be removed and set the right extension, for content type / mime type pass in the last param like mimeType: | default value like pattern: "openapi/v1" format: JSON mimeType: "application/json" ( 0-0)/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Depends on user expectation and the type of experience that we want to provide. TOML isn't a supported OpenAPI document format so we could decide to throw if we encounter an extension and it's not associated with the supported set of formats.
With this behavior, #3 would be a breaking change between .NET 9 and .NET 10 but I think we can live with it.
Yeah, I considered this option as well but opted to be more conservative in the amount of new API that was introduced. It also feels that given there are only two formats supported currently, we might want to support a simpler API for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've opted to avoid the breaking change for now. We've got runaway to revisit this if needed. |
||
context.Response.ContentType = "application/json;charset=utf-8"; | ||
} | ||
await context.Response.BodyWriter.WriteAsync(output.ToArray(), context.RequestAborted); | ||
await context.Response.BodyWriter.FlushAsync(context.RequestAborted); | ||
} | ||
|
@@ -63,4 +71,8 @@ public static IEndpointConventionBuilder MapOpenApi(this IEndpointRouteBuilder e | |
} | ||
}).ExcludeFromDescription(); | ||
} | ||
|
||
private static bool UseYaml(string pattern) => | ||
pattern.EndsWith(".yaml", StringComparison.OrdinalIgnoreCase) || | ||
pattern.EndsWith(".yml", StringComparison.OrdinalIgnoreCase); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,11 +29,13 @@ public void MapOpenApi_ReturnsEndpointConventionBuilder() | |
Assert.IsAssignableFrom<IEndpointConventionBuilder>(returnedBuilder); | ||
} | ||
|
||
[Fact] | ||
public void MapOpenApi_SupportsCustomizingPath() | ||
[Theory] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do any of these tests validate it is actually YAML? They check the content type and that a OpenAPI parser can parse the content, but if you replaced the writer implementation in the YAML block with the JSON one would they still pass? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, it would still pass if the writer used was a JSON one. Since JSON is valid YAML, if we wanted to verify that the emitted content was actually YAML we'd have to perhaps do some string-based check to see if it contained the correct starting characters compared to a JSON output? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL JSON is YML, but that still feels wrong to me that the tests wouldn't detect the content not being the requested format. |
||
[InlineData("/custom/{documentName}/openapi.json")] | ||
[InlineData("/custom/{documentName}/openapi.yaml")] | ||
[InlineData("/custom/{documentName}/openapi.yml")] | ||
public void MapOpenApi_SupportsCustomizingPath(string expectedPath) | ||
{ | ||
// Arrange | ||
var expectedPath = "/custom/{documentName}/openapi.json"; | ||
var serviceProvider = CreateServiceProvider(); | ||
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); | ||
|
||
|
@@ -72,13 +74,17 @@ public async Task MapOpenApi_ReturnsRenderedDocument() | |
}); | ||
} | ||
|
||
[Fact] | ||
public async Task MapOpenApi_ReturnsDefaultDocumentIfNoNameProvided() | ||
[Theory] | ||
[InlineData("/openapi.json", "application/json;charset=utf-8", false)] | ||
[InlineData("/openapi.toml", "application/json;charset=utf-8", false)] | ||
[InlineData("/openapi.yaml", "text/plain+yaml;charset=utf-8", true)] | ||
[InlineData("/openapi.yml", "text/plain+yaml;charset=utf-8", true)] | ||
public async Task MapOpenApi_ReturnsDefaultDocumentIfNoNameProvided(string expectedPath, string expectedContentType, bool isYaml) | ||
{ | ||
// Arrange | ||
var serviceProvider = CreateServiceProvider(); | ||
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); | ||
builder.MapOpenApi("/openapi.json"); | ||
builder.MapOpenApi(expectedPath); | ||
var context = new DefaultHttpContext(); | ||
var responseBodyStream = new MemoryStream(); | ||
context.Response.Body = responseBodyStream; | ||
|
@@ -91,6 +97,11 @@ public async Task MapOpenApi_ReturnsDefaultDocumentIfNoNameProvided() | |
|
||
// Assert | ||
Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); | ||
Assert.Equal(expectedContentType, context.Response.ContentType); | ||
var responseString = Encoding.UTF8.GetString(responseBodyStream.ToArray()); | ||
// String check to validate that generated document starts with YAML syntax | ||
Assert.Equal(isYaml, responseString.StartsWith("openapi: 3.0.1", StringComparison.OrdinalIgnoreCase)); | ||
responseBodyStream.Position = 0; | ||
ValidateOpenApiDocument(responseBodyStream, document => | ||
{ | ||
Assert.Equal("OpenApiEndpointRouteBuilderExtensionsTests | v1", document.Info.Title); | ||
|
@@ -121,16 +132,19 @@ public async Task MapOpenApi_Returns404ForUnresolvedDocument() | |
Assert.Equal("No OpenAPI document with the name 'v2' was found.", Encoding.UTF8.GetString(responseBodyStream.ToArray())); | ||
} | ||
|
||
[Fact] | ||
public async Task MapOpenApi_ReturnsDocumentIfNameProvidedInQuery() | ||
[Theory] | ||
[InlineData("/openapi.json", "application/json;charset=utf-8", false)] | ||
[InlineData("/openapi.yaml", "text/plain+yaml;charset=utf-8", true)] | ||
[InlineData("/openapi.yml", "text/plain+yaml;charset=utf-8", true)] | ||
public async Task MapOpenApi_ReturnsDocumentIfNameProvidedInQuery(string expectedPath, string expectedContentType, bool isYaml) | ||
{ | ||
// Arrange | ||
var documentName = "v2"; | ||
var hostEnvironment = new HostEnvironment() { ApplicationName = nameof(OpenApiEndpointRouteBuilderExtensionsTests) }; | ||
var serviceProviderIsService = new ServiceProviderIsService(); | ||
var serviceProvider = CreateServiceProvider(documentName); | ||
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); | ||
builder.MapOpenApi("/openapi.json"); | ||
builder.MapOpenApi(expectedPath); | ||
var context = new DefaultHttpContext(); | ||
var responseBodyStream = new MemoryStream(); | ||
context.Response.Body = responseBodyStream; | ||
|
@@ -144,6 +158,11 @@ public async Task MapOpenApi_ReturnsDocumentIfNameProvidedInQuery() | |
|
||
// Assert | ||
Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); | ||
Assert.Equal(expectedContentType, context.Response.ContentType); | ||
var responseString = Encoding.UTF8.GetString(responseBodyStream.ToArray()); | ||
// String check to validate that generated document starts with YAML syntax | ||
Assert.Equal(isYaml, responseString.StartsWith("openapi: 3.0.1", StringComparison.OrdinalIgnoreCase)); | ||
responseBodyStream.Position = 0; | ||
ValidateOpenApiDocument(responseBodyStream, document => | ||
{ | ||
Assert.Equal($"OpenApiEndpointRouteBuilderExtensionsTests | {documentName}", document.Info.Title); | ||
|
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.
I assume
OpenApiYamlWriter
does smart things to guard against, e.g., the Norway Problem?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.
OpenAPI recommends YAML 1.2 of the spec, which prohibits non-boolean string literals from being interpreted as booleans, AFAIK.
The serialization itself seems to emit it as a string correctly. For example:
becomes