Repository: zeromicro/go-zero
Pull request #2928 (2023-02-26 13:12:47)

chore: refine rest validator

codecov[bot]: # [Codecov](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#2928](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (8b05b38) into [master](https://codecov.io/gh/zeromicro/go-zero/commit/92c8899f47eac8827ac2da06cd2a762629bdb4ab?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (92c8899) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 8b05b38 differs from pull request most recent head 9b1498d. Consider uploading reports for the commit 9b1498d to get more accurate results


| [Impacted Files](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [rest/httpx/requests.go](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cmVzdC9odHRweC9yZXF1ZXN0cy5nbw==) | `94.54% <100.00%> (+9.09%)` | :arrow_up: |
| [core/stores/sqlx/tx.go](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvc3FseC90eC5nbw==) | `19.41% <0.00%> (-3.89%)` | :arrow_down: |
| [core/logx/rotatelogger.go](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9sb2d4L3JvdGF0ZWxvZ2dlci5nbw==) | `72.69% <0.00%> (-1.11%)` | :arrow_down: |
| [core/stores/sqlx/sqlconn.go](https://codecov.io/gh/zeromicro/go-zero/pull/2928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvc3FseC9zcWxjb25uLmdv) | `57.84% <0.00%> (+1.47%)` | :arrow_up: |


Patch:
diff --git a/rest/httpx/requests.go b/rest/httpx/requests.go
index b923d1a2..cd088bb0 100644
--- a/rest/httpx/requests.go
+++ b/rest/httpx/requests.go
@@ -4,6 +4,7 @@ import (
 	"io"
 	"net/http"
 	"strings"
+	"sync/atomic"
 
 	"github.com/zeromicro/go-zero/core/mapping"
 	"github.com/zeromicro/go-zero/rest/internal/encoding"
@@ -23,15 +24,13 @@ const (
 var (
 	formUnmarshaler = mapping.NewUnmarshaler(formKey, mapping.WithStringValues())
 	pathUnmarshaler = mapping.NewUnmarshaler(pathKey, mapping.WithStringValues())
-	xValidator      Validator
+	validator       atomic.Value
 )
 
+// Validator defines the interface for validating the request.
 type Validator interface {
-	Validate(data interface{}, lang string) error
-}
-
-func SetValidator(validator Validator) {
-	xValidator = validator
+	// Validate validates the request and parsed data.
+	Validate(r *http.Request, data any) error
 }
 
 // Parse parses the request.
@@ -52,9 +51,10 @@ func Parse(r *http.Request, v any) error {
 		return err
 	}
 
-	if xValidator != nil {
-		return xValidator.Validate(v, r.Header.Get("Accept-Language"))
+	if val := validator.Load(); val != nil {
+		return val.(Validator).Validate(r, v)
 	}
+
 	return nil
 }
 
@@ -117,6 +117,13 @@ func ParsePath(r *http.Request, v any) error {
 	return pathUnmarshaler.Unmarshal(m, v)
 }
 
+// SetValidator sets the validator.
+// The validator is used to validate the request, only called in Parse,
+// not in ParseHeaders, ParseForm, ParseHeader, ParseJsonBody, ParsePath.
+func SetValidator(val Validator) {
+	validator.Store(val)
+}
+
 func withJsonBody(r *http.Request) bool {
 	return r.ContentLength > 0 && strings.Contains(r.Header.Get(header.ContentType), header.ApplicationJson)
 }


Test patch:
diff --git a/rest/httpx/requests_test.go b/rest/httpx/requests_test.go
index d6601f33..fa7dd9dc 100644
--- a/rest/httpx/requests_test.go
+++ b/rest/httpx/requests_test.go
@@ -1,8 +1,10 @@
 package httpx
 
 import (
+	"errors"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"strconv"
 	"strings"
 	"testing"
@@ -207,9 +209,23 @@ func TestParseJsonBody(t *testing.T) {
 		r := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
 		r.Header.Set(ContentType, header.JsonContentType)
 
-		assert.Nil(t, Parse(r, &v))
-		assert.Equal(t, "kevin", v.Name)
-		assert.Equal(t, 18, v.Age)
+		if assert.NoError(t, Parse(r, &v)) {
+			assert.Equal(t, "kevin", v.Name)
+			assert.Equal(t, 18, v.Age)
+		}
+	})
+
+	t.Run("bad body", func(t *testing.T) {
+		var v struct {
+			Name string `json:"name"`
+			Age  int    `json:"age"`
+		}
+
+		body := `{"name":"kevin", "ag": 18}`
+		r := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
+		r.Header.Set(ContentType, header.JsonContentType)
+
+		assert.Error(t, Parse(r, &v))
 	})
 
 	t.Run("hasn't body", func(t *testing.T) {
@@ -308,6 +324,36 @@ func TestParseHeaders_Error(t *testing.T) {
 	assert.NotNil(t, Parse(r, &v))
 }
 
+func TestParseWithValidator(t *testing.T) {
+	SetValidator(mockValidator{})
+	var v struct {
+		Name    string  `form:"name"`
+		Age     int     `form:"age"`
+		Percent float64 `form:"percent,optional"`
+	}
+
+	r, err := http.NewRequest(http.MethodGet, "/a?name=hello&age=18&percent=3.4", http.NoBody)
+	assert.Nil(t, err)
+	if assert.NoError(t, Parse(r, &v)) {
+		assert.Equal(t, "hello", v.Name)
+		assert.Equal(t, 18, v.Age)
+		assert.Equal(t, 3.4, v.Percent)
+	}
+}
+
+func TestParseWithValidatorWithError(t *testing.T) {
+	SetValidator(mockValidator{})
+	var v struct {
+		Name    string  `form:"name"`
+		Age     int     `form:"age"`
+		Percent float64 `form:"percent,optional"`
+	}
+
+	r, err := http.NewRequest(http.MethodGet, "/a?name=world&age=18&percent=3.4", http.NoBody)
+	assert.Nil(t, err)
+	assert.Error(t, Parse(r, &v))
+}
+
 func BenchmarkParseRaw(b *testing.B) {
 	r, err := http.NewRequest(http.MethodGet, "http://hello.com/a?name=hello&age=18&percent=3.4", http.NoBody)
 	if err != nil {
@@ -351,3 +397,16 @@ func BenchmarkParseAuto(b *testing.B) {
 		}
 	}
 }
+
+type mockValidator struct{}
+
+func (m mockValidator) Validate(r *http.Request, data any) error {
+	if r.URL.Path == "/a" {
+		val := reflect.ValueOf(data).Elem().FieldByName("Name").String()
+		if val != "hello" {
+			return errors.New("name is not hello")
+		}
+	}
+
+	return nil
+}