From d470886977ff9188d05f382cf9da2cb5eef49419 Mon Sep 17 00:00:00 2001 From: jessetang <1430482733@qq.com> Date: Sun, 26 Mar 2023 22:54:08 +0800 Subject: [PATCH] refactor(transport): format import and init slice cap (#2741) * fix(transport): format import and init slice cap * fix lint: local-prefixes: github.com/go-kratos --------- Co-authored-by: haiyux --- .golangci.yml | 2 +- contrib/opensergo/opensergo.go | 4 +- transport/grpc/client.go | 10 +-- transport/grpc/resolver/discovery/builder.go | 4 +- transport/grpc/resolver/discovery/resolver.go | 21 ++--- transport/grpc/server.go | 18 ++-- transport/http/binding/bind.go | 3 +- transport/http/binding/bind_test.go | 88 +++++++++---------- transport/http/binding/encode.go | 7 +- transport/http/client_test.go | 4 +- transport/http/codec.go | 4 +- transport/http/resolver.go | 8 +- transport/http/server.go | 7 +- 13 files changed, 84 insertions(+), 96 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 06ee200c6..1408787ef 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -68,4 +68,4 @@ linters-settings: # recommend 10-20 min-complexity: 50 goimports: - local-prefixes: github.com/go-kratos/kratos # Put imports beginning with prefix after 3rd-party packages + local-prefixes: github.com/go-kratos # Put imports beginning with prefix after 3rd-party packages diff --git a/contrib/opensergo/opensergo.go b/contrib/opensergo/opensergo.go index e8f38c7bf..56de2536e 100644 --- a/contrib/opensergo/opensergo.go +++ b/contrib/opensergo/opensergo.go @@ -9,8 +9,6 @@ import ( "strconv" "time" - "github.com/go-kratos/kratos/v2" - v1 "github.com/opensergo/opensergo-go/proto/service_contract/v1" "golang.org/x/net/context" "google.golang.org/genproto/googleapis/api/annotations" @@ -19,6 +17,8 @@ import ( "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" + + "github.com/go-kratos/kratos/v2" ) type Option func(*options) diff --git a/transport/grpc/client.go b/transport/grpc/client.go index a7a00910e..3c8a17d75 100644 --- a/transport/grpc/client.go +++ b/transport/grpc/client.go @@ -6,6 +6,11 @@ import ( "fmt" "time" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" + grpcinsecure "google.golang.org/grpc/credentials/insecure" + grpcmd "google.golang.org/grpc/metadata" + "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/registry" @@ -16,11 +21,6 @@ import ( // init resolver _ "github.com/go-kratos/kratos/v2/transport/grpc/resolver/direct" - - "google.golang.org/grpc" - "google.golang.org/grpc/credentials" - grpcinsecure "google.golang.org/grpc/credentials/insecure" - grpcmd "google.golang.org/grpc/metadata" ) func init() { diff --git a/transport/grpc/resolver/discovery/builder.go b/transport/grpc/resolver/discovery/builder.go index 28aff384e..3d147cd83 100644 --- a/transport/grpc/resolver/discovery/builder.go +++ b/transport/grpc/resolver/discovery/builder.go @@ -6,10 +6,10 @@ import ( "strings" "time" - "github.com/go-kratos/kratos/v2/registry" - "github.com/google/uuid" "google.golang.org/grpc/resolver" + + "github.com/go-kratos/kratos/v2/registry" ) const name = "discovery" diff --git a/transport/grpc/resolver/discovery/resolver.go b/transport/grpc/resolver/discovery/resolver.go index 03c76664d..10775d749 100644 --- a/transport/grpc/resolver/discovery/resolver.go +++ b/transport/grpc/resolver/discovery/resolver.go @@ -6,13 +6,13 @@ import ( "errors" "time" + "google.golang.org/grpc/attributes" + "google.golang.org/grpc/resolver" + + "github.com/go-kratos/aegis/subset" "github.com/go-kratos/kratos/v2/internal/endpoint" "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/registry" - - "github.com/go-kratos/aegis/subset" - "google.golang.org/grpc/attributes" - "google.golang.org/grpc/resolver" ) type discoveryResolver struct { @@ -49,9 +49,10 @@ func (r *discoveryResolver) watch() { } func (r *discoveryResolver) update(ins []*registry.ServiceInstance) { - addrs := make([]resolver.Address, 0) - endpoints := make(map[string]struct{}) - filtered := make([]*registry.ServiceInstance, 0, len(ins)) + var ( + endpoints = make(map[string]struct{}) + filtered = make([]*registry.ServiceInstance, 0, len(ins)) + ) for _, in := range ins { ept, err := endpoint.ParseEndpoint(in.Endpoints, endpoint.Scheme("grpc", !r.insecure)) if err != nil { @@ -70,15 +71,16 @@ func (r *discoveryResolver) update(ins []*registry.ServiceInstance) { if r.subsetSize != 0 { filtered = subset.Subset(r.selecterKey, filtered, r.subsetSize) } + + addrs := make([]resolver.Address, 0, len(filtered)) for _, in := range filtered { ept, _ := endpoint.ParseEndpoint(in.Endpoints, endpoint.Scheme("grpc", !r.insecure)) endpoints[ept] = struct{}{} addr := resolver.Address{ ServerName: in.Name, - Attributes: parseAttributes(in.Metadata), + Attributes: parseAttributes(in.Metadata).WithValue("rawServiceInstance", in), Addr: ept, } - addr.Attributes = addr.Attributes.WithValue("rawServiceInstance", in) addrs = append(addrs, addr) } if len(addrs) == 0 { @@ -89,7 +91,6 @@ func (r *discoveryResolver) update(ins []*registry.ServiceInstance) { if err != nil { log.Errorf("[resolver] failed to update state: %s", err) } - if r.debugLog { b, _ := json.Marshal(filtered) log.Infof("[resolver] update instances: %s", b) diff --git a/transport/grpc/server.go b/transport/grpc/server.go index 9a8344bfe..6e281b671 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -7,22 +7,20 @@ import ( "net/url" "time" - "github.com/go-kratos/kratos/v2/internal/endpoint" - "github.com/go-kratos/kratos/v2/internal/matcher" - - apimd "github.com/go-kratos/kratos/v2/api/metadata" - - "github.com/go-kratos/kratos/v2/internal/host" - "github.com/go-kratos/kratos/v2/log" - "github.com/go-kratos/kratos/v2/middleware" - "github.com/go-kratos/kratos/v2/transport" - "google.golang.org/grpc" "google.golang.org/grpc/admin" "google.golang.org/grpc/credentials" "google.golang.org/grpc/health" "google.golang.org/grpc/health/grpc_health_v1" "google.golang.org/grpc/reflection" + + apimd "github.com/go-kratos/kratos/v2/api/metadata" + "github.com/go-kratos/kratos/v2/internal/endpoint" + "github.com/go-kratos/kratos/v2/internal/host" + "github.com/go-kratos/kratos/v2/internal/matcher" + "github.com/go-kratos/kratos/v2/log" + "github.com/go-kratos/kratos/v2/middleware" + "github.com/go-kratos/kratos/v2/transport" ) var ( diff --git a/transport/http/binding/bind.go b/transport/http/binding/bind.go index 100073673..69534ada6 100644 --- a/transport/http/binding/bind.go +++ b/transport/http/binding/bind.go @@ -4,10 +4,9 @@ import ( "net/http" "net/url" - "github.com/go-kratos/kratos/v2/errors" - "github.com/go-kratos/kratos/v2/encoding" "github.com/go-kratos/kratos/v2/encoding/form" + "github.com/go-kratos/kratos/v2/errors" ) // BindQuery bind vars parameters to target. diff --git a/transport/http/binding/bind_test.go b/transport/http/binding/bind_test.go index 6cd8b6d5a..962292c4a 100644 --- a/transport/http/binding/bind_test.go +++ b/transport/http/binding/bind_test.go @@ -1,6 +1,7 @@ package binding import ( + "errors" "io" "net/http" "net/url" @@ -8,47 +9,47 @@ import ( "strings" "testing" - "github.com/go-kratos/kratos/v2/errors" + kratoserror "github.com/go-kratos/kratos/v2/errors" ) -func TestBindQuery(t *testing.T) { - type TestBind struct { +type ( + TestBind struct { Name string `json:"name"` URL string `json:"url"` } - - type TestBind2 struct { + TestBind2 struct { Age int `json:"age"` } - p1 := TestBind{} - p2 := TestBind2{} +) + +func TestBindQuery(t *testing.T) { type args struct { vars url.Values target interface{} } + tests := []struct { - name string - args args - wantErr bool - want interface{} + name string + args args + err error + want interface{} }{ { name: "test", args: args{ vars: map[string][]string{"name": {"kratos"}, "url": {"https://go-kratos.dev/"}}, - target: &p1, + target: &TestBind{}, }, - wantErr: false, - want: &TestBind{"kratos", "https://go-kratos.dev/"}, + err: nil, + want: &TestBind{"kratos", "https://go-kratos.dev/"}, }, { name: "test1", args: args{ vars: map[string][]string{"age": {"kratos"}, "url": {"https://go-kratos.dev/"}}, - target: &p2, + target: &TestBind2{}, }, - wantErr: true, - want: errors.BadRequest("CODEC", ""), + err: kratoserror.BadRequest("CODEC", "Field Namespace:age ERROR:Invalid Integer Value 'kratos' Type 'int' Namespace 'age'"), }, { name: "test2", @@ -56,18 +57,17 @@ func TestBindQuery(t *testing.T) { vars: map[string][]string{"age": {"1"}, "url": {"https://go-kratos.dev/"}}, target: &TestBind2{}, }, - wantErr: false, - want: &TestBind2{Age: 1}, + err: nil, + want: &TestBind2{Age: 1}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := BindQuery(tt.args.vars, tt.args.target); (err != nil) != tt.wantErr { - t.Errorf("BindQuery() error = %v, wantErr %v", err, tt.wantErr) - } else { - t.Log(err) + err := BindQuery(tt.args.vars, tt.args.target) + if !kratoserror.Is(err, tt.err) { + t.Fatalf("BindQuery() error = %v, err %v", err, tt.err) } - if !tt.wantErr && !reflect.DeepEqual(tt.args.target, tt.want) { + if err == nil && !reflect.DeepEqual(tt.args.target, tt.want) { t.Errorf("BindQuery() target = %v, want %v", tt.args.target, tt.want) } }) @@ -75,32 +75,25 @@ func TestBindQuery(t *testing.T) { } func TestBindForm(t *testing.T) { - type TestBind struct { - Name string `json:"name"` - URL string `json:"url"` - } - type TestBind2 struct { - Age int `json:"age"` - } - p1 := TestBind{} type args struct { req *http.Request target interface{} } + tests := []struct { - name string - args args - wantErr bool - want interface{} + name string + args args + err error + want *TestBind }{ { name: "error not nil", args: args{ req: &http.Request{Method: http.MethodPost}, - target: &p1, + target: &TestBind{}, }, - wantErr: true, - want: nil, + err: errors.New("missing form body"), + want: nil, }, { name: "error is nil", @@ -110,10 +103,10 @@ func TestBindForm(t *testing.T) { Header: http.Header{"Content-Type": {"application/x-www-form-urlencoded; param=value"}}, Body: io.NopCloser(strings.NewReader("name=kratos&url=https://go-kratos.dev/")), }, - target: &p1, + target: &TestBind{}, }, - wantErr: false, - want: &TestBind{"kratos", "https://go-kratos.dev/"}, + err: nil, + want: &TestBind{"kratos", "https://go-kratos.dev/"}, }, { name: "error BadRequest", @@ -125,18 +118,17 @@ func TestBindForm(t *testing.T) { }, target: &TestBind2{}, }, - wantErr: true, - want: nil, + err: kratoserror.BadRequest("CODEC", "Field Namespace:age ERROR:Invalid Integer Value 'a' Type 'int' Namespace 'age'"), + want: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := BindForm(tt.args.req, tt.args.target) - if (err != nil) != tt.wantErr { - t.Errorf("BindForm() error = %v, wantErr %v", err, tt.wantErr) + if !reflect.DeepEqual(err, tt.err) { + t.Fatalf("BindForm() error = %v, err %v", err, tt.err) } - t.Log(err) - if !tt.wantErr && !reflect.DeepEqual(tt.args.target, tt.want) { + if err == nil && !reflect.DeepEqual(tt.args.target, tt.want) { t.Errorf("BindForm() target = %v, want %v", tt.args.target, tt.want) } }) diff --git a/transport/http/binding/encode.go b/transport/http/binding/encode.go index 72afd5085..5f12cf370 100644 --- a/transport/http/binding/encode.go +++ b/transport/http/binding/encode.go @@ -20,13 +20,12 @@ func EncodeURL(pathTemplate string, msg interface{}, needQuery bool) string { 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 {} - //if len(in) < 4 { //nolint:gomnd // ** explain the 4 number here :-) ** + // if len(in) < 4 { //nolint:gomnd // ** explain the 4 number here :-) ** // return in - //} + // } key := in[1 : len(in)-1] - value := queryParams.Get(key) pathParams[key] = struct{}{} - return value + return queryParams.Get(key) }) if !needQuery { if v, ok := msg.(proto.Message); ok { diff --git a/transport/http/client_test.go b/transport/http/client_test.go index 063878af8..25ae9a8ec 100644 --- a/transport/http/client_test.go +++ b/transport/http/client_test.go @@ -227,7 +227,7 @@ func TestDefaultResponseDecoder(t *testing.T) { resp1 := &http.Response{ Header: make(http.Header), StatusCode: 200, - Body: io.NopCloser(bytes.NewBufferString("{\"a\":\"1\", \"b\": 2}")), + Body: io.NopCloser(bytes.NewBufferString(`{"a":"1", "b": 2}`)), } v1 := &struct { A string `json:"a"` @@ -279,7 +279,7 @@ func TestDefaultErrorDecoder(t *testing.T) { resp2 := &http.Response{ Header: make(http.Header), StatusCode: 500, - Body: io.NopCloser(bytes.NewBufferString("{\"code\":54321, \"message\": \"hi\", \"reason\": \"FOO\"}")), + Body: io.NopCloser(bytes.NewBufferString(`{"code":54321, "message": "hi", "reason": "FOO"}`)), } err := DefaultErrorDecoder(context.TODO(), resp2) if err == nil { diff --git a/transport/http/codec.go b/transport/http/codec.go index 5aef7995c..39db4d211 100644 --- a/transport/http/codec.go +++ b/transport/http/codec.go @@ -7,12 +7,12 @@ import ( "net/http" "net/url" + "github.com/gorilla/mux" + "github.com/go-kratos/kratos/v2/encoding" "github.com/go-kratos/kratos/v2/errors" "github.com/go-kratos/kratos/v2/internal/httputil" "github.com/go-kratos/kratos/v2/transport/http/binding" - - "github.com/gorilla/mux" ) // SupportPackageIsVersion1 These constants should not be referenced from any other code. diff --git a/transport/http/resolver.go b/transport/http/resolver.go index dbb2211f0..bcbf8f3b3 100644 --- a/transport/http/resolver.go +++ b/transport/http/resolver.go @@ -7,13 +7,13 @@ import ( "strings" "time" + "github.com/google/uuid" + + "github.com/go-kratos/aegis/subset" "github.com/go-kratos/kratos/v2/internal/endpoint" "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/registry" "github.com/go-kratos/kratos/v2/selector" - - "github.com/go-kratos/aegis/subset" - "github.com/google/uuid" ) // Target is resolver target @@ -135,7 +135,7 @@ func (r *resolver) update(services []*registry.ServiceInstance) bool { if r.subsetSize != 0 { filtered = subset.Subset(r.selecterKey, filtered, r.subsetSize) } - nodes := make([]selector.Node, 0) + nodes := make([]selector.Node, 0, len(filtered)) for _, ins := range filtered { ept, _ := endpoint.ParseEndpoint(ins.Endpoints, endpoint.Scheme("http", !r.insecure)) nodes = append(nodes, selector.NewNode("http", ept, ins)) diff --git a/transport/http/server.go b/transport/http/server.go index 7b27d326e..8bece9afe 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -9,15 +9,14 @@ import ( "net/url" "time" - "github.com/go-kratos/kratos/v2/internal/endpoint" - "github.com/go-kratos/kratos/v2/internal/matcher" + "github.com/gorilla/mux" + "github.com/go-kratos/kratos/v2/internal/endpoint" "github.com/go-kratos/kratos/v2/internal/host" + "github.com/go-kratos/kratos/v2/internal/matcher" "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/transport" - - "github.com/gorilla/mux" ) var (