search

java-json-tools / json-schema-validator

A JSON Schema validation implementation in pure Java, which aims for correctness and performance, in that order
http://json-schema-validator.herokuapp.com/
Other
1.63k stars 399 forks source link

Remove joda-time library and use Java 8 time instead #355

Open chriskilding opened 3 years ago

chriskilding commented 3 years ago

Remove joda-time library and use Java 8 time instead.

chriskilding commented 3 years ago

This change might be an alternative solution to https://github.com/java-json-tools/json-schema-validator/issues/142

chriskilding commented 3 years ago

Starting with a largely mechanical line-by-line translation of Joda Time API calls to their Java 8 Time equivalents, to see what works and what doesn't.

chriskilding commented 3 years ago

First note is that Java 8's DateTimeFormatter#appendFraction accepts a maximum fraction width of 9 rather than 12. This is just a limitation of Java 8 time that we'd have to live with.

Several of the tests fail only because they use fraction widths longer than 9, so we'd need to delete those.

chriskilding commented 3 years ago

Just 3 test failures to go...

+HHmm offsets are rejected while +HH:mm offsets are allowed (in other words, the new parsing has strict RFC3339 offset format requirements everywhere, whereas parsing with Joda time was lenient):

Gradle suite > Gradle test > com.github.fge.jsonschema.format.common.DateTimeTest > instanceIsCorrectlyAnalyzed[0]("2012-12-02T13:05:00+0100", true, null, null) FAILED
    org.mockito.exceptions.verification.NoInteractionsWanted

Java time tolerates the 30th February whereas Joda rejects it:

Gradle suite > Gradle test > com.github.fge.jsonschema.format.draftv3.DateTest > instanceIsCorrectlyAnalyzed[5]("1922-02-30", false, string "1922-02-30" is invalid against requested date format(s) yyyy-MM-dd, {"value":"1922-02-30","expected":"yyyy-MM-dd"}) FAILED
    org.mockito.exceptions.verification.WantedButNotInvoked

Java time tolerates "24:00:00" whereas Joda rejects it:

Gradle suite > Gradle test > com.github.fge.jsonschema.format.draftv3.TimeTest > instanceIsCorrectlyAnalyzed[3]("24:00:00", false, string "24:00:00" is invalid against requested date format(s) HH:mm:ss, {"value":"24:00:00","expected":"HH:mm:ss"}) FAILED
    org.mockito.exceptions.verification.WantedButNotInvoked
adetunjiakintundeakinde commented 3 years ago

Do you need help with the PR. I am willing to help as we need this change for one of our projects

chriskilding commented 3 years ago

I do need suggestions on what should be done about the remaining 3 tests - whether each Java 8 time behaviour difference is acceptable (so we need to update or delete the relevant tests) or not (so we need to fine tune the implementation). The library maintainers need to provide their opinion on this as well.

Also if you can think of relevant missing test cases then please do suggest/add them :)

chriskilding commented 3 years ago

@Capstan would you (or one of the other maintainers) be able to take a look at this?

chriskilding commented 3 years ago

@ajorg-aws would you (or another maintainer) be able to review this?

ajorg-aws commented 3 years ago

@ajorg-aws would you (or another maintainer) be able to review this?

Hello! I'm not a maintainer, just a contributor. The commit you see from me was contributed in #351

chriskilding commented 3 years ago

@Capstan would you (or another maintainer) be able to take a look at this?

xp-vit commented 3 years ago

@chriskilding That is a really nice and necessary change! Shall we contact maintainers?

chriskilding commented 3 years ago

@huggsboson I believe you are one of the other maintainers in the java-json-tools org. Would you be able to review this and give me some design guidance to resolve the last failing tests, and then we can see about merging this?