test: improve guarantees for validate function call ordering

Signed-off-by: Dan Hudlow <[email protected]>

packages/utilities/src/utils/validate-composed-schemas.js

@@ -28,9 +28,13 @@ const SchemaPath = require('./schema-path');
  * - `schema` — the composed schema
  * - `path` — the array of path segments to locate the composed schema within the resolved document
  *
- * The provided `validate()` function is guaranteed to be called for a schema before any of its
- * composed schemas. However, it is not guaranteed that the `validate()` function is called in any
- * particular order for a schema's composed schemas.
+ * The provided `validate()` function is guaranteed to be called:
+ * - for a schema before any of its composed schemas
+ * - more recently for the schema that composes it (its "composition parent") than for that schema's
+ *    siblings (or their descendants) in the composition tree
+ *
+ * However, it is not guaranteed that the `validate()` function is called in any particular order
+ * for a schema's directly composed schemas.
  *
  * WARNING: It is only safe to use this function for a "resolved" schema — it cannot traverse `$ref`
  * references.

packages/utilities/src/utils/validate-nested-schemas.js

@@ -39,9 +39,13 @@ const SchemaPath = require('./schema-path');
  *    Within the logical path, an arbitrary array item is represented by `[]` and an arbitrary
  *    dictionary property is represented by `*`.
  *
- * The provided `validate()` function is guaranteed to be called for a schema before any of its
- * nested schemas. However, it is not guaranteed that the `validate()` function is called in any
- * particular order for a schema's nested schemas.
+ * The provided `validate()` function is guaranteed to be called:
+ * - for a schema before any of its nested schemas
+ * - more recently for the schema that nests it (its "nesting parent") than for that schema's
+ *    siblings (or their descendants) in the nesting tree
+ *
+ * However, it is not guaranteed that the `validate()` function is called in any particular order
+ * for a schema's directly nested schemas.
  *
  * WARNING: It is only safe to use this function for a "resolved" schema — it cannot traverse `$ref`
  * references.
@@ -120,6 +124,23 @@ function validateNestedSchemas(
     );
   }
 
+  if (
+    schema.patternProperties &&
+    typeof schema.patternProperties === 'object'
+  ) {
+    for (const entry of Object.entries(schema.patternProperties)) {
+      errors.push(
+        ...validateNestedSchemas(
+          entry[1],
+          path.withPatternProperty(entry[0]),
+          validate,
+          true,
+          includeNot
+        )
+      );
+    }
+  }
+
   if (includeNot && schema.not) {
     errors.push(
       ...validateNestedSchemas(
@@ -148,23 +169,6 @@ function validateNestedSchemas(
     }
   }
 
-  if (
-    schema.patternProperties &&
-    typeof schema.patternProperties === 'object'
-  ) {
-    for (const entry of Object.entries(schema.patternProperties)) {
-      errors.push(
-        ...validateNestedSchemas(
-          entry[1],
-          path.withPatternProperty(entry[0]),
-          validate,
-          true,
-          includeNot
-        )
-      );
-    }
-  }
-
   return errors;
 }
 

packages/utilities/src/utils/validate-subschemas.js

@@ -42,9 +42,15 @@ const validateNestedSchemas = require('./validate-nested-schemas');
  * The provided `validate()` function is guaranteed to be called:
  * - for a schema before any of its composed schemas
  * - for a schema before any of its nested schemas
+ * - more recently for the schema that composes it (its "composition parent") than for that schema's
+ *    siblings (or their descendants) in the portion of composition tree local to the composition
+ *    parent's branch of the nesting tree
+ * - more recently for the schema that nests it (its "nesting parent") than for that schema's
+ *    siblings (or their descendants) in the portion of nesting tree local to the nesting parent's
+ *    branch of the composition tree
  *
  * However, it is not guaranteed that the `validate()` function is called in any particular order
- * for a schema's composed or nested schemas.
+ * for a schema's directly composed or directly nested schemas.
  *
  * WARNING: It is only safe to use this function for a "resolved" schema — it cannot traverse `$ref`
  * references.

packages/utilities/test/schema-path.test.js

@@ -38,63 +38,111 @@ describe('Utility class: SchemaPath', () => {
       expect(new SchemaPath(['one', 'two'], []).logical).toEqual([]);
     });
     it('logical path is preserved', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).logical).toEqual(['foo', 'bar']);
+      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).logical).toEqual([
+        'foo',
+        'bar',
+      ]);
     });
     it('logical path is immutable', async () => {
       const path = new SchemaPath(['one', 'two'], ['foo', 'bar']);
-      expect(() => path.logical = []).toThrow();
+      expect(() => (path.logical = [])).toThrow();
       expect(() => path.logical.push('three')).toThrow();
     });
   });
   describe('withProperty()', () => {
     it('physical path is correctly appended', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withProperty('three')]).toEqual(['one', 'two', 'properties', 'three']);
+      expect([
+        ...new SchemaPath(['one', 'two'], ['foo', 'bar']).withProperty('three'),
+      ]).toEqual(['one', 'two', 'properties', 'three']);
     });
     it('logical path is correctly appended', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).withProperty('three').logical).toEqual(['foo', 'bar', 'three']);
+      expect(
+        new SchemaPath(['one', 'two'], ['foo', 'bar']).withProperty('three')
+          .logical
+      ).toEqual(['foo', 'bar', 'three']);
     });
   });
   describe('withAdditionalProperty()', () => {
     it('physical path is correctly appended', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withAdditionalProperty()]).toEqual(['one', 'two', 'additionalProperties']);
+      expect([
+        ...new SchemaPath(
+          ['one', 'two'],
+          ['foo', 'bar']
+        ).withAdditionalProperty(),
+      ]).toEqual(['one', 'two', 'additionalProperties']);
     });
     it('logical path is correctly appended', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).withAdditionalProperty('three').logical).toEqual(['foo', 'bar', '*']);
+      expect(
+        new SchemaPath(['one', 'two'], ['foo', 'bar']).withAdditionalProperty(
+          'three'
+        ).logical
+      ).toEqual(['foo', 'bar', '*']);
     });
   });
   describe('withPatternProperty()', () => {
     it('physical path is correctly appended', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withPatternProperty('baz')]).toEqual(['one', 'two', 'patternProperties', 'baz']);
+      expect([
+        ...new SchemaPath(['one', 'two'], ['foo', 'bar']).withPatternProperty(
+          'baz'
+        ),
+      ]).toEqual(['one', 'two', 'patternProperties', 'baz']);
     });
     it('logical path is correctly appended', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).withPatternProperty('baz').logical).toEqual(['foo', 'bar', '*']);
+      expect(
+        new SchemaPath(['one', 'two'], ['foo', 'bar']).withPatternProperty(
+          'baz'
+        ).logical
+      ).toEqual(['foo', 'bar', '*']);
     });
   });
   describe('withArrayItem()', () => {
     it('physical path is correctly appended', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withArrayItem()]).toEqual(['one', 'two', 'items']);
+      expect([
+        ...new SchemaPath(['one', 'two'], ['foo', 'bar']).withArrayItem(),
+      ]).toEqual(['one', 'two', 'items']);
     });
     it('logical path is correctly appended', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).withArrayItem().logical).toEqual(['foo', 'bar', '[]']);
+      expect(
+        new SchemaPath(['one', 'two'], ['foo', 'bar']).withArrayItem().logical
+      ).toEqual(['foo', 'bar', '[]']);
     });
   });
   describe('withApplicator()', () => {
     it('physical path is correctly appended', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withApplicator('oneOf', '0')]).toEqual(['one', 'two', 'oneOf', '0']);
+      expect([
+        ...new SchemaPath(['one', 'two'], ['foo', 'bar']).withApplicator(
+          'oneOf',
+          '0'
+        ),
+      ]).toEqual(['one', 'two', 'oneOf', '0']);
     });
     it('logical path is correctly (not) appended', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).withApplicator('oneOf', '0').logical).toEqual(['foo', 'bar']);
+      expect(
+        new SchemaPath(['one', 'two'], ['foo', 'bar']).withApplicator(
+          'oneOf',
+          '0'
+        ).logical
+      ).toEqual(['foo', 'bar']);
     });
     it('numeric index is coerced to string', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withApplicator('oneOf', 0)]).toEqual(['one', 'two', 'oneOf', '0']);
+      expect([
+        ...new SchemaPath(['one', 'two'], ['foo', 'bar']).withApplicator(
+          'oneOf',
+          0
+        ),
+      ]).toEqual(['one', 'two', 'oneOf', '0']);
     });
   });
   describe('withNot()', () => {
     it('physical path is correctly appended', async () => {
-      expect([...new SchemaPath(['one', 'two'], ['foo', 'bar']).withNot()]).toEqual(['one', 'two', 'not']);
+      expect([
+        ...new SchemaPath(['one', 'two'], ['foo', 'bar']).withNot(),
+      ]).toEqual(['one', 'two', 'not']);
     });
     it('logical path is correctly (not) appended', async () => {
-      expect(new SchemaPath(['one', 'two'], ['foo', 'bar']).withNot().logical).toEqual(['foo', 'bar']);
+      expect(
+        new SchemaPath(['one', 'two'], ['foo', 'bar']).withNot().logical
+      ).toEqual(['foo', 'bar']);
     });
   });
 });

packages/utilities/test/validate-composed-schemas.test.js

@@ -201,6 +201,44 @@ describe('Utility function: validateComposedSchemas()', () => {
     expect(results).toEqual([1, 2, 3, 4, 5]);
   });
 
+  it("should validate a schema's composition parent more recently than its parent's siblings", async () => {
+    const schema = {
+      allOf: [{ oneOf: [{}] }, { oneOf: [{}] }],
+      oneOf: [{ anyOf: [{}] }, { anyOf: [{}] }],
+      anyOf: [{ not: {} }, { not: {} }],
+      not: { allOf: [{}] },
+    };
+
+    const visitedPaths = [];
+
+    validateComposedSchemas(schema, [], (s, p) => {
+      visitedPaths.push(p.join('.'));
+      return [];
+    });
+
+    expect(visitedPaths.indexOf('allOf.0.oneOf.0') - 1).toEqual(
+      visitedPaths.indexOf('allOf.0')
+    );
+    expect(visitedPaths.indexOf('allOf.1.oneOf.0') - 1).toEqual(
+      visitedPaths.indexOf('allOf.1')
+    );
+    expect(visitedPaths.indexOf('oneOf.0.anyOf.0') - 1).toEqual(
+      visitedPaths.indexOf('oneOf.0')
+    );
+    expect(visitedPaths.indexOf('oneOf.1.anyOf.0') - 1).toEqual(
+      visitedPaths.indexOf('oneOf.1')
+    );
+    expect(visitedPaths.indexOf('anyOf.0.not') - 1).toEqual(
+      visitedPaths.indexOf('anyOf.0')
+    );
+    expect(visitedPaths.indexOf('anyOf.1.not') - 1).toEqual(
+      visitedPaths.indexOf('anyOf.1')
+    );
+    expect(visitedPaths.indexOf('not.allOf.0') - 1).toEqual(
+      visitedPaths.indexOf('not')
+    );
+  });
+
   it('recorded paths are accurate for each schema', async () => {
     const schema = {
       allOf: [{ oneOf: [{ anyOf: [{ not: {} }] }] }],

packages/utilities/test/validate-nested-schemas.test.js

@@ -228,6 +228,42 @@ describe('Utility function: validateNestedSchemas()', () => {
     expect(results.sort()).toEqual([1, 2, 3, 4, 5, 6]);
   });
 
+  it("should validate a schema's nesting parent more recently than its parent's siblings", async () => {
+    const schema = {
+      items: { additionalProperties: {} },
+      additionalProperties: { patternProperties: { '^baz$': {} } },
+      patternProperties: {
+        '^foo$': { properties: { baz: {} } },
+        '^bar$': { properties: { baz: {} } },
+      },
+      properties: {
+        a: {},
+        one: { items: {} },
+        z: {},
+      },
+    };
+
+    const visitedPaths = [];
+
+    validateNestedSchemas(schema, [], (s, p) => {
+      visitedPaths.push(p.join('.'));
+      return [];
+    });
+
+    expect(visitedPaths.indexOf('items.additionalProperties') - 1).toEqual(
+      visitedPaths.indexOf('items')
+    );
+    expect(
+      visitedPaths.indexOf('additionalProperties.patternProperties.^baz$') - 1
+    ).toEqual(visitedPaths.indexOf('additionalProperties'));
+    expect(
+      visitedPaths.indexOf('patternProperties.^foo$.properties.baz') - 1
+    ).toEqual(visitedPaths.indexOf('patternProperties.^foo$'));
+    expect(visitedPaths.indexOf('properties.one.items') - 1).toEqual(
+      visitedPaths.indexOf('properties.one')
+    );
+  });
+
   it('should validate through `allOf` schema', async () => {
     const schema = {
       allOf: [{ properties: { inside_all_of: {} } }],