From cd289465fd02f19452977536c568579c6f26fbc7 Mon Sep 17 00:00:00 2001 From: Kevin Wan Date: Wed, 22 Dec 2021 21:43:37 +0800 Subject: [PATCH] chore: coding style and comments (#1361) * chore: coding style and comments * chore: optimize `ParseJsonBody` (#1353) * chore: optimize `ParseJsonBody` * chore: optimize `ParseJsonBody` * fix: fix a test * chore: optimize `ParseJsonBody` * fix a test * chore: add comment * chore: refactor Co-authored-by: chenquan --- core/mapping/jsonunmarshaler_test.go | 47 +++++++++++++++++++++++ rest/handler/timeouthandler.go | 2 + rest/httpx/requests.go | 7 +--- rest/httpx/requests_test.go | 6 --- rest/internal/security/contentsecurity.go | 10 +---- tools/goctl/bug/env.go | 2 +- tools/goctl/bug/issue.go | 2 +- 7 files changed, 54 insertions(+), 22 deletions(-) diff --git a/core/mapping/jsonunmarshaler_test.go b/core/mapping/jsonunmarshaler_test.go index c98244c7..ccaf6b6c 100644 --- a/core/mapping/jsonunmarshaler_test.go +++ b/core/mapping/jsonunmarshaler_test.go @@ -871,3 +871,50 @@ func TestUnmarshalReaderError(t *testing.T) { assert.NotNil(t, err) assert.True(t, strings.Contains(err.Error(), payload)) } + +func TestUnmarshalMap(t *testing.T) { + t.Run("nil map and valid", func(t *testing.T) { + var m map[string]interface{} + var v struct { + Any string `json:",optional"` + } + + err := UnmarshalJsonMap(m, &v) + assert.Nil(t, err) + assert.True(t, len(v.Any) == 0) + }) + + t.Run("empty map but not valid", func(t *testing.T) { + m := map[string]interface{}{} + var v struct { + Any string + } + + err := UnmarshalJsonMap(m, &v) + assert.NotNil(t, err) + }) + + t.Run("empty map and valid", func(t *testing.T) { + m := map[string]interface{}{} + var v struct { + Any string `json:",optional"` + } + + err := UnmarshalJsonMap(m, &v) + assert.Nil(t, err) + assert.True(t, len(v.Any) == 0) + }) + + t.Run("valid map", func(t *testing.T) { + m := map[string]interface{}{ + "Any": "foo", + } + var v struct { + Any string + } + + err := UnmarshalJsonMap(m, &v) + assert.Nil(t, err) + assert.Equal(t, "foo", v.Any) + }) +} diff --git a/rest/handler/timeouthandler.go b/rest/handler/timeouthandler.go index 0e81c161..1b9532cf 100644 --- a/rest/handler/timeouthandler.go +++ b/rest/handler/timeouthandler.go @@ -89,6 +89,8 @@ func (h *timeoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { case <-ctx.Done(): tw.mu.Lock() defer tw.mu.Unlock() + // there isn't any user-defined middleware before TimoutHandler, + // so we can guarantee that cancelation in biz related code won't come here. if errors.Is(ctx.Err(), context.Canceled) { w.WriteHeader(statusClientClosedRequest) } else { diff --git a/rest/httpx/requests.go b/rest/httpx/requests.go index 2609030d..170e31c9 100644 --- a/rest/httpx/requests.go +++ b/rest/httpx/requests.go @@ -25,8 +25,6 @@ var ( pathUnmarshaler = mapping.NewUnmarshaler(pathKey, mapping.WithStringValues()) headerUnmarshaler = mapping.NewUnmarshaler(headerKey, mapping.WithStringValues(), mapping.WithCanonicalKeyFunc(textproto.CanonicalMIMEHeaderKey)) - - emptyMap = map[string]interface{}{} ) // Parse parses the request. @@ -107,13 +105,12 @@ func ParseHeader(headerValue string) map[string]string { // ParseJsonBody parses the post request which contains json in body. func ParseJsonBody(r *http.Request, v interface{}) error { - var reader io.Reader if withJsonBody(r) { - reader = io.LimitReader(r.Body, maxBodyLen) + reader := io.LimitReader(r.Body, maxBodyLen) return mapping.UnmarshalJsonReader(reader, v) } - return mapping.UnmarshalJsonMap(emptyMap, v) + return mapping.UnmarshalJsonMap(nil, v) } // ParsePath parses the symbols reside in url path. diff --git a/rest/httpx/requests_test.go b/rest/httpx/requests_test.go index 76cbe346..7e40fb94 100644 --- a/rest/httpx/requests_test.go +++ b/rest/httpx/requests_test.go @@ -196,9 +196,7 @@ Content-Disposition: form-data; name="age" } func TestParseJsonBody(t *testing.T) { - t.Run("has body", func(t *testing.T) { - var v struct { Name string `json:"name"` Age int `json:"age"` @@ -211,11 +209,9 @@ func TestParseJsonBody(t *testing.T) { assert.Nil(t, Parse(r, &v)) assert.Equal(t, "kevin", v.Name) assert.Equal(t, 18, v.Age) - }) t.Run("hasn't body", func(t *testing.T) { - var v struct { Name string `json:"name,optional"` Age int `json:"age,optional"` @@ -225,9 +221,7 @@ func TestParseJsonBody(t *testing.T) { assert.Nil(t, Parse(r, &v)) assert.Equal(t, "", v.Name) assert.Equal(t, 0, v.Age) - }) - } func TestParseRequired(t *testing.T) { diff --git a/rest/internal/security/contentsecurity.go b/rest/internal/security/contentsecurity.go index ec66f04c..4ed4237a 100644 --- a/rest/internal/security/contentsecurity.go +++ b/rest/internal/security/contentsecurity.go @@ -119,18 +119,10 @@ func VerifySignature(r *http.Request, securityHeader *ContentSecurityHeader, tol }, "\n") actualSignature := codec.HmacBase64(securityHeader.Key, signContent) - /*passed := securityHeader.Signature == actualSignature - if !passed { - logx.Infof("signature different, expect: %s, actual: %s", - securityHeader.Signature, actualSignature) - } - - if passed { - return httpx.CodeSignaturePass - }*/ if securityHeader.Signature == actualSignature { return httpx.CodeSignaturePass } + logx.Infof("signature different, expect: %s, actual: %s", securityHeader.Signature, actualSignature) diff --git a/tools/goctl/bug/env.go b/tools/goctl/bug/env.go index 6f954520..da66e8bd 100644 --- a/tools/goctl/bug/env.go +++ b/tools/goctl/bug/env.go @@ -21,7 +21,7 @@ func (e env) string() string { w.WriteString(fmt.Sprintf("%s = %q\n", k, v)) } - return strings.TrimSuffix(w.String(),"\n") + return strings.TrimSuffix(w.String(), "\n") } func getEnv() env { diff --git a/tools/goctl/bug/issue.go b/tools/goctl/bug/issue.go index c3cd0981..0c623aad 100644 --- a/tools/goctl/bug/issue.go +++ b/tools/goctl/bug/issue.go @@ -1,6 +1,6 @@ package bug -const issueTemplate=` +const issueTemplate = ` ### What category of issue (goctl or sdk)?