search

NorfairKing / autodocodec

self(auto)- documenting encoders and decoders
MIT License
121 stars 20 forks source link

Allow number codecs to specify that their values must be integers #36

Closed dmoverton closed 4 months ago

dmoverton commented 1 year ago

Add a field to the NumberCodec that specifies whether or not the value is required to be an integer. The main reason for doing this is to ensure that more precise schemas are generated for OpenAPI, Swagger and JSON Schema.

OpenAPI, Swagger and JSON Schema all support "type": "integer" as a type specifier, but because NumberCodec was not previously able to differentiate between integral and non-integral types it was generating schemas with "type": "number" for integral types.

NorfairKing commented 1 year ago

Just so we don't get in trouble with this again: are there any other numbers than integers and not integers?

CI fails as well, so that definitely needs to be fixed first, but I like the idea of this PR.

dmoverton commented 1 year ago

Just so we don't get in trouble with this again: are there any other numbers than integers and not integers?

Looking at the OpenAPI 3 spec, there is an optional format property which for number can be float or double, and for integer can be int32 or int64. We want to use the OpenAPI 3 spec to generate Rust types, so I suspect this extra information would be useful. I'll have a go at that.

The ToSchema instances in the Haskell openapi3 library actually generate the format property for the Haskell types Float, Double, Int32 and Int64. Unfortunately, it's not easy for Autodocodec to use those instances.

CI fails as well, so that definitely needs to be fixed first, but I like the idea of this PR.

It's failing with

Error: Unable to locate executable file: cachix

I'm not sure why my changes would be causing that error. Are you able to investigate that?

NorfairKing commented 1 year ago

Looking at the OpenAPI 3 spec, there is an optional format property which for number can be float or double, and for integer can be int32 or int64. We want to use the OpenAPI 3 spec to generate Rust types, so I suspect this extra information would be useful. I'll have a go at that.

In that case we could actually encode this information in the number codec.

Are you able to investigate that?

Yes we probably just need to bump the github actions' versions. As long as nix flake check passes, that's fine by me.

NorfairKing commented 1 year ago

Ping @dmoverton

dmoverton commented 1 year ago

Ping @dmoverton

Hi. I did do some work on using the "format" string for number, but I wasn't completely happy with it and then I got distracted by something else and never got back to it. I'll see if I can get something to you in the next day or so.

NorfairKing commented 1 year ago

@dmoverton Any news?

NorfairKing commented 4 months ago

This has now been implemented in the latest release.