Skip to content

Commit

Permalink
Improve type validation of integrals (#496)
Browse files Browse the repository at this point in the history
* Improve type validation of integrals

Instead of doing string comparison, use new jackson method to determine if a number is an integral.

The `javaSemantics` config option was added in PR #343 which partially addressed issue #334. In the notes for this PR:
> Once jackson-databind 2.12.0 is out, I'll replace my solution with a call to canConvertToExactIntegral

jackson-databind has been updated to 2.12.1 so this is available but the change has not yet been made.

PR #450 which addressed #446 missed this location which is used when calling `JsonSchemaFactory.getSchema`.

Issue #344 requested coercion of various types but the only type implemented in PR #379 was lossless narrowing, set with configuration option `losslessNarrowing`. I believe that setting is unnecessary now as this implementation of `javaSemantics` addresses the original issue, but left it for backwards compatibility. It also allows you to set `javaSemantics=false` and `losslessNarrowing=true` to achieve only this specific case rather than anything that `javaSemantics` is used for in the future. At this time, these properties do exactly the same thing.

- Change from string comparison to `canConvertToExactIntegral` for `javaSemantics` and `losslessNarrowing`
- Add missing documentation around `losslessNarrowing`
- Add more test cases around integrals

* Update changelog
  • Loading branch information
christi-square authored Jan 14, 2022
1 parent 7f96315 commit 4172360
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- fixes #446 maxLength and minLength do not work when a number with a zero decimal is used

## 1.0.65 - 2022-01-07

### Changed
Expand Down
8 changes: 8 additions & 0 deletions doc/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,11 @@ When set to true, use Java-specific semantics rather than native JavaScript sema
For example, if the node type is `number` per JS semantics where the value can be losslesly interpreted as `java.lang.Long`, the validator would use `integer` as the node type instead of `number`. This is useful when schema type is `integer`, since validation would fail otherwise.

For more details, please refer to this [issue](https://github.com/networknt/json-schema-validator/issues/334).

* losslessNarrowing

When set to true, can interpret round doubles as integers.

Note that setting `javaSemantics = true` will achieve the same functionality at this time.

For more details, please refer to this [issue](https://github.com/networknt/json-schema-validator/issues/344).
4 changes: 2 additions & 2 deletions src/main/java/com/networknt/schema/TypeFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ public static JsonType getValueNodeType(JsonNode node, SchemaValidatorsConfig co
if (node.isIntegralNumber())
return JsonType.INTEGER;
if (node.isNumber())
if (config != null && config.isJavaSemantics() && node.canConvertToLong() && (node.asText().indexOf('.') == -1))
if (config != null && config.isJavaSemantics() && node.canConvertToExactIntegral())
return JsonType.INTEGER;
else if (config!= null && config.isLosslessNarrowing() && node.asText().endsWith(".0"))
else if (config != null && config.isLosslessNarrowing() && node.canConvertToExactIntegral())
return JsonType.INTEGER;
else
return JsonType.NUMBER;
Expand Down
22 changes: 18 additions & 4 deletions src/test/java/com/networknt/schema/TypeFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,43 @@ public class TypeFactoryTest {

private static final String[] validIntegralValues = {
"1", "-1", "0E+1", "0E1", "-0E+1", "-0E1", "10.1E+1", "10.1E1", "-10.1E+1", "-10.1E1", "1E+0", "1E-0",
"1E0", "1E18", "9223372036854775807", "-9223372036854775808"
"1E0", "1E18", "9223372036854775807", "-9223372036854775808", "1.0", "1.00", "-1.0", "-1.00"
};

private static final String[] validNonIntegralNumberValues = {
"1.1", "-1.1", "1.10"
};

private final SchemaValidatorsConfig schemaValidatorsConfig = new SchemaValidatorsConfig();

@Test
public void testValidIntegralValuesWithJavaSemantics() {
public void testIntegralValuesWithJavaSemantics() {
schemaValidatorsConfig.setJavaSemantics(true);
for (String validValue : validIntegralValues) {
assertSame(JsonType.INTEGER,
getValueNodeType(DecimalNode.valueOf(new BigDecimal(validValue)), schemaValidatorsConfig),
validValue);
}
for (String validValue : validNonIntegralNumberValues) {
assertSame(JsonType.NUMBER,
getValueNodeType(DecimalNode.valueOf(new BigDecimal(validValue)), schemaValidatorsConfig),
validValue);
}
}

@Test
public void testValidIntegralValuesWithoutJavaSemantics() {
public void testIntegralValuesWithoutJavaSemantics() {
schemaValidatorsConfig.setJavaSemantics(false);
for (String validValue : validIntegralValues) {
assertSame(JsonType.NUMBER,
getValueNodeType(DecimalNode.valueOf(new BigDecimal(validValue)), schemaValidatorsConfig),
validValue);
}
for (String validValue : validNonIntegralNumberValues) {
assertSame(JsonType.NUMBER,
getValueNodeType(DecimalNode.valueOf(new BigDecimal(validValue)), schemaValidatorsConfig),
validValue);
}
}


Expand Down Expand Up @@ -82,4 +96,4 @@ public void testWithoutLosslessNarrowing() {
}

}
}
}

0 comments on commit 4172360

Please sign in to comment.