Repository: networknt/json-schema-validator
Pull request #485 (2021-11-22 17:43:09)

Issue386: FailFast should not cause exception on if
Fix for https://github.com/networknt/json-schema-validator/issues/386

When failFast is enabled the code that builds the ValidationMessages throws an exception rather than returning a Set.
In if/else/then blocks this could cause the validation to fail whenever an 'if' was false.
I've a basic change here to just wrap the if validation in a try/catch if that's appropriate?

Btw, when running the tests I found that Issue366Test wasn't working, as it was looking in the wrong location (but otherwise is not related to this change)


Patch:
diff --git a/pom.xml b/pom.xml
index a6efcc5..9a55940 100644
--- a/pom.xml
+++ b/pom.xml
@@ -106,6 +106,12 @@
             <version>${version.junit}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+            <version>${version.junit}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>
diff --git a/src/main/java/com/networknt/schema/IfValidator.java b/src/main/java/com/networknt/schema/IfValidator.java
index 85269b3..908df06 100644
--- a/src/main/java/com/networknt/schema/IfValidator.java
+++ b/src/main/java/com/networknt/schema/IfValidator.java
@@ -59,10 +59,18 @@ public class IfValidator extends BaseJsonValidator implements JsonValidator {
 
         Set<ValidationMessage> errors = new LinkedHashSet<ValidationMessage>();
 
-        Set<ValidationMessage> ifErrors = ifSchema.validate(node, rootNode, at);
-        if (ifErrors.isEmpty() && thenSchema != null) {
+        boolean ifConditionPassed;
+        try {
+            ifConditionPassed = ifSchema.validate(node, rootNode, at).isEmpty();
+        } catch (JsonSchemaException ex) {
+            // When failFast is enabled, validations are thrown as exceptions.
+            // An exception means the condition failed
+            ifConditionPassed = false;
+        }
+
+        if (ifConditionPassed && thenSchema != null) {
             errors.addAll(thenSchema.validate(node, rootNode, at));
-        } else if (!ifErrors.isEmpty() && elseSchema != null) {
+        } else if (!ifConditionPassed && elseSchema != null) {
             errors.addAll(elseSchema.validate(node, rootNode, at));
         }
 


Test patch:
diff --git a/src/test/java/com/networknt/schema/Issue366FailFast.java b/src/test/java/com/networknt/schema/Issue366FailFastTest.java
similarity index 95%
rename from src/test/java/com/networknt/schema/Issue366FailFast.java
rename to src/test/java/com/networknt/schema/Issue366FailFastTest.java
index 51737e4..0511597 100644
--- a/src/test/java/com/networknt/schema/Issue366FailFast.java
+++ b/src/test/java/com/networknt/schema/Issue366FailFastTest.java
@@ -13,7 +13,7 @@ import java.util.Set;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-public class Issue366FailFast {
+public class Issue366FailFastTest {
 
   @BeforeEach
   public void setup() throws IOException {
@@ -35,7 +35,7 @@ public class Issue366FailFast {
 
     URI uri = getSchema();
 
-    InputStream in = getClass().getResourceAsStream("/draft7/issue366_schema.json");
+    InputStream in = getClass().getResourceAsStream("/schema/issue366_schema.json");
     JsonNode testCases = objectMapper.readValue(in, JsonNode.class);
     this.jsonSchema = schemaFactory.getSchema(uri, testCases,schemaValidatorsConfig);
   }
@@ -75,21 +75,21 @@ public class Issue366FailFast {
   @Test
   public void bothValid() throws Exception {
     String dataPath = "/data/issue366.json";
-    
+
     assertThrows(JsonSchemaException.class, () -> {
         InputStream dataInputStream = getClass().getResourceAsStream(dataPath);
         JsonNode node = getJsonNodeFromStreamContent(dataInputStream);
         List<JsonNode> testNodes = node.findValues("tests");
         JsonNode testNode = testNodes.get(0).get(2);
         JsonNode dataNode = testNode.get("data");
-        jsonSchema.validate(dataNode);        
+        jsonSchema.validate(dataNode);
     });
   }
 
   @Test
   public void neitherValid() throws Exception {
     String dataPath = "/data/issue366.json";
-    
+
     assertThrows(JsonSchemaException.class, () -> {
         InputStream dataInputStream = getClass().getResourceAsStream(dataPath);
         JsonNode node = getJsonNodeFromStreamContent(dataInputStream);
diff --git a/src/test/java/com/networknt/schema/Issue366FailSlow.java b/src/test/java/com/networknt/schema/Issue366FailSlowTest.java
similarity index 99%
rename from src/test/java/com/networknt/schema/Issue366FailSlow.java
rename to src/test/java/com/networknt/schema/Issue366FailSlowTest.java
index 1a8542b..3c0d27f 100644
--- a/src/test/java/com/networknt/schema/Issue366FailSlow.java
+++ b/src/test/java/com/networknt/schema/Issue366FailSlowTest.java
@@ -13,7 +13,7 @@ import java.util.Set;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-public class Issue366FailSlow {
+public class Issue366FailSlowTest {
 
   @BeforeEach
   public void setup() throws IOException {
diff --git a/src/test/java/com/networknt/schema/Issue386Test.java b/src/test/java/com/networknt/schema/Issue386Test.java
new file mode 100644
index 0000000..8f7bebf
--- /dev/null
+++ b/src/test/java/com/networknt/schema/Issue386Test.java
@@ -0,0 +1,75 @@
+package com.networknt.schema;
+
+import java.io.InputStream;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+public class Issue386Test {
+    protected JsonSchema getJsonSchemaFromPathV7(String schemaPath, boolean failFast) {
+        InputStream schemaInputStream = getClass().getResourceAsStream(schemaPath);
+        JsonSchemaFactory factory = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V7);
+        SchemaValidatorsConfig config = new SchemaValidatorsConfig();
+        config.setFailFast(failFast);
+        return factory.getSchema(schemaInputStream, config);
+    }
+
+    protected JsonNode getJsonNodeFromPath(String dataPath) throws Exception {
+        InputStream dataInputStream = getClass().getResourceAsStream(dataPath);
+        ObjectMapper mapper = new ObjectMapper();
+        JsonNode node = mapper.readTree(dataInputStream);
+        return node;
+    }
+
+    @ParameterizedTest
+    @ValueSource(booleans = { true, false } )
+    public void dataIsValid(boolean failFast) throws Exception {
+        String schemaPath = "/schema/issue386-v7.json";
+        String dataPath = "/data/issue386.json";
+        JsonSchema schema = getJsonSchemaFromPathV7(schemaPath, failFast);
+        JsonNode node = getJsonNodeFromPath(dataPath).get("valid");
+        node.forEach(testNode -> {
+            Set<ValidationMessage> errors = schema.validate(testNode.get("data"));
+            Assertions.assertEquals(0, errors.size(), "Expected no errors for " + testNode.get("data"));
+        });
+    }
+
+    @Test
+    public void dataIsInvalidFailFast() throws Exception {
+        String schemaPath = "/schema/issue386-v7.json";
+        String dataPath = "/data/issue386.json";
+        JsonSchema schema = getJsonSchemaFromPathV7(schemaPath, true);
+        JsonNode node = getJsonNodeFromPath(dataPath).get("invalid");
+        node.forEach(testNode -> {
+            try {
+                schema.validate(testNode.get("data"));
+                Assertions.fail();
+            } catch (JsonSchemaException e) {
+                Assertions.assertEquals(testNode.get("expectedErrors").get(0).asText(), e.getMessage());
+            }
+        });
+    }
+
+    @Test
+    public void dataIsInvalidFailSlow() throws Exception {
+        String schemaPath = "/schema/issue386-v7.json";
+        String dataPath = "/data/issue386.json";
+        JsonSchema schema = getJsonSchemaFromPathV7(schemaPath, false);
+        JsonNode node = getJsonNodeFromPath(dataPath).get("invalid");
+        node.forEach(testNode -> {
+            Set<ValidationMessage> errors = schema.validate(testNode.get("data"));
+            List<String> errorMessages = errors.stream().map(x -> x.getMessage()).collect(Collectors.toList());
+            testNode.get("expectedErrors").forEach(expectedError -> {
+                Assertions.assertTrue(errorMessages.contains(expectedError.asText()));
+            });
+        });
+    }
+}
diff --git a/src/test/resources/data/issue386.json b/src/test/resources/data/issue386.json
new file mode 100644
index 0000000..a70a950
--- /dev/null
+++ b/src/test/resources/data/issue386.json
@@ -0,0 +1,49 @@
+{
+  "valid": [
+    {
+      "data": {
+        "street_address": "1600 Pennsylvania Avenue NW",
+        "country": "United States of America",
+        "postal_code": "20500"
+      }
+    },
+    {
+      "data": {
+        "street_address": "1600 Pennsylvania Avenue NW",
+        "postal_code": "20500"
+      }
+    },
+    {
+      "data": {
+        "street_address": "24 Sussex Drive",
+        "country": "Canada",
+        "postal_code": "K1M 1M4"
+      }
+    },
+    {
+      "data": {
+        "street_address": "Adriaan Goekooplaan",
+        "country": "Netherlands",
+        "postal_code": "2517 JX"
+      }
+    }
+  ],
+  "invalid": [
+    {
+      "data": {
+        "street_address": "24 Sussex Drive",
+        "country": "Canada",
+        "postal_code": "10000"
+      },
+      "expectedErrors": ["$.postal_code: does not match the regex pattern [A-Z][0-9][A-Z] [0-9][A-Z][0-9]"]
+    },
+    {
+      "description": "invalid through first then",
+      "data": {
+        "street_address": "1600 Pennsylvania Avenue NW",
+        "postal_code": "K1M 1M4"
+      },
+      "expectedErrors": ["$.postal_code: does not match the regex pattern [0-9]{5}(-[0-9]{4})?"]
+    }
+  ]
+}
\ No newline at end of file
diff --git a/src/test/resources/draft2019-09/if-then-else.json b/src/test/resources/draft2019-09/if-then-else.json
index 1cdeeb9..7b014ae 100644
--- a/src/test/resources/draft2019-09/if-then-else.json
+++ b/src/test/resources/draft2019-09/if-then-else.json
@@ -184,5 +184,183 @@
         "valid": true
       }
     ]
+  },
+  {
+    "description": "conditions by properties",
+    "schema": {
+      "type": "object",
+      "properties": {
+        "street_address": {
+          "type": "string"
+        },
+        "country": {
+          "enum": ["United States of America", "Canada"]
+        }
+      },
+      "if": {
+        "properties": {
+          "country": {
+            "const": "United States of America"
+          }
+        }
+      },
+      "then": {
+        "properties": {
+          "postal_code": {
+            "pattern": "[0-9]{5}(-[0-9]{4})?"
+          }
+        }
+      },
+      "else": {
+        "properties": {
+          "postal_code": {
+            "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]"
+          }
+        }
+      }
+    },
+    "tests": [
+      {
+        "description": "valid through then",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "country": "United States of America",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid through then, alternative match",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid through else",
+        "data": {
+          "street_address": "24 Sussex Drive",
+          "country": "Canada",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": true
+      },
+      {
+        "description": "invalid through else",
+        "data": {
+          "street_address": "24 Sussex Drive",
+          "country": "Canada",
+          "postal_code": "10000"
+        },
+        "valid": false
+      }
+      ,
+      {
+        "description": "invalid through then",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": false
+      }
+    ]
+  },
+  {
+    "description": "conditions by allOf properties",
+    "schema": {
+      "type": "object",
+      "properties": {
+        "street_address": {
+          "type": "string"
+        },
+        "country": {
+          "default": "United States of America",
+          "enum": ["United States of America", "Canada", "Netherlands"]
+        }
+      },
+      "allOf": [
+        {
+          "if": {
+            "properties": { "country": { "const": "United States of America" } }
+          },
+          "then": {
+            "properties": { "postal_code": { "pattern": "[0-9]{5}(-[0-9]{4})?" } }
+          }
+        },
+        {
+          "if": {
+            "properties": { "country": { "const": "Canada" } },
+            "required": ["country"]
+          },
+          "then": {
+            "properties": { "postal_code": { "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]" } }
+          }
+        },
+        {
+          "if": {
+            "properties": { "country": { "const": "Netherlands" } },
+            "required": ["country"]
+          },
+          "then": {
+            "properties": { "postal_code": { "pattern": "[0-9]{4} [A-Z]{2}" } }
+          }
+        }
+      ]
+    },
+    "tests": [
+      {
+        "description": "valid first if",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "country": "United States of America",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid first if, alternative match",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid second if",
+        "data": {
+          "street_address": "24 Sussex Drive",
+          "country": "Canada",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid third if",
+        "data": {
+          "street_address": "Adriaan Goekooplaan",
+          "country": "Netherlands",
+          "postal_code": "2517 JX"
+        },
+        "valid": true
+      },
+      {
+        "description": "invalid through second then",
+        "data": {
+          "street_address": "24 Sussex Drive",
+          "country": "Canada",
+          "postal_code": "10000"
+        },
+        "valid": false
+      },
+      {
+        "description": "invalid through first then",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": false
+      }
+    ]
   }
 ]
diff --git a/src/test/resources/draft7/if-then-else.json b/src/test/resources/draft7/if-then-else.json
index 634aa06..7b014ae 100644
--- a/src/test/resources/draft7/if-then-else.json
+++ b/src/test/resources/draft7/if-then-else.json
@@ -198,24 +198,24 @@
         }
       },
       "if": {
-        "properties": { 
-          "country": { 
-            "const": "United States of America" 
-          } 
+        "properties": {
+          "country": {
+            "const": "United States of America"
+          }
         }
       },
       "then": {
-        "properties": { 
-          "postal_code": { 
-            "pattern": "[0-9]{5}(-[0-9]{4})?" 
-          } 
+        "properties": {
+          "postal_code": {
+            "pattern": "[0-9]{5}(-[0-9]{4})?"
+          }
         }
       },
       "else": {
-        "properties": { 
-          "postal_code": { 
-            "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]" 
-          } 
+        "properties": {
+          "postal_code": {
+            "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]"
+          }
         }
       }
     },
@@ -229,6 +229,14 @@
         },
         "valid": true
       },
+      {
+        "description": "valid through then, alternative match",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
       {
         "description": "valid through else",
         "data": {
@@ -239,7 +247,7 @@
         "valid": true
       },
       {
-        "description": "invalid",
+        "description": "invalid through else",
         "data": {
           "street_address": "24 Sussex Drive",
           "country": "Canada",
@@ -247,6 +255,112 @@
         },
         "valid": false
       }
+      ,
+      {
+        "description": "invalid through then",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": false
+      }
+    ]
+  },
+  {
+    "description": "conditions by allOf properties",
+    "schema": {
+      "type": "object",
+      "properties": {
+        "street_address": {
+          "type": "string"
+        },
+        "country": {
+          "default": "United States of America",
+          "enum": ["United States of America", "Canada", "Netherlands"]
+        }
+      },
+      "allOf": [
+        {
+          "if": {
+            "properties": { "country": { "const": "United States of America" } }
+          },
+          "then": {
+            "properties": { "postal_code": { "pattern": "[0-9]{5}(-[0-9]{4})?" } }
+          }
+        },
+        {
+          "if": {
+            "properties": { "country": { "const": "Canada" } },
+            "required": ["country"]
+          },
+          "then": {
+            "properties": { "postal_code": { "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]" } }
+          }
+        },
+        {
+          "if": {
+            "properties": { "country": { "const": "Netherlands" } },
+            "required": ["country"]
+          },
+          "then": {
+            "properties": { "postal_code": { "pattern": "[0-9]{4} [A-Z]{2}" } }
+          }
+        }
+      ]
+    },
+    "tests": [
+      {
+        "description": "valid first if",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "country": "United States of America",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid first if, alternative match",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "20500"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid second if",
+        "data": {
+          "street_address": "24 Sussex Drive",
+          "country": "Canada",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": true
+      },
+      {
+        "description": "valid third if",
+        "data": {
+          "street_address": "Adriaan Goekooplaan",
+          "country": "Netherlands",
+          "postal_code": "2517 JX"
+        },
+        "valid": true
+      },
+      {
+        "description": "invalid through second then",
+        "data": {
+          "street_address": "24 Sussex Drive",
+          "country": "Canada",
+          "postal_code": "10000"
+        },
+        "valid": false
+      },
+      {
+        "description": "invalid through first then",
+        "data": {
+          "street_address": "1600 Pennsylvania Avenue NW",
+          "postal_code": "K1M 1M4"
+        },
+        "valid": false
+      }
     ]
   }
 ]
diff --git a/src/test/resources/schema/issue386-v7.json b/src/test/resources/schema/issue386-v7.json
new file mode 100644
index 0000000..a63d0b6
--- /dev/null
+++ b/src/test/resources/schema/issue386-v7.json
@@ -0,0 +1,40 @@
+{
+  "type": "object",
+  "properties": {
+    "street_address": {
+      "type": "string"
+    },
+    "country": {
+      "default": "United States of America",
+      "enum": ["United States of America", "Canada", "Netherlands"]
+    }
+  },
+  "allOf": [
+    {
+      "if": {
+        "properties": { "country": { "const": "United States of America" } }
+      },
+      "then": {
+        "properties": { "postal_code": { "pattern": "[0-9]{5}(-[0-9]{4})?" } }
+      }
+    },
+    {
+      "if": {
+        "properties": { "country": { "const": "Canada" } },
+        "required": ["country"]
+      },
+      "then": {
+        "properties": { "postal_code": { "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]" } }
+      }
+    },
+    {
+      "if": {
+        "properties": { "country": { "const": "Netherlands" } },
+        "required": ["country"]
+      },
+      "then": {
+        "properties": { "postal_code": { "pattern": "[0-9]{4} [A-Z]{2}" } }
+      }
+    }
+  ]
+}
\ No newline at end of file