Repository navigation
Allow range be Float - #905
Conversation
|
@artyomb … yeah, makes sense to allow any kind of number please rebase against master, it fixes the rubocop stuff |
|
Sorry, if I broke something. I don't know how to rebase with this pull request. |
|
I did "sync fork". Hope it helped? |
|
no … please have a look on the failures, it seems your change is related to it |
|
Ok, sorry. I don't know how complete the request. Please close/reject it. Maybe move to the issues. |
|
The patch is still actual, is it possible to accept the request? |
Danger ReportNo issues found. |
There was a problem hiding this comment.
🟡 Changes recommended
The parameter-range guard can raise on beginless ranges and silently omit BigDecimal bounds, leaving valid Swagger minimum and maximum values undocumented.
1 open issue
What changed in this PR
This PR lets float ranges in a parameter's values option produce minimum and maximum in the generated Swagger output. Before, only integer ranges did. The Swagger spec allows numeric bounds, and the linked issue #904 asks for the same change.
Changes:
- The range check in
parse_enum_or_range_valuesnow accepts ranges whose first value is aFloatas well as anInteger. - The float range spec in
param_values_spec.rbnow expectsminimum: -5.0andmaximum: 5.0instead of a string range. - A new unit spec in
parse_params_spec.rbcovers-5.0..5.0, and a CHANGELOG entry records the fix.
| File | Description |
|---|---|
| spec/swagger_v2/param_values_spec.rb | Updates the float range expectation to check numeric minimum and maximum values. |
| spec/lib/parse_params_spec.rb | Adds a unit test for a float range passed to parse_enum_or_range_values. |
| lib/grape-swagger/doc_methods/parse_params.rb | Widens the range check to accept float ranges. |
| CHANGELOG.md | Adds a fix entry for float range support. |
Review effort: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate edge-case defects remain: non-real Complex range bounds can abort Swagger generation, and exclusive float ranges can be documented with an incorrectly inclusive maximum.
2 open issues
1 resolved since last review
Review effort: Lite
The Float range support documents minimum and maximum, which an existing spec still asserted as absent. Update it, add a unit test for the Float case and record the change.
A Float range with an infinite bound, such as 1.0..Float::INFINITY, put Infinity into the parameter and made /swagger_doc fail to serialize. Only finite numeric bounds are emitted now. Beginless ranges no longer raise, and BigDecimal ranges, which map to a Swagger double, are documented as floats.
An exclusive range such as 1.0...5.0 was documented with an inclusive maximum, so the excluded end looked valid. It now also sets exclusiveMaximum, which survives in body definitions. A range with a Complex bound no longer aborts Swagger generation.
Range bounds that are not Integer or Float were always rounded through Float, so a whole BigDecimal such as 9007199254740993 lost precision and 1 was documented as 1.0. Whole values now stay Integers; only fractional ones become Floats. Specs also cover Rational, NaN bounds, and exclusiveMaximum on the items of a query array.
A range with a real start and a non-real end, such as 1..Complex(2, 0), was documented with only its start because the check looked at a single bound. Checking all bounds drops the whole range instead, matching how non-real bounds are already handled.
#904 (comment)
As described in https://docs.swagger.io/spec.html
minimum | string | number, integer | The minimum valid value for the type, inclusive. If this field is used in conjunction with the defaultValue field, then the default value MUST be higher than or equal to this value. The value type is string and should represent the minimum numeric value. Note: This will change to a numeric value in the future.
The values may be number (float or double)?