Skip to content

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

Conversation

paulmendix
Copy link
Contributor

@paulmendix paulmendix commented Apr 13, 2025

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 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.

…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`.
@paulmendix paulmendix requested a review from a team as a code owner April 13, 2025 22:22
@paulmendix
Copy link
Contributor Author

I will contact the right people at my work so I can sign the CLA, this will take some time.

Copy link
Member

@baywet baywet left a 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.

@paulmendix
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Mendix"

baywet
baywet previously approved these changes May 12, 2025
Copy link
Member

@baywet baywet left a 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

@baywet
Copy link
Member

baywet commented May 12, 2025

@paulmendix can you please handle the conflict before we review another time?
@MaggieKimani1 when you merge, please ensure you squash, and that nowhere in the commit message we have !:, otherwise release please will try to major bump...

@paulmendix
Copy link
Contributor Author

Thanks. I addressed the conflict. Ready for final review @MaggieKimani1

@baywet baywet changed the title fix!: handle deserializing and writing empty security requirements #1426 fix: handle deserializing and writing empty security requirements #1426 May 12, 2025
@MaggieKimani1 MaggieKimani1 merged commit 962e0e4 into microsoft:main May 13, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No difference between no security requirement and empty security requirement on Operations
3 participants