ASCOMInitiative / ASCOMRemote

The ASCOM REST based Remote Driver Server and Remote Access Clients
GNU General Public License v3.0
57 stars 15 forks source link

uint32 is not a Swagger type #48

Closed bretmckee closed 1 year ago

bretmckee commented 1 year ago

The Swagger definition contains format: uint32, but uint32 is not a legal value for integers according to the Swagger Spec. I figured this out by Exporting go code Swagger Hub and getting errors which I try to compile it (the max values are too big because the generated code is treating the value as a 32 bit integer).

It could either be changed to reduce the max value to 2G (so it fits into a signed 32 bit value), or by changing the size to be int64 and keep the max the same. Either of those would technically be "breaking changes", but making the int 64 bits seems unlikely to actually break anything (since with 32 bit unsigned integers the max is technically a no-opt, and isn't clear what such code would do today if the user send 40G in one of these fields since it won't fit).

I'm happy to submit a PR with the changes if agreement is reached on what the right fix is.

Peter-Simpson commented 1 year ago

Hi Bret,

Our intent is to keep the documentation as close to the API definition as possible and it is unfortunate that OpenAPI and programming languages differ in how they represent numbers such as 32 bit signed integer and 32 bit unsigned integer.

We'd prefer not to start calling unsigned integers "integers" because this may mislead some developers when they actually come to implementing the APIs by hand as opposed to using creative tools that use the swagger definition.

We feel that the current presentation is the best compromise for developers even though the UInt32 format description is not OpenAPI compliant.

Best wishes, Peter

RReverser commented 1 year ago

For what it's worth, the response from OpenAPI spec authors on a relevant feature request was:

As per the JSON Schema spec format is an open-ended keyword, so you can define your own values.

So while Swagger itself might be limiting list of formats, arbitrary formats like uint32 are still in line with the OpenAPI spec.

Here someone even collected list of OpenAPI formats used in the wild, and it shows an even larger diversity: https://github.com/Mermade/openapi-specification-extensions/blob/main/formats/2021/formats.tsv

Peter-Simpson commented 1 year ago

Thanks for adding this update it's very helpful to know that we are in line with the OpenAPI specification and that Swagger is choosing to use only a specified subset of type descriptions.

Best wishes, Peter