From ee0b21c0f94df614941e89d7f1573b45e1c1ce48 Mon Sep 17 00:00:00 2001 From: Balloon Date: Wed, 5 Jun 2019 14:01:39 -0400 Subject: [PATCH 1/7] -fixed max validator when have an integer type but with a double max value -added test cases --- .../networknt/schema/MaximumValidator.java | 4 +- .../schema/MaximumValidatorTest.java | 60 +++++++++++++++---- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/networknt/schema/MaximumValidator.java b/src/main/java/com/networknt/schema/MaximumValidator.java index 14e7ee809..3a586eef8 100644 --- a/src/main/java/com/networknt/schema/MaximumValidator.java +++ b/src/main/java/com/networknt/schema/MaximumValidator.java @@ -46,8 +46,8 @@ public MaximumValidator(String schemaPath, JsonNode schemaNode, JsonSchema paren } parseErrorCode(getValidatorType().getErrorCodeKey()); - - if (!JsonType.INTEGER.toString().equals(getNodeFieldType())) { + //if the schema type is not integer, or the maximum value is not an integer, try to compare using double values; + if (!JsonType.INTEGER.toString().equals(getNodeFieldType()) || !JsonType.INTEGER.toString().equals(schemaNode.getNodeType())) { // "number" or no type // by default treat value as double: compatible with previous behavior final double dm = schemaNode.doubleValue(); diff --git a/src/test/java/com/networknt/schema/MaximumValidatorTest.java b/src/test/java/com/networknt/schema/MaximumValidatorTest.java index 5cdcb12a3..3d6190e26 100644 --- a/src/test/java/com/networknt/schema/MaximumValidatorTest.java +++ b/src/test/java/com/networknt/schema/MaximumValidatorTest.java @@ -63,7 +63,7 @@ public void doubleValueOverflow() throws IOException { JsonNode doc = mapper.readTree(value); Set messages = v.validate(doc); - assertFalse(format("Expecing validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); + assertFalse(format("Expecting validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); } } @@ -86,7 +86,7 @@ public void documentParsedWithBigDecimal() throws IOException { JsonNode doc = bigDecimalMapper.readTree(value); Set messages = v.validate(doc); - assertFalse(format("Expecing validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); + assertFalse(format("Expecting validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); } } @@ -109,7 +109,7 @@ public void documentAndSchemaParsedWithBigDecimal() throws IOException { JsonNode doc = bigDecimalMapper.readTree(value); Set messages = v.validate(doc); - assertFalse(format("Expecing validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); + assertFalse(format("Expecting validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); } } @@ -198,7 +198,7 @@ public void longUnderMaxValueOverflow() throws IOException { JsonNode doc = mapper.readTree(value); Set messages = v.validate(doc); - assertFalse(format("Expecing validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); + assertFalse(format("Expecting validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); } } @@ -218,7 +218,7 @@ public void longValueOverflowWithInverseEffect() throws IOException { JsonNode doc = mapper.readTree(value); Set messages = v.validate(doc); - assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); } } @@ -238,7 +238,7 @@ public void BigIntegerBothWithinLongRangePositive() throws IOException { JsonNode doc = bigIntegerMapper.readTree(value); Set messages = v.validate(doc); - assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); } } @@ -298,7 +298,7 @@ public void BigIntegerNotOverflow() throws IOException { JsonNode doc = bigIntegerMapper.readTree(value); Set messages = v.validate(doc); - assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); } } @@ -318,7 +318,7 @@ public void BigIntegerBothAboveLongRangePositive() throws IOException { JsonNode doc = bigIntegerMapper.readTree(value); Set messages = v.validate(doc); - assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); } } @@ -358,7 +358,7 @@ public void BigIntegerNotOverflowOnLongRangeEdge() throws IOException { JsonNode doc = bigIntegerMapper.readTree(value); Set messages = v.validate(doc); - assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); } } @@ -378,7 +378,47 @@ public void BigIntegerOverflowOnLongRangeEdge() throws IOException { JsonNode doc = bigIntegerMapper.readTree(value); Set messages = v.validate(doc); - assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + } + } + + @Test + public void testIntegerTypeWithFloatMaxPositive() throws IOException { + String[][] values = { +// maximum, value + {"37.7", "37"} + }; + + for(String[] aTestCycle : values) { + String maximum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"integer\", \"maximum\": %s, \"exclusiveMaximum\": false}", maximum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = bigIntegerMapper.readTree(value); + + Set messages = v.validate(doc); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + } + } + + @Test + public void testIntegerTypeWithFloatMaxNegative() throws IOException { + String[][] values = { +// maximum, value + {"37.7", "38"} + }; + + for(String[] aTestCycle : values) { + String maximum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"integer\", \"maximum\": %s, \"exclusiveMaximum\": false}", maximum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = bigIntegerMapper.readTree(value); + + Set messages = v.validate(doc); + assertFalse(format("Expecting validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); } } } From a456ff53d507b5a4ab4e07658152b4ed78568fca Mon Sep 17 00:00:00 2001 From: Balloon Date: Tue, 11 Jun 2019 13:13:17 -0400 Subject: [PATCH 2/7] -only compares with Long value when the schema type is int/long and the compare node is integer type, otherwise means one of schema or compare node is with decimal, so convert it to BigDecimal to do the compare. -change some edge test cases, due to the way of measuring is changed, some edge scenarios don't apply anymore. --- .../networknt/schema/MaximumValidator.java | 39 ++----- .../schema/MaximumValidatorTest.java | 107 +++++++++++++++++- 2 files changed, 113 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/networknt/schema/MaximumValidator.java b/src/main/java/com/networknt/schema/MaximumValidator.java index 3a586eef8..85f3c32aa 100644 --- a/src/main/java/com/networknt/schema/MaximumValidator.java +++ b/src/main/java/com/networknt/schema/MaximumValidator.java @@ -20,6 +20,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.math.BigDecimal; import java.math.BigInteger; import java.util.Collections; import java.util.Set; @@ -46,34 +47,17 @@ public MaximumValidator(String schemaPath, JsonNode schemaNode, JsonSchema paren } parseErrorCode(getValidatorType().getErrorCodeKey()); - //if the schema type is not integer, or the maximum value is not an integer, try to compare using double values; - if (!JsonType.INTEGER.toString().equals(getNodeFieldType()) || !JsonType.INTEGER.toString().equals(schemaNode.getNodeType())) { - // "number" or no type - // by default treat value as double: compatible with previous behavior - final double dm = schemaNode.doubleValue(); - typedMaximum = new ThresholdMixin() { - @Override - public boolean crossesThreshold(JsonNode node) { - double value = node.asDouble(); - return greaterThan(value, dm) || (excludeEqual && MaximumValidator.this.equals(value, dm)); - } - - @Override - public String thresholdValue() { - return String.valueOf(dm); - } - }; - } else if ( schemaNode.isLong() || schemaNode.isInt() ) { + if (( schemaNode.isLong() || schemaNode.isInt() ) && (JsonType.INTEGER.toString().equals(getNodeFieldType()))) { // "integer", and within long range final long lm = schemaNode.asLong(); typedMaximum = new ThresholdMixin() { @Override public boolean crossesThreshold(JsonNode node) { long val = node.asLong(); - if(node.isBigInteger()) { + if (node.isBigInteger()) { //node.isBigInteger is not trustable, the type BigInteger doesn't mean it is a big number. - if(node.bigIntegerValue().compareTo(new BigInteger(String.valueOf(Long.MAX_VALUE))) > 0) { + if (node.bigIntegerValue().compareTo(new BigInteger(String.valueOf(Long.MAX_VALUE))) > 0) { return true; } } @@ -85,20 +69,22 @@ public String thresholdValue() { return String.valueOf(lm); } }; - } else { - // "integer" outside long range - final BigInteger bim = new BigInteger(schemaNode.asText()); typedMaximum = new ThresholdMixin() { @Override public boolean crossesThreshold(JsonNode node) { - int cmp = bim.compareTo(node.bigIntegerValue()); - return cmp < 0 || (excludeEqual && cmp <= 0); + if(schemaNode.doubleValue() == Double.POSITIVE_INFINITY) { + return false; + } + final BigDecimal max = new BigDecimal(schemaNode.asText()); + if(node.doubleValue() == Double.POSITIVE_INFINITY) {return true;} + BigDecimal value = new BigDecimal(node.asText()); + return value.compareTo(max) > 0 || (excludeEqual && value.compareTo(max) == 0); } @Override public String thresholdValue() { - return String.valueOf(bim); + return schemaNode.asText(); } }; } @@ -117,5 +103,4 @@ public Set validate(JsonNode node, JsonNode rootNode, String } return Collections.emptySet(); } - } \ No newline at end of file diff --git a/src/test/java/com/networknt/schema/MaximumValidatorTest.java b/src/test/java/com/networknt/schema/MaximumValidatorTest.java index 3d6190e26..4b2cac565 100644 --- a/src/test/java/com/networknt/schema/MaximumValidatorTest.java +++ b/src/test/java/com/networknt/schema/MaximumValidatorTest.java @@ -40,6 +40,8 @@ public class MaximumValidatorTest { @Before public void setUp() { mapper = new ObjectMapper(); + // due to a jackson bug, a float number which is larger than Double.POSITIVE_INFINITY cannot be convert to BigDecimal correctly + // https://github.com/FasterXML/jackson-databind/issues/1770 bigDecimalMapper = new ObjectMapper().enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS); bigIntegerMapper = new ObjectMapper().enable(DeserializationFeature.USE_BIG_INTEGER_FOR_INTS); } @@ -137,14 +139,25 @@ public void negativeDoubleOverflowTest() throws IOException { assertTrue(format("Maximum %s and value %s are interpreted as Infinity, thus no schema violation should be reported", maximum, value), messages.isEmpty()); // document parsed with BigDecimal + doc = bigDecimalMapper.readTree(value); - messages = v.validate(doc); - assertTrue(format("Maximum %s and value %s are interpreted as Infinity, thus no schema violation should be reported", maximum, value), messages.isEmpty()); + Set messages2 = v.validate(doc); + if(Double.valueOf(maximum) == Double.POSITIVE_INFINITY) { + assertTrue(format("Maximum %s and value %s are equal, thus no schema violation should be reported", maximum, value), messages2.isEmpty()); + } else { + assertFalse(format("Maximum %s is smaller than value %s , should be validation error reported", maximum, value), messages2.isEmpty()); + } + // schema and document parsed with BigDecimal v = factory.getSchema(bigDecimalMapper.readTree(schema)); - messages = v.validate(doc); - assertTrue(format("Maximum %s and value %s are interpreted as Infinity, thus no schema violation should be reported", maximum, value), messages.isEmpty()); + Set messages3 = v.validate(doc); + //when the schema and value are both using BigDecimal, the value should be parsed in same mechanism. + if(maximum.toLowerCase().equals(value.toLowerCase()) || Double.valueOf(maximum) == Double.POSITIVE_INFINITY) { + assertTrue(format("Maximum %s and value %s are equal, thus no schema violation should be reported", maximum, value), messages3.isEmpty()); + } else { + assertFalse(format("Maximum %s is smaller than value %s , should be validation error reported", maximum, value), messages3.isEmpty()); + } } } @@ -165,7 +178,9 @@ public void doubleValueCoarsing() throws IOException { doc = bigDecimalMapper.readTree(content); messages = v.validate(doc); - assertTrue("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); + // "1.7976931348623158e+308" == "1.7976931348623157e+308" == Double.MAX_VALUE + // new BigDecimal("1.7976931348623158e+308").compareTo(new BigDecimal("1.7976931348623157e+308")) > 0 + assertFalse("Validation should not succeed because content is using bigDecimalMapper, and bigger than the maximum", messages.isEmpty()); /** * Note: technically this is where 1.7976931348623158e+308 rounding to 1.7976931348623157e+308 could be spotted, @@ -176,7 +191,7 @@ public void doubleValueCoarsing() throws IOException { */ v = factory.getSchema(bigDecimalMapper.readTree(schema)); messages = v.validate(doc); - assertTrue("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); + assertFalse("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); } @Test @@ -421,6 +436,86 @@ public void testIntegerTypeWithFloatMaxNegative() throws IOException { assertFalse(format("Expecting validation error with maximum %s and value %s", maximum, value), messages.isEmpty()); } } + + @Test + public void testMaximumDoubleValue() throws IOException { + String[][] values = { +// maximum, value + {"1E39", "1000"} + }; + + for(String[] aTestCycle : values) { + String maximum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"integer\", \"maximum\": %s, \"exclusiveMaximum\": false}", maximum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + } + } + + @Test + public void testMaximumDoubleValueNegative() throws IOException { + String[][] values = { +// maximum, value + {"1000", "1E39"} + }; + + for(String[] aTestCycle : values) { + String maximum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"integer\", \"maximum\": %s, \"exclusiveMaximum\": false}", maximum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertFalse(format("Expecting validation errors as value %s is greater than maximum %s", maximum, value), messages.isEmpty()); + } + } + + @Test + public void testMaximumDoubleValueWithNumberType() throws IOException { + String[][] values = { +// maximum, value + {"1000.1", "1000"} + }; + + for(String[] aTestCycle : values) { + String maximum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"number\", \"maximum\": %s, \"exclusiveMaximum\": false}", maximum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertTrue(format("Expecting no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); + } + } + + @Test + public void testMaximumDoubleValueWithNumberTypeNegative() throws IOException { + String[][] values = { +// maximum, value + {"1000", "1000.1"} + }; + + for(String[] aTestCycle : values) { + String maximum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"number\", \"maximum\": %s, \"exclusiveMaximum\": false}", maximum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertFalse(format("Expecting validation errors as value %s is greater than maximum %s", maximum, value), messages.isEmpty()); + } + } } From 70968903f5a5d12849aad1da4efb2d13a524af5e Mon Sep 17 00:00:00 2001 From: Balloon Date: Tue, 11 Jun 2019 13:19:49 -0400 Subject: [PATCH 3/7] -add final to schemaNode -change logback-test from debug to error level due to it may affect performance test --- .../networknt/schema/MaximumValidator.java | 2 +- src/test/resources/logback-test.xml | 62 +++++++++---------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/networknt/schema/MaximumValidator.java b/src/main/java/com/networknt/schema/MaximumValidator.java index 85f3c32aa..5288fe5cc 100644 --- a/src/main/java/com/networknt/schema/MaximumValidator.java +++ b/src/main/java/com/networknt/schema/MaximumValidator.java @@ -34,7 +34,7 @@ public class MaximumValidator extends BaseJsonValidator implements JsonValidator private final ThresholdMixin typedMaximum; - public MaximumValidator(String schemaPath, JsonNode schemaNode, JsonSchema parentSchema, ValidationContext validationContext) { + public MaximumValidator(String schemaPath, final JsonNode schemaNode, JsonSchema parentSchema, ValidationContext validationContext) { super(schemaPath, schemaNode, parentSchema, ValidatorTypeCode.MAXIMUM, validationContext); if (!schemaNode.isNumber()) { diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml index bfcfd9f9b..d61b04f59 100644 --- a/src/test/resources/logback-test.xml +++ b/src/test/resources/logback-test.xml @@ -1,31 +1,31 @@ - - - - - - - - - %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n - - - - - - - + + + + + + + + + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n + + + + + + + From 40dc57e27fc48f71229c7480ca4c32654211b3cb Mon Sep 17 00:00:00 2001 From: Balloon Date: Tue, 11 Jun 2019 13:23:19 -0400 Subject: [PATCH 4/7] -added a performance test to execute test to compare with double type. the time consuming diff is very small after the change using BigDecimal --- .../schema/MaximumValidatorPerfTest.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java diff --git a/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java b/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java new file mode 100644 index 000000000..6fe951fdc --- /dev/null +++ b/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java @@ -0,0 +1,62 @@ +package com.networknt.schema; + +import org.junit.Test; + +import java.lang.annotation.Annotation; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +public class MaximumValidatorPerfTest { + MaximumValidatorTest test = new MaximumValidatorTest(); + + @Test + public void testTime() throws InvocationTargetException, IllegalAccessException { + test.setUp(); + String[] testMethodsToBeExecuted = {"testMaximumDoubleValue"}; + List testMethods = getTestMethods(testMethodsToBeExecuted); + long start = System.currentTimeMillis(); + executeTests(testMethods, 200000); + long end = System.currentTimeMillis(); + System.out.println("time to execute all tests using:" + (end - start) +"ms"); + } + + public void executeTests(List methods, int executeTimes) throws InvocationTargetException, IllegalAccessException { + + for(int i = 0; i < executeTimes; i++) { + for(Method testMethod : methods) { + testMethod.invoke(test); + } + } + + } + + public List getTestMethods(String[] methodNames) { + Method[] methods = test.getClass().getMethods(); + List testMethods = new ArrayList(); + if(methodNames.length > 0) { + for(String name : methodNames) { + testMethods.addAll(Arrays.stream(methods).filter(m -> m.getName().equals(name)).collect(Collectors.toList())); + } + return testMethods; + } + for (Method m : methods) { + Annotation[] annotations = m.getDeclaredAnnotations(); + boolean isTestMethod = false; + for(Annotation annotation : annotations) { + if(annotation.annotationType() == Test.class) { + isTestMethod = true; + } + } + if(isTestMethod) { + //filter out incompatible test cases. + if(!m.getName().equals("doubleValueCoarsing") && !m.getName().equals("negativeDoubleOverflowTest")) + testMethods.add(m); + } + } + return testMethods; + } +} From f31f69885a5523ad308ba12f188b4bfc7bb3af75 Mon Sep 17 00:00:00 2001 From: Balloon Date: Tue, 11 Jun 2019 14:16:59 -0400 Subject: [PATCH 5/7] -change minimum validator to use BigDecimal to do comparsion with similar logic to MaximumValidator -added test cases --- .../networknt/schema/MinimumValidator.java | 33 ++--- .../schema/MaximumValidatorTest.java | 32 +++++ .../schema/MinimumValidatorTest.java | 128 +++++++++++++++++- 3 files changed, 165 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/networknt/schema/MinimumValidator.java b/src/main/java/com/networknt/schema/MinimumValidator.java index 672380f09..a01595906 100644 --- a/src/main/java/com/networknt/schema/MinimumValidator.java +++ b/src/main/java/com/networknt/schema/MinimumValidator.java @@ -20,6 +20,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.math.BigDecimal; import java.math.BigInteger; import java.util.Collections; import java.util.Set; @@ -50,24 +51,7 @@ public MinimumValidator(String schemaPath, JsonNode schemaNode, JsonSchema paren parseErrorCode(getValidatorType().getErrorCodeKey()); - if (!JsonType.INTEGER.toString().equals(getNodeFieldType())) { - // "number" or no type - // by default treat value as double: compatible with previous behavior - final double dmin = schemaNode.doubleValue(); - typedMinimum = new ThresholdMixin() { - @Override - public boolean crossesThreshold(JsonNode node) { - double value = node.asDouble(); - return lessThan(value, dmin) || (excluded && MinimumValidator.this.equals(value, dmin)); - } - - @Override - public String thresholdValue() { - return String.valueOf(dmin); - } - }; - - } else if ( schemaNode.isLong() || schemaNode.isInt() ) { + if ( schemaNode.isLong() || schemaNode.isInt() && JsonType.INTEGER.toString().equals(getNodeFieldType())) { // "integer", and within long range final long lmin = schemaNode.asLong(); typedMinimum = new ThresholdMixin() { @@ -90,18 +74,21 @@ public String thresholdValue() { }; } else { - // "integer" outside long range - final BigInteger bimin = new BigInteger(schemaNode.asText()); typedMinimum = new ThresholdMixin() { @Override public boolean crossesThreshold(JsonNode node) { - int cmp = bimin.compareTo(node.bigIntegerValue()); - return cmp > 0 || (excluded && cmp >= 0); + if(schemaNode.doubleValue() == Double.NEGATIVE_INFINITY) { + return false; + } + final BigDecimal min = new BigDecimal(schemaNode.asText()); + if(node.doubleValue() == Double.NEGATIVE_INFINITY) {return true;} + BigDecimal value = new BigDecimal(node.asText()); + return value.compareTo(min) < 0 || (excluded && value.compareTo(min) == 0); } @Override public String thresholdValue() { - return String.valueOf(bimin); + return schemaNode.asText(); } }; } diff --git a/src/test/java/com/networknt/schema/MaximumValidatorTest.java b/src/test/java/com/networknt/schema/MaximumValidatorTest.java index 4b2cac565..79e85a3c7 100644 --- a/src/test/java/com/networknt/schema/MaximumValidatorTest.java +++ b/src/test/java/com/networknt/schema/MaximumValidatorTest.java @@ -194,6 +194,38 @@ public void doubleValueCoarsing() throws IOException { assertFalse("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); } + /** + * BigDecimalMapper issue, it doesn't work as expected, it will treat 1.7976931348623159e+308 as INFINITY instead of as it is. + */ + @Test + public void doubleValueCoarsingExceedRange() throws IOException { + String schema = "{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"number\", \"maximum\": 1.7976931348623159e+308 }"; + String content = "1.7976931348623160e+308"; + + JsonNode doc = mapper.readTree(content); + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + + Set messages = v.validate(doc); + assertTrue("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); + + doc = bigDecimalMapper.readTree(content); + messages = v.validate(doc); + // "1.7976931348623158e+308" == "1.7976931348623157e+308" == Double.MAX_VALUE + // new BigDecimal("1.7976931348623158e+308").compareTo(new BigDecimal("1.7976931348623157e+308")) > 0 + assertTrue("Validation should success because the bug of bigDecimalMapper, it will treat 1.7976931348623159e+308 as INFINITY", messages.isEmpty()); + + /** + * Note: technically this is where 1.7976931348623158e+308 rounding to 1.7976931348623157e+308 could be spotted, + * yet it requires a dedicated case of comparison BigDecimal to BigDecimal. Since values above + * 1.7976931348623158e+308 are parsed as Infinity anyways (jackson uses double as primary type with later + * "upcasting" to BigDecimal, if property is set) adding a dedicated code block just for this one case + * seems infeasible. + */ + v = factory.getSchema(bigDecimalMapper.readTree(schema)); + messages = v.validate(doc); + assertTrue("Validation should success because the bug of bigDecimalMapper, it will treat 1.7976931348623159e+308 as INFINITY", messages.isEmpty()); + } + @Test public void longUnderMaxValueOverflow() throws IOException { String[][] values = { diff --git a/src/test/java/com/networknt/schema/MinimumValidatorTest.java b/src/test/java/com/networknt/schema/MinimumValidatorTest.java index a197f842d..2410fae0f 100644 --- a/src/test/java/com/networknt/schema/MinimumValidatorTest.java +++ b/src/test/java/com/networknt/schema/MinimumValidatorTest.java @@ -139,13 +139,28 @@ public void negativeDoubleOverflowTest() throws IOException { // document parsed with BigDecimal doc = bigDecimalMapper.readTree(value); - messages = v.validate(doc); - assertTrue(format("Minimum %s and value %s are interpreted as Infinity, thus no schema violation should be reported", minimum, value), messages.isEmpty()); + Set messages2 = v.validate(doc); + + //when the schema and value are both using BigDecimal, the value should be parsed in same mechanism. + if(Double.valueOf(minimum) == Double.NEGATIVE_INFINITY) { + /** + * {"-1.000000000000000000000001E+308", "-1.000000000000000000000001E+308"} will be false + * because the different between two mappers, without using big decimal, it loses some precises. + */ + assertTrue(format("Minimum %s and value %s are equal, thus no schema violation should be reported", minimum, value), messages2.isEmpty()); + } else { + assertFalse(format("Minimum %s is larger than value %s , should be validation error reported", minimum, value), messages2.isEmpty()); + } // schema and document parsed with BigDecimal v = factory.getSchema(bigDecimalMapper.readTree(schema)); - messages = v.validate(doc); - assertTrue(format("Minimum %s and value %s are interpreted as Infinity, thus no schema violation should be reported", minimum, value), messages.isEmpty()); + Set messages3 = v.validate(doc); + //when the schema and value are both using BigDecimal, the value should be parsed in same mechanism. + if(minimum.toLowerCase().equals(value.toLowerCase()) || Double.valueOf(minimum) == Double.NEGATIVE_INFINITY) { + assertTrue(format("Minimum %s and value %s are equal, thus no schema violation should be reported", minimum, value), messages3.isEmpty()); + } else { + assertFalse(format("Minimum %s is larger than value %s , should be validation error reported", minimum, value), messages3.isEmpty()); + } } } @@ -166,7 +181,7 @@ public void doubleValueCoarsing() throws IOException { doc = bigDecimalMapper.readTree(content); messages = v.validate(doc); - assertTrue("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); + assertFalse("Validation should not succeed because content is using bigDecimalMapper, and smaller than the minimum", messages.isEmpty()); /** * Note: technically this is where -1.7976931348623158e+308 rounding to -1.7976931348623157e+308 could be @@ -177,7 +192,30 @@ public void doubleValueCoarsing() throws IOException { */ v = factory.getSchema(bigDecimalMapper.readTree(schema)); messages = v.validate(doc); + assertFalse("Validation should not succeed because content is using bigDecimalMapper, and smaller than the minimum", messages.isEmpty()); + } + + /** + * BigDecimalMapper issue, it doesn't work as expected, it will treat -1.7976931348623157e+309 as INFINITY instead of as it is. + */ + @Test + public void doubleValueCoarsingExceedRange() throws IOException { + String schema = "{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"number\", \"minimum\": -1.7976931348623159e+308 }"; + String content = "-1.7976931348623160e+308"; + + JsonNode doc = mapper.readTree(content); + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + + Set messages = v.validate(doc); assertTrue("Validation should succeed as by default double values are used by mapper", messages.isEmpty()); + + doc = bigDecimalMapper.readTree(content); + messages = v.validate(doc); + assertTrue("Validation should succeed due to the bug of BigDecimal option of mapper", messages.isEmpty()); + + v = factory.getSchema(bigDecimalMapper.readTree(schema)); + messages = v.validate(doc); + assertTrue("Validation should succeed due to the bug of BigDecimal option of mapper", messages.isEmpty()); } @Test @@ -381,6 +419,86 @@ public void BigIntegerOverflowOnLongRangeEdge() throws IOException { assertTrue(format("Expecing no validation errors as maximum %s is greater than value %s", maximum, value), messages.isEmpty()); } } + + @Test + public void testMinimumDoubleValue() throws IOException { + String[][] values = { +// minimum, value + {"-1E309", "-1000"} + }; + + for(String[] aTestCycle : values) { + String minimum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"integer\", \"minimum\": %s, \"exclusiveMinimum\": false}", minimum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertTrue(format("Expecting no validation errors as value %s is greater than minimum %s", value, minimum), messages.isEmpty()); + } + } + + @Test + public void testMinimumDoubleValueNegative() throws IOException { + String[][] values = { +// minimum, value + {"-1000", "-1E309"} + }; + + for(String[] aTestCycle : values) { + String minimum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"integer\", \"minimum\": %s, \"exclusiveMinimum\": false}", minimum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertFalse(format("Expecting validation errors as value %s is smaller than minimum %s", value, minimum), messages.isEmpty()); + } + } + + @Test + public void testMinimumDoubleValueWithNumberType() throws IOException { + String[][] values = { +// minimum, value + {"1000", "1000.1"} + }; + + for(String[] aTestCycle : values) { + String minimum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"number\", \"minimum\": %s, \"exclusiveMinimum\": false}", minimum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertTrue(format("Expecting no validation errors as value %s is greater than minimum %s", value, minimum), messages.isEmpty()); + } + } + + @Test + public void testMinimumDoubleValueWithNumberTypeNegative() throws IOException { + String[][] values = { +// minimum, value + {"1000.1", "1000"} + }; + + for(String[] aTestCycle : values) { + String minimum = aTestCycle[0]; + String value = aTestCycle[1]; + String schema = format("{ \"$schema\":\"http://json-schema.org/draft-04/schema#\", \"type\": \"number\", \"minimum\": %s, \"exclusiveMinimum\": false}", minimum); + + JsonSchema v = factory.getSchema(mapper.readTree(schema)); + JsonNode doc = mapper.readTree(value); + + Set messages = v.validate(doc); + assertFalse(format("Expecting validation errors as value %s is smaller than minimum %s", value, minimum), messages.isEmpty()); + } + } } From d96ac6e360513aa705defc8cb50ca2e5aaf45275 Mon Sep 17 00:00:00 2001 From: Balloon Date: Tue, 11 Jun 2019 14:18:59 -0400 Subject: [PATCH 6/7] -upgrade java.testversion so that support lambda in test cases --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 59ede70ed..3a1404c94 100644 --- a/pom.xml +++ b/pom.xml @@ -59,7 +59,7 @@ 1.8 - 1.7 + 1.8 UTF-8 2.9.9 1.7.25 From 7ce7f9858d0daa6add34c3657d683d1676c17139 Mon Sep 17 00:00:00 2001 From: Balloon Date: Thu, 13 Jun 2019 13:06:43 -0400 Subject: [PATCH 7/7] -ignore performance test, shouldn't include in build. --- .../java/com/networknt/schema/MaximumValidatorPerfTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java b/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java index 6fe951fdc..4d2b18041 100644 --- a/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java +++ b/src/test/java/com/networknt/schema/MaximumValidatorPerfTest.java @@ -1,5 +1,6 @@ package com.networknt.schema; +import org.junit.Ignore; import org.junit.Test; import java.lang.annotation.Annotation; @@ -9,7 +10,7 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; - +@Ignore public class MaximumValidatorPerfTest { MaximumValidatorTest test = new MaximumValidatorTest();