-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
fix: schema enum validation #464
Conversation
WalkthroughThe change in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #464 +/- ##
=======================================
Coverage 92.84% 92.84%
=======================================
Files 22 22
Lines 3761 3762 +1
=======================================
+ Hits 3492 3493 +1
Misses 225 225
Partials 44 44 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (6)
validate.go (6)
Line range hint
748-748
: Undefined typeRegistry
.Please ensure that the
Registry
type is defined within the scope of this project or imported correctly.
Line range hint
184-184
: Undefined typeSchema
.The
Schema
type is referenced but not defined or imported in this file. Please define it or include the appropriate package.
Line range hint
262-262
: Undefined typeRegistry
in multiple locations.The
Registry
type is used but not defined or imported. This issue appears in multiple locations and needs to be addressed to ensure the code compiles.Also applies to: 280-280
Line range hint
162-162
: Undefined typeErrorDetail
.The
ErrorDetail
type is used in error handling but is not defined or imported. This needs to be resolved to properly handle errors.Also applies to: 172-172
Line range hint
345-345
: Undefined types for schema validation.Several types such as
TypeBoolean
,TypeNumber
,TypeString
,TypeArray
, andTypeObject
are used for schema validation but are not defined or imported. These should be declared or the appropriate packages should be imported.Also applies to: 350-350, 408-408, 444-444, 477-477
Line range hint
757-757
: Undefined functionNewMapRegistry
.The function
NewMapRegistry
is called but not defined or imported in this file. Please ensure it is available within the project's scope.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- validate.go (1 hunks)
Additional context used
golangci-lint
validate.go
748-748: undefined: Registry
184-184: undefined: Schema
262-262: undefined: Registry
280-280: undefined: Registry
162-162: undefined: ErrorDetail
172-172: undefined: ErrorDetail
345-345: undefined: TypeBoolean
350-350: undefined: TypeNumber
408-408: undefined: TypeString
444-444: undefined: TypeArray
477-477: undefined: TypeObject
757-757: undefined: NewMapRegistry
Additional comments not posted (1)
validate.go (1)
490-492
: Ensure consistent enum value comparison by converting to string.This change aligns with the PR's objective to fix schema enum validation by ensuring that the comparison is done using string representations, which matches the JSON error message expectations.
@danielgtaylor i think this issue relates to #438. A good solution on #438 solves the problem here and avoids the use of a fmt.Sprint which allocates and seems a bit hacky |
@danielgtaylor is there any chances to get support for enum types? type VehicleType string
const (
VehicleTypeCar VehicleType = "car"
VehicleTypeTruck VehicleType = "truck"
VehicleTypeSpecial VehicleType = "special"
)
func (*VehicleType) Schema(r huma.Registry) *huma.Schema {
schema := huma.SchemaFromType(r, reflect.TypeFor[string]()) // always complains that the validation failed
schema.Enum = []any{VehicleTypeCar, VehicleTypeTruck, VehicleTypeSpecial}
return schema
} func (*VehicleType) Schema(r huma.Registry) *huma.Schema {
schema := huma.SchemaFromType(r, reflect.TypeFor[VehicleType]()) // panics
schema.Enum = []any{VehicleTypeCar, VehicleTypeTruck, VehicleTypeSpecial}
return schema
} Actually enums are quite common in API schemas |
@codercms have a try func (*VehicleType) Schema(r huma.Registry) *huma.Schema {
schema := huma.SchemaFromType(r, reflect.TypeFor[string]()) // always complains that the validation failed
schema.Enum = []any{string(VehicleTypeCar), string(VehicleTypeTruck), string(VehicleTypeSpecial)}
return schema
} |
@fourcels it works, thanks :) |
Summary by CodeRabbit