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

refactor(form_mapping.go): mapping multipart request #1829

Merged
merged 5 commits into from
Apr 2, 2019
Merged

refactor(form_mapping.go): mapping multipart request #1829

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cdc534f
refactor(form_mapping.go): mapping multipart request
vkd Mar 26, 2019
adff101
add checkers for a types to match with the setter interface
vkd Mar 30, 2019
6c5e1da
form_mapping.go: rename method name on setter interface, add comments
vkd Apr 1, 2019
fe96cf6
fix style of comments
vkd Apr 1, 2019
0235b7c
Merge branch 'master' into refactor_mapping_multipart
thinkerou Apr 1, 2019
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
31 changes: 26 additions & 5 deletions binding/form.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

package binding

import "net/http"
import (
"mime/multipart"
"net/http"
"reflect"
)

const defaultMemory = 32 * 1024 * 1024

Expand Down Expand Up @@ -53,13 +57,30 @@ func (formMultipartBinding) Bind(req *http.Request, obj interface{}) error {
if err := req.ParseMultipartForm(defaultMemory); err != nil {
return err
}
if err := mapForm(obj, req.MultipartForm.Value); err != nil {
if err := mappingByPtr(obj, (*multipartRequest)(req), "form"); err != nil {
return err
}

if err := mapFiles(obj, req); err != nil {
return err
return validate(obj)
}

type multipartRequest http.Request

var (
multipartFileHeaderStructType = reflect.TypeOf(multipart.FileHeader{})
)

func (r *multipartRequest) Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) {
Copy link
Member

@thinkerou thinkerou Mar 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

check: var _ setter = &multipartRequest
and add node for export method/class/interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean when you talk about "node"?
Btw, this method is not exported because the type multipartRequest is not exported, and I see no warnings about this from any linters.

Copy link
Member

Choose a reason for hiding this comment

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

@vkd sorry, word typo, note.
Yes, it's not exported. We should add some usage note for new and important interface/method/class even if un-exported. thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thanks. I've added a couple of comments.

if value.Type() == multipartFileHeaderStructType {
_, file, err := (*http.Request)(r).FormFile(key)
if err != nil {
return false, err
}
if file != nil {
value.Set(reflect.ValueOf(*file))
return true, nil
}
}

return validate(obj)
return setByForm(value, field, r.MultipartForm.Value, key, opt)
}
80 changes: 37 additions & 43 deletions binding/form_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package binding
import (
"errors"
"fmt"
"net/http"
"reflect"
"strconv"
"strings"
Expand All @@ -16,34 +15,6 @@ import (
"github.com/gin-gonic/gin/internal/json"
)

func mapFiles(ptr interface{}, req *http.Request) error {
typ := reflect.TypeOf(ptr).Elem()
val := reflect.ValueOf(ptr).Elem()
for i := 0; i < typ.NumField(); i++ {
typeField := typ.Field(i)
structField := val.Field(i)

t := fmt.Sprintf("%s", typeField.Type)
if string(t) != "*multipart.FileHeader" {
continue
}

inputFieldName := typeField.Tag.Get("form")
if inputFieldName == "" {
inputFieldName = typeField.Name
}

_, fileHeader, err := req.FormFile(inputFieldName)
if err != nil {
return err
}

structField.Set(reflect.ValueOf(fileHeader))

}
return nil
}

var errUnknownType = errors.New("Unknown type")

func mapUri(ptr interface{}, m map[string][]string) error {
Expand All @@ -57,11 +28,25 @@ func mapForm(ptr interface{}, form map[string][]string) error {
var emptyField = reflect.StructField{}

func mapFormByTag(ptr interface{}, form map[string][]string, tag string) error {
_, err := mapping(reflect.ValueOf(ptr), emptyField, form, tag)
return mappingByPtr(ptr, formSource(form), tag)
}

type setter interface {
Copy link
Member

@thinkerou thinkerou Mar 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

add some notes?

Copy link
Member

Choose a reason for hiding this comment

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

please not use // setter - style, thanks!

Set(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error)
}

type formSource map[string][]string

func (form formSource) Set(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) {
Copy link
Member

@thinkerou thinkerou Mar 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

check: var _ setter = &formSource
and add note for export method/class/interface.

return setByForm(value, field, form, tagValue, opt)
}

func mappingByPtr(ptr interface{}, setter setter, tag string) error {
_, err := mapping(reflect.ValueOf(ptr), emptyField, setter, tag)
return err
}

func mapping(value reflect.Value, field reflect.StructField, form map[string][]string, tag string) (bool, error) {
func mapping(value reflect.Value, field reflect.StructField, setter setter, tag string) (bool, error) {
var vKind = value.Kind()

if vKind == reflect.Ptr {
Expand All @@ -71,7 +56,7 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s
isNew = true
vPtr = reflect.New(value.Type().Elem())
}
isSetted, err := mapping(vPtr.Elem(), field, form, tag)
isSetted, err := mapping(vPtr.Elem(), field, setter, tag)
if err != nil {
return false, err
}
Expand All @@ -81,7 +66,7 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s
return isSetted, nil
}

ok, err := tryToSetValue(value, field, form, tag)
ok, err := tryToSetValue(value, field, setter, tag)
if err != nil {
return false, err
}
Expand All @@ -97,7 +82,7 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s
if !value.Field(i).CanSet() {
continue
}
ok, err := mapping(value.Field(i), tValue.Field(i), form, tag)
ok, err := mapping(value.Field(i), tValue.Field(i), setter, tag)
if err != nil {
return false, err
}
Expand All @@ -108,9 +93,14 @@ func mapping(value reflect.Value, field reflect.StructField, form map[string][]s
return false, nil
}

func tryToSetValue(value reflect.Value, field reflect.StructField, form map[string][]string, tag string) (bool, error) {
var tagValue, defaultValue string
var isDefaultExists bool
type setOptions struct {
isDefaultExists bool
defaultValue string
}

func tryToSetValue(value reflect.Value, field reflect.StructField, setter setter, tag string) (bool, error) {
var tagValue string
var setOpt setOptions

tagValue = field.Tag.Get(tag)
tagValue, opts := head(tagValue, ",")
Expand All @@ -132,25 +122,29 @@ func tryToSetValue(value reflect.Value, field reflect.StructField, form map[stri
k, v := head(opt, "=")
switch k {
case "default":
isDefaultExists = true
defaultValue = v
setOpt.isDefaultExists = true
setOpt.defaultValue = v
}
}

return setter.Set(value, field, tagValue, setOpt)
}

func setByForm(value reflect.Value, field reflect.StructField, form map[string][]string, tagValue string, opt setOptions) (isSetted bool, err error) {
vs, ok := form[tagValue]
if !ok && !isDefaultExists {
if !ok && !opt.isDefaultExists {
return false, nil
}

switch value.Kind() {
case reflect.Slice:
if !ok {
vs = []string{defaultValue}
vs = []string{opt.defaultValue}
}
return true, setSlice(vs, value, field)
case reflect.Array:
if !ok {
vs = []string{defaultValue}
vs = []string{opt.defaultValue}
}
if len(vs) != value.Len() {
return false, fmt.Errorf("%q is not valid value for %s", vs, value.Type().String())
Expand All @@ -159,7 +153,7 @@ func tryToSetValue(value reflect.Value, field reflect.StructField, form map[stri
default:
var val string
if !ok {
val = defaultValue
val = opt.defaultValue
}

if len(vs) > 0 {
Expand Down