Skip to content

Commit abcc92c

Browse files
committed
fix(ibm-api-symmetry): allow for dictionary properties when appropriate
Before, this rule took a strict approach to dictionaries - any dictionary defined on a variant schema could cause the rule to produce a warning. However, in cases where the dictionary property actually matches a dictionary property on the canonical schema, this is too strict. When canonical/variant schemas have identical dictionary schemas, we should not disqualify the variant from being a graph fragment of the canonical. Signed-off-by: Dustin Popp <[email protected]>
1 parent 6709f07 commit abcc92c

File tree

2 files changed

+62
-7
lines changed

2 files changed

+62
-7
lines changed

packages/ruleset/src/functions/api-symmetry.js

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* SPDX-License-Identifier: Apache2.0
44
*/
55

6+
const { isEqual } = require('lodash');
7+
68
const {
79
getSchemaType,
810
isObject,
@@ -128,7 +130,8 @@ function checkApiForSymmetry(apidef, nodes) {
128130
/**
129131
* Determine if the variant schema is indeed a proper graph fragment of the
130132
* canonical schema, using the following conditions:
131-
* - The variant does not define additional or pattern properties
133+
* - The variant does not define additional or pattern properties that are not
134+
* defined identically on the canonical schema.
132135
* - The variant does not define any properties that do not exist on the
133136
* canonical schema (exists = present and has same type)
134137
* - The variant omits at least one property from the canonical schema
@@ -163,17 +166,23 @@ function checkForGraphFragmentPattern(
163166
);
164167
let result = true;
165168

166-
// A variant schema cannot allow extraneous properties and still be
167-
// an explicit graph fragment of the canonical schema.
168-
if (variant.additionalProperties) {
169+
// A variant schema cannot allow extraneous properties that aren't allowed
170+
// on the canonical schema and still be a graph fragment.
171+
if (
172+
variant.additionalProperties &&
173+
!isEqual(variant.additionalProperties, canonical.additionalProperties)
174+
) {
169175
logger.info(
170-
`${ruleId}: schema defines 'additionalProperties' - it is not a proper graph fragment`
176+
`${ruleId}: 'additionalProperties' on variant does not match canonical - it is not a proper graph fragment`
171177
);
172178
result = false;
173179
}
174-
if (variant.patternProperties) {
180+
if (
181+
variant.patternProperties &&
182+
!isEqual(variant.patternProperties, canonical.patternProperties)
183+
) {
175184
logger.info(
176-
`${ruleId}: schema defines 'patternProperties' - it is not a proper graph fragment`
185+
`${ruleId}: 'patternProperties' on variant does not match canonical - it is not a proper graph fragment`
177186
);
178187
result = false;
179188
}

packages/ruleset/test/rules/api-symmetry.test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,52 @@ describe(`Spectral rule: ${ruleId}`, () => {
623623
expect(results).toHaveLength(0);
624624
});
625625

626+
it('prototype schema with nested additionalProperties is compliant', async () => {
627+
const testDocument = makeCopy(rootDocument);
628+
testDocument.components.schemas.Movie.properties.personnel = {
629+
type: 'object',
630+
properties: {
631+
cast: {
632+
description: 'key-value pairs mapping name to role',
633+
type: 'object',
634+
additionalProperties: {
635+
type: 'string',
636+
},
637+
},
638+
},
639+
};
640+
641+
testDocument.components.schemas.MoviePrototype.properties.personnel =
642+
testDocument.components.schemas.Movie.properties.personnel;
643+
644+
const results = await testRule(ruleId, rule, testDocument);
645+
expect(results).toHaveLength(0);
646+
});
647+
648+
it('prototype schema with nested patternProperties is compliant', async () => {
649+
const testDocument = makeCopy(rootDocument);
650+
testDocument.components.schemas.Movie.properties.personnel = {
651+
type: 'object',
652+
properties: {
653+
cast: {
654+
description: 'key-value pairs mapping name to role',
655+
type: 'object',
656+
patternProperties: {
657+
'^[A-Z][a-z]+( ?[A-Z][a-z]+)*$': {
658+
type: 'string',
659+
},
660+
},
661+
},
662+
},
663+
};
664+
665+
testDocument.components.schemas.MoviePrototype.properties.personnel =
666+
testDocument.components.schemas.Movie.properties.personnel;
667+
668+
const results = await testRule(ruleId, rule, testDocument);
669+
expect(results).toHaveLength(0);
670+
});
671+
626672
// Already covered in root document:
627673
// - Valid Prototype schemas
628674
// - Valid Patch schemas

0 commit comments

Comments
 (0)