search

astm-utm / Protocol

ASTM UTM Protocol (API and sequence diagrams)
16 stars 11 forks source link

Appropriate use of exclusiveMinimum and exclusiveMaximum #65

Open nasajoey opened 3 years ago

nasajoey commented 3 years ago

https://github.com/astm-utm/Protocol/blob/c005a5838c2b1be6a50d31bb2a7cbde8dc721c31/utm.yaml#L175

OpenAPI 3.0.3 (and I think 3.0.2) defer the definition of the exclusiveMaximum and exclusiveMinimum fields to the JSON schema specification. There (https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.6.2) the definition is:

6.2.3. exclusiveMaximum The value of "exclusiveMaximum" MUST be a number, representing an exclusive upper limit for a numeric instance.

If the instance is a number, then the instance is valid only if it has a value strictly less than (not equal to) "exclusiveMaximum".

The current astm-utm yaml uses booleans. I think this is a holdover from OAS 2.0 (https://swagger.io/specification/v2/) that defined them as such.

This issue/task would be to disposition all of the exclusiveMinimum and exclusiveMaximum fields. This may mean removing them, or updating them with a numeric value per JSON Schema specification. Note that if it is the latter, there may be a need to remove or modify any adjacent "minimum"/"maximum" fields.

This change is important for tooling related to code generation and validation of the schema. For example, trying to re-use or reference these schemas from other schemas causes errors due to lack of JSON Schema adherence.

nasajoey commented 3 years ago

This may be messy. I think OAS 3.0.3 is pinned to an older version of JSON Spec (https://datatracker.ietf.org/doc/html/draft-wright-json-schema-validation-00). Whereas the latest JSON Spec has the change noted in the issue. Our problem is that we are adapting the API to asyncAPI and that specification pins to a slightly later version of JSON Schema (https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-validation-01#section-6.2.3).

Great.

nasajoey commented 3 years ago

A related discussion: https://github.com/tiangolo/fastapi/issues/240

nasajoey commented 3 years ago

Alright... down the rabbit hole and back out. OAS3.1 would address this issue. Will leave this issue open for any discussion, but likely should be ultimately closed with no action. Other than to potentially open a separate issue requesting upgrade to OAS3.1. I'm not clear on all the implications with that (other than exclusiveMinimum and max will work the same way as in AsyncAPI).

https://spec.openapis.org/oas/v3.1.0

BenjaminPelletier commented 3 years ago

IIRC, OAS3.1 is an enormous upgrade even if only for the benefit that description is allowed alongside $ref; that will let us remove all the problematic allOf/anyOf keywords. I had looked at upgrading a couple months back, but at that time, there was essentially no tooling that supported 3.1. The key tooling we want to make sure supports the format used here includes ReDoc (the primary viewer used by most people not actively code'gening something from the API) and SwaggerHub. SwaggerHub is apparently "preparing" to support 3.1. Redocly may have implemented support in May? I thought I looked more recently than May and they didn't support when I looked, but I'm not sure. Once both those tools have 3.1 support, I very much look forward to eliminating allOf/anyOf, and taking advantage of the other benefits as well.