Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

property described with multipleOf does not pass the validation #817

Open
asia111307 opened this issue Jun 28, 2023 · 3 comments
Open

property described with multipleOf does not pass the validation #817

asia111307 opened this issue Jun 28, 2023 · 3 comments

Comments

@asia111307
Copy link

In my openapi file, I have one property that is described as follows:

Quantity:
    type: number
    maximum: 999999999.99
    minimum: -999999999.99
    multipleOf: 0.01

The problem is that not every value provided as Quantity passes validation.

This part of lib code seems to be responsible for random outcome: kin-openapi/openapi3/schema.go#L1508 as the division value/0.01 sometimes gives a rounded value (so it’s Int and it passes validation), but sometimes it gives decimal outcome and the validation returns an error.

Here are some examples of Quantity for which division by 0.01 returns Int, so the validation passes:
8.11
68.15
2130.46
5895083

And here are some examples of Quantity for which division by 0.01 returns non-Int, so the validation fails:
8.1
2.07
19628.87
323.39
40428.2

I believe some adjustments to this validation should be provided.

@fenollp
Copy link
Collaborator

fenollp commented Jun 28, 2023

Hello! Indeed IEEE floats seem to be getting in the way. https://pkg.go.dev/math/big#Rat should be used instead.
If you're down to open a PR with this fix as well as with regression tests, I'll gladly review it! Thanks

@maxxant
Copy link

maxxant commented Sep 20, 2023

Good discussion and proposal for golang "decimal float types (IEEE 754-2008)" :
golang/go#19787

Good explanation "Why don't you just use big.Rat" for money and trading or financial application:
https://github.com/shopspring/decimal#why-dont-you-just-use-bigrat

another decimal implementation woodsbury/decimal128 that can do a few things faster:
woodsbury/decimal128#4

@fenollp
Copy link
Collaborator

fenollp commented Nov 22, 2023

Hey @maxxant thanks for doing this!

Indeed big.Rat usage is full of footguns. decimal is too short on precision for JSON (float64). decimal128 looks good enough to go with it!
We should keep that choice of lib hidden (to the API) so as to be able to switch to a future golang/std impl.
This means tweaking openapi3.Schema de-serialization of Min Max and MultipleOf, also json.Number.
If we can't manage this well we'll that'd just be a breaking change and new version.

Feel free to publish your attempts at this! I'll help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants