fix(openapi): support kin-openapi exclusive bounds - #7607
Conversation
WalkthroughUpdates OpenAPI dependencies and refactors numeric example generation to support kin-openapi exclusive bounds, including OpenAPI 3.0 and 3.1 cases and impossible integer ranges. ChangesOpenAPI example compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/input/formats/openapi/examples_test.go (1)
11-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for the documented scenarios; consider adding a narrow-range case.
These cases correctly validate boolean (3.0) and numeric (3.1) exclusive bounds. Consider adding a case like
Min=0(exclusive) +Max=1(inclusive) for an integer schema — this narrow range currently trips up the midpoint-selection logic innumericExample(see comment onexamples.go), so it would make a useful regression test once that's fixed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/input/formats/openapi/examples_test.go` around lines 11 - 53, The existing exclusive-bound test table lacks coverage for a narrow integer range. Add a regression case to TestOpenAPIExampleExclusiveBounds using an exclusive minimum of 0 and inclusive maximum of 1, and assert the valid generated example is 1; keep the case focused on the numericExample midpoint-selection behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/input/formats/openapi/examples.go`:
- Around line 277-300: Update the midpoint handling in the example-generation
logic, including the branches for belowMinimum with maximum and aboveMaximum
with minimum, so integer candidates respect exclusive bounds and narrow ranges
return a valid boundary value when the midpoint truncates onto an excluded
boundary. Prefer the boundary-adjusted candidate and use the midpoint only when
it satisfies all bounds; add regression coverage in examples_test.go for both
asymmetric narrow exclusive-integer cases.
---
Nitpick comments:
In `@pkg/input/formats/openapi/examples_test.go`:
- Around line 11-53: The existing exclusive-bound test table lacks coverage for
a narrow integer range. Add a regression case to
TestOpenAPIExampleExclusiveBounds using an exclusive minimum of 0 and inclusive
maximum of 1, and assert the valid generated example is 1; keep the case focused
on the numericExample midpoint-selection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d5477e45-5754-446d-adf2-18b552f2445f
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modpkg/input/formats/openapi/examples.gopkg/input/formats/openapi/examples_test.go
| switch { | ||
| case belowMinimum && maximum != nil: | ||
| value = (*minimum + *maximum) / 2.0 | ||
| case belowMinimum: | ||
| value = *minimum | ||
| if minimumExclusive { | ||
| if schema.Type.Is("integer") { | ||
| value = math.Floor(value) + 1 | ||
| } else { | ||
| value = math.Nextafter(value, math.Inf(1)) | ||
| } | ||
| } | ||
| case aboveMaximum && minimum != nil: | ||
| value = (*minimum + *maximum) / 2.0 | ||
| case aboveMaximum: | ||
| value = *maximum | ||
| if maximumExclusive { | ||
| if schema.Type.Is("integer") { | ||
| value = math.Ceil(value) - 1 | ||
| } else { | ||
| value = math.Nextafter(value, math.Inf(-1)) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Midpoint branches mis-handle narrow exclusive integer ranges — valid schemas incorrectly return ErrNoExample.
For schema.Type=integer, Min=0 with exclusive minimum, Max=1 (inclusive) — i.e. {"type":"integer","minimum":0,"exclusiveMinimum":true,"maximum":1} — the only valid value is 1, but the current logic returns ErrNoExample:
belowMinimum && maximum != nil→value = (0+1)/2 = 0.5math.Trunc(0.5)→0- Final check:
minimumExclusive && 0 <= 0→ErrNoExample
The same defect is symmetric for aboveMaximum && minimum != nil (e.g. minimum=-2, exclusiveMaximum at -1, integer — valid answer -2 is also rejected). Unlike the boundary-only branches (belowMinimum, aboveMaximum), the midpoint branches never apply the Floor+1/Ceil-1 integer-exclusivity adjustment, nor do they fall back if the midpoint truncates onto an excluded boundary. This directly undermines the exclusive-bound support this PR is meant to add, and isn't covered by the added tests (which all use ranges wide enough that the midpoint truncation happens to land safely inside the bounds).
🐛 Proposed fix: try the boundary-adjusted candidate first, fall back to midpoint only if it doesn't fit
switch {
case belowMinimum && maximum != nil:
- value = (*minimum + *maximum) / 2.0
+ value = *minimum
+ if minimumExclusive {
+ if schema.Type.Is("integer") {
+ value = math.Floor(value) + 1
+ } else {
+ value = math.Nextafter(value, math.Inf(1))
+ }
+ }
+ if value > *maximum || (maximumExclusive && value >= *maximum) {
+ value = (*minimum + *maximum) / 2.0
+ }
case belowMinimum:
value = *minimum
if minimumExclusive {
if schema.Type.Is("integer") {
value = math.Floor(value) + 1
} else {
value = math.Nextafter(value, math.Inf(1))
}
}
case aboveMaximum && minimum != nil:
- value = (*minimum + *maximum) / 2.0
+ value = *maximum
+ if maximumExclusive {
+ if schema.Type.Is("integer") {
+ value = math.Ceil(value) - 1
+ } else {
+ value = math.Nextafter(value, math.Inf(-1))
+ }
+ }
+ if value < *minimum || (minimumExclusive && value <= *minimum) {
+ value = (*minimum + *maximum) / 2.0
+ }
case aboveMaximum:
value = *maximum
if maximumExclusive {
if schema.Type.Is("integer") {
value = math.Ceil(value) - 1
} else {
value = math.Nextafter(value, math.Inf(-1))
}
}
}Please also add a regression test for this narrow-range case in examples_test.go once fixed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch { | |
| case belowMinimum && maximum != nil: | |
| value = (*minimum + *maximum) / 2.0 | |
| case belowMinimum: | |
| value = *minimum | |
| if minimumExclusive { | |
| if schema.Type.Is("integer") { | |
| value = math.Floor(value) + 1 | |
| } else { | |
| value = math.Nextafter(value, math.Inf(1)) | |
| } | |
| } | |
| case aboveMaximum && minimum != nil: | |
| value = (*minimum + *maximum) / 2.0 | |
| case aboveMaximum: | |
| value = *maximum | |
| if maximumExclusive { | |
| if schema.Type.Is("integer") { | |
| value = math.Ceil(value) - 1 | |
| } else { | |
| value = math.Nextafter(value, math.Inf(-1)) | |
| } | |
| } | |
| } | |
| switch { | |
| case belowMinimum && maximum != nil: | |
| value = *minimum | |
| if minimumExclusive { | |
| if schema.Type.Is("integer") { | |
| value = math.Floor(value) + 1 | |
| } else { | |
| value = math.Nextafter(value, math.Inf(1)) | |
| } | |
| } | |
| if value > *maximum || (maximumExclusive && value >= *maximum) { | |
| value = (*minimum + *maximum) / 2.0 | |
| } | |
| case belowMinimum: | |
| value = *minimum | |
| if minimumExclusive { | |
| if schema.Type.Is("integer") { | |
| value = math.Floor(value) + 1 | |
| } else { | |
| value = math.Nextafter(value, math.Inf(1)) | |
| } | |
| } | |
| case aboveMaximum && minimum != nil: | |
| value = *maximum | |
| if maximumExclusive { | |
| if schema.Type.Is("integer") { | |
| value = math.Ceil(value) - 1 | |
| } else { | |
| value = math.Nextafter(value, math.Inf(-1)) | |
| } | |
| } | |
| if value < *minimum || (minimumExclusive && value <= *minimum) { | |
| value = (*minimum + *maximum) / 2.0 | |
| } | |
| case aboveMaximum: | |
| value = *maximum | |
| if maximumExclusive { | |
| if schema.Type.Is("integer") { | |
| value = math.Ceil(value) - 1 | |
| } else { | |
| value = math.Nextafter(value, math.Inf(-1)) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/input/formats/openapi/examples.go` around lines 277 - 300, Update the
midpoint handling in the example-generation logic, including the branches for
belowMinimum with maximum and aboveMaximum with minimum, so integer candidates
respect exclusive bounds and narrow ranges return a valid boundary value when
the midpoint truncates onto an excluded boundary. Prefer the boundary-adjusted
candidate and use the midpoint only when it satisfies all bounds; add regression
coverage in examples_test.go for both asymmetric narrow exclusive-integer cases.
|
Superseded by #7609. |
2 participants
Proposed changes
Update kin-openapi to v0.145.0 and adapt OpenAPI example generation to its version-aware ExclusiveBound type. OpenAPI 3.0 boolean exclusive bounds retain their existing behavior, while OpenAPI 3.1 numeric exclusiveMinimum and exclusiveMaximum values are now used as the effective bounds. Impossible exclusive integer ranges return ErrNoExample.
Fixes #7606.
Proof
Checklist
Summary by CodeRabbit
Bug Fixes
Tests