Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,305 @@
import testRule from './__helpers__/testRule.js';
import { DiagnosticSeverity } from '@stoplight/types';

testRule('xgen-IPA-123-enum-values-should-not-exceed-20', [
{
name: 'valid when enum has exactly 20 values',
document: {
components: {
schemas: {
TestSchema: {
properties: {
status: {
type: 'string',
enum: [
'VALUE_1',
'VALUE_2',
'VALUE_3',
'VALUE_4',
'VALUE_5',
'VALUE_6',
'VALUE_7',
'VALUE_8',
'VALUE_9',
'VALUE_10',
'VALUE_11',
'VALUE_12',
'VALUE_13',
'VALUE_14',
'VALUE_15',
'VALUE_16',
'VALUE_17',
'VALUE_18',
'VALUE_19',
'VALUE_20',
],
},
},
},
},
},
},
errors: [],
},
{
name: 'valid when enum has fewer than 20 values',
document: {
components: {
schemas: {
TestSchema: {
properties: {
status: {
type: 'string',
enum: ['PENDING', 'ACTIVE', 'COMPLETE', 'FAILED'],
},
},
},
},
},
},
errors: [],
},
{
name: 'invalid when enum has more than 20 values',
document: {
components: {
schemas: {
TestSchema: {
properties: {
status: {
type: 'string',
enum: [
'VALUE_1',
'VALUE_2',
'VALUE_3',
'VALUE_4',
'VALUE_5',
'VALUE_6',
'VALUE_7',
'VALUE_8',
'VALUE_9',
'VALUE_10',
'VALUE_11',
'VALUE_12',
'VALUE_13',
'VALUE_14',
'VALUE_15',
'VALUE_16',
'VALUE_17',
'VALUE_18',
'VALUE_19',
'VALUE_20',
'VALUE_21',
],
},
},
},
},
},
},
errors: [
{
code: 'xgen-IPA-123-enum-values-should-not-exceed-20',
message: 'Enum arrays should not exceed 20 values. Current count: 21',
path: ['components', 'schemas', 'TestSchema', 'properties', 'status', 'enum'],
severity: DiagnosticSeverity.Warning,
},
],
},
{
name: 'exception on schema',
document: {
components: {
schemas: {
TestSchema: {
properties: {
status: {
type: 'string',
enum: [
'VALUE_1',
'VALUE_2',
'VALUE_3',
'VALUE_4',
'VALUE_5',
'VALUE_6',
'VALUE_7',
'VALUE_8',
'VALUE_9',
'VALUE_10',
'VALUE_11',
'VALUE_12',
'VALUE_13',
'VALUE_14',
'VALUE_15',
'VALUE_16',
'VALUE_17',
'VALUE_18',
'VALUE_19',
'VALUE_20',
'VALUE_21',
'VALUE_22',
'VALUE_23',
'VALUE_24',
'VALUE_25',
],
'x-xgen-IPA-exception': {
'xgen-IPA-123-enum-values-should-not-exceed-20': 'Legacy enum with more than 20 values',
},
},
},
},
},
},
},
errors: [],
},
{
name: 'invalid with integer enum values exceeding limit',
document: {
components: {
schemas: {
TestSchema: {
properties: {
priority: {
type: 'integer',
enum: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21],
},
},
},
},
},
},
errors: [
{
code: 'xgen-IPA-123-enum-values-should-not-exceed-20',
message: 'Enum arrays should not exceed 20 values. Current count: 21',
path: ['components', 'schemas', 'TestSchema', 'properties', 'priority', 'enum'],
severity: DiagnosticSeverity.Warning,
},
],
},
{
name: 'valid for parameters in path operation',
document: {
paths: {
'/resources': {
get: {
parameters: [
{
name: 'status',
in: 'query',
schema: {
type: 'string',
enum: ['ACTIVE', 'INACTIVE', 'PENDING'],
},
},
],
},
},
},
},
errors: [],
},
{
name: 'invalid for parameters in path operation exceeding limit',
document: {
paths: {
'/resources': {
get: {
parameters: [
{
name: 'status',
in: 'query',
schema: {
type: 'string',
enum: [
'VAL_1',
'VAL_2',
'VAL_3',
'VAL_4',
'VAL_5',
'VAL_6',
'VAL_7',
'VAL_8',
'VAL_9',
'VAL_10',
'VAL_11',
'VAL_12',
'VAL_13',
'VAL_14',
'VAL_15',
'VAL_16',
'VAL_17',
'VAL_18',
'VAL_19',
'VAL_20',
'VAL_21',
],
},
},
],
},
},
},
},
errors: [
{
code: 'xgen-IPA-123-enum-values-should-not-exceed-20',
message: 'Enum arrays should not exceed 20 values. Current count: 21',
path: ['paths', '/resources', 'get', 'parameters', '0', 'schema', 'enum'],
severity: DiagnosticSeverity.Warning,
},
],
},
{
name: 'exception on parameter schema',
document: {
paths: {
'/resources': {
get: {
parameters: [
{
name: 'status',
in: 'query',
schema: {
type: 'string',
enum: [
'VAL_1',
'VAL_2',
'VAL_3',
'VAL_4',
'VAL_5',
'VAL_6',
'VAL_7',
'VAL_8',
'VAL_9',
'VAL_10',
'VAL_11',
'VAL_12',
'VAL_13',
'VAL_14',
'VAL_15',
'VAL_16',
'VAL_17',
'VAL_18',
'VAL_19',
'VAL_20',
'VAL_21',
'VAL_22',
'VAL_23',
'VAL_24',
'VAL_25',
],
'x-xgen-IPA-exception': {
'xgen-IPA-123-enum-values-should-not-exceed-20': 'Parameter with many possible values',
},
},
},
],
},
},
},
},
errors: [],
},
]);
17 changes: 17 additions & 0 deletions tools/spectral/ipa/rulesets/IPA-123.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

functions:
- IPA123EachEnumValueMustBeUpperSnakeCase
- IPA123EnumValuesShouldNotExceed20

rules:
xgen-IPA-123-enum-values-must-be-upper-snake-case:
Expand All @@ -17,6 +18,22 @@ rules:
- Skips validation if the schema has an exception defined for this rule
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-123-enum-values-must-be-upper-snake-case'
severity: error
resolved: false
Copy link
Collaborator Author

@yelizhenden-mdb yelizhenden-mdb Apr 2, 2025

Choose a reason for hiding this comment

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

FYI: drive-by fix, not to duplicate the check for referenced schemas. The rule validates only the root component

given: '$..enum'
then:
function: 'IPA123EachEnumValueMustBeUpperSnakeCase'
xgen-IPA-123-enum-values-should-not-exceed-20:
description: |
Enum values should not exceed 20 entries.

##### Implementation details
Rule checks for the following conditions:
- Applies to all enum value arrays defined in the OpenAPI schema
- Validates that each enum array has 20 or fewer values
- Skips validation if the schema has an exception defined for this rule
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-123-enum-values-should-not-exceed-20'
severity: warn
resolved: false
given: '$..enum'
Copy link
Member

Choose a reason for hiding this comment

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

❤️

then:
function: 'IPA123EnumValuesShouldNotExceed20'
11 changes: 11 additions & 0 deletions tools/spectral/ipa/rulesets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,17 @@ Rule checks for the following conditions:
- Validates each enum value individually against the UPPER_SNAKE_CASE pattern
- Skips validation if the schema has an exception defined for this rule

#### xgen-IPA-123-enum-values-should-not-exceed-20

![warn](https://img.shields.io/badge/warning-yellow)
Enum values should not exceed 20 entries.

##### Implementation details
Rule checks for the following conditions:
- Applies to all enum value arrays defined in the OpenAPI schema
- Validates that each enum array has 20 or fewer values
- Skips validation if the schema has an exception defined for this rule



### IPA-125
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { hasException } from './utils/exceptions.js';
import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
import { getSchemaPathFromEnumPath } from './utils/schemaUtils.js';
import { resolveObject } from './utils/componentUtils.js';

const RULE_NAME = 'xgen-IPA-123-enum-values-should-not-exceed-20';
const ERROR_MESSAGE = 'Enum arrays should not exceed 20 values. Current count: ';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const ERROR_MESSAGE = 'Enum arrays should not exceed 20 values. Current count: ';
const ERROR_MESSAGE = 'Enum arrays should not exceed 20 values, consider opting for a string instead. Current count: ';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also thought about including it, but not sure if producers would understand the difference, because type is already string. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I took it right from the IPAs, so if it's unclear we should probably update the IPA too to explain. No string opinion though, I think in the context it makes sense

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Until the investigation for the reusable enum schemas are finalized, I think we should not give any suggestions here. I will note that the relevant IPA and error message here should be refined according to the investigation results

Copy link
Member

Choose a reason for hiding this comment

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

IMHO it is premature to put recomendation for using string. We moving problem to area we also want to fix (long strings).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

@wtrocki wtrocki Apr 3, 2025

Choose a reason for hiding this comment

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

To be clear - I agree that each IPA should have a clear message how to workaround the problem.
For this IPA we agreed that we're going to delegate it outside this PR as more investigation is required.

const MAX_ENUM_VALUES = 20;

export default (input, options, { path, documentInventory }) => {
const oas = documentInventory.resolved;
const schemaPath = getSchemaPathFromEnumPath(path);
const schemaObject = resolveObject(oas, schemaPath);

// Check for exceptions
if (hasException(schemaObject, RULE_NAME)) {
collectException(schemaObject, RULE_NAME, path);
return;
}

if (!Array.isArray(input)) {
return;
}

if (input.length > MAX_ENUM_VALUES) {
return collectAndReturnViolation(path, RULE_NAME, [
{
path,
message: `${ERROR_MESSAGE}${input.length}`,
},
]);
}

collectAdoption(path, RULE_NAME);
};
Loading