-
Notifications
You must be signed in to change notification settings - Fork 259
fix: handle deserializing and writing empty security requirements #1426 #2323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: handle deserializing and writing empty security requirements #1426 #2323
Conversation
…crosoft#1426 make distinction between empty security requirements and no security requirements on an operation. empty security requirements are read as an empty list, no security requirements are read as null for OpenAPI v2/v3/v3.1. This is a breaking change, previously both cases were read as an empty list. also includes a change to OpenApiOperation.SerializeInternal so it can serialize these two cases separately. this required a new method OpenApiWriterExtensions.WriteOptionalOrEmptyCollection. includes unit tests, change to PublicApi.approved.txt to include the new method, and I removed a couple of unused usings and a typo in test name `SerializeDocWithSecuritySchemeWithInlineReferencesWorks`.
I will contact the right people at my work so I can sign the CLA, this will take some time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
A couple of suggestions while we wait for the CLA.
src/Microsoft.OpenApi/Reader/V31/OpenApiOperationDeserializer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi/Reader/V2/OpenApiOperationDeserializer.cs
Outdated
Show resolved
Hide resolved
@microsoft-github-policy-service agree company="Mendix" |
test/Microsoft.OpenApi.Tests/Models/Samples/docWithEmptyOperationSecurity.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
@MaggieKimani1 for final review and merge
@paulmendix can you please handle the conflict before we review another time? |
…ity-requirements-on-operation
Thanks. I addressed the conflict. Ready for final review @MaggieKimani1 |
fix #1426.
make distinction between empty security requirements and no security requirements on an operation. empty security requirements are read as an empty list, no security requirements are read as null for OpenAPI v2/v3/v3.1. This is a breaking change, previously both cases were read as an empty list.
also includes a change to
OpenApiOperation.SerializeInternal
so it can serialize these two cases separately. this required a new methodOpenApiWriterExtensions.WriteOptionalOrEmptyCollection
.includes unit tests, change to PublicApi.approved.txt to include the new method, and I removed a couple of unused usings and a typo in test name
SerializeDocWithSecuritySchemeWithInlineReferencesWorks
.