Add support for "duration" string format.

[lcharlois-neotys]

Fix #116

Motivation:

"duration" format is valid since RFC 2019-09 and in OpenApi 3.0.

[pk-work]

Thanks for the PR! This looks very good. We have also tests for duration in the test suite [1]. At the moment these tests are disabled, could you please remove related lines in this file [2] to activate the tests?

[1] https://github.com/eclipse-vertx/vertx-json-schema/blob/master/src/test/resources/test-suite-tck.json
[2] https://github.com/eclipse-vertx/vertx-json-schema/blob/master/src/test/resources/unsupported-tck-tests.properties

[lcharlois-neotys]

@pk-work It's done.
I had to change 2 test definitions (see my comment).

[pk-work]

Where is the comment? I think the test suite we are using is the official one from JSON Schema. We should not change the tests, we should change the code that it fits to the tests.

[lcharlois-neotys]

@pk-work I had to change these test definition.
They were failling and they looked wrong to me as PT1D is not a valid duration in ISO 8601.
I let you confirm I'm not wrong.

[lcharlois-neotys]

Where is the comment? I think the test suite we are using is the official one from JSON Schema. We should not change the tests, we should change the code that it fits to the tests.

Sorry, I forgot to publih my comment.
Yes, I'm not telling it must be changed, perhaps I don't understand correctly the test definition.

[lcharlois-neotys]

Hello @pk-work,

What's your feedback regarding my last comment?

Best,

[pk-work]

@lcharlois-neotys Hi, unfortunately I was ill, but I will try to have a look on Friday. I need to understand it a little bit better, because I don't know yet if the test data is correct or not.

[lcharlois-neotys]

@lcharlois-neotys Hi, unfortunately I was ill, but I will try to have a look on Friday. I need to understand it a little bit better, because I don't know yet if the test data is correct or not.

Oh sorry, I was not sure if you have missed the comment or if you were not able to answer.

I took a look since last time.
What I see is that the same test is defined on JSON Schema repository.

So my change looks definetely wrong.
What I don't understand is what is the meaning of this test.

[pk-work]

@lcharlois-neotys, I had now time to look into this problem.

According to this link [1] JSON Schema is using ISO_8601 [2] and in particular ISO 8601 Collected ABNF [3], which states

ISO 8601 states that the "T" may be omitted under some circumstances. This grammar requires the "T" to avoid ambiguity.

Which explains PT3D which is a period of 3 days.

[1] https://json-schema.org/understanding-json-schema/reference/string#dates-and-times
[2] https://en.wikipedia.org/wiki/ISO_8601
[3] https://datatracker.ietf.org/doc/html/rfc3339#appendix-A

[lcharlois-neotys]

@pk-work Thanks for your detailed answer.

I don't share your analysis and still think that PT1D is not a valid duration, but I think this is not the real problem here.

Looking more attentively to the project I've seen that the library is able to correctly validate the duration format.
In fact the tck tests are already active and are green.

The implmentation relies on the class io.vertx.json.schema.impl.SchemaValidatorImpl.
So here I don't understand why openapi3 is not supporting duration format validation.

I can still take the time to fix, but here I will need more guidance on how to enable duration validation for openapi3.

PS: Regarding the PT1D case, the best answer is that tck test explicitely defines it as invalid.

[pk-work]

@lcharlois-neotys You are right, I just concentrated on the "T", but the value "1D" (1 day), must be added directly after the "P" not the "T".

I now looked into the other test cases, and found that "P1YT" is also invalid. But as I mentioned above I thought the "T" is required. So I'm a little bit confused ...

But for now I would say the test suite is the truth.

[lcharlois-neotys]

@pk-work From what I understand:
T is only required if it have a value in (hours, minutes or seconds), if it has no value it must not be present.
P is always required and if it doesn't have a value (in years, months, weeks or days) it must be followed directly by a valid T part.

[pk-work]

"duration" format is valid since RFC 2019-09 and in OpenApi 3.0.

I think OpenAPI 3.0 is using Draft4 [1]. Which would explain why "duration" is not available.

[1] https://github.com/OAI/OpenAPI-Specification/blob/78170608af208da8165ab095715e5cb9ff715f47/schemas/v3.0/schema.yaml#L2

[lcharlois-neotys]

@pk-work You're right.
I was wrong in my original issue. I based my issue on the wrong specification (I based it on 3.1.0).
I'm sorry for this.
I'm closing the PR and the issue too wuth a comment.

[kennethaasan]

Hey, this was a great discussion. I just made ajv-validator/ajv-formats#87 for ajv, and we need support for negative duration for our Java users in this library. If we make a PR that allows that, will that be accepted?

Thanks, Kenneth

[pk-work]

Is a "negative duration" something somewhere mentioned in the official spec? Haven't found it so far.

[kennethaasan]

It's not easy to find as the ISO is behind a paywall... But I managed to find https://www.postgresql.org/message-id/9q0ftb37dv7.fsf%40gmx.us which explains that this is covered by the 2019 edition of ISO-8601. It's supported by java.time.Duration, and date-fns/moment in the JS world.

Another good discussion here: tc39/proposal-temporal#782