[OpenAPI] Use invariant culture for TextWriter

[martincostello]

Use invariant culture for TextWriter

Use culture-invariant TextWriter implementation for OpenAPI.

Description

• Ensure OpenAPI documents are written to a culture-invariant TextWriter implementation.
• Add/update unit/integration tests to ensure the invariant culture is used to generate the OpenAPI document regardless of the current culture.
• Add support for emitting exclusiveMinimum/exclusiveMaximum instead of minimum/maximum where relevant based on the values of RangeAttribute properties.

I couldn't replicate this issue in .NET 10, I'm guessing Microsoft.OpenApi has had some internal refactoring that means that it's no longer using an overload on TextWriter that was hitting this (unless I messed up my test to repro it somehow).

• Contributes to MapOpenApi writes invalid json if example contains OpenApiDouble and CurrentCulture formats doubles with comma #60628.
• Contributes to [OpenApi] .NET 10, XML schema transformer applies property comments on referenced schema #61965.

[Copilot]

Pull Request Overview

This PR updates the OpenAPI implementation to enforce the usage of an invariant culture when writing documents using TextWriter. Key changes include adding a new constructor overload to Utf8BufferTextWriter that accepts an IFormatProvider, updating OpenAPI endpoint routing to explicitly use CultureInfo.InvariantCulture, and adding tests and a sample endpoint ("/getcultureinvariant") to verify the culture invariance behavior.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/SignalR/common/Shared/Utf8BufferTextWriter.cs	Added a new constructor overload to inject an IFormatProvider.
src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/...	Updated snapshots to include a new "/getcultureinvariant" endpoint.
src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/...	Updated snapshots to include a new "/getcultureinvariant" endpoint.
src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentIntegrationTests.cs	Refactored test data setup using MemberData and updated the verification call.
src/OpenApi/src/Extensions/OpenApiEndpointRouteBuilderExtensions.cs	Modified the instantiation of Utf8BufferTextWriter to use CultureInfo.InvariantCulture.
src/OpenApi/sample/Program.cs	Added middleware to change the current culture for testing invariant behavior.
src/OpenApi/sample/Controllers/TestController.cs	Introduced a new GET endpoint and record type to support culture-invariant document generation.

Comments suppressed due to low confidence (2)

src/OpenApi/sample/Controllers/TestController.cs:35

• [nitpick] The method name 'PostTypedResult' is potentially misleading for a GET endpoint. Consider renaming it to something like 'GetCultureInvariant' to reflect its purpose more clearly.

[HttpGet]

src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/OpenApiDocumentIntegrationTests.cs:49

• The change from 'Verifier.Verify(json)' to 'Verify(json)' should be confirmed as intentional, as it alters the API call pattern. If this change is designed to utilize a new verification mechanism, consider adding a brief comment for clarity.

await Verify(json)

[desjoerd]

I think this change should also be applied for GetDocument, but I am not sure.

[martincostello]

GetDocument

Where do you mean exactly?

[desjoerd]

It's not in the filtered solution, but the dotnet tool/executable which extracts the document during build.

[desjoerd]

Found it: https://github.com/dotnet/aspnetcore/blob/faf269645b0338a9f4a7ecc88b46053f80f6f602/src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommandWorker.cs

[martincostello]

Yep you're right.

The code here creates a TextWriter (StreamWriter):

aspnetcore/src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommandWorker.cs

Lines 333 to 353 in faf2696

	using (var writer = new StreamWriter(stream, _utf8EncodingWithoutBOM, bufferSize: 1024, leaveOpen: true))
	{
	var targetMethod = generateWithVersionMethod ?? generateMethod;
	object[] arguments = [documentName, writer];
	if (generateWithVersionMethod != null)
	{
	_reporter.WriteInformation(Resources.VersionedGenerateMethod);
	if (Enum.TryParse<OpenApiSpecVersion>(_context.OpenApiVersion, out var version))
	{
	arguments = [documentName, writer, version];
	}
	else
	{
	if (!string.IsNullOrWhiteSpace(_context.OpenApiVersion))
	{
	_reporter.WriteWarning(Resources.FormatInvalidOpenApiVersion(_context.OpenApiVersion));
	}
	arguments = [documentName, writer, OpenApiSpecVersion.OpenApi3_1];
	}
	}
	using var resultTask = (Task)InvokeMethod(targetMethod, service, arguments);

Then that gets used here:

aspnetcore/src/OpenApi/src/Services/OpenApiDocumentProvider.cs

Lines 58 to 60 in faf2696

	var document = await targetDocumentService.GetOpenApiDocumentAsync(scopedService.ServiceProvider);
	var jsonWriter = new OpenApiJsonWriter(writer);
	await document.SerializeAsync(jsonWriter, openApiSpecVersion);

I'll update that to use the invariant culture too.

[captainsafia]

I couldn't replicate this issue in .NET 10, I'm guessing Microsoft.OpenApi has had some internal refactoring that means that it's no longer using an overload on TextWriter that was hitting this (unless I messed up my test to repro it somehow).

Do you mean that the new tests pass without any of the code changes in this PR?

It might be good to track the change on the Microsoft.OpenApi side that caused this.

[martincostello]

Yeah, they still pass without the fix being present.

My hunch is that they stopped using an overload like Write(double) or similar, which would have gone through a code path that would have used the current culture formatter.

I did try to test the theory by making an equivalent change to the release/9.0 branch, but I had issues running restore.cmd for some reason. I might try that again later this week.

[martincostello]

@captainsafia I worked out why the repro wasn't happening and fixed it.

TL;DR - because the snapshots directly got the document and self-serialized it, it didn't go through the code path for the endpoint that was causing the issue (the tests didn't have the same bug in them).

[captainsafia]

Feels like something spooky happened with the new test data setup. These should still target OpenApi v3.

[martincostello]

Oh yes, I see what I've done.

There's no mechanism to pass the OpenAPI version to use on to the endpoint now that it's doing it over HTTP instead of using the service directly.

[martincostello]

Right, I've fixed this now, but I'm not particularly happy with it.

Unless I've missed something, it's not possible to dynamically determine the OpenAPI version to return without encoding it into the document's name, so I've had to expose the endpoints on multiple URLs each to cover 3.0 and 3.1, which then means I need to customise ShouldInclude so the paths still get returned.

Is it worth adding a capability somehow to allow the user to dynamically compute the OpenAPI version to use at runtime? I tried to do it dynamically with IPostConfigureOptions<T>, but as it uses IOptionsMonitor<T> it only runs once and keeps the first request's preference.

Maybe something like this?

public OpenApiOptions
{
+   public Func<HttpContext, OpenApiSpecVersion?>? OpenApiVersionSelector { get; set; }
}

If you think it's worth proposing I'll spin up an API Suggestion issue (and am happy to take a hand at implementing it if approved), assuming there's still time to get it into 10.0.

[martincostello]

Example: martincostello@65f941f

[martincostello]

#62984

[martincostello]

While working on a related Swashbuckle change I realised that if RangeAttribute set MaximumIsExclusive or MinimumIsExclusive to true, the OpenAPI schema wasn't being emitted in a way that respected that.

I've fixed that and added unit tests for various scenarios. I've also refactored the code to avoid the round-trip string format/parse where possible.

[martincostello]

@captainsafia I've moved the localization stuff out into its own text fixture as discussed offline. I've left the changes to extend the sample to have different OpenAPI versions covered in the snapshots.

Merging with main has uncovered a failing test added by #61399 so I need to investigate what's going on there. It fails in main too, so maybe it's another localisation-related thing that my en-GB machine has flushed out.

 OpenApiSchemaServiceTests.RespectsEnumDefaultValueInControllerFormParameters
   Source: OpenApiSchemaService.RequestBodySchemas.cs line 715
   Duration: 11 ms

  Message: 
System.InvalidCastException : Unable to cast object of type 'Status' to type 'System.String'.

  Stack Trace: 
JsonSerializer.UnboxOnWrite[T](Object value)
JsonTypeInfo`1.SerializeAsObject(Utf8JsonWriter writer, Object rootValue)
JsonSerializer.WriteNodeAsObject(Object value, JsonTypeInfo jsonTypeInfo)
JsonNodeSchemaExtensions.ApplyDefaultValue(JsonNode schema, Object defaultValue, JsonTypeInfo jsonTypeInfo) line 166
JsonNodeSchemaExtensions.ApplyParameterInfo(JsonNode schema, ApiParameterDescription parameterDescription, JsonTypeInfo jsonTypeInfo) line 316
OpenApiSchemaService.GetOrCreateUnresolvedSchemaAsync(OpenApiDocument document, Type type, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, ApiParameterDescription parameterDescription, CancellationToken cancellationToken) line 128
OpenApiSchemaService.GetOrCreateSchemaAsync(OpenApiDocument document, Type type, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, ApiParameterDescription parameterDescription, CancellationToken cancellationToken) line 141
OpenApiDocumentService.GetFormRequestBody(OpenApiDocument document, IList`1 supportedRequestFormats, IEnumerable`1 formParameters, IList`1 endpointMetadata, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) line 567
OpenApiDocumentService.GetRequestBodyAsync(OpenApiDocument document, ApiDescription description, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) line 516
OpenApiDocumentService.GetOperationAsync(ApiDescription description, OpenApiDocument document, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) line 312
OpenApiDocumentService.GetOperationsAsync(IGrouping`2 descriptions, OpenApiDocument document, IServiceProvider scopedServiceProvider, IOpenApiOperationTransformer[] operationTransformers, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) line 269
OpenApiDocumentService.GetOpenApiPathsAsync(OpenApiDocument document, IServiceProvider scopedServiceProvider, IOpenApiOperationTransformer[] operationTransformers, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken) line 253
OpenApiDocumentService.GetOpenApiDocumentAsync(IServiceProvider scopedServiceProvider, HttpRequest httpRequest, CancellationToken cancellationToken) line 75
OpenApiDocumentServiceTestBase.VerifyOpenApiDocument(ActionDescriptor action, Action`1 verifyOpenApiDocument, CancellationToken cancellationToken) line 46
OpenApiSchemaServiceTests.RespectsEnumDefaultValueInControllerFormParameters() line 721
--- End of stack trace from previous location ---

[martincostello]

I need to investigate what's going on there

Worked it out - MVC isn't part of the OpenAPI solution filter, so when I rebased it hadn't been rebuilt with the fix, but the tests were being rebuilt because they are in the filter.

Running build.cmd in a terminal to refresh everything fixed it.

[captainsafia]

Bleh, had these comments in pending and never hit submit 😭

[captainsafia]

Can we keep the logic that queries the OpenApiDocumentService directly and only run through the HTTP request based path for the localization tests?

[captainsafia]

Instead of running the localization tests on every document, can we create a new document specifically for localization scenarios then update this test to only send a request to that one?

[captainsafia]

Thanks for reacting to all the feedback here! 😃