gin
gin copied to clipboard
Map validation, contextual validation, validator tags for custom map/slice types
This pull request offers a few simple, but very useful improvements:
- Adds support for validating maps, including registering tags for map types (as well as slice types).
- Adds support for contextual validation. Custom validators (e.g. those registered with
(*Validator).RegisterValidationCtxor(*Validator).RegisterStructValidationCtx) will be passed the Gin context (ascontext.Context). Resolves https://github.com/gin-gonic/gin/issues/2741 - Returns slice and map validation errors are regular
validator.ValidationErrors, without any custom types. This way callers can handle all validation errors in the same manner. Related to: https://github.com/gin-gonic/gin/pull/2777
A further improvement would be to merge this PR: https://github.com/gin-gonic/gin/pull/2825. That will enable getting the actual *gin.Context in custom validators, resolving https://github.com/gin-gonic/gin/issues/1489.
Codecov Report
Merging #2877 (b626f63) into master (efa3175) will increase coverage by
0.02%. The diff coverage is100.00%.
@@ Coverage Diff @@
## master #2877 +/- ##
==========================================
+ Coverage 98.73% 98.76% +0.02%
==========================================
Files 41 41
Lines 3080 3150 +70
==========================================
+ Hits 3041 3111 +70
Misses 27 27
Partials 12 12
| Flag | Coverage Δ | |
|---|---|---|
| go-1.13 | 98.76% <100.00%> (+0.02%) |
:arrow_up: |
| go-1.14 | 98.60% <100.00%> (+0.03%) |
:arrow_up: |
| go-1.15 | 98.60% <100.00%> (+0.03%) |
:arrow_up: |
| go-1.16 | 98.60% <100.00%> (+0.03%) |
:arrow_up: |
| go-1.17 | 98.50% <100.00%> (+0.03%) |
:arrow_up: |
| macos-latest | 98.76% <100.00%> (+0.02%) |
:arrow_up: |
| nomsgpack | 98.74% <100.00%> (+0.02%) |
:arrow_up: |
| ubuntu-latest | 98.76% <100.00%> (+0.02%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| binding/binding.go | 100.00% <100.00%> (ø) |
|
| binding/binding_nomsgpack.go | 100.00% <100.00%> (ø) |
|
| binding/default_validator.go | 100.00% <100.00%> (ø) |
|
| binding/form.go | 100.00% <100.00%> (ø) |
|
| binding/header.go | 100.00% <100.00%> (ø) |
|
| binding/json.go | 100.00% <100.00%> (ø) |
|
| binding/msgpack.go | 100.00% <100.00%> (ø) |
|
| binding/query.go | 100.00% <100.00%> (ø) |
|
| binding/uri.go | 100.00% <100.00%> (ø) |
|
| binding/xml.go | 100.00% <100.00%> (ø) |
|
| ... and 4 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update efa3175...b626f63. Read the comment docs.
@appleboy @thinkerou Would you have the time to review this PR? I suggest looking at it commit by commit, since each commit is self-contained.
I did some renaming (Ctx -> Context) and fixed my omission in ShouldBindWith and ShouldBindBodyWith, which should call the contextual binding methods, but didn't. I also rebased onto latest master.
Turns out that this may break custom validator tags if they use FieldLevel.Top(). I'm working on a version which will avoid that.
I have reworked my implementation. Now it does not rely on Validate.Var/Validate.VarCtx, except for validating custom slice/array/map types. It will behave similar to the existing implementation, except the returned error is always of type validator.ValidationErrors and can be inspected to get the failing element index (or key in the case of maps).
how?
how?
Hey, what exactly are you asking about?
I have found an edge case when validating non-nil pointers with nil value. It can be fixed by changing
case reflect.Ptr:
return v.ValidateStructContext(ctx, value.Elem().Interface())
to
case reflect.Ptr:
if value.IsNil() {
return nil
}
return v.ValidateStructContext(ctx, value.Elem().Interface())
in ValidateStructContext.
I'm not updating the commits, though, because the maintainers don't seem to be particularly interested in this PR anyway.
Great feature, that will allow me to refactor a bunch of validation code. Hope this gets merged soon! 🙏
Great feature, that will allow me to refactor a bunch of validation code. Hope this gets merged soon! 🙏
This PR has been here for a long time. I kept it open just in case, but I'm not even updating it anymore. The chances of getting it merged seem slim. I could try to rebase it, though, if the maintainers are interested in merging it.
Any chance we can get this rolling?