-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Implement nested form binding for structs and arrays #2834
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
base: master
Are you sure you want to change the base?
Implement nested form binding for structs and arrays #2834
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2834 +/- ##
==========================================
- Coverage 93.25% 93.18% -0.07%
==========================================
Files 39 39
Lines 4652 4753 +101
==========================================
+ Hits 4338 4429 +91
- Misses 218 225 +7
- Partials 96 99 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@aldas |
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.
I am not very fond of this feature as it is potentially quite complex (in maintenance perspective). I did only rudimentary checks and these index issues make me little bit uneasy.
have you considered creating separate library out of it?
| } | ||
| case MIMEApplicationXML, MIMETextXML: | ||
| if err = xml.NewDecoder(req.Body).Decode(i); err != nil { | ||
| if ute, ok := err.(*xml.UnsupportedTypeError); ok { |
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.
why this is deleted?
| } | ||
| } | ||
|
|
||
| for key, values := range data { |
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.
This function is also called for header, params and query binding - should it run for them also?
| func parseFieldPath(key string) []interface{} { | ||
| var parts []interface{} | ||
| var buf strings.Builder | ||
| for i := 0; i < len(key); i++ { |
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.
Could it be simpler to iterate over key and stop when .[] is encountered cut these parts out of key. part = key[previous:i] this buf parts seems little bit too much.
| for ; j < len(key) && key[j] != ']'; j++ { | ||
| buf.WriteByte(key[j]) | ||
| } | ||
| index, _ := strconv.Atoi(buf.String()) |
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.
what if key is 18446744073709551615 (math.MaxUint64)?
or -1?
I am sure bind will panic with negative numbers
| return setWithProperType(fv.Kind(), value, fv) | ||
| } | ||
| return setValueByParts(fv, ft.Type, parts[1:], value) | ||
| case int: |
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.
I think these keys should be treated not as indexes but something like grouping key and only allocate so many elements in arrays/slices as there are distinct grouping keys.
just to guard against shenanigans with huge indexes
Summary
Addresses issue #2746 by adding support for binding nested structs, arrays, and pointer fields from form data in Echo.
This enhancement allows developers to seamlessly bind complex, deeply nested web form data (such as
team.members[0].name=Alice) directly to Go structs with nested slices, pointer fields, and multiple levels of depth.Changes
team.members[0].name).winner.players[1].role=Forwardandloser.leaders[0].name=Charlie.Benefits
Test plan