-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP-306579: Repeated Fields #653
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
| - Validates that each enum array has 20 or fewer values | ||
| - Reusable enum schemas will be ignored | ||
| - Skips validation if the schema has an exception defined for this rule | ||
| - This validation threshold can be adjusted by changing the functionOptions.maxEnumValues parameter |
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.
FYI: drive-by fix
| rules: | ||
| xgen-IPA-124-array-max-items: | ||
| description: | | ||
| Array fields must have a maxItems property defined to enforce an upper bound on the number of items (default: 100). |
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.
| Array fields must have a maxItems property defined to enforce an upper bound on the number of items (default: 100). | |
| Array fields must have a maxItems property defined to enforce an upper bound on the number of items (recommended max: 100). If repeated data has the chance of being too large, the API should use a sub-resource instead. |
[Nit] Include guidance on option instead of very large array
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.
I changed repeated data with array field to be consistent, and kept the rest. Let me know how it looks 👍
|
|
||
| testRule('xgen-IPA-124-array-max-items', [ | ||
| { | ||
| name: 'valid array with maxItems set to 100', |
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.
Can you also add a case with valid maxItems: <value lower than 100>?
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.
Addressed, let me know how it looks 👍
| properties: { | ||
| outerArray: { | ||
| type: 'array', | ||
| maxItems: 100, | ||
| arrayProperty: { | ||
| type: 'array', | ||
| maxItems: 101, | ||
| items: { | ||
| type: 'string', | ||
| }, | ||
| }, |
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.
Can you add a case where both the outer and nested array has a too large value?
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.
Addressed, let me know how it looks 👍
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.
TY!
| rules: | ||
| xgen-IPA-124-array-max-items: | ||
| description: | | ||
| Array fields must have a maxItems property defined to enforce an upper bound on the number of items (recommended max: 100). If the array field has the chance of being too large, the API should use a sub-resource instead.. |
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.
| Array fields must have a maxItems property defined to enforce an upper bound on the number of items (recommended max: 100). If the array field has the chance of being too large, the API should use a sub-resource instead.. | |
| Array fields must have a maxItems property defined to enforce an upper bound on the number of items (recommended max: 100). If the array field has the chance of being too large, the API should use a sub-resource instead. |
Proposed changes
Jira ticket: CLOUDP-306579
Found 426 violations to fix.
Planned fix: No exceptions, but adding maxItems for the violating array fields
Checklist
Changes to Spectral
Further comments