diff --git a/packages/ruleset/src/functions/use-date-based-format.js b/packages/ruleset/src/functions/use-date-based-format.js index f4279586..72588736 100644 --- a/packages/ruleset/src/functions/use-date-based-format.js +++ b/packages/ruleset/src/functions/use-date-based-format.js @@ -4,14 +4,12 @@ */ const { - isArraySchema, + getExamplesForSchema, isDateSchema, isDateTimeSchema, isIntegerSchema, isObject, - isObjectSchema, isStringSchema, - schemaHasConstraint, validateNestedSchemas, } = require('@ibm-cloud/openapi-ruleset-utilities'); @@ -75,64 +73,35 @@ module.exports = function (schema, _opts, context) { * @returns an array containing the violations found or [] if no violations */ function checkForDateBasedFormat(s, p, apidef) { - const examples = []; - const propertyPath = []; + // Map connecting a list of examples for a schema to its logical path. + const examples = {}; // Check for any examples outside of the schema path - they may be in - // request bodies, response bodies, or parameters. - examples.push(...checkForIndirectExamples(p, apidef)); + // request bodies, response bodies, or parameters. Store these separately. + const indirectExamples = checkForIndirectExamples(p, apidef); - return validateNestedSchemas(s, p, (schema, path) => { + return validateNestedSchemas(s, p, (schema, path, logicalPath) => { logger.debug(`${ruleId}: checking schema at location: ${path.join('.')}`); + logger.debug( + `${ruleId}: logical schema path is : ${logicalPathForLogger(logicalPath)}` + ); - // We can look at primitive schemas directly but for objects and arrays, - // we need more specialized handling in case we need to find a particular - // property within their examples. - if (isObjectSchema(schema) || isArraySchema(schema)) { - // Maintain a running path to each schema that we look at. This will be - // used to determine where to look for a property value within an example - // object, relative to that example's location. - if (isSchemaProperty(path)) { - propertyPath.push(path.at(-1)); - } - - // Keep a running hierarchy of all examples we find as we look through - // the schemas in the API. Nested properties may only have an example - // value defined within a parent schema example. - if (schema.example) { - logger.debug( - `${ruleId}: adding example for schema at location: ${path.join('.')}` - ); - logger.debug( - `${ruleId}: - the example path will be : ${propertyPath.join('.')}` - ); - - examples.push({ - example: schema.example, - examplePath: [ ...propertyPath ], - }); - } + // Use a composition-aware utility to gather any examples relevant to this + // schema, including those defined on applicator schemas in oneOf, etc. + const schemaExamples = getExamplesForSchema(schema); + logger.debug(`${ruleId}: ${schemaExamples.length} examples found`); - // Add sentinels for arrays/dictionaries to the running path, - // to assist the example-parsing logic later on. This must come - // after we push the example to the list. - if (isSchemaProperty(path)) { - if (isArraySchema(schema)) { - propertyPath.push('[]'); - } - - if (isDictionarySchema(schema)) { - propertyPath.push('{}'); - } - } + if (schemaExamples.length) { + // Index the examples with the stringified logical path. + examples[logicalPath.join('.')] = schemaExamples; } return performValidation( schema, path, - apidef, - propertyPath, - examples + logicalPath, + examples, + indirectExamples ); }); } @@ -143,7 +112,13 @@ function checkForDateBasedFormat(s, p, apidef) { // this function implements the checks: 1) see if the name of a property // indicates that it is a date-based schema and 2) see if the example value for // a schema indicates that it is a date-based schema. -function performValidation(schema, path, apidef, propertyPath, examples) { +function performValidation( + schema, + path, + logicalPath, + examples, + indirectExamples +) { // If this is already a date or date-time schema, no need to check if it should be. if (isDateSchema(schema) || isDateTimeSchema(schema)) { logger.debug( @@ -186,19 +161,10 @@ function performValidation(schema, path, apidef, propertyPath, examples) { // Check example values for string schemas. if (isStringSchema(schema)) { - // Make a modifiable copy of the path. - propertyPath = [ ...propertyPath ]; - - // If this is a property, we need to include its name in the path. - if (isSchemaProperty(path)) { - propertyPath.push(path.at(-1)); - } - - // Either use the schema example directly or search the list of examples - // for an example object that contains a value for this property. - // !!! look for schemas own example needs to be composition-aware - can use Dan's utility - const exampleValue = schema.example || findExample(propertyPath, examples); - if (exampleValue) { + // Search the list of examples for an example object that contains a value + // for this property. + const exampleValue = findExample(logicalPath, examples, indirectExamples); + if (exampleValue !== undefined) { logger.debug( `${ruleId}: example value found for string schema at location ${path.join( '.' @@ -223,44 +189,100 @@ function performValidation(schema, path, apidef, propertyPath, examples) { // This function checks all of the examples we've gathered while processing // schemas to check if once of them defines a value for the specific property // or string schema that we are looking at. It returns the first value found. -function findExample(propertyPath, examples) { - let exampleValue; - - // According to the OpenAPI specification, Media Type/Parameter examples - // override any examples defined on the schemas themselves. Going "in order" - // through this loop ensures we prioritize those examples, followed by - // higher-level schema examples. If it turns out that we should prioritize - // nested examples, we can simply reverse this loop. - for (const { example, examplePath } of examples) { - // First thing is to find the relevant segment of the property path relative - // to the example path, which should be the first element where they differ. - const index = propertyPath.findIndex((prop, i) => prop !== examplePath[i]); - const value = getObjectValueAtPath(example, propertyPath.slice(index)); +function findExample(logicalPath, examples, indirectExamples) { + // First, look at the indirect examples (those included through OpenAPI + // fields) as they are given priority by OpenAPI over native schema examples. + if (indirectExamples.length) { + logger.debug(`${ruleId}: Looking for value within indirect examples.`); + const value = findValueInExamples( + indirectExamples, + logicalPath, + // Indirect examples, like primary schemas, need an empty path array. + [] + ); - // If we find a value, go ahead and break from the loop. - if (value) { + // If we find a value, go ahead and return. + if (value !== undefined) { logger.debug( - `${ruleId}: value found in example at location: ${examplePath.join( - '.' - )}` + `${ruleId}: example value found in indirect example: ${value}` ); - logger.debug(`${ruleId}: property path is : ${propertyPath.join('.')}`); - exampleValue = value; - break; + return value; } } - if (exampleValue === undefined) { + // Look for examples at different paths up the logical path. + const examplePath = [...logicalPath]; + + // Look at each level of hierarchy of the example path, INCLUDING examples on + // primary schemas, which will have an example path of an empty array. For + // that reason, we can't loop through the example path elements, as we would + // miss the iteration with no elements in the array. Using a simple for-loop + // that executes one more time than there are segments in the example path, + // we execute this logic for the empty path scenario as well, and thereby + // properly handle primary schemas. The length of the example path is used to + // initialize the counter, rather than in the conditional, since the array + // will change length as we progress. + for (let i = examplePath.length; i > -1; i--) { logger.debug( - `${ruleId}: no example value found for schema at location: ${propertyPath.join( - '.' + `${ruleId}: Looking at example at logical path ${logicalPathForLogger( + examplePath )}` ); + const value = findValueInExamples( + examples[examplePath.join('.')], + logicalPath, + examplePath + ); + + // If we find a value, go ahead and return. + if (value !== undefined) { + logger.debug( + `${ruleId}: example value found in schema example at logical path: ${logicalPathForLogger( + examplePath + )}: ${value}` + ); + + return value; + } + + // Pop the last element to keep looking up the path. + examplePath.pop(); } // This will return `undefined` if we never find a value; - return exampleValue; + logger.debug( + `${ruleId}: no example value found for schema at logical path: ${logicalPathForLogger( + logicalPath + )}` + ); +} + +function findValueInExamples(examples, logicalPath, examplePath) { + if (examples && examples.length) { + // First thing is to find the relevant segment of the logical path relative + // to the example path, which should be the first element where they differ. + const index = logicalPath.findIndex((prop, i) => prop !== examplePath[i]); + + // This means that the logical paths are the same, i.e. the examples are + // primitives defined directly on the schema. + if (index === -1) { + // Return the first defined example. + return examples.find(e => e !== undefined); + } + + const relativeLogicalPath = logicalPath.slice(index); + for (const example of examples) { + const value = getObjectValueAtPath(example, relativeLogicalPath); + + // If we find a value, go ahead and return it. + if (value !== undefined) { + return value; + } + } + } + + return undefined; } // This function takes an object, as well as a path to a specific value, and @@ -279,7 +301,7 @@ function getObjectValueAtPath(obj, pathToValue) { } // Make a modifiable copy. - pathToValue = [ ...pathToValue ]; + pathToValue = [...pathToValue]; const p = pathToValue.shift(); @@ -289,7 +311,7 @@ function getObjectValueAtPath(obj, pathToValue) { } // Check for sentinel indicating a dictionary. - if (p === '{}' && isObject(obj) && Object.values(obj).length) { + if (p === '*' && isObject(obj) && Object.values(obj).length) { return getObjectValueAtPath(Object.values(obj)[0], pathToValue); } @@ -304,6 +326,12 @@ function getObjectValueAtPath(obj, pathToValue) { // "Indirect" examples are those coming from request bodies, response bodies, and parameters. function checkForIndirectExamples(path, apidef) { + logger.debug( + `${ruleId}: checking indirect examples for schema at location: ${path.join( + '.' + )}` + ); + // Parameter and Media Type objects have the same format when it comes // to examples, so we can treat all of these scenarios the same way. if ( @@ -333,12 +361,7 @@ function checkForIndirectExamples(path, apidef) { ); // Put the examples in the format the downstream algorithm for this rule needs. - return examples.map(example => { - return { - example, - examplePath: [], // All top-level examples get an empty array for the path. - }; - }); + return examples; } return []; @@ -368,12 +391,11 @@ function getOpenApiExamples(artifact) { return []; } -// This function determines if a schema is a "dictionary" (as opposed to a -// standard model with static properties) based on the presence of either -// `additionalProperties` or `patternProperties` (OpenAPI 3.1 only). -function isDictionarySchema(schema) { - return schemaHasConstraint( - schema, - s => isObjectSchema(s) && (s.additionalProperties || s.patternProperties) - ); +// Format the logical path in a way that makes sense when the array is empty. +function logicalPathForLogger(logicalPath) { + if (!logicalPath.length) { + return `'' (primary schema)`; + } + + return `'${logicalPath.join('.')}'`; } diff --git a/packages/ruleset/test/rules/use-date-based-format.test.js b/packages/ruleset/test/rules/use-date-based-format.test.js index c268ad0b..bf1a33ef 100644 --- a/packages/ruleset/test/rules/use-date-based-format.test.js +++ b/packages/ruleset/test/rules/use-date-based-format.test.js @@ -3012,10 +3012,10 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.properties.first_completed.oneOf.0', - 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.first_completed.oneOf.0', - 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.properties.first_completed.oneOf.0', - 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.properties.first_completed.oneOf.0', + 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.properties.first_completed', + 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.first_completed', + 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.properties.first_completed', + 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.properties.first_completed', ]; for (const i in results) {