support bind http header param #1956 #1957

guonaihong · 2019-06-18T11:32:52Z

update #1956

package main

import (
	"fmt"
	"github.com/gin-gonic/gin"
)

type testHeader struct {
	Rate   int    `header:"Rate"`
	Domain string `header:"Domain"`
}

func main() {
	r := gin.Default()
	r.GET("/", func(c *gin.Context) {
		h := testHeader{}

		if err := c.ShouldBindHeader(&h); err != nil {
			c.JSON(200, err)
		}

		fmt.Printf("%#v\n", h)
		c.JSON(200, gin.H{"Rate": h.Rate, "Domain": h.Domain})
	})

	r.Run()

// client
// curl -H "rate:300" -H "domain:music" 127.0.0.1:8080/
// output
// {"Domain":"music","Rate":300}
}

update gin-gonic#1956 ``` package main import ( "fmt" "github.com/gin-gonic/gin" ) type testHeader struct { Rate int `header:"Rate"` Domain string `header:"Domain"` } func main() { r := gin.Default() r.GET("/", func(c *gin.Context) { h := testHeader{} if err := c.ShouldBindHeader(&h); err != nil { c.JSON(200, err) } fmt.Printf("%#v\n", h) c.JSON(200, gin.H{"Rate": h.Rate, "Domain": h.Domain}) }) r.Run() // client // curl -H "rate:300" -H "domain:music" 127.0.0.1:8080/ // output // {"Domain":"music","Rate":300} } ```

codecov · 2019-06-18T11:39:09Z

Codecov Report

Merging #1957 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1957      +/-   ##
==========================================
+ Coverage   98.77%   98.77%   +<.01%     
==========================================
  Files          39       40       +1     
  Lines        2198     2212      +14     
==========================================
+ Hits         2171     2185      +14     
  Misses         15       15              
  Partials       12       12

Impacted Files	Coverage Δ
binding/binding.go	`100% <ø> (ø)`	⬆️
binding/header.go	`100% <100%> (ø)`
context.go	`98.44% <100%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09a3650...adef0a8. Read the comment docs.

appleboy · 2019-06-20T03:52:31Z

@guonaihong Please write the unit test.

guonaihong · 2019-06-20T11:24:22Z

@appleboy ok

When the http header is obtained in the standard library, the key value will be modified by the CanonicalMIMEHeaderKey function, and finally the value of the http header will be obtained from the map. As follows. ```go func (h MIMEHeader) Get(key string) string { // ... v := h[CanonicalMIMEHeaderKey(key)] // ... } ``` This pr also follows this modification

vkd · 2019-06-23T14:17:00Z

binding/form_mapping.go

+	)
+
+	if opt.isHeader {
+		vs, ok = form[textproto.CanonicalMIMEHeaderKey(tagValue)]


Good idea to implement a header mapping.

In my mind, you complicate a little the current implementation of the form mapping.

This is my suggestion of an implementation (add to your new binding/header.go file):

func mapHeader(ptr interface{}, h map[string][]string) error { return mappingByPtr(ptr, headerSource(h), "header") } type headerSource map[string][]string var _ setter = headerSource(nil) func (hs headerSource) TrySet(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) { return setByForm(value, field, hs, textproto.CanonicalMIMEHeaderKey(tagValue), opt) }

Very good idea, I am going to modify it at night

Perfect, thanks!

You can even uncomment the //var _ setter = headerSource(nil) line, it is legal =)

vkd · 2019-06-23T14:18:41Z

binding/header.go

+	return "header"
+}
+
+func (headerBinding) Bind(req *http.Request, obj interface{}) error {


I think, is a good idea to add unit tests inside of binding package to cover it by 100% inside.

Please make a coverage of binding package to 100%. (binding/header.go)

This can help you:

cd binding go test -coverprofile=cover.prof go tool cover -html=cover.prof

Very good way，have submitted changes

env GOPATH=`pwd` go test github.com/gin-gonic/gin/binding -coverprofile=cover.prof ok github.com/gin-gonic/gin/binding 0.015s coverage: 100.0% of statements

vkd · 2019-06-25T13:42:14Z

binding/form_mapping.go

@@ -38,8 +38,6 @@ type setter interface {

 type formSource map[string][]string

-var _ setter = formSource(nil)


Please, return back this line.
And it will be perfect, in my mind, if you add var _ setter = headerSource(nil) after this line. Thanks.

I don't quite understand the meaning of this line of code.
Var _ setter = formSource(nil)
Can you tell me why?
Doesn't it work?

check formSource implement setter interface.

It is just checking in compile time, what this type is implemented that interface.

This line is optional, but it is for a good self-documented code. It simply shows why this new type was added. This is a good practice when you add a new type only for a specific interface.

demo

import ( "reflect" ) type setOptions struct { isDefaultExists bool defaultValue string } type setter interface { TrySet(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) } func mappingByPtr(ptr interface{}, setter setter, tag string) error { //_, err := mapping(reflect.ValueOf(ptr), emptyField, setter, tag) return nil } type formSource map[string][]string func (f formSource) TrySet(value reflect.Value, field reflect.StructField, key string, opt setOptions) (isSetted bool, err error) { return true, nil } type headerSource map[string][]string func main() { mappingByPtr(reflect.ValueOf(nil), formSource(map[string][]string{}), "test") mappingByPtr(reflect.ValueOf(nil), headerSource(map[string][]string{}), "test") }

output

./interface.go:32:49: cannot use headerSource(map[string][]string literal) (type headerSource) as type setter in argument to mappingByPtr:
headerSource does not implement setter (missing TrySet method)

idea

The compiler has checked when the parameter is assigned. It doesn't seem to be necessary code.
I will check the code tomorrow morning.

thinkerou · 2019-06-26T05:12:35Z

@guonaihong please add use case to README.md, thanks!

vkd

Excellent, thanks!

* support bind http header param gin-gonic#1956 update gin-gonic#1956 ``` package main import ( "fmt" "github.com/gin-gonic/gin" ) type testHeader struct { Rate int `header:"Rate"` Domain string `header:"Domain"` } func main() { r := gin.Default() r.GET("/", func(c *gin.Context) { h := testHeader{} if err := c.ShouldBindHeader(&h); err != nil { c.JSON(200, err) } fmt.Printf("%#v\n", h) c.JSON(200, gin.H{"Rate": h.Rate, "Domain": h.Domain}) }) r.Run() // client // curl -H "rate:300" -H "domain:music" 127.0.0.1:8080/ // output // {"Domain":"music","Rate":300} } ``` * add unit test * Modify the code to get the http header When the http header is obtained in the standard library, the key value will be modified by the CanonicalMIMEHeaderKey function, and finally the value of the http header will be obtained from the map. As follows. ```go func (h MIMEHeader) Get(key string) string { // ... v := h[CanonicalMIMEHeaderKey(key)] // ... } ``` This pr also follows this modification * Thanks to vkd for suggestions, modifying code * Increase test coverage env GOPATH=`pwd` go test github.com/gin-gonic/gin/binding -coverprofile=cover.prof ok github.com/gin-gonic/gin/binding 0.015s coverage: 100.0% of statements * Rollback check code * add use case to README.md

NeverBehave · 2022-07-25T22:20:37Z

binding/header.go

+var _ setter = headerSource(nil)
+
+func (hs headerSource) TrySet(value reflect.Value, field reflect.StructField, tagValue string, opt setOptions) (isSetted bool, err error) {
+	return setByForm(value, field, hs, textproto.CanonicalMIMEHeaderKey(tagValue), opt)


the tag value of CanonicalMIMEHeaderKey is used instead of the actual tag value. This should be mentioned in the docs.

appleboy added the feature label Jun 20, 2019

appleboy added this to the 1.5 milestone Jun 20, 2019

add unit test

f439a1b

vkd reviewed Jun 23, 2019

View reviewed changes

guonaihong added 2 commits June 24, 2019 22:20

Thanks to vkd for suggestions, modifying code

159224b

Increase test coverage

c438fc1

env GOPATH=`pwd` go test github.com/gin-gonic/gin/binding -coverprofile=cover.prof ok github.com/gin-gonic/gin/binding 0.015s coverage: 100.0% of statements

vkd reviewed Jun 25, 2019

View reviewed changes

Rollback check code

8a7e38a

thinkerou requested review from appleboy, thinkerou and javierprovecho June 26, 2019 05:12

add use case to README.md

f9f0f9d

vkd approved these changes Jun 26, 2019

View reviewed changes

appleboy approved these changes Jun 27, 2019

View reviewed changes

Merge branch 'master' into 1956-feature-support-bind-http-header

adef0a8

thinkerou merged commit f98b339 into gin-gonic:master Jun 27, 2019

NeverBehave reviewed Jul 25, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support bind http header param #1956 #1957

support bind http header param #1956 #1957

guonaihong commented Jun 18, 2019

codecov bot commented Jun 18, 2019 •

edited

Loading

appleboy commented Jun 20, 2019

guonaihong commented Jun 20, 2019

vkd Jun 23, 2019

guonaihong Jun 24, 2019

vkd Jun 25, 2019

vkd Jun 23, 2019

guonaihong Jun 24, 2019

vkd Jun 25, 2019

guonaihong Jun 25, 2019

vkd Jun 25, 2019

guonaihong Jun 25, 2019

thinkerou Jun 25, 2019

vkd Jun 25, 2019

guonaihong Jun 25, 2019

thinkerou commented Jun 26, 2019

vkd left a comment

NeverBehave Jul 25, 2022

		@@ -38,8 +38,6 @@ type setter interface {

		type formSource map[string][]string

		var _ setter = formSource(nil)

support bind http header param #1956 #1957

support bind http header param #1956 #1957

Conversation

guonaihong commented Jun 18, 2019

codecov bot commented Jun 18, 2019 • edited Loading

Codecov Report

appleboy commented Jun 20, 2019

guonaihong commented Jun 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

demo

output

idea

thinkerou commented Jun 26, 2019

vkd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 18, 2019 •

edited

Loading