Merge pull request #52 from hyperjump-io/discriminator

Add support for discriminator identification for anyOf

Author: arpitkuriyal

package-lock.json

@@ -39,6 +39,7 @@
   "dependencies": {
     "@fluent/bundle": "^0.19.1",
     "@hyperjump/browser": "^1.3.1",
+    "@hyperjump/json-pointer": "^1.1.1",
     "@hyperjump/json-schema": "^1.16.0",
     "@hyperjump/pact": "^1.4.0",
     "leven": "^4.0.0"

package.json

@@ -1,31 +1,38 @@
 import { getSchema } from "@hyperjump/json-schema/experimental";
-import * as Schema from "@hyperjump/browser";
 import * as Instance from "@hyperjump/json-schema/instance/experimental";
+import * as Schema from "@hyperjump/browser";
+import * as JsonPointer from "@hyperjump/json-pointer";
 import { getErrors } from "../error-handling.js";
 
 /**
  * @import { ErrorHandler, ErrorObject, NormalizedOutput } from "../index.d.ts"
  */
 
 /** @type ErrorHandler */
-const anyOf = async (normalizedErrors, instance, localization) => {
+const anyOfErrorHandler = async (normalizedErrors, instance, localization) => {
   /** @type ErrorObject[] */
   const errors = [];
 
   if (normalizedErrors["https://json-schema.org/keyword/anyOf"]) {
     for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/anyOf"]) {
+      const allAlternatives = normalizedErrors["https://json-schema.org/keyword/anyOf"][schemaLocation];
+      if (typeof allAlternatives === "boolean") {
+        continue;
+      }
+
       /** @type NormalizedOutput[] */
       const alternatives = [];
-      const allAlternatives = /** @type NormalizedOutput[] */ (normalizedErrors["https://json-schema.org/keyword/anyOf"][schemaLocation]);
       for (const alternative of allAlternatives) {
-        if (Object.values(alternative[Instance.uri(instance)]["https://json-schema.org/keyword/type"]).every((valid) => valid)) {
+        if (Object.values(alternative[Instance.uri(instance)]["https://json-schema.org/keyword/type"] ?? {}).every((valid) => valid)) {
           alternatives.push(alternative);
         }
       }
-      // case 1 where no. alternative matched the type of the instance.
+
+      // No alternative matched the type of the instance.
       if (alternatives.length === 0) {
         /** @type Set<string> */
         const expectedTypes = new Set();
+
         for (const alternative of allAlternatives) {
           for (const instanceLocation in alternative) {
             if (instanceLocation === Instance.uri(instance)) {
@@ -37,28 +44,114 @@ const anyOf = async (normalizedErrors, instance, localization) => {
             }
           }
         }
+
         errors.push({
           message: localization.getTypeErrorMessage([...expectedTypes], Instance.typeOf(instance)),
           instanceLocation: Instance.uri(instance),
           schemaLocation: schemaLocation
         });
-      } else if (alternatives.length === 1) { // case 2 when only one type match
-        return getErrors(alternatives[0], instance, localization);
-      } else if (instance.type === "object") {
-        let targetAlternativeIndex = -1;
-        for (const alternative of alternatives) {
-          targetAlternativeIndex++;
+        continue;
+      }
+
+      // Only one alternative matches the type of the instance
+      if (alternatives.length === 1) {
+        errors.push(...await getErrors(alternatives[0], instance, localization));
+        continue;
+      }
+
+      if (instance.type === "object") {
+        const definedProperties = allAlternatives.map((alternative) => {
+          /** @type Set<string> */
+          const alternativeProperties = new Set();
+
           for (const instanceLocation in alternative) {
-            if (instanceLocation !== "#") {
-              return getErrors(alternatives[targetAlternativeIndex], instance, localization);
+            const pointer = instanceLocation.slice(Instance.uri(instance).length + 1);
+            if (pointer.length > 0) {
+              const position = pointer.indexOf("/");
+              const propertyName = pointer.slice(0, position === -1 ? undefined : position);
+              const location = JsonPointer.append(propertyName, Instance.uri(instance));
+              alternativeProperties.add(location);
             }
           }
+
+          return alternativeProperties;
+        });
+
+        const discriminator = definedProperties.reduce((acc, properties) => {
+          return acc.intersection(properties);
+        }, definedProperties[0]);
+
+        const discriminatedAlternatives = alternatives.filter((alternative) => {
+          for (const instanceLocation in alternative) {
+            if (!discriminator.has(instanceLocation)) {
+              continue;
+            }
+
+            let valid = true;
+            for (const keyword in alternative[instanceLocation]) {
+              for (const schemaLocation in alternative[instanceLocation][keyword]) {
+                if (alternative[instanceLocation][keyword][schemaLocation] !== true) {
+                  valid = false;
+                  break;
+                }
+              }
+            }
+            if (valid) {
+              return true;
+            }
+          }
+          return false;
+        });
+
+        // Discriminator match
+        if (discriminatedAlternatives.length === 1) {
+          errors.push(...await getErrors(discriminatedAlternatives[0], instance, localization));
+          continue;
         }
+
+        // Discriminator identified, but none of the alternatives match
+        if (discriminatedAlternatives.length === 0) {
+          // TODO: How do we handle this case?
+        }
+
+        // Last resort, select the alternative with the most properties matching the instance
+        // TODO: We shouldn't use this strategy if alternatives have the same number of matching instances
+        const instanceProperties = new Set(Instance.values(instance)
+          .map((node) => Instance.uri(node)));
+        let maxMatches = -1;
+        let selectedIndex = 0;
+        let index = -1;
+        for (const alternativeProperties of definedProperties) {
+          index++;
+          const matches = alternativeProperties.intersection(instanceProperties).size;
+          if (matches > maxMatches) {
+            selectedIndex = index;
+          }
+        }
+
+        errors.push(...await getErrors(alternatives[selectedIndex], instance, localization));
+        continue;
       }
+
+      // TODO: Handle alternatives with const
+      // TODO: Handle alternatives with enum
+      // TODO: Handle null alternatives
+      // TODO: Handle boolean alternatives
+      // TODO: Handle string alternatives
+      // TODO: Handle array alternatives
+      // TODO: Handle alternatives without a type
+
+      // TODO: If we get here, we don't know what else to do and give a very generic message
+      // Ideally this should be replace by something that can handle whatever case is missing.
+      errors.push({
+        message: localization.getAnyOfErrorMessage(),
+        instanceLocation: Instance.uri(instance),
+        schemaLocation: schemaLocation
+      });
     }
   }
 
   return errors;
 };
 
-export default anyOf;
+export default anyOfErrorHandler;

src/error-handlers/anyOf.js

@@ -961,7 +961,7 @@ describe("Error messages", async () => {
     ]);
   });
 
-  test.skip("anyOf - const-based discriminator mismatch", async () => {
+  test.skip("anyOf - discriminator with no matches", async () => {
     registerSchema({
       $schema: "https://json-schema.org/draft/2020-12/schema",
       anyOf: [
@@ -1022,6 +1022,65 @@ describe("Error messages", async () => {
     ]);
   });
 
+  test("anyOf - discriminator with one match", async () => {
+    registerSchema({
+      $schema: "https://json-schema.org/draft/2020-12/schema",
+      anyOf: [
+        {
+          type: "object",
+          properties: {
+            type: { const: "a" },
+            apple: { type: "string" }
+          },
+          required: ["type"]
+        },
+        {
+          type: "object",
+          properties: {
+            type: { const: "b" },
+            banana: { type: "string" }
+          },
+          required: ["type"]
+        }
+      ]
+    }, schemaUri);
+
+    const instance = {
+      type: "a",
+      apple: 42,
+      banana: "yellow"
+    };
+
+    /** @type OutputFormat */
+    const output = {
+      valid: false,
+      errors: [
+        {
+          absoluteKeywordLocation: `https://example.com/main#/anyOf/0/properties/apple/type`,
+          instanceLocation: "#/apple"
+        },
+        {
+          absoluteKeywordLocation: `https://example.com/main#/anyOf/1/properties/type/const`,
+          instanceLocation: "#/type"
+        },
+        {
+          absoluteKeywordLocation: `https://example.com/main#/anyOf`,
+          instanceLocation: "#"
+        }
+      ]
+    };
+
+    const result = await betterJsonSchemaErrors(output, schemaUri, instance);
+
+    expect(result.errors).to.eql([
+      {
+        schemaLocation: `https://example.com/main#/anyOf/0/properties/apple/type`,
+        instanceLocation: "#/apple",
+        message: localization.getTypeErrorMessage("string", "number")
+      }
+    ]);
+  });
+
   test("anyOf - using $ref in alternatives", async () => {
     const subjectUri = "https://example.com/main";
 

src/keyword-error-message.test.js

@@ -259,4 +259,9 @@ export class Localization {
 
     return this._formatMessage("enum-error", formattedArgs);
   }
+
+  /** @type () => string */
+  getAnyOfErrorMessage() {
+    return this._formatMessage("anyOf-error");
+  }
 }

src/localization.js

@@ -1,5 +1,6 @@
-import { evaluateSchema } from "../normalized-output.js";
 import * as Instance from "@hyperjump/json-schema/instance/experimental";
+import * as JsonPointer from "@hyperjump/json-pointer";
+import { evaluateSchema } from "../normalized-output.js";
 
 /**
  * @import { KeywordHandler, NormalizedOutput } from "../index.d.ts"
@@ -15,10 +16,13 @@ const properties = {
     for (const propertyName in properties) {
       const propertyNode = Instance.step(propertyName, instance);
       if (!propertyNode) {
-        continue;
+        errors.push({
+          [JsonPointer.append(propertyName, Instance.uri(instance))]: {}
+        });
+      } else {
+        errors.push(evaluateSchema(properties[propertyName], propertyNode, context));
+        context.evaluatedProperties?.add(propertyName);
       }
-      errors.push(evaluateSchema(properties[propertyName], propertyNode, context));
-      context.evaluatedProperties?.add(propertyName);
     }
 
     return errors;

src/normalization-handlers/properties.js

@@ -85,8 +85,8 @@ export const evaluateSchema = (schemaLocation, instance, context) => {
 /** @type (a: API.NormalizedOutput, b: API.NormalizedOutput) => void */
 const mergeOutput = (a, b) => {
   for (const instanceLocation in b) {
+    a[instanceLocation] ??= {};
     for (const keywordUri in b[instanceLocation]) {
-      a[instanceLocation] ??= {};
       a[instanceLocation][keywordUri] ??= {};
 
       Object.assign(a[instanceLocation][keywordUri], b[instanceLocation][keywordUri]);

src/normalized-output.js

@@ -138,6 +138,7 @@ describe("Error Output Normalization", async () => {
           "https://example.com/main#/type": true
         }
       },
+      "#/name": {},
       "#/age": {
         "https://json-schema.org/keyword/type": {
           "https://example.com/main#/properties/age/type": false
@@ -213,7 +214,8 @@ describe("Error Output Normalization", async () => {
         "https://json-schema.org/keyword/type": {
           "https://example.com/main#/$defs/profile/properties/name/type": false
         }
-      }
+      },
+      "#/profile/age": {}
     });
   });
 
@@ -313,7 +315,8 @@ describe("Error Output Normalization", async () => {
         "https://json-schema.org/keyword/type": {
           "https://example.com/main#/$defs/profile/properties/name/type": false
         }
-      }
+      },
+      "#/profile/age": {}
     });
   });
 

src/normalized-output.test.js

@@ -1,32 +1,38 @@
+# Non-type specific messages
 type-error = The instance should be of type {$expected} but found {$actual}.
+const-error = The instance should be equal to {$expectedValue}.
+enum-error = { $variant ->
+	[suggestion] Unexpected value {$instanceValue}. Did you mean {$suggestion}?
+	*[fallback] Unexpected value {$instanceValue}. Expected one of: {$allowedValues}.
+}
 
+# String messages
 string-error = Expected a string {$constraints}.
 string-error-minLength = at least {$minLength} characters long
 string-error-maxLength = at most {$maxLength} characters long
+pattern-error = The instance should match the pattern: {$pattern}.
+format-error = The instance should match the format: {$format}.
 
+# Number messages
 number-error = Expected a number {$constraints}.
 number-error-minimum = greater than {$minimum}
 number-error-exclusive-minimum = greater than or equal to {$minimum}
 number-error-maximum = less than {$maximum}
 number-error-exclusive-maximum = less than or equal to {$maximum}
-
-required-error = This instance is missing required property(s): {$missingProperties}.
 multiple-of-error = The instance should be a multiple of {$divisor}.
 
+# Object messages
 properties-error = Expected object to have {$constraints}
 properties-error-max = at most {$maxProperties} properties.
 properties-error-min = at least {$minProperties} properties.
+required-error = This instance is missing required property(s): {$missingProperties}.
+additional-properties-error = The property "{$propertyName}" is not allowed.
 
-const-error = The instance should be equal to {$expectedValue}.
-
+# Array messages
 array-error = Expected the array to have {$constraints}.
 array-error-min = at least {$minItems} items
 array-error-max = at most {$maxItems} items
-
 unique-items-error = The instance should have unique items in the array.
-format-error = The instance should match the format: {$format}.
-pattern-error = The instance should match the pattern: {$pattern}.
-
 contains-error-min = The array must contain at least {$minContains -> 
   [one] item that passes
 	*[other] items that pass
@@ -36,9 +42,6 @@ contains-error-min-max = The array must contain at least {$minContains} and at m
 	*[other] items that pass
 } the 'contains' schema.
 
+# Conditional messages
+anyOf-error = The instance must pass at least one of the given schemas.
 not-error = The instance is not allowed to be used in this schema.
-additional-properties-error = The property "{$propertyName}" is not allowed.
-enum-error = { $variant ->
-	[suggestion] Unexpected value {$instanceValue}. Did you mean {$suggestion}?
-	*[fallback] Unexpected value {$instanceValue}. Expected one of: {$allowedValues}.
-}