Conversation
...enApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=schemas-by-ref.verified.txt
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This pull request adds support for OpenAPI 3.2.0 by upgrading the Microsoft.OpenApi NuGet packages from version 2.0.0 to 3.3.1 and updating the default OpenAPI specification version from 3.1 to 3.2. The PR makes necessary code adaptations to work with the upgraded library's API changes.
Changes:
- Upgrades Microsoft.OpenApi and Microsoft.OpenApi.YamlReader packages from 2.0.0 to 3.3.1
- Changes default OpenAPI specification version from OpenApi3_1 to OpenApi3_2
- Adapts code to API changes in the library (Dictionary<string, OpenApiMediaType> → Dictionary<string, IOpenApiMediaType>)
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/Versions.props | Updates Microsoft.OpenApi package versions from 2.0.0 to 3.3.1 |
| src/OpenApi/src/Services/OpenApiOptions.cs | Changes default OpenAPI version from OpenApi3_1 to OpenApi3_2 |
| src/OpenApi/src/Services/OpenApiGenerator.cs | Adapts to library API change for Content dictionary types |
| src/OpenApi/src/Services/OpenApiDocumentService.cs | Adapts to library API change for Content dictionary types |
| src/OpenApi/gen/XmlCommentGenerator.Emitter.cs | Adds .OfType() filtering when iterating Content values |
| src/Tools/GetDocumentInsider/src/Commands/GetDocumentCommandWorker.cs | Updates default fallback version to OpenApi3_2 |
| src/Tools/GetDocumentInsider/tests/GetDocumentTests.cs | Updates test assertions to expect OpenApi3_2 |
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Transformers/CustomSchemaTransformerTests.cs | Adapts test setup to use IOpenApiMediaType dictionary type |
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Extensions/OpenApiEndpointRouteBuilderExtensionsTests.cs | Updates version string assertions from "3.1.1" to "3.1.2" |
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.SourceGenerators.Tests/snapshots/*.cs | Updates generated code to use .OfType() pattern |
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_1/*.txt | Updates version string from "3.1.1" to "3.1.2" in snapshots |
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApi3_0/OpenApiDocumentIntegrationTests.VerifyOpenApiDocument_documentName=schemas-by-ref.verified.txt | Updates nullable schema structure from library changes |
| src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Integration/snapshots/OpenApiDocumentLocalizationTests.VerifyOpenApiDocumentIsInvariant.verified.txt | Updates version from "3.1.1" to "3.2.0" for default version |
.../Microsoft.AspNetCore.OpenApi.Tests/Extensions/OpenApiEndpointRouteBuilderExtensionsTests.cs
Show resolved
Hide resolved
.../Microsoft.AspNetCore.OpenApi.Tests/Extensions/OpenApiEndpointRouteBuilderExtensionsTests.cs
Show resolved
Hide resolved
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
…or now. Needs to be reverted before GA. Signed-off-by: Vincent Biret <vibiret@microsoft.com>
halter73
left a comment
There was a problem hiding this comment.
Is there anything we should include in our breaking change announcement beyond linking to https://github.com/microsoft/OpenAPI.NET/blob/main/docs/upgrade-guide-3.md?
@halter73 maybe for people to make sure they upgrade swashbuckle if they use it? (version upcoming) and any code that customizes the OpenAPI description might need to be updated as well? Also, since I'm not a part of this GH org, you'll need to approve the workflows run :) (as well as merge once they complete) |
|
@martincostello I saw you mention that you already fired up a .NET 11 branch for Swashbuckle. Is the plan to release a preview Swashbuckle.AspNetCore package with support for Microsoft.OpenApi 3.3.1? I think we'll try to release this as part of preview2 which is slated for release on March 10th. I think it would have been better if the 3.x versions of the Microsoft.OpenApi had been release under a different name like Microsoft.OpenApi.v3 to avoid issues with people accidently over-upgrading Microsoft.OpenApi. While not perfect, I wonder if it would still be worthwhile to create a new package name for the .NET 11 branch of Swashbuckle. It might be enough to stop most of the accidental upgrading people are likely to run into when .NET 11 releases since more people reference Swashbucke rather than Microsoft.OpenApi directly in their project files. |
|
On second thought, given that the major versions of the Swashbuckle.AspNetCore packages align with the supported ASP.NET Core versions, maybe a new package name is overkill. Hopefully, not too many people will try to use Swashbuckle.AspNetCore 11 with ASP.NET Core 10, but I'm not sure. I certainly wouldn't blame people for thinking it might be okay especially when NuGet does not complain. |
|
@halter73 is the only reason why nuget does not complain because aspnet.openapi 10 does not have an upper bound for microsoft.openapi? |
For .NET 10 we just had PR builds with prereleases available in MyGet, rather than pushing them to NuGet.org as "official" previews. My aim was to keep the long-lived PR up-to-date as the .NET 11 previews land. |
Possibly. I'm normally against upper bounds, because it could be that in certain scenarios, the breakages are irrelevant to the application, but you prevent the developer from upgrading a dependency without scary suppressions. However, if we know for a fact that it's impossible for any application that uses aspnet.openapi 10 to work with microsoft.openapi >= 3, we can consider it. We wouldn't be able to do that for packages we've already released though. We'd be counting on people updating to the latest 10.x patch before upgrading to 11. |
|
@baywet It looks like some of the Microsoft.AspNetCore.OpenApi.Tests are failing on the CI. Can you look into them? We might just need to regenerate baselines again. |
fixes #63754
a subsequent pull request(s) will be required to take advantage of the new capabilities like item schema for streaming events