Fix for required annotations for evaluation not collected (#944)

justin-tay · web-flow · commit 2795d79d170d · 2024-02-01T12:40:54.000-05:00
* Fix for required annotations for evaluation not collected due to cross-draft scenario

* Fix incorrect access modifier on setAnnotationCollectionEnabled

* Fix reporting for unevaluated properties
diff --git a/README.md b/README.md
@@ -361,7 +361,7 @@ JsonSchema schema = factory.getSchema(SchemaLocation.of("https://json-schema.org
         
 OutputUnit outputUnit = schema.validate(inputData, InputFormat.JSON, OutputFormat.HIERARCHICAL, executionContext -> {
     executionContext.getExecutionConfig().setAnnotationCollectionEnabled(true);
-    executionContext.getExecutionConfig().setAnnotationCollectionPredicate(keyword -> true);
+    executionContext.getExecutionConfig().setAnnotationCollectionFilter(keyword -> true);
 });
 ```
 The following is sample output from the Hierarchical format.
@@ -438,7 +438,7 @@ The following is sample output from the Hierarchical format.
 | Name                           | Description                                                                                                                                                                                                                       | Default Value
 |--------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------
 | `annotationCollectionEnabled`  | Controls whether annotations are collected during processing. Note that collecting annotations will adversely affect performance.                                                                                                 | `false`
-| `annotationCollectionPredicate`| The predicate used to control which keyword to collect and report annotations for. This requires `annotationCollectionEnabled` to be `true`.                                                                                      | `keyword -> false`
+| `annotationCollectionFilter`   | The predicate used to control which keyword to collect and report annotations for. This requires `annotationCollectionEnabled` to be `true`.                                                                                      | `keyword -> false`
 | `locale`                       | The locale to use for generating messages in the `ValidationMessage`. Note that this value is copied from `SchemaValidatorsConfig` for each execution.                                                                            | `Locale.getDefault()`
 | `failFast`                     | Whether to return failure immediately when an assertion is generated. Note that this value is copied from `SchemaValidatorsConfig` for each execution but is automatically set to `true` for the Boolean and Flag output formats. | `false`
 | `formatAssertionsEnabled`      | The default is to generate format assertions from Draft 4 to Draft 7 and to only generate annotations from Draft 2019-09. Setting to `true` or `false` will override the default behavior.                                        | `null`
diff --git a/src/main/java/com/networknt/schema/AbstractJsonValidator.java b/src/main/java/com/networknt/schema/AbstractJsonValidator.java
@@ -93,7 +93,7 @@ protected boolean collectAnnotations(ExecutionContext executionContext) {
      */
     protected boolean collectAnnotations(ExecutionContext executionContext, String keyword) {
         return executionContext.getExecutionConfig().isAnnotationCollectionEnabled()
-                && executionContext.getExecutionConfig().getAnnotationCollectionPredicate().test(keyword);
+                && executionContext.getExecutionConfig().getAnnotationCollectionFilter().test(keyword);
     }
 
     /**
diff --git a/src/main/java/com/networknt/schema/BaseJsonValidator.java b/src/main/java/com/networknt/schema/BaseJsonValidator.java
@@ -315,27 +315,37 @@ public String toString() {
         return getEvaluationPath().getName(-1);
     }
 
+    /**
+     * Determines if the keyword exists adjacent in the evaluation path.
+     * <p>
+     * This does not check if the keyword exists in the current meta schema as this
+     * can be a cross-draft case where the properties keyword is in a Draft 7 schema
+     * and the unevaluatedProperties keyword is in an outer Draft 2020-12 schema.
+     * <p>
+     * The fact that the validator exists in the evaluation path implies that the
+     * keyword was valid in whatever meta schema for that schema it was created for.
+     * 
+     * @param keyword the keyword to check
+     * @return true if found
+     */
     protected boolean hasAdjacentKeywordInEvaluationPath(String keyword) {
-        boolean hasValidator = validationContext.getMetaSchema().getKeywords()
-                .get(keyword) != null;
-        if (hasValidator) {
-            JsonSchema schema = getEvaluationParentSchema();
-            while (schema != null) {
-                for (JsonValidator validator : schema.getValidators()) {
-                    if (keyword.equals(validator.getKeyword())) {
-                        hasValidator = true;
-                        break;
-                    }
-                }
-                if (hasValidator) {
+        boolean hasValidator = false;
+        JsonSchema schema = getEvaluationParentSchema();
+        while (schema != null) {
+            for (JsonValidator validator : schema.getValidators()) {
+                if (keyword.equals(validator.getKeyword())) {
+                    hasValidator = true;
                     break;
                 }
-                schema = schema.getEvaluationParentSchema();
             }
+            if (hasValidator) {
+                break;
+            }
+            schema = schema.getEvaluationParentSchema();
         }
         return hasValidator;
     }
-    
+
     @Override
     protected MessageSourceValidationMessage.Builder message() {
         return super.message().schemaNode(this.schemaNode);
@@ -360,7 +370,7 @@ protected boolean collectAnnotations(ExecutionContext executionContext) {
      */
     protected boolean collectAnnotations(ExecutionContext executionContext, String keyword) {
         return executionContext.getExecutionConfig().isAnnotationCollectionEnabled()
-                && executionContext.getExecutionConfig().getAnnotationCollectionPredicate().test(keyword);
+                && executionContext.getExecutionConfig().getAnnotationCollectionFilter().test(keyword);
     }
 
     /**
diff --git a/src/main/java/com/networknt/schema/ExecutionConfig.java b/src/main/java/com/networknt/schema/ExecutionConfig.java
@@ -43,7 +43,7 @@ public class ExecutionConfig {
      * This does not affect annotation collection required for evaluating keywords
      * such as unevaluatedItems or unevaluatedProperties and only affects reporting.
      */
-    private Predicate<String> annotationCollectionPredicate = keyword -> false;
+    private Predicate<String> annotationCollectionFilter = keyword -> false;
 
     /**
      * Since Draft 2019-09 format assertions are not enabled by default.
@@ -126,12 +126,12 @@ public void setFailFast(boolean failFast) {
      * 
      * @return if annotation collection is enabled
      */
-    protected boolean isAnnotationCollectionEnabled() {
+    public boolean isAnnotationCollectionEnabled() {
         return annotationCollectionEnabled;
     }
 
     /**
-     * Sets whether to annotation collection is enabled.
+     * Sets whether the annotation collection is enabled.
      * <p>
      * This does not affect annotation collection required for evaluating keywords
      * such as unevaluatedItems or unevaluatedProperties and only affects reporting.
@@ -141,7 +141,7 @@ protected boolean isAnnotationCollectionEnabled() {
      * 
      * @param annotationCollectionEnabled true to enable annotation collection
      */
-    protected void setAnnotationCollectionEnabled(boolean annotationCollectionEnabled) {
+    public void setAnnotationCollectionEnabled(boolean annotationCollectionEnabled) {
         this.annotationCollectionEnabled = annotationCollectionEnabled;
     }
 
@@ -159,8 +159,8 @@ protected void setAnnotationCollectionEnabled(boolean annotationCollectionEnable
      * @return the predicate to determine if annotation collection is allowed for
      *         the keyword
      */
-    public Predicate<String> getAnnotationCollectionPredicate() {
-        return annotationCollectionPredicate;
+    public Predicate<String> getAnnotationCollectionFilter() {
+        return annotationCollectionFilter;
     }
 
     /**
@@ -173,11 +173,11 @@ public Predicate<String> getAnnotationCollectionPredicate() {
      * This does not affect annotation collection required for evaluating keywords
      * such as unevaluatedItems or unevaluatedProperties and only affects reporting.
      *
-     * @param annotationCollectionPredicate the predicate accepting the keyword
+     * @param annotationCollectionFilter the predicate accepting the keyword
      */
-    public void setAnnotationCollectionPredicate(Predicate<String> annotationCollectionPredicate) {
-        this.annotationCollectionPredicate = Objects.requireNonNull(annotationCollectionPredicate,
-                "annotationCollectionPredicate must not be null");
+    public void setAnnotationCollectionFilter(Predicate<String> annotationCollectionFilter) {
+        this.annotationCollectionFilter = Objects.requireNonNull(annotationCollectionFilter,
+                "annotationCollectionFilter must not be null");
     }
 
 }
diff --git a/src/main/java/com/networknt/schema/UnevaluatedItemsValidator.java b/src/main/java/com/networknt/schema/UnevaluatedItemsValidator.java
@@ -185,10 +185,11 @@ public Set<ValidationMessage> validate(ExecutionContext executionContext, JsonNo
             if (messages.isEmpty()) {
                 valid = true;
             } else {
-                // Report these as unevaluated paths or not matching the unevalutedItems schema
+                // Report these as unevaluated paths or not matching the unevaluatedItems schema
                 messages = messages.stream()
-                        .map(m -> message().instanceNode(node).instanceLocation(m.getInstanceLocation())
+                        .map(m -> message().instanceNode(node).instanceLocation(instanceLocation)
                                 .locale(executionContext.getExecutionConfig().getLocale())
+                                .arguments(m.getInstanceLocation().getName(-1))
                                 .failFast(executionContext.getExecutionConfig().isFailFast()).build())
                         .collect(Collectors.toCollection(LinkedHashSet::new));
             }
diff --git a/src/main/java/com/networknt/schema/UnevaluatedPropertiesValidator.java b/src/main/java/com/networknt/schema/UnevaluatedPropertiesValidator.java
@@ -128,8 +128,10 @@ public Set<ValidationMessage> validate(ExecutionContext executionContext, JsonNo
             // Report these as unevaluated paths or not matching the unevaluatedProperties
             // schema
             messages = messages.stream()
-                    .map(m -> message().instanceNode(node).instanceLocation(m.getInstanceLocation())
+                    .map(m -> message().instanceNode(node).instanceLocation(instanceLocation)
                             .locale(executionContext.getExecutionConfig().getLocale())
+                            .arguments(m.getInstanceLocation().getName(-1))
+                            .property(m.getInstanceLocation().getName(-1))
                             .failFast(executionContext.getExecutionConfig().isFailFast()).build())
                     .collect(Collectors.toCollection(LinkedHashSet::new));
         }
diff --git a/src/main/java/com/networknt/schema/output/HierarchicalOutputUnitFormatter.java b/src/main/java/com/networknt/schema/output/HierarchicalOutputUnitFormatter.java
@@ -47,7 +47,7 @@ public static OutputUnit format(JsonSchema jsonSchema, Set<ValidationMessage> va
         OutputUnitData data = OutputUnitData.from(validationMessages, executionContext);
 
         Map<OutputUnitKey, Boolean> valid = data.getValid();
-        Map<OutputUnitKey, Map<String, String>> errors = data.getErrors();
+        Map<OutputUnitKey, Map<String, Object>> errors = data.getErrors();
         Map<OutputUnitKey, Map<String, Object>> annotations = data.getAnnotations();
         Map<OutputUnitKey, Map<String, Object>> droppedAnnotations = data.getDroppedAnnotations();
         
@@ -66,7 +66,7 @@ public static OutputUnit format(JsonSchema jsonSchema, Set<ValidationMessage> va
         droppedAnnotations.keySet().stream().forEach(k -> buildIndex(k, index, keys, root));
         
         // Process all the data
-        for (Entry<OutputUnitKey, Map<String, String>> error : errors.entrySet()) {
+        for (Entry<OutputUnitKey, Map<String, Object>> error : errors.entrySet()) {
             OutputUnitKey key = error.getKey();
             OutputUnit unit = index.get(key.getEvaluationPath());
             unit.setInstanceLocation(key.getInstanceLocation().toString());
diff --git a/src/main/java/com/networknt/schema/output/ListOutputUnitFormatter.java b/src/main/java/com/networknt/schema/output/ListOutputUnitFormatter.java
@@ -38,7 +38,7 @@ public static OutputUnit format(Set<ValidationMessage> validationMessages, Execu
         OutputUnitData data = OutputUnitData.from(validationMessages, executionContext);
 
         Map<OutputUnitKey, Boolean> valid = data.getValid();
-        Map<OutputUnitKey, Map<String, String>> errors = data.getErrors();
+        Map<OutputUnitKey, Map<String, Object>> errors = data.getErrors();
         Map<OutputUnitKey, Map<String, Object>> annotations = data.getAnnotations();
         Map<OutputUnitKey, Map<String, Object>> droppedAnnotations = data.getDroppedAnnotations();
 
@@ -52,12 +52,12 @@ public static OutputUnit format(Set<ValidationMessage> validationMessages, Execu
             output.setInstanceLocation(key.getInstanceLocation().toString());
 
             // Errors
-            Map<String, String> errorMap = errors.get(key);
+            Map<String, Object> errorMap = errors.get(key);
             if (errorMap != null && !errorMap.isEmpty()) {
                 if (output.getErrors() == null) {
                     output.setErrors(new LinkedHashMap<>());
                 }
-                for (Entry<String, String> errorEntry : errorMap.entrySet()) {
+                for (Entry<String, Object> errorEntry : errorMap.entrySet()) {
                     output.getErrors().put(errorEntry.getKey(), errorEntry.getValue());
                 }
             }
diff --git a/src/main/java/com/networknt/schema/output/OutputUnit.java b/src/main/java/com/networknt/schema/output/OutputUnit.java
@@ -43,7 +43,7 @@ public class OutputUnit {
     private String schemaLocation = null;
     private String instanceLocation = null;
 
-    private Map<String, String> errors = null;
+    private Map<String, Object> errors = null;
 
     private Map<String, Object> annotations = null;
 
@@ -83,11 +83,11 @@ public void setInstanceLocation(String instanceLocation) {
         this.instanceLocation = instanceLocation;
     }
 
-    public Map<String, String> getErrors() {
+    public Map<String, Object> getErrors() {
         return errors;
     }
 
-    public void setErrors(Map<String, String> errors) {
+    public void setErrors(Map<String, Object> errors) {
         this.errors = errors;
     }
 
diff --git a/src/main/java/com/networknt/schema/output/OutputUnitData.java b/src/main/java/com/networknt/schema/output/OutputUnitData.java
@@ -15,6 +15,7 @@
  */
 package com.networknt.schema.output;
 
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -30,15 +31,15 @@
  */
 public class OutputUnitData {
     private final Map<OutputUnitKey, Boolean> valid = new LinkedHashMap<>();
-    private final Map<OutputUnitKey, Map<String, String>> errors = new LinkedHashMap<>();
+    private final Map<OutputUnitKey, Map<String, Object>> errors = new LinkedHashMap<>();
     private final Map<OutputUnitKey, Map<String, Object>> annotations = new LinkedHashMap<>();
     private final Map<OutputUnitKey, Map<String, Object>> droppedAnnotations = new LinkedHashMap<>();
 
     public Map<OutputUnitKey, Boolean> getValid() {
         return valid;
     }
 
-    public Map<OutputUnitKey, Map<String, String>> getErrors() {
+    public Map<OutputUnitKey, Map<String, Object>> getErrors() {
         return errors;
     }
 
@@ -66,11 +67,12 @@ public static String formatMessage(String message) {
         return message;
     }
 
+    @SuppressWarnings("unchecked")
     public static OutputUnitData from(Set<ValidationMessage> validationMessages, ExecutionContext executionContext) {
         OutputUnitData data = new OutputUnitData();
 
         Map<OutputUnitKey, Boolean> valid = data.valid;
-        Map<OutputUnitKey, Map<String, String>> errors = data.errors;
+        Map<OutputUnitKey, Map<String, Object>> errors = data.errors;
         Map<OutputUnitKey, Map<String, Object>> annotations = data.annotations;
         Map<OutputUnitKey, Map<String, Object>> droppedAnnotations = data.droppedAnnotations;
 
@@ -80,15 +82,28 @@ public static OutputUnitData from(Set<ValidationMessage> validationMessages, Exe
             OutputUnitKey key = new OutputUnitKey(assertion.getEvaluationPath().getParent(),
                     assertionSchemaLocation, assertion.getInstanceLocation());
             valid.put(key, false);
-            Map<String, String> errorMap = errors.computeIfAbsent(key, k -> new LinkedHashMap<>());
-            errorMap.put(assertion.getType(), formatMessage(assertion.getMessage()));
+            Map<String, Object> errorMap = errors.computeIfAbsent(key, k -> new LinkedHashMap<>());
+            Object value = errorMap.get(assertion.getType());
+            if (value == null) {
+                errorMap.put(assertion.getType(), formatMessage(assertion.getMessage()));
+            } else {
+                // Existing error, make it into a list
+                if (value instanceof List) {
+                    ((List<String>) value).add(formatMessage(assertion.getMessage()));
+                } else {
+                    List<String> values = new ArrayList<>();
+                    values.add(value.toString());
+                    values.add(formatMessage(assertion.getMessage()));
+                    errorMap.put(assertion.getType(), values);
+                }
+            }
         }
 
         for (List<JsonNodeAnnotation> annotationsResult : executionContext.getAnnotations().asMap().values()) {
             for (JsonNodeAnnotation annotation : annotationsResult) {
                 // As some annotations are required for computation, filter those that are not
                 // required for reporting
-                if (executionContext.getExecutionConfig().getAnnotationCollectionPredicate()
+                if (executionContext.getExecutionConfig().getAnnotationCollectionFilter()
                         .test(annotation.getKeyword())) {
                     SchemaLocation annotationSchemaLocation = new SchemaLocation(
                             annotation.getSchemaLocation().getAbsoluteIri(),
diff --git a/src/main/resources/jsv-messages.properties b/src/main/resources/jsv-messages.properties
@@ -42,8 +42,8 @@ propertyNames = Property name {0} is not valid for validation: {1}
 readOnly = {0}: is a readonly field, it cannot be changed
 required = {0}: required property ''{1}'' not found
 type = {0}: {1} found, {2} expected
-unevaluatedItems = {0}: must not have unevaluated items or must match unevaluated items schema
-unevaluatedProperties = {0}: must not have unevaluated properties
+unevaluatedItems = {0}: item ''{1}'' must not be unevaluated or must match unevaluated items schema
+unevaluatedProperties = {0}: property ''{1}'' must not be unevaluated
 unionType = {0}: {1} found, but {2} is required
 uniqueItems = {0}: the items in the array must be unique
 uuid = {0}: {1} is an invalid {2}
diff --git a/src/test/java/com/networknt/schema/ContentSchemaValidatorTest.java b/src/test/java/com/networknt/schema/ContentSchemaValidatorTest.java
@@ -58,12 +58,12 @@ void annotationCollection() throws JsonProcessingException {
         SchemaValidatorsConfig config = new SchemaValidatorsConfig();
         config.setPathType(PathType.JSON_POINTER);
         JsonSchema schema = factory.getSchema(schemaData, config);
-        
+
         String inputData = "\"helloworld\"";
-        
+
         OutputUnit outputUnit = schema.validate(inputData, InputFormat.JSON, OutputFormat.LIST, executionConfiguration -> {
             executionConfiguration.getExecutionConfig().setAnnotationCollectionEnabled(true);
-            executionConfiguration.getExecutionConfig().setAnnotationCollectionPredicate(keyword -> true);
+            executionConfiguration.getExecutionConfig().setAnnotationCollectionFilter(keyword -> true);
         });
         String output = JsonMapperFactory.getInstance().writeValueAsString(outputUnit);
         String expected = "{\"valid\":true,\"details\":[{\"valid\":true,\"evaluationPath\":\"\",\"schemaLocation\":\"#\",\"instanceLocation\":\"\",\"annotations\":{\"contentMediaType\":\"application/jwt\",\"contentSchema\":{\"type\":\"array\",\"minItems\":2,\"prefixItems\":[{\"const\":{\"typ\":\"JWT\",\"alg\":\"HS256\"}},{\"type\":\"object\",\"required\":[\"iss\",\"exp\"],\"properties\":{\"iss\":{\"type\":\"string\"},\"exp\":{\"type\":\"integer\"}}}]}}}]}";
diff --git a/src/test/java/com/networknt/schema/Issue943Test.java b/src/test/java/com/networknt/schema/Issue943Test.java
diff --git a/src/test/java/com/networknt/schema/OutputUnitTest.java b/src/test/java/com/networknt/schema/OutputUnitTest.java
diff --git a/src/test/resources/schema/unevaluatedTests/unevaluated-tests.json b/src/test/resources/schema/unevaluatedTests/unevaluated-tests.json

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ protected boolean collectAnnotations(ExecutionContext executionContext) {`
`93`	`93`	`*/`
`94`	`94`	`protected boolean collectAnnotations(ExecutionContext executionContext, String keyword) {`
`95`	`95`	`return executionContext.getExecutionConfig().isAnnotationCollectionEnabled()`
`96`		`- && executionContext.getExecutionConfig().getAnnotationCollectionPredicate().test(keyword);`
	`96`	`+ && executionContext.getExecutionConfig().getAnnotationCollectionFilter().test(keyword);`
`97`	`97`	`}`
`98`	`98`
`99`	`99`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ public class OutputUnit {`
`43`	`43`	`private String schemaLocation = null;`
`44`	`44`	`private String instanceLocation = null;`
`45`	`45`
`46`		`- private Map<String, String> errors = null;`
	`46`	`+ private Map<String, Object> errors = null;`
`47`	`47`
`48`	`48`	`private Map<String, Object> annotations = null;`
`49`	`49`
`@@ -83,11 +83,11 @@ public void setInstanceLocation(String instanceLocation) {`
`83`	`83`	`this.instanceLocation = instanceLocation;`
`84`	`84`	`}`
`85`	`85`
`86`		`- public Map<String, String> getErrors() {`
	`86`	`+ public Map<String, Object> getErrors() {`
`87`	`87`	`return errors;`
`88`	`88`	`}`
`89`	`89`
`90`		`- public void setErrors(Map<String, String> errors) {`
	`90`	`+ public void setErrors(Map<String, Object> errors) {`
`91`	`91`	`this.errors = errors;`
`92`	`92`	`}`
`93`	`93`