fix(ibm-api-symmetry): maintain paths when resolving reference schemas (

#658) The `ibm-api-symmetry` code is losing track of the path to the canonical schema when "reference" schemas are resolved during processing, leading to false positives. This resolves the issue. Signed-off-by: Dustin Popp <[email protected]>
IBM · Apr 3, 2024 · db7eda1 · db7eda1
1 parent 67676ad
commit db7eda1
Show file tree

Hide file tree

Showing 2 changed files with 114 additions and 30 deletions.
diff --git a/packages/ruleset/src/functions/api-symmetry.js b/packages/ruleset/src/functions/api-symmetry.js
@@ -215,15 +215,15 @@ function checkForGraphFragmentPattern(
             canonical,
             canonicalPath,
             schemaFinder,
-            s =>
+            (schema, path) =>
               // Note that these first two conditions are guaranteed to be met at
               // least once by the first call to `canonicalSchemaMeetsConstraint`
-              'properties' in s &&
-              isObject(s.properties[name]) &&
+              'properties' in schema &&
+              isObject(schema.properties[name]) &&
               isGraphFragment(
                 prop,
-                s.properties[name],
-                [...canonicalPath, 'properties', name],
+                schema.properties[name],
+                [...path, 'properties', name],
                 false
               )
           )
@@ -243,16 +243,16 @@ function checkForGraphFragmentPattern(
             canonical,
             canonicalPath,
             schemaFinder,
-            s =>
+            (schema, path) =>
               // Note that these first two conditions are guaranteed to be met at
               // least once by the first call to `canonicalSchemaMeetsConstraint`
-              'properties' in s &&
-              isObject(s.properties[name]) &&
-              isObject(s.properties[name].items) &&
+              'properties' in schema &&
+              isObject(schema.properties[name]) &&
+              isObject(schema.properties[name].items) &&
               isGraphFragment(
                 prop.items,
-                s.properties[name].items,
-                [...canonicalPath, 'properties', name, 'items'],
+                schema.properties[name].items,
+                [...path, 'properties', name, 'items'],
                 false
               )
           )
@@ -270,21 +270,18 @@ function checkForGraphFragmentPattern(
     ['allOf', 'oneOf', 'anyOf'].forEach(applicator => {
       if (
         Array.isArray(variant[applicator]) &&
-        variant[applicator].length > 0
+        variant[applicator].length > 0 &&
+        !variant[applicator].reduce(
+          (previousResult, v) =>
+            previousResult &&
+            isGraphFragment(v, canonical, canonicalPath, true),
+          true
+        )
       ) {
-        if (
-          !variant[applicator].reduce(
-            (previousResult, v) =>
-              previousResult &&
-              isGraphFragment(v, canonical, canonicalPath, true),
-            true
-          )
-        ) {
-          logger.info(
-            `${ruleId}: variant schema applicator '${applicator}' is not a graph fragment of the canonical schema`
-          );
-          result = false;
-        }
+        logger.info(
+          `${ruleId}: variant schema applicator '${applicator}' is not a graph fragment of the canonical schema`
+        );
+        result = false;
       }
     });
 
@@ -313,13 +310,13 @@ function checkForGraphFragmentPattern(
           for (const [name, prop] of Object.entries(c.properties)) {
             logger.debug(`${ruleId}: checking canonical property '${name}'`);
 
-            const included = schemaHasLooseConstraint(variant, s => {
-              return (
+            const included = schemaHasLooseConstraint(
+              variant,
+              s =>
                 'properties' in s &&
                 isObject(s.properties[name]) &&
                 getSchemaType(s.properties[name]) === getSchemaType(prop)
-              );
-            });
+            );
 
             logger.debug(
               `${ruleId}: finished checking canonical property '${name}', included=${included}`
@@ -417,7 +414,7 @@ function canonicalSchemaMeetsConstraint(
     path = ['components', 'schemas', canonicalVersion];
   }
 
-  if (hasConstraint(schema)) {
+  if (hasConstraint(schema, path)) {
     return true;
   }
 

diff --git a/packages/ruleset/test/api-symmetry.test.js b/packages/ruleset/test/api-symmetry.test.js
@@ -531,6 +531,93 @@ describe(`Spectral rule: ${ruleId}`, () => {
       expect(results).toHaveLength(0);
     });
 
+    it('complex prototype schema with multiple, nested reference schemas is compliant', async () => {
+      const testDocument = makeCopy(rootDocument);
+      testDocument.components.schemas.Filmmaker = {
+        type: 'object',
+        properties: {
+          id: { type: 'string' },
+          name: { type: 'string' },
+          age: { type: 'integer' },
+          education: {
+            allOf: [
+              { $ref: '#/components/schemas/SchoolReference' },
+              { description: 'overwritten description for education' },
+            ],
+          },
+        },
+      };
+      testDocument.components.schemas.FilmmakerReference = {
+        type: 'object',
+        properties: {
+          id: { type: 'string' },
+          name: { type: 'string' },
+        },
+      };
+      testDocument.components.schemas.School = {
+        type: 'object',
+        properties: {
+          id: { type: 'string' },
+          name: { type: 'string' },
+          location: { type: 'string' },
+        },
+      };
+      testDocument.components.schemas.SchoolReference = {
+        type: 'object',
+        properties: {
+          id: { type: 'string' },
+          name: { type: 'string' },
+        },
+      };
+
+      testDocument.components.schemas.GeneralIdentity = {
+        type: 'object',
+        properties: {
+          id: { type: 'string' },
+        },
+      };
+      testDocument.components.schemas.DirectorPrototype = {
+        oneOf: [
+          { $ref: '#/components/schemas/GeneralIdentity' },
+          { $ref: '#/components/schemas/DirectorContext' },
+        ],
+      };
+      testDocument.components.schemas.DirectorContext = {
+        type: 'object',
+        properties: {
+          name: { type: 'string' },
+          education: { $ref: '#/components/schemas/SchoolPrototype' },
+        },
+      };
+      testDocument.components.schemas.SchoolPrototype = {
+        oneOf: [
+          { $ref: '#/components/schemas/GeneralIdentity' },
+          { $ref: '#/components/schemas/SchoolContext' },
+        ],
+      };
+      testDocument.components.schemas.SchoolContext = {
+        type: 'object',
+        properties: {
+          name: { type: 'string' },
+          location: { type: 'string' },
+        },
+      };
+
+      testDocument.components.schemas.Movie.properties.director = {
+        $ref: '#/components/schemas/FilmmakerReference',
+      };
+
+      testDocument.components.schemas.MoviePrototype.properties.director = {
+        allOf: [
+          { $ref: '#/components/schemas/DirectorPrototype' },
+          { description: 'Director type for creating a movie' },
+        ],
+      };
+
+      const results = await testRule(ruleId, rule, testDocument);
+      expect(results).toHaveLength(0);
+    });
+
     // Already covered in root document:
     // - Valid Prototype schemas
     // - Valid Patch schemas