Skip to content

Commit

Permalink
fix(ibm-well-defined-dictionaries): include patternProperties in vali…
Browse files Browse the repository at this point in the history
…dation (#713)

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 <dpopp07@gmail.com>
  • Loading branch information
dpopp07 authored Jan 10, 2025
1 parent f0dc756 commit ad7134d
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 67 deletions.
2 changes: 1 addition & 1 deletion .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"files": "package-lock.json|^.secrets.baseline$",
"lines": null
},
"generated_at": "2024-12-19T16:14:03Z",
"generated_at": "2025-01-09T19:49:59Z",
"plugins_used": [
{
"name": "AWSKeyDetector"
Expand Down
4 changes: 2 additions & 2 deletions docs/ibm-cloud-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
If used, the `(additional|pattern)Properties` 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.
</td>
</tr>
<tr>
Expand Down
80 changes: 51 additions & 29 deletions packages/ruleset/src/functions/well-defined-dictionaries.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
/**
* Copyright 2024 IBM Corporation.
* Copyright 2024 - 2025 IBM Corporation.
* SPDX-License-Identifier: Apache2.0
*/

const {
isObject,
isObjectSchema,
schemaHasConstraint,
schemaLooselyHasConstraint,
validateNestedSchemas,
} = require('@ibm-cloud/openapi-ruleset-utilities');
const { LoggerFactory } = require('../utils');

let ruleId;
let logger;

/**
* The implementation for this rule makes assumptions that are dependent on the
* presence of the following other rules:
*
* - ibm-pattern-properties: patternProperties isn't empty or the wrong type
*/

module.exports = function (schema, _opts, context) {
if (!logger) {
ruleId = context.rule.name;
Expand All @@ -32,23 +38,23 @@ function wellDefinedDictionaries(schema, path) {

// We will flag dictionaries of dictionaries, so we can skip
// providing guidance for directly nested dictionaries.
if (path.at(-1) === 'additionalProperties') {
if (isDictionaryValueSchema(path)) {
return [];
}

logger.debug(
`${ruleId}: checking object schema at location: ${path.join('.')}`
);

// Dictionaries should have additionalProperties defined on them.
// If the schema doesn't, make sure it has properties and then
// abandon the check.
if (!schemaDefinesField(schema, 'additionalProperties')) {
// Dictionaries should have additionalProperties or patternProperties
// defined on them. If the schema doesn't, make sure it has properties
// and then abandon the check.
if (!isDictionarySchema(schema)) {
if (!schemaDefinesField(schema, 'properties')) {
return [
{
message:
'Object schemas must define either properties, or additionalProperties with a concrete type',
'Object schemas must define either properties, or (additional/pattern)Properties with a concrete type',
path,
},
];
Expand Down Expand Up @@ -79,8 +85,7 @@ function wellDefinedDictionaries(schema, path) {
// more strict in the future but this meets our current purposes.
if (schemaHasConstraint(schema, isAmbiguousDictionary)) {
errors.push({
message:
'Dictionary schemas must have a single, well-defined value type in `additionalProperties`',
message: 'Dictionary schemas must have a single, well-defined value type',
path,
});
}
Expand All @@ -89,7 +94,7 @@ function wellDefinedDictionaries(schema, path) {
// should not be dictionaries themselves.
if (schemaHasConstraint(schema, isDictionaryOfDictionaries)) {
errors.push({
message: 'Dictionaries must not have values that are also dictionaries.',
message: 'Dictionaries must not have values that are also dictionaries',
path,
});
}
Expand All @@ -102,29 +107,46 @@ function schemaDefinesField(schema, field) {
}

function isAmbiguousDictionary(schema) {
if (!schema.additionalProperties) {
return false;
}

// additionalProperties must be an object (not a boolean) value
// and must define a `type` field in order to be considered an
// unambiguous dictionary.
return (
!isObject(schema.additionalProperties) ||
!schemaDefinesField(schema.additionalProperties, 'type')
return dictionaryValuesHaveConstraint(
schema,
valueSchema =>
!isObject(valueSchema) || !schemaDefinesField(valueSchema, 'type')
);
}

function isDictionaryOfDictionaries(schema) {
if (!isObject(schema.additionalProperties)) {
return dictionaryValuesHaveConstraint(
schema,
valueSchema => isObject(valueSchema) && isDictionarySchema(valueSchema)
);
}

function dictionaryValuesHaveConstraint(schema, hasConstraint) {
return schemaHasConstraint(schema, s => {
if (s.additionalProperties !== undefined) {
return hasConstraint(s.additionalProperties);
}

if (s.patternProperties !== undefined) {
return Object.values(s.patternProperties).some(p => hasConstraint(p));
}

return false;
}
});
}

// We don't want any schema that may define the dictionary values
// to also be a dictionary, so we use a looser constraint that
// checks against any oneOf/anyOf schema.
return schemaLooselyHasConstraint(
schema.additionalProperties,
s => !!s['additionalProperties']
// Check, *by path*, if the current schema is a dictionary value schema.
function isDictionaryValueSchema(path) {
return (
path.at(-1) === 'additionalProperties' ||
path.at(-2) === 'patternProperties'
);
}

// Check, *by object fields* if the current schema is a dictionary or not.
function isDictionarySchema(schema) {
return (
schemaDefinesField(schema, 'additionalProperties') ||
schemaDefinesField(schema, 'patternProperties')
);
}
Loading

0 comments on commit ad7134d

Please sign in to comment.