Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changed from isIntegralNumber to canConvertToExactIntegral to support… #450

Merged
merged 1 commit into from
Sep 18, 2021

Conversation

mohsin-sq
Copy link
Contributor

… numbers with 0 decimal value as valid integers for max/min lengths.

Related to: #446

… numbers with 0 decimal value as valid integers for max/min lengths.
@stevehu stevehu merged commit 7522176 into networknt:master Sep 18, 2021
stevehu pushed a commit that referenced this pull request Jan 14, 2022
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants