From b88c5973ce6ebb5e43519c23ab9986ff235092fe Mon Sep 17 00:00:00 2001 From: Gareth Robinson Date: Mon, 22 Nov 2021 17:40:12 +0000 Subject: [PATCH 1/2] Issue386: FailFast should not cause exception on 'if' --- .../com/networknt/schema/IfValidator.java | 14 ++++-- .../networknt/schema/Issue366FailFast.java | 8 ++-- .../com/networknt/schema/Issue386Test.java | 47 +++++++++++++++++++ src/test/resources/data/issue386.json | 5 ++ src/test/resources/schema/issue386-v7.json | 37 +++++++++++++++ 5 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/networknt/schema/Issue386Test.java create mode 100644 src/test/resources/data/issue386.json create mode 100644 src/test/resources/schema/issue386-v7.json diff --git a/src/main/java/com/networknt/schema/IfValidator.java b/src/main/java/com/networknt/schema/IfValidator.java index 85269b390..908df067b 100644 --- a/src/main/java/com/networknt/schema/IfValidator.java +++ b/src/main/java/com/networknt/schema/IfValidator.java @@ -59,10 +59,18 @@ public Set validate(JsonNode node, JsonNode rootNode, String Set errors = new LinkedHashSet(); - Set 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)); } diff --git a/src/test/java/com/networknt/schema/Issue366FailFast.java b/src/test/java/com/networknt/schema/Issue366FailFast.java index 51737e406..e6d92fe9a 100644 --- a/src/test/java/com/networknt/schema/Issue366FailFast.java +++ b/src/test/java/com/networknt/schema/Issue366FailFast.java @@ -35,7 +35,7 @@ private void setupSchema() throws IOException { 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 void secondOneValid() throws Exception { @Test public void bothValid() throws Exception { String dataPath = "/data/issue366.json"; - + assertThrows(JsonSchemaException.class, () -> { InputStream dataInputStream = getClass().getResourceAsStream(dataPath); JsonNode node = getJsonNodeFromStreamContent(dataInputStream); List 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/Issue386Test.java b/src/test/java/com/networknt/schema/Issue386Test.java new file mode 100644 index 000000000..9c690ea83 --- /dev/null +++ b/src/test/java/com/networknt/schema/Issue386Test.java @@ -0,0 +1,47 @@ +package com.networknt.schema; + +import java.io.InputStream; +import java.util.Set; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +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; + } + + @Test + public void dataIsValidFailAll() throws Exception { + String schemaPath = "/schema/issue386-v7.json"; + String dataPath = "/data/issue386.json"; + JsonSchema schema = getJsonSchemaFromPathV7(schemaPath, false); + JsonNode node = getJsonNodeFromPath(dataPath); + Set errors = schema.validate(node); + Assertions.assertEquals(0, errors.size()); + } + + @Test + public void dataIsValidFailFast() throws Exception { + String schemaPath = "/schema/issue386-v7.json"; + String dataPath = "/data/issue386.json"; + JsonSchema schema = getJsonSchemaFromPathV7(schemaPath, true); + JsonNode node = getJsonNodeFromPath(dataPath); + Set errors = schema.validate(node); + Assertions.assertEquals(0, errors.size()); + } +} diff --git a/src/test/resources/data/issue386.json b/src/test/resources/data/issue386.json new file mode 100644 index 000000000..24a7f0b17 --- /dev/null +++ b/src/test/resources/data/issue386.json @@ -0,0 +1,5 @@ +{ + "street_address": "1600 Pennsylvania Avenue NW", + "country": "United States of America", + "postal_code": "20500" +} \ No newline at end of file diff --git a/src/test/resources/schema/issue386-v7.json b/src/test/resources/schema/issue386-v7.json new file mode 100644 index 000000000..547043cb5 --- /dev/null +++ b/src/test/resources/schema/issue386-v7.json @@ -0,0 +1,37 @@ +{ + "type": "object", + "properties": { + "street_address": { + "type": "string" + }, + "country": { + "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" } } + }, + "then": { + "properties": { "postal_code": { "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]" } } + } + }, + { + "if": { + "properties": { "country": { "const": "Netherlands" } } + }, + "then": { + "properties": { "postal_code": { "pattern": "[0-9]{4} [A-Z]{2}" } } + } + } + ] +} \ No newline at end of file From d7a814406aa6b7e2d42dc6b781709b5930cde5e8 Mon Sep 17 00:00:00 2001 From: Gareth Robinson Date: Wed, 24 Nov 2021 11:36:38 +0000 Subject: [PATCH 2/2] Added additional fail-fast and if-else tests --- pom.xml | 6 + ...ailFast.java => Issue366FailFastTest.java} | 2 +- ...ailSlow.java => Issue366FailSlowTest.java} | 2 +- .../com/networknt/schema/Issue386Test.java | 58 ++++-- src/test/resources/data/issue386.json | 50 ++++- .../resources/draft2019-09/if-then-else.json | 178 ++++++++++++++++++ src/test/resources/draft7/if-then-else.json | 140 ++++++++++++-- src/test/resources/schema/issue386-v7.json | 7 +- 8 files changed, 408 insertions(+), 35 deletions(-) rename src/test/java/com/networknt/schema/{Issue366FailFast.java => Issue366FailFastTest.java} (99%) rename src/test/java/com/networknt/schema/{Issue366FailSlow.java => Issue366FailSlowTest.java} (99%) diff --git a/pom.xml b/pom.xml index a6efcc515..9a5594091 100644 --- a/pom.xml +++ b/pom.xml @@ -106,6 +106,12 @@ ${version.junit} test + + org.junit.jupiter + junit-jupiter-params + ${version.junit} + test + org.mockito mockito-core diff --git a/src/test/java/com/networknt/schema/Issue366FailFast.java b/src/test/java/com/networknt/schema/Issue366FailFastTest.java similarity index 99% rename from src/test/java/com/networknt/schema/Issue366FailFast.java rename to src/test/java/com/networknt/schema/Issue366FailFastTest.java index e6d92fe9a..051159765 100644 --- a/src/test/java/com/networknt/schema/Issue366FailFast.java +++ b/src/test/java/com/networknt/schema/Issue366FailFastTest.java @@ -13,7 +13,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class Issue366FailFast { +public class Issue366FailFastTest { @BeforeEach public void setup() throws IOException { 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 1a8542b9e..3c0d27fae 100644 --- a/src/test/java/com/networknt/schema/Issue366FailSlow.java +++ b/src/test/java/com/networknt/schema/Issue366FailSlowTest.java @@ -13,7 +13,7 @@ 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 index 9c690ea83..8f7bebfdf 100644 --- a/src/test/java/com/networknt/schema/Issue386Test.java +++ b/src/test/java/com/networknt/schema/Issue386Test.java @@ -1,10 +1,14 @@ 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; @@ -12,36 +16,60 @@ 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); + 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(); + InputStream dataInputStream = getClass().getResourceAsStream(dataPath); + ObjectMapper mapper = new ObjectMapper(); JsonNode node = mapper.readTree(dataInputStream); return node; } - @Test - public void dataIsValidFailAll() throws Exception { + @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, false); - JsonNode node = getJsonNodeFromPath(dataPath); - Set errors = schema.validate(node); - Assertions.assertEquals(0, errors.size()); + JsonSchema schema = getJsonSchemaFromPathV7(schemaPath, failFast); + JsonNode node = getJsonNodeFromPath(dataPath).get("valid"); + node.forEach(testNode -> { + Set errors = schema.validate(testNode.get("data")); + Assertions.assertEquals(0, errors.size(), "Expected no errors for " + testNode.get("data")); + }); } @Test - public void dataIsValidFailFast() throws Exception { + 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); - Set errors = schema.validate(node); - Assertions.assertEquals(0, errors.size()); + 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 errors = schema.validate(testNode.get("data")); + List 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 index 24a7f0b17..a70a95066 100644 --- a/src/test/resources/data/issue386.json +++ b/src/test/resources/data/issue386.json @@ -1,5 +1,49 @@ { - "street_address": "1600 Pennsylvania Avenue NW", - "country": "United States of America", - "postal_code": "20500" + "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 1cdeeb9e1..7b014aeac 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 634aa0664..7b014aeac 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 index 547043cb5..a63d0b649 100644 --- a/src/test/resources/schema/issue386-v7.json +++ b/src/test/resources/schema/issue386-v7.json @@ -5,6 +5,7 @@ "type": "string" }, "country": { + "default": "United States of America", "enum": ["United States of America", "Canada", "Netherlands"] } }, @@ -19,7 +20,8 @@ }, { "if": { - "properties": { "country": { "const": "Canada" } } + "properties": { "country": { "const": "Canada" } }, + "required": ["country"] }, "then": { "properties": { "postal_code": { "pattern": "[A-Z][0-9][A-Z] [0-9][A-Z][0-9]" } } @@ -27,7 +29,8 @@ }, { "if": { - "properties": { "country": { "const": "Netherlands" } } + "properties": { "country": { "const": "Netherlands" } }, + "required": ["country"] }, "then": { "properties": { "postal_code": { "pattern": "[0-9]{4} [A-Z]{2}" } }