Improve type validation of numeric values

[oguzhanunlu]

Issue
TypeFactory uses jackson-databind's JsonNode.isIntegralNumber and JsonNode.isNumber to see if the node's type is integer or number, respectively. This result is used when comparing node type and schema type which is not precise enough in some cases.

json-schema-validator/src/main/java/com/networknt/schema/TypeFactory.java

Lines 72 to 75 in 4ba09d5

	if (node.isIntegralNumber())
	return JsonType.INTEGER;
	if (node.isNumber())
	return JsonType.NUMBER;

Scenario
Assume that we have a schema where a property's type is integer.
Given a DecimalNode(v) where v is a value that can fit into Long, json-schema-validator thinks that node type is number and this results in type failure as number found, integer expected

Proposal
Update line 72-73 in the snippet above as following (canConvertToLong() is defined for DecimalNode at jackson-databind)

if (node.isIntegralNumber() || node.canConvertToLong())
    return JsonType.INTEGER; 

so that we can have a validator with better precision

[stevehu]

@oguzhanunlu Thanks a lot for raising the issue and provide a solution. Let me add some background information here so that others can see where this issue is from and why we should adopt your solution as a Java-specific implementation.

Warning

The precise treatment of the “integer” type may depend on the implementation of your JSON Schema validator. JavaScript (and thus also JSON) does not have distinct types for integers and floating-point values. Therefore, JSON Schema can not use type alone to distinguish between integers and non-integers. The JSON Schema specification recommends, but does not require, that validators use the mathematical value to determine whether a number is an integer, and not the type alone. Therefore, there is some disagreement between validators on this point. For example, a JavaScript-based validator may accept 1.0 as an integer, whereas the Python-based jsonschema does not.

Clever use of the multipleOf keyword (see Multiples) can be used to get around this discrepancy. For example, the following likely has the same behavior on all JSON Schema implementations:

{ "type": "number", "multipleOf": 1.0 }

Given the above quote from the official site, I think we should leverage the Java language feature to support the Long type; however, the change might make the validator different than the Javascript implementation. As this is a community-driven implementation, I want to know the opinion of @everybody. The best solution is to support both and use the configuration to differentiate it. We just need to make the decision of which implementation is the default.

[oguzhanunlu]

Thanks for the quick response @stevehu !

Looking at the implementation of DecimalNode.canConvertToLong in jackson-databind, it seems that floating values smaller than maximum long, e.g. 1.4, will be considered as integer, if we use the proposal I shared above, which would be wrong, so we need to use a better logical predicate to check if the node value can fit into a Long and it doesn't have a fractional value.

Instead of the initial proposal, perhaps something as following:

if (node.isIntegralNumber() || (node.canConvertToLong() && !node.asText().contains('.')))
    return JsonType.INTEGER; 

[oguzhanunlu]

Hi @stevehu , is there any update on this one? Or is there anything I can do to proceed here?

[stevehu]

@oguzhanunlu I think we should go ahead with the implementation. Just make sure to keep the existing behaviour as default and the new feature can be enabled from the configuration. Could you submit a PR for this? Thanks.

[oguzhanunlu]

sure @stevehu , I'll open a PR, thanks!

[oguzhanunlu]

hey @stevehu , fyi, I just opened the PR and wanted to share here as well in case you don't get notified

[stevehu]

@oguzhanunlu Thanks a lot for your help. I am working with customers these days and very busy. I am reviewing in my spare time and it will take a bit longer.

[oguzhanunlu]

I understand it @stevehu , no worries at all! I hope jackson-databind publishes 2.12.0 in the meantime.