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

src/OpenApi/src/Services/OpenApiDocumentService.cs

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); }],

martincostello · 2025-04-24T08:47:06Z

I'm guessing this missed the ⛵ for preview 4?

BrennanConroy · 2025-04-24T17:52:45Z

I'm guessing this missed the ⛵ for preview 4?

Oh, yeah, it's too late unless we really need it. I should have merged it for her since she's out...

martincostello · 2025-04-24T18:53:15Z

It would be nice as there's a big dependency chain for stuff like Swashbuckle.AspNetCore where we can't use newer previews of Microsoft.OpenApi due to various binary-breaking changes along the way since p11, so without it we can't test anything newer until June (domaindrivendev/Swashbuckle.AspNetCore#3283 (comment))

BrennanConroy · 2025-04-24T19:05:11Z

/backport to release/10.0-preview4

github-actions · 2025-04-24T19:05:24Z

Started backporting to release/10.0-preview4: https://github.com/dotnet/aspnetcore/actions/runs/14649645511

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

10 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

BrennanConroy approved these changes Apr 21, 2025

View reviewed changes

BrennanConroy merged commit bc1b3b2 into main Apr 24, 2025
27 checks passed

BrennanConroy deleted the safia/openapi-p4-bump branch April 24, 2025 19:04

github-actions bot mentioned this pull request Apr 24, 2025

[release/10.0-preview4] Update to Microsoft.OpenApi v2.0.0-preview.17 #61675

Merged

10 tasks

dotnet-policy-service bot added this to the 10.0-preview5 milestone Apr 24, 2025

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

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

martincostello commented Apr 24, 2025

BrennanConroy commented Apr 24, 2025

martincostello commented Apr 24, 2025 •

edited

Loading

BrennanConroy commented Apr 24, 2025

github-actions bot commented Apr 24, 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

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

martincostello commented Apr 24, 2025

BrennanConroy commented Apr 24, 2025

martincostello commented Apr 24, 2025 • edited Loading

BrennanConroy commented Apr 24, 2025

github-actions bot commented Apr 24, 2025

captainsafia commented Apr 17, 2025 •

edited

Loading

martincostello commented Apr 24, 2025 •

edited

Loading