Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bind: Maintain backwards compatibility for map[string]interface{} binding #2656

Merged
merged 2 commits into from
Jul 22, 2024
Merged

bind: Maintain backwards compatibility for map[string]interface{} binding #2656

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9622474
bind: Maintain backwards compatibility for map[string]interface{} bin…
thesaltree Jul 9, 2024
62fe8cd
bind to single string for map[string]interface{}{}
thesaltree Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions bind.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri
for k, v := range data {
if isElemString {
val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v[0]))
} else if isElemInterface {
if len(v) == 1 {
Copy link
Contributor

@aldas aldas Jul 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not certain that this behavior is not creating more problems. Number of query params does not need to be fixed. You could have situations where user send request with multiple values and some time one. now at backend you are forced to real with situation that you could be dealing with slice of strings or element of string type. This functionality could be some kind of search form with checkboxes etc.

I think if we want have backwards compatibility we need a check for map[string]interface{}{} that is produces always strings and not slices of strings

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aldas thanks for pointing this out. I agree having to deal with slice of strings and single string would be inconvenient.

I made changes as per your suggestions. Now for interface type, it will produce always first string. Please review again.

val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v[0]))
} else {
val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v))
}
} else {
val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v))
}
Expand Down
4 changes: 2 additions & 2 deletions bind_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ func TestDefaultBinder_bindDataToMap(t *testing.T) {
assert.Equal(t,
map[string]interface{}{
"multiple": []string{"1", "2"},
"single": []string{"3"},
"single": "3",
},
dest,
)
Expand All @@ -510,7 +510,7 @@ func TestDefaultBinder_bindDataToMap(t *testing.T) {
assert.Equal(t,
map[string]interface{}{
"multiple": []string{"1", "2"},
"single": []string{"3"},
"single": "3",
},
dest,
)
Expand Down
Loading