Upgrade to Microsoft.OpenApi 2.x and support OpenAPI v3.1 #59480
Conversation
dotnet-issue-labeler
bot
added
the
area-web-frameworks
captainsafia
added
area-mvc
| <MicrosoftOpenApiVersion>1.6.17</MicrosoftOpenApiVersion> | ||
| <MicrosoftOpenApiReadersVersion>1.6.17</MicrosoftOpenApiReadersVersion> | ||
| <MicrosoftOpenApiVersion>2.0.0-preview2</MicrosoftOpenApiVersion> | ||
| <MicrosoftOpenApiReadersVersion>2.0.0-preview2</MicrosoftOpenApiReadersVersion> |
There was a problem hiding this comment.
Are we planning to upgrade to 2.0.0 RTM once it's available? The build will eventually complain if we're still on preview2 when we stabilize for 10.0
There was a problem hiding this comment.
Yep! There will eventually be an RTM of the package but we're doing this work now to provide feedback to the Microsoft.OpenApi team about the APIs and iterate on things.
The build will eventually complain if we're still on preview2 when we stabilize for 10.0
When do we expect this to happen?
There was a problem hiding this comment.
When do we expect this to happen?
The second half of next year some time, so we've got plenty of time
There was a problem hiding this comment.
Copilot reviewed 38 out of 53 changed files in this pull request and generated no comments.
Files not reviewed (15)
- eng/Versions.props: Language not supported
- src/OpenApi/src/Comparers/OpenApiReferenceComparer.cs: Evaluated as low risk
- src/OpenApi/src/Extensions/OpenApiSchemaExtensions.cs: Evaluated as low risk
- src/OpenApi/perf/Microbenchmarks/OpenApiSchemaComparerBenchmark.cs: Evaluated as low risk
- src/OpenApi/src/Comparers/OpenApiSchemaComparer.cs: Evaluated as low risk
- src/OpenApi/src/Comparers/ComparerHelpers.cs: Evaluated as low risk
- src/OpenApi/src/Comparers/OpenApiAnyComparer.cs: Evaluated as low risk
- src/OpenApi/src/Comparers/OpenApiDiscriminatorComparer.cs: Evaluated as low risk
- src/OpenApi/src/Comparers/OpenApiExternalDocsComparer.cs: Evaluated as low risk
- src/OpenApi/src/Comparers/OpenApiXmlComparer.cs: Evaluated as low risk
- src/OpenApi/src/Services/OpenApiOptions.cs: Evaluated as low risk
- src/OpenApi/sample/Transformers/AddExternalDocsTransformer.cs: Evaluated as low risk
- src/OpenApi/perf/Microbenchmarks/TransformersBenchmark.cs: Evaluated as low risk
- src/OpenApi/src/Extensions/OpenApiServiceCollectionExtensions.cs: Evaluated as low risk
- src/OpenApi/src/Schemas/OpenApiJsonSchema.Helpers.cs: Evaluated as low risk
| }; | ||
| document.Paths = await GetOpenApiPathsAsync(document, scopedServiceProvider, operationTransformers, schemaTransformers, cancellationToken); | ||
| document.Tags = document.Tags?.Distinct(OpenApiTagComparer.Instance).ToList(); |
There was a problem hiding this comment.
I filed microsoft/OpenAPI.NET#1983 to eventually make this a lot smoother.
| // resolution of references in the document. | ||
| document.Workspace ??= new(); | ||
| document.Workspace.RegisterComponents(document); | ||
| if (document.Components?.Schemas is not null) |
There was a problem hiding this comment.
We do this to ensure that the schemas are always ordered lexicographically by reference ID in the document. This was previously done in the OpenApiSchemaReferenceTransformer but since that's been removed in favor of using OpenApiSchemaReference directly, we need to do the sorting somewhere.
| @@ -199,7 +200,7 @@ await VerifyOpenApiDocument(builder, options, document => | |||
| }); | |||
| } | |||
|
|
|||
| [Fact] | |||
| [ConditionalFact(Skip = "https://github.com/dotnet/aspnetcore/issues/58619")] | |||
| public async Task HandlesDuplicateSchemaReferenceIdsGeneratedByOverload() | |||
There was a problem hiding this comment.
This is one of the tests we have to temporarily skip while we sort out how schema deduping looks in the new model.
| @@ -51,13 +51,13 @@ public async Task GetOpenApiParameters_RouteParametersAreAlwaysRequired() | |||
| // Assert | |||
| await VerifyOpenApiDocument(builder, document => | |||
| { | |||
| var pathParameter = Assert.Single(document.Paths["/api/todos/{id}"].Operations[OperationType.Get].Parameters); | |||
| var pathParameter = Assert.Single(document.Paths["/api/todos/{id}"].Operations[OperationType.Get].Parameters!); | |||
There was a problem hiding this comment.
Microsoft.OpenApi marks the parameter list property as nullable in the new model so we use a null suppression here and elsewhere to quiet nullability warnings.
I used Assert.NotNull in other places so it's not the most consistent but 🤷🏽 .
Not sure why we are getting these warnings now. They are coming from the ApiDescription.Server and adjacent dependencies. We're planning on changing this in .NET 10 anyways (see #58353), so I'll go ahead and remove the out-of-support runtime target in this PR. |
Contributes towards #58619.
This PR updates our Microsoft.OpenApi dependency to the new major version (v2) to support Open API v3.1.
Changes in this PR include:
JsonSchemaTypeenum to represent schema typesIOpenApiAnyin favor ofJsonNodeper the change in the Microsoft.OpenApi libraryThe biggest functional change in this PR is the removal of the
OpenApiSchemaStoreclass in favor of the newOpenApiSchemaReferencetype. TheOpenApiSchemaStorewas our previous strategy of maintaining resolved references to OpenAPI schemas and their reference IDs. TheOpenApiSchemaReferencetype supports this behavior by allowing you to hold a reference to a schema and support resolving that schema in one.This change means that we can store schemas directly in the
OpenApiDocument.Components.Schemasproperty and provide resolved references to them for use in transformers without additional overhead on our part.This does temporarily introduce a regression into the logic we had for disambiguating duplicate schemas. Moving forward, I'd like to use this as a forcing function to implement the comparers in the Microsoft.OpenApi library (see microsoft/OpenAPI.NET#414) and avoid the overhead of having to maintain the comparers in our own codebase (removed here).
Another note: this change just lays the groundwork for this upgrade. There's still some more work required to support actual JSON schema features, like changing the way that nullability is modeled to use the
typeproperty or taking advantage of theconstkeyword. These changes will happen in follow up PRs.