Update to Microsoft.OpenApi v2.0.0-preview.17 #61541

captainsafia · 2025-04-17T21:46:09Z

React to nullable annotations changes in types
React to OperationType -> HttpMethod move
React to Annotations -> Metadata rename
React to discriminator mapping using OpenApiSchemaReference
React to Reader package being renamed to YamlReader

src/OpenApi/gen/XmlCommentGenerator.Emitter.cs

BrennanConroy · 2025-04-17T22:34:26Z

src/OpenApi/src/Services/OpenApiDocumentService.cs

+            // Sort schemas by key name for better readability and consistency
+            // This works around an API change in OpenAPI.NET
+            document.Components.Schemas = new Dictionary<string, IOpenApiSchema>(
+                document.Components.Schemas.OrderBy(kvp => kvp.Key),


I don't think this does anything once you insert the items into the dictionary?

This is done as the final step before returning the document to the consumer. The expectation is that new schemas aren't inserted into the dictionary after that. We could use FrozenDictionary/ImmutableDictionary to help codify these but the new APIs use Dictionary instead of IDictionary so we're constrained here. :/

BrennanConroy · 2025-04-17T22:56:50Z

src/OpenApi/src/Services/OpenApiDocumentService.cs

@@ -661,7 +677,7 @@ private async Task<OpenApiRequestBody> GetFormRequestBody(
            var contentType = requestFormat.MediaType;
            requestBody.Content[contentType] = new OpenApiMediaType
            {
-                Schema = schema
+                Schema = complexTypeSchema ?? schema


Not following this change. Can you explain it? And does it need a new test?

I'm working a bit around some of the new strictness in the APIs. The IOpenApiSchema has two implementations OpenApiSchema which represents a resolved schema and has set/get on all properties and OpenApiSchemaaReference which is a pointer to a schema and only exposes a get on properties.

The schema variable represents a resolved schema that we construct and populate with form parameters depending on some specific rules. For example, if you have multiple parameters annotated with [FromForm] like in this test we generate a schema that uses allOf to indicate that all of the properties in each type should appear in a single form payload.

When there is a single parameter annotated with [FromForm] and it is a complex type then the binding implementation resolves from its properties directly. This test validates that behavior.

src/OpenApi/gen/XmlCommentGenerator.Emitter.cs

martincostello · 2025-04-18T06:18:17Z

src/OpenApi/gen/XmlCommentGenerator.Emitter.cs

@@ -383,6 +387,29 @@ public Task TransformAsync(OpenApiOperation operation, OpenApiOperationTransform

            return Task.CompletedTask;
        }
+
+        private static OpenApiParameter UnwrapOpenApiParameter(IOpenApiParameter sourceParameter)


I like this - I might borrow it for Swashbuckle 🙂

Hmmm -- I wonder if it might be worth integrating into the Microsoft.OpenAPI APIs? Let me know how it shakes out for you in practice and we can consider moving it to the base library?

martincostello · 2025-04-18T06:20:16Z

src/OpenApi/src/Extensions/ApiDescriptionExtensions.cs

+            "PATCH" => HttpMethod.Patch,
+            "HEAD" => HttpMethod.Head,
+            "OPTIONS" => HttpMethod.Options,
+            "TRACE" => HttpMethod.Trace,
            _ => throw new InvalidOperationException($"Unsupported HTTP method: {apiDescription.HttpMethod}"),


IIRC there's a bug logged somewhere about an exception being throw for the empty string in MVC instead of being a GET. If you're touching this area maybe fix that here?

I think the right fix for that bug is to fix the ApiExplorer layer so that GET is the assumed default for MVC controller actions that don't explicitly set it. That way we can retain the behavior of throwing for actually invalid HTTP methods here.

martincostello · 2025-04-18T06:23:18Z

src/OpenApi/src/Services/OpenApiConstants.cs

+        HttpMethod.Options,
+        HttpMethod.Head,
+        HttpMethod.Patch,
+        HttpMethod.Trace


I've seen an issue that there's a new QUERY method coming soon, so that might need adding shortly.

dotnet/runtime#114489 (comment)

AFAIK, the OpenAPI spec doesn't yet support all HTTP methods. There's been discussion of loosening this requirement in the past but at the moment it only supports this fixed set.

src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs

...pNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ResponseSchemas.cs

martincostello · 2025-04-18T06:36:58Z

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

@@ -31,17 +31,25 @@ public void MapRelativePathToItemPath_ReturnsItemPathForApiDescription(string re
        Assert.Equal(expectedItemPath, itemPath);
    }

+    public static class HttpMethodTestData
+    {
+        public static IEnumerable<object[]> TestCases => new List<object[]>


Regarding my comment about QUERY, is it worth adding a reflection-based test that checks for HttpMethod having any new static property to flush out the need to react to any addition?

I think we can probably forgo on this until the spec supports more HTTP methods. From my understanding, moving from OperationType to HttpMethod in this breaking change was a setup for that.

martincostello · 2025-04-18T16:16:01Z

src/OpenApi/gen/XmlCommentGenerator.Emitter.cs

@@ -407,7 +407,7 @@ private static OpenApiParameter UnwrapOpenApiParameter(IOpenApiParameter sourceP
            }
            else
            {
-                throw new InvalidOperationException("The input schema must be an OpenApiParameter or OpenApiParameterReference.");
+                throw new InvalidOperationException("The input schema must be an {nameof(OpenApiParameter)} or {nameof(OpenApiParameterReference)}.");


Suggested change

throw new InvalidOperationException("The input schema must be an {nameof(OpenApiParameter)} or {nameof(OpenApiParameterReference)}.");

throw new InvalidOperationException($"The input schema must be an {nameof(OpenApiParameter)} or {nameof(OpenApiParameterReference)}.");

omg I hate myself

This one did make me feel bad 😅 when I spotted it in the snapshots

it's ok I groaned harder on the other whitespace issue

I really wish there was a nicer templating story for source generated files so you could catch these kinds of things more easily

martincostello · 2025-04-18T16:16:29Z

...NetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs

@@ -338,6 +338,8 @@ await VerifyOpenApiDocument(action, document =>
        [([MinLength(2)] int[] id) => {}, (OpenApiSchema schema) => Assert.Equal(2, schema.MinItems)],
        [([Length(4, 8)] int[] id) => {}, (OpenApiSchema schema) => { Assert.Equal(4, schema.MinItems); Assert.Equal(8, schema.MaxItems); }],
        [([Range(4, 8)]int id) => {}, (OpenApiSchema schema) => { Assert.Equal("4", schema.Minimum); Assert.Equal("8", schema.Maximum); }],
+        [([Range(1234, 5678)]int id) => {}, (OpenApiSchema schema) => { Assert.Equal("1234", schema.Minimum); Assert.Equal("5678", schema.Maximum); }],
+         [([Range(1234.56, 7891.1)] decimal id) => {}, (OpenApiSchema schema) => { Assert.Equal("1234.56", schema.Minimum); Assert.Equal("7891.1", schema.Maximum); }],


Suggested change

[([Range(1234.56, 7891.1)] decimal id) => {}, (OpenApiSchema schema) => { Assert.Equal("1234.56", schema.Minimum); Assert.Equal("7891.1", schema.Maximum); }],

[([Range(1234.56, 7891.1)] decimal id) => {}, (OpenApiSchema schema) => { Assert.Equal("1234.56", schema.Minimum); Assert.Equal("7891.1", schema.Maximum); }],

Update to Microsoft.OpenApi v2.0.0-preview.17

a06f8b0

captainsafia requested review from BrennanConroy and mikekistler April 17, 2025 21:46

captainsafia requested review from a team and wtgodbe as code owners April 17, 2025 21:46

dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Apr 17, 2025

BrennanConroy reviewed Apr 17, 2025

View reviewed changes

src/OpenApi/gen/XmlCommentGenerator.Emitter.cs Outdated Show resolved Hide resolved

BrennanConroy reviewed Apr 17, 2025

View reviewed changes

build-analysis bot mentioned this pull request Apr 18, 2025

System.TimeoutException : The operation has timed out. dotnet/dnceng#5279

Open

3 tasks

Fix exception message

daa8e00

martincostello reviewed Apr 18, 2025

View reviewed changes

Address more feedback

4c45af2

martincostello mentioned this pull request Apr 18, 2025

Support .NET 10 domaindrivendev/Swashbuckle.AspNetCore#3283

Draft

9 tasks

martincostello reviewed Apr 18, 2025

View reviewed changes

captainsafia added 2 commits April 18, 2025 09:24

Address more feedback

ef254f4

Fix initialization in TransformersBenchmark

b9841dc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to Microsoft.OpenApi v2.0.0-preview.17 #61541

Update to Microsoft.OpenApi v2.0.0-preview.17 #61541

captainsafia commented Apr 17, 2025 •

edited

Loading

BrennanConroy Apr 17, 2025

captainsafia Apr 18, 2025

BrennanConroy Apr 17, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

martincostello Apr 18, 2025

captainsafia Apr 18, 2025

	throw new InvalidOperationException("The input schema must be an {nameof(OpenApiParameter)} or {nameof(OpenApiParameterReference)}.");
	throw new InvalidOperationException($"The input schema must be an {nameof(OpenApiParameter)} or {nameof(OpenApiParameterReference)}.");

	[([Range(1234.56, 7891.1)] decimal id) => {}, (OpenApiSchema schema) => { Assert.Equal("1234.56", schema.Minimum); Assert.Equal("7891.1", schema.Maximum); }],
	[([Range(1234.56, 7891.1)] decimal id) => {}, (OpenApiSchema schema) => { Assert.Equal("1234.56", schema.Minimum); Assert.Equal("7891.1", schema.Maximum); }],

Update to Microsoft.OpenApi v2.0.0-preview.17 #61541

Are you sure you want to change the base?

Update to Microsoft.OpenApi v2.0.0-preview.17 #61541

Conversation

captainsafia commented Apr 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainsafia commented Apr 17, 2025 •

edited

Loading