Repository: go-kratos/kratos
Pull request #2391 (2022-09-20 06:55:29)

fix(registry/consul): use health check option to control custom checks
<!--
🎉 Thanks for sending a pull request to Kratos! Here are some tips for you:

1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
3. If the PR is unfinished, you may need to mark it as a WIP(Work In Progress) PR or draft PR
4. Please use a semantic commits format title, such as `<type>[optional scope]: <description>`, see: https://go-kratos.dev/docs/community/contribution#type
5. at the same time, please note that similar work should be submitted in one PR as far as possible to reduce the workload of reviewers. Do not split a work into multiple PR unless it should.
-->

<!--
🎉 感谢您向 Kratos 发送 PR 请求！以下是一些提示：
如果这是你第一次为 Kratos 贡献，请阅读我们的贡献指南：https://go-kratos.dev/en/docs/community/contribution/
2、确保您已经为您的 PR 添加或运行了适当的测试和lint，请在提交PR之前使用“make lint”和“make test”，使用“make clean”整理您的 go.mod。
3、如果请购单未完成，您可能需要将其标记为 WIP（在制品）PR 或 draft PR
4、请使用语义提交格式标题，如“<类型>[可选范围]：<说明>`，请参阅：https://go-kratos.dev/docs/community/contribution#type
5. 同时请注意，同类的工作请尽量在一个PR中提交，以减轻 review 者的工作负担，不要把一项工作拆分成很多个PR，除非它应该这样做。
-->

<!--
-->

#### Description (what this PR does / why we need it):
<!--
* The description should include the motivation for this PR or contrast this with previous behavior
-->


#### Which issue(s) this PR fixes (resolves / be part of):
<!--
* Automatically closes linked issue when PR is merged.
* If your PR is not fully resolved the issue, please use `part of #<issue number>` instead.

Usage: `fixes/resolves #<issue number>`, or `fixes/resolves (paste link of issue)`.
-->


#### Other special notes for the reviewers:
<!--
* Somethings that need extra attention for the reviewers
* Some additional notes, TODO list, etc.
-->


codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2391?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#2391](https://codecov.io/gh/go-kratos/kratos/pull/2391?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (df8ef00) into [main](https://codecov.io/gh/go-kratos/kratos/commit/080165f2ee466177aeb894d82adb24874cba4e75?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (080165f) will **not change** coverage.
> The diff coverage is `n/a`.

```diff
@@           Coverage Diff           @@
##             main    #2391   +/-   ##
=======================================
  Coverage   85.44%   85.44%           
=======================================
  Files          88       88           
  Lines        3780     3780           
=======================================
  Hits         3230     3230           
  Misses        383      383           
  Partials      167      167           
```



Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)


Patch:
diff --git a/contrib/registry/consul/client.go b/contrib/registry/consul/client.go
index 55425dff..c1364bb3 100644
--- a/contrib/registry/consul/client.go
+++ b/contrib/registry/consul/client.go
@@ -132,6 +132,8 @@ func (c *Client) Register(_ context.Context, svc *registry.ServiceInstance, enab
 				Timeout:                        "5s",
 			})
 		}
+		// custom checks
+		asr.Checks = append(asr.Checks, c.serviceChecks...)
 	}
 	if c.heartbeat {
 		asr.Checks = append(asr.Checks, &api.AgentServiceCheck{
@@ -141,9 +143,6 @@ func (c *Client) Register(_ context.Context, svc *registry.ServiceInstance, enab
 		})
 	}
 
-	// custom checks
-	asr.Checks = append(asr.Checks, c.serviceChecks...)
-
 	err := c.cli.Agent().ServiceRegister(asr)
 	if err != nil {
 		return err
diff --git a/encoding/form/form.go b/encoding/form/form.go
index 77c4237b..823aeb8f 100644
--- a/encoding/form/form.go
+++ b/encoding/form/form.go
@@ -17,10 +17,13 @@ const (
 	nullStr = "null"
 )
 
+var (
+	encoder = form.NewEncoder()
+	decoder = form.NewDecoder()
+)
+
 func init() {
-	decoder := form.NewDecoder()
 	decoder.SetTagName("json")
-	encoder := form.NewEncoder()
 	encoder.SetTagName("json")
 	encoding.RegisterCodec(codec{encoder: encoder, decoder: decoder})
 }
diff --git a/encoding/form/proto_encode.go b/encoding/form/proto_encode.go
index 449fca0d..28ebb0bc 100644
--- a/encoding/form/proto_encode.go
+++ b/encoding/form/proto_encode.go
@@ -14,16 +14,19 @@ import (
 )
 
 // EncodeValues encode a message into url values.
-func EncodeValues(msg proto.Message) (url.Values, error) {
+func EncodeValues(msg interface{}) (url.Values, error) {
 	if msg == nil || (reflect.ValueOf(msg).Kind() == reflect.Ptr && reflect.ValueOf(msg).IsNil()) {
 		return url.Values{}, nil
 	}
-	u := make(url.Values)
-	err := encodeByField(u, "", msg.ProtoReflect())
-	if err != nil {
-		return nil, err
+	if v, ok := msg.(proto.Message); ok {
+		u := make(url.Values)
+		err := encodeByField(u, "", v.ProtoReflect())
+		if err != nil {
+			return nil, err
+		}
+		return u, nil
 	}
-	return u, nil
+	return encoder.Encode(msg)
 }
 
 func encodeByField(u url.Values, path string, m protoreflect.Message) (finalErr error) {
diff --git a/transport/http/binding/encode.go b/transport/http/binding/encode.go
index 3129bbe5..122ec1db 100644
--- a/transport/http/binding/encode.go
+++ b/transport/http/binding/encode.go
@@ -1,24 +1,21 @@
 package binding
 
 import (
-	"fmt"
 	"reflect"
 	"regexp"
-	"strings"
 
 	"github.com/go-kratos/kratos/v2/encoding/form"
-
 	"google.golang.org/protobuf/proto"
-	protoreflect "google.golang.org/protobuf/reflect/protoreflect"
 )
 
 var reg = regexp.MustCompile(`/{[\\.\w]+}`)
 
 // EncodeURL encode proto message to url path.
-func EncodeURL(pathTemplate string, msg proto.Message, needQuery bool) string {
+func EncodeURL(pathTemplate string, msg interface{}, needQuery bool) string {
 	if msg == nil || (reflect.ValueOf(msg).Kind() == reflect.Ptr && reflect.ValueOf(msg).IsNil()) {
 		return pathTemplate
 	}
+	queryParams, _ := form.EncodeValues(msg)
 	pathParams := make(map[string]struct{})
 	path := reg.ReplaceAllStringFunc(pathTemplate, func(in string) string {
 		// it's unreachable because the reg means that must have more than one char in {}
@@ -26,50 +23,25 @@ func EncodeURL(pathTemplate string, msg proto.Message, needQuery bool) string {
 		//	return in
 		//}
 		key := in[2 : len(in)-1]
-		vars := strings.Split(key, ".")
-		value, err := getValueByField(msg.ProtoReflect(), vars)
-		if err != nil {
-			return in
-		}
+		value := queryParams.Get(key)
 		pathParams[key] = struct{}{}
 		return "/" + value
 	})
 	if !needQuery {
-		if query := form.EncodeFieldMask(msg.ProtoReflect()); query != "" {
-			return path + "?" + query
+		if v, ok := msg.(proto.Message); ok {
+			if query := form.EncodeFieldMask(v.ProtoReflect()); query != "" {
+				return path + "?" + query
+			}
 		}
 		return path
 	}
-	u, err := form.EncodeValues(msg)
-	if err == nil && len(u) > 0 {
+	if len(queryParams) > 0 {
 		for key := range pathParams {
-			delete(u, key)
+			delete(queryParams, key)
 		}
-		query := u.Encode()
-		if query != "" {
+		if query := queryParams.Encode(); query != "" {
 			path += "?" + query
 		}
 	}
 	return path
 }
-
-func getValueByField(v protoreflect.Message, fieldPath []string) (string, error) {
-	var fd protoreflect.FieldDescriptor
-	for i, fieldName := range fieldPath {
-		fields := v.Descriptor().Fields()
-		if fd = fields.ByJSONName(fieldName); fd == nil {
-			fd = fields.ByName(protoreflect.Name(fieldName))
-			if fd == nil {
-				return "", fmt.Errorf("field path not found: %q", fieldName)
-			}
-		}
-		if i == len(fieldPath)-1 {
-			break
-		}
-		if fd.Message() == nil || fd.Cardinality() == protoreflect.Repeated {
-			return "", fmt.Errorf("invalid path: %q is not a message", fieldName)
-		}
-		v = v.Get(fd).Message()
-	}
-	return form.EncodeField(fd, v.Get(fd))
-}


Test patch:
diff --git a/transport/http/binding/encode_test.go b/transport/http/binding/encode_test.go
index fdc2bb78..16c7cf0e 100644
--- a/transport/http/binding/encode_test.go
+++ b/transport/http/binding/encode_test.go
@@ -9,29 +9,34 @@ import (
 )
 
 func TestProtoPath(t *testing.T) {
-	url := EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!"}}, false)
-	fmt.Println(url)
+	url := EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "2233!!!"},
+	}, false)
 	if url != `http://helloworld.Greeter/helloworld/test/sub/2233!!!` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}", nil, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", nil, false)
 	fmt.Println(url)
-	if url != "http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}" {
+	if url != "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.name}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.naming}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "hello"},
+	}, false)
 	fmt.Println(url)
 	if url != "http://helloworld.Greeter/helloworld/{}/sub/hello" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "hello"},
+	}, false)
 	fmt.Println(url)
-	if url != "http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}" {
+	if url != "http://helloworld.Greeter/helloworld/{}/sub/" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
 	url = EncodeURL(
-		"http://helloworld.Greeter/helloworld/{}/sub/{test_repeated.1}",
+		"http://helloworld.Greeter/helloworld/{}/sub/{test_repeated}",
 		&binding.HelloRequest{
 			Name: "test", Sub: &binding.Sub{Name: "hello"},
 			TestRepeated: []string{"123", "456"},
@@ -39,17 +44,21 @@ func TestProtoPath(t *testing.T) {
 		false,
 	)
 	fmt.Println(url)
-	if url != "http://helloworld.Greeter/helloworld/{}/sub/{test_repeated.1}" {
+	if url != "http://helloworld.Greeter/helloworld/{}/sub/123" {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "5566!!!"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "5566!!!"},
+	}, false)
 	fmt.Println(url)
 	if url != `http://helloworld.Greeter/helloworld/test/sub/5566!!!` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/sub", &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!"}}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/sub", &binding.HelloRequest{
+		Name: "test", Sub: &binding.Sub{Name: "2233!!!"},
+	}, false)
 	fmt.Println(url)
 	if url != `http://helloworld.Greeter/helloworld/sub` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
@@ -61,9 +70,9 @@ func TestProtoPath(t *testing.T) {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name33}", &binding.HelloRequest{Name: "test"}, false)
+	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}", &binding.HelloRequest{Name: "test"}, false)
 	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/test/sub/{sub.name33}` {
+	if url != `http://helloworld.Greeter/helloworld/test/sub/` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
@@ -76,12 +85,12 @@ func TestProtoPath(t *testing.T) {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 
-	url = EncodeURL("http://helloworld.Greeter/helloworld/sub/{sub.name}", &binding.HelloRequest{
+	url = EncodeURL("http://helloworld.Greeter/helloworld/sub/{sub.naming}", &binding.HelloRequest{
 		Sub:        &binding.Sub{Name: "kratos"},
-		UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.name"}},
+		UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.naming"}},
 	}, false)
 	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.name` {
+	if url != `http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.naming` {
 		t.Fatalf("proto path not expected!actual: %s ", url)
 	}
 }