Skip to content
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(ibm-well-defined-dictionaries): include patternProperties in validation #713

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

dpopp07
Copy link
Member

@dpopp07 dpopp07 commented Jan 10, 2025

Currently, the rule only considers dictionaries defined with additionalProperties. OpenAPI 3.1.x supports defining dictionaries with patternProperties, so this commit adds consideration for this field in its validation, in addition to additionalProperties.

PR summary

PR Checklist

General checklist

Please make sure that your PR fulfills the following requirements:

  • The commit message follows the Angular Commit Message Guidelines.
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Dependencies have been updated as needed
  • .secrets.baseline has been updated as needed
  • npm run generate-utilities-docs has been run if any files in packages/utilities/src have been updated

@dpopp07 dpopp07 requested a review from padamstx January 10, 2025 19:42
…dation

Currently, the rule only considers dictionaries defined with
`additionalProperties`. OpenAPI 3.1.x supports defining dictionaries
with `patternProperties`, so this commit adds consideration for this
field in its validation, in addition to `additionalProperties`.

Signed-off-by: Dustin Popp <[email protected]>
@dpopp07 dpopp07 force-pushed the dp/fix-well-defined-dictionaries branch from 43008a9 to 2f2ce01 Compare January 10, 2025 21:28
Copy link
Member

@padamstx padamstx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -7328,8 +7328,8 @@ paths:
This rule validates that any dictionary schemas are well defined and that all values share a single type.
Dictionaries are defined as object type schemas that have variable key names. They are distinct from model types,
which are objects with pre-defined properties. A schema must not define both concrete properties and variable key names.
Practically, this means a schema must explicitly define a `properties` object or an `additionalProperties` schema, but not both.
If used, the `additionalProperties` schema must define a concrete type. The concrete type of the values must not be a dictionary itself. See the <a href="https://cloud.ibm.com/docs/api-handbook?topic=api-handbook-types">IBM Cloud API Handbook documentation on types</a> for more info.
Practically, this means a schema must explicitly define a `properties` object or an `(additional|pattern)Properties` schema, but not both.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never realized that our API handbook was this opinionated on "properties" vs "additionalProperties" 😂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's new-ish guidance 🙂 but good guidance, I think!

@dpopp07 dpopp07 merged commit ad7134d into main Jan 10, 2025
7 checks passed
@dpopp07 dpopp07 deleted the dp/fix-well-defined-dictionaries branch January 10, 2025 21:55
ibm-devx-sdk pushed a commit that referenced this pull request Jan 10, 2025
## @ibm-cloud/openapi-ruleset [1.28.1](https://github.com/IBM/openapi-validator/compare/@ibm-cloud/[email protected]...@ibm-cloud/[email protected]) (2025-01-10)

### Bug Fixes

* **ibm-well-defined-dictionaries:** include patternProperties in validation ([#713](#713)) ([ad7134d](ad7134d))
@ibm-devx-sdk
Copy link

🎉 This PR is included in version 1.28.1 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@ibm-devx-sdk
Copy link

🎉 This PR is included in version 1.32.1 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants