Skip to content

Commit ccd88e4

Browse files
sander1095captainsafia
authored andcommitted
More thorough support for retrieving OpenAPI routes in a case-sensitive manner (#59568)
* When retrieving optione, use the lowercase document name, too * Added tests for checking if options monitor uses case-insensitive document name for OpenAPI version retrieval * Add XML comments to prevent bugs in the future * Improve comments on existing test * Add new tests for DocumentProvider which should also work case-insensitively * Fix build by adding necessary XML comment * Fix build of tests * Improve comments * Fix incorrect tests * Fix bugs around case-sensitive document names * Implemented PR comments
1 parent cd7b123 commit ccd88e4

File tree

5 files changed

+117
-16
lines changed

5 files changed

+117
-16
lines changed

src/OpenApi/src/Extensions/OpenApiEndpointRouteBuilderExtensions.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ public static IEndpointConventionBuilder MapOpenApi(this IEndpointRouteBuilder e
3030
var options = endpoints.ServiceProvider.GetRequiredService<IOptionsMonitor<OpenApiOptions>>();
3131
return endpoints.MapGet(pattern, async (HttpContext context, string documentName = OpenApiConstants.DefaultDocumentName) =>
3232
{
33-
// We need to retrieve the document name in a case-insensitive manner
34-
// to support case-insensitive document name resolution.
35-
// Keyed Services are case-sensitive by default, which doesn't work well for document names in ASP.NET Core
36-
// as routing in ASP.NET Core is case-insensitive by default.
33+
// We need to retrieve the document name in a case-insensitive manner to support case-insensitive document name resolution.
34+
// The document service is registered with a key equal to the document name, but in lowercase.
35+
// The GetRequiredKeyedService() method is case-sensitive, which doesn't work well for OpenAPI document names here,
36+
// as the document name is also used as the route to retrieve the document, so we need to ensure this is lowercased to achieve consistency with ASP.NET Core routing.
37+
// The same goes for the document options below, which is also case-sensitive, and thus we need to pass in a case-insensitive document name.
38+
// See OpenApiServiceCollectionExtensions.cs for more info.
3739
var lowercasedDocumentName = documentName.ToLowerInvariant();
3840

3941
// It would be ideal to use the `HttpResponseStreamWriter` to
@@ -50,7 +52,7 @@ public static IEndpointConventionBuilder MapOpenApi(this IEndpointRouteBuilder e
5052
else
5153
{
5254
var document = await documentService.GetOpenApiDocumentAsync(context.RequestServices, context.RequestAborted);
53-
var documentOptions = options.Get(documentName);
55+
var documentOptions = options.Get(lowercasedDocumentName);
5456
using var output = MemoryBufferWriter.Get();
5557
using var writer = Utf8BufferTextWriter.Get(output);
5658
try

src/OpenApi/src/Extensions/OpenApiServiceCollectionExtensions.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,9 @@ public static IServiceCollection AddOpenApi(this IServiceCollection services, st
5757
ArgumentNullException.ThrowIfNull(services);
5858
ArgumentNullException.ThrowIfNull(configureOptions);
5959

60-
// We need to store the document name in a case-insensitive manner
61-
// to support case-insensitive document name resolution.
62-
// Keyed Services are case-sensitive by default, which doesn't work well for document names in ASP.NET Core
63-
// as routing in ASP.NET Core is case-insensitive by default.
60+
// We need to register the document name in a case-insensitive manner to support case-insensitive document name resolution.
61+
// The document name is used to store and retrieve keyed services and configuration options, which are all case-sensitive.
62+
// To achieve parity with ASP.NET Core routing, which is case-insensitive, we need to ensure the document name is lowercased.
6463
var lowercasedDocumentName = documentName.ToLowerInvariant();
6564

6665
services.AddOpenApiCore(lowercasedDocumentName);

src/OpenApi/src/Services/OpenApiDocumentProvider.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@ internal sealed class OpenApiDocumentProvider(IServiceProvider serviceProvider)
2525
/// <param name="writer">A text writer associated with the document to write to.</param>
2626
public async Task GenerateAsync(string documentName, TextWriter writer)
2727
{
28+
// See OpenApiServiceCollectionExtensions.cs to learn why we lowercase the document name
29+
var lowercasedDocumentName = documentName.ToLowerInvariant();
30+
2831
var options = serviceProvider.GetRequiredService<IOptionsMonitor<OpenApiOptions>>();
29-
var namedOption = options.Get(documentName);
32+
var namedOption = options.Get(lowercasedDocumentName);
3033
var resolvedOpenApiVersion = namedOption.OpenApiVersion;
31-
await GenerateAsync(documentName, writer, resolvedOpenApiVersion);
34+
await GenerateAsync(lowercasedDocumentName, writer, resolvedOpenApiVersion);
3235
}
3336

3437
/// <summary>
@@ -40,10 +43,17 @@ public async Task GenerateAsync(string documentName, TextWriter writer)
4043
/// <param name="openApiSpecVersion">The OpenAPI specification version to use when serializing the document.</param>
4144
public async Task GenerateAsync(string documentName, TextWriter writer, OpenApiSpecVersion openApiSpecVersion)
4245
{
46+
// We need to retrieve the document name in a case-insensitive manner to support case-insensitive document name resolution.
47+
// The document service is registered with a key equal to the document name, but in lowercase.
48+
// The GetRequiredKeyedService() method is case-sensitive, which doesn't work well for OpenAPI document names here,
49+
// as the document name is also used as the route to retrieve the document, so we need to ensure this is lowercased to achieve consistency with ASP.NET Core routing.
50+
// See OpenApiServiceCollectionExtensions.cs for more info.
51+
var lowercasedDocumentName = documentName.ToLowerInvariant();
52+
4353
// Microsoft.OpenAPI does not provide async APIs for writing the JSON
4454
// document to a file. See https://github.com/microsoft/OpenAPI.NET/issues/421 for
4555
// more info.
46-
var targetDocumentService = serviceProvider.GetRequiredKeyedService<OpenApiDocumentService>(documentName);
56+
var targetDocumentService = serviceProvider.GetRequiredKeyedService<OpenApiDocumentService>(lowercasedDocumentName);
4757
using var scopedService = serviceProvider.CreateScope();
4858
var document = await targetDocumentService.GetOpenApiDocumentAsync(scopedService.ServiceProvider);
4959
var jsonWriter = new OpenApiJsonWriter(writer);

src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Extensions/OpenApiEndpointRouteBuilderExtensionsTests.cs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.OpenApi.Models;
1313
using Microsoft.OpenApi.Readers;
1414
using System.Text;
15+
using Microsoft.OpenApi;
1516

1617
public class OpenApiEndpointRouteBuilderExtensionsTests : OpenApiDocumentServiceTestBase
1718
{
@@ -156,6 +157,42 @@ public async Task MapOpenApi_ReturnsDocumentWhenPathIsCaseSensitive(string regis
156157
Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);
157158
}
158159

160+
[Fact]
161+
public async Task MapOpenApi_ShouldRetrieveOptionsInACaseInsensitiveManner()
162+
{
163+
// Arrange
164+
var hostEnvironment = new HostEnvironment() { ApplicationName = nameof(OpenApiEndpointRouteBuilderExtensionsTests) };
165+
var serviceProviderIsService = new ServiceProviderIsService();
166+
var serviceProvider = CreateServiceProvider("casesensitive", OpenApiSpecVersion.OpenApi2_0);
167+
var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider));
168+
builder.MapOpenApi("/openapi/{documentName}.json");
169+
var context = new DefaultHttpContext();
170+
var responseBodyStream = new MemoryStream();
171+
context.Response.Body = responseBodyStream;
172+
context.RequestServices = serviceProvider;
173+
context.Request.RouteValues.Add("documentName", "CaseSensitive");
174+
var endpoint = builder.DataSources.First().Endpoints[0];
175+
176+
// Act
177+
var requestDelegate = endpoint.RequestDelegate;
178+
await requestDelegate(context);
179+
180+
// Assert
181+
Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);
182+
var responseString = Encoding.UTF8.GetString(responseBodyStream.ToArray());
183+
184+
// When we receive an OpenAPI document, we use an OptionsMonitor to retrieve OpenAPI options which are stored with a key equal the requested document name.
185+
// This key is case-sensitive. If the document doesn't exist, the options monitor return a default instance, in which the OpenAPI version is set to v3.
186+
// This could cause bugs! You'd get your document, but depending on the casing you used in the document name you passed to the function, you'll receive different OpenAPI document versions.
187+
// We want to prevent this from happening. Therefore:
188+
// By setting up a v2 document on the "casesensitive" route and requesting it on "CaseSensitive",
189+
// we can test that the we've configured the options monitor to retrieve the options in a case-insensitive manner.
190+
// If it is case-sensitive, it would return a default instance with OpenAPI version v3, which would cause this test to fail!
191+
// However, if it would return the v2 instance, which was configured on the lowercase - case-insensitive - documentname, the test would pass!
192+
// For more info, see OpenApiEndpointRouteBuilderExtensions.cs
193+
Assert.StartsWith("{\n \"swagger\": \"2.0\"", responseString);
194+
}
195+
159196
[Theory]
160197
[InlineData("/openapi.json", "application/json;charset=utf-8", false)]
161198
[InlineData("/openapi.yaml", "text/plain+yaml;charset=utf-8", true)]
@@ -201,7 +238,7 @@ private static void ValidateOpenApiDocument(MemoryStream documentStream, Action<
201238
action(document);
202239
}
203240

204-
private static IServiceProvider CreateServiceProvider(string documentName = Microsoft.AspNetCore.OpenApi.OpenApiConstants.DefaultDocumentName)
241+
private static IServiceProvider CreateServiceProvider(string documentName = Microsoft.AspNetCore.OpenApi.OpenApiConstants.DefaultDocumentName, OpenApiSpecVersion openApiSpecVersion = OpenApiSpecVersion.OpenApi3_0)
205242
{
206243
var hostEnvironment = new HostEnvironment() { ApplicationName = nameof(OpenApiEndpointRouteBuilderExtensionsTests) };
207244
var serviceProviderIsService = new ServiceProviderIsService();
@@ -210,7 +247,7 @@ private static IServiceProvider CreateServiceProvider(string documentName = Micr
210247
.AddSingleton<IHostEnvironment>(hostEnvironment)
211248
.AddSingleton(CreateApiDescriptionGroupCollectionProvider())
212249
.AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance)
213-
.AddOpenApi(documentName)
250+
.AddOpenApi(documentName, x => x.OpenApiVersion = openApiSpecVersion)
214251
.BuildServiceProvider();
215252
return serviceProvider;
216253
}

src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentProviderTests.cs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Microsoft.Extensions.ApiDescriptions;
55
using Microsoft.Extensions.DependencyInjection;
66
using Microsoft.Extensions.Hosting;
7+
using Microsoft.OpenApi;
78
using Microsoft.OpenApi.Models;
89
using Microsoft.OpenApi.Readers;
910
using static Microsoft.AspNetCore.OpenApi.Tests.OpenApiOperationGeneratorTests;
@@ -30,6 +31,58 @@ public async Task GenerateAsync_ReturnsDocument()
3031
});
3132
}
3233

34+
[Fact]
35+
public async Task GenerateAsync_ShouldRetrieveOptionsInACaseInsensitiveManner()
36+
{
37+
// Arrange
38+
var documentName = "CaseSensitive";
39+
var serviceProvider = CreateServiceProvider(["casesensitive"], OpenApiSpecVersion.OpenApi2_0);
40+
var documentProvider = new OpenApiDocumentProvider(serviceProvider);
41+
var stringWriter = new StringWriter();
42+
43+
// Act
44+
await documentProvider.GenerateAsync(documentName, stringWriter);
45+
46+
// Assert
47+
var document = stringWriter.ToString();
48+
49+
// When we generate an OpenAPI document, we use an OptionsMonitor to retrieve OpenAPI options which are stored with a key equal the requested document name.
50+
// This key is case-sensitive. If the document doesn't exist, the options monitor return a default instance, in which the OpenAPI version is set to v3.
51+
// This could cause bugs! You'd get your document, but depending on the casing you used in the document name you passed to the function, you'll receive different OpenAPI document versions.
52+
// We want to prevent this from happening. Therefore:
53+
// By setting up a v2 document on the "casesensitive" route and requesting it on "CaseSensitive",
54+
// we can test that the we've configured the options monitor to retrieve the options in a case-insensitive manner.
55+
// If it is case-sensitive, it would return a default instance with OpenAPI version v3, which would cause this test to fail!
56+
// However, if it would return the v2 instance, which was configured on the lowercase - case-insensitive - documentname, the test would pass!
57+
Assert.StartsWith("{\n \"swagger\": \"2.0\"", document);
58+
}
59+
60+
[Fact]
61+
public async Task GenerateAsync_ShouldRetrieveOpenApiDocumentServiceWithACaseInsensitiveKey()
62+
{
63+
// Arrange
64+
var documentName = "CaseSensitive";
65+
var serviceProvider = CreateServiceProvider(["casesensitive"]);
66+
var documentProvider = new OpenApiDocumentProvider(serviceProvider);
67+
var stringWriter = new StringWriter();
68+
69+
// Act
70+
await documentProvider.GenerateAsync(documentName, stringWriter, OpenApiSpecVersion.OpenApi3_0);
71+
72+
// Assert
73+
var document = stringWriter.ToString();
74+
75+
// If the Document Service is retrieved with a non-existent (case-sensitive) key, it would throw:
76+
// https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.serviceproviderkeyedserviceextensions.getrequiredkeyedservice?view=net-9.0-pp
77+
78+
// In this test's case, we're testing that the document service is retrieved with a case-insensitive key.
79+
// It's registered as "casesensitive" but we're passing in "CaseSensitive", which doesn't exist.
80+
// Therefore, if the test doesn't throw, we know it has passed correctly.
81+
// We still do a small check to validate the document, just in case. But the main test is that it doesn't throw.
82+
ValidateOpenApiDocument(stringWriter, _ => { });
83+
Assert.StartsWith("{\n \"openapi\": \"3.0.1\"", document);
84+
}
85+
3386
[Fact]
3487
public void GetDocumentNames_ReturnsAllRegisteredDocumentName()
3588
{
@@ -56,7 +109,7 @@ private static void ValidateOpenApiDocument(StringWriter stringWriter, Action<Op
56109
action(document);
57110
}
58111

59-
private static IServiceProvider CreateServiceProvider(string[] documentNames)
112+
private static IServiceProvider CreateServiceProvider(string[] documentNames, OpenApiSpecVersion openApiSpecVersion = OpenApiSpecVersion.OpenApi3_0)
60113
{
61114
var hostEnvironment = new HostEnvironment() { ApplicationName = nameof(OpenApiDocumentProviderTests) };
62115
var serviceProviderIsService = new ServiceProviderIsService();
@@ -66,7 +119,7 @@ private static IServiceProvider CreateServiceProvider(string[] documentNames)
66119
.AddSingleton(CreateApiDescriptionGroupCollectionProvider());
67120
foreach (var documentName in documentNames)
68121
{
69-
serviceCollection.AddOpenApi(documentName);
122+
serviceCollection.AddOpenApi(documentName, x => x.OpenApiVersion = openApiSpecVersion);
70123
}
71124
var serviceProvider = serviceCollection.BuildServiceProvider(validateScopes: true);
72125
return serviceProvider;

0 commit comments

Comments
 (0)