fix(http/binding): fix http encode url (#2400)

`/api/v1/[name]`
2 years ago · add67beeb3
parent 7866ff75fd
commit add67beeb3
2 changed files with 127 additions and 85 deletions
--- a/transport/http/binding/encode.go
+++ b/transport/http/binding/encode.go
@ -8,7 +8,7 @@ import (
 	"google.golang.org/protobuf/proto"
 )

-var reg = regexp.MustCompile(`/{[\\.\w]+}`)
+var reg = regexp.MustCompile(`{[\\.\w]+}`)

 // EncodeURL encode proto message to url path.
 func EncodeURL(pathTemplate string, msg interface{}, needQuery bool) string {
@ -22,10 +22,10 @@ func EncodeURL(pathTemplate string, msg interface{}, needQuery bool) string {
 		//if len(in) < 4 { //nolint:gomnd // **  explain the 4 number here :-) **
 		//	return in
 		//}
-		key := in[2 : len(in)-1]
+		key := in[1 : len(in)-1]
 		value := queryParams.Get(key)
 		pathParams[key] = struct{}{}
-		return "/" + value
+		return value
 	})
 	if !needQuery {
 		if v, ok := msg.(proto.Message); ok {
--- a/transport/http/binding/encode_test.go
+++ b/transport/http/binding/encode_test.go
@ -1,7 +1,6 @@
 package binding

 import (
-	"fmt"
 	"testing"

 	"github.com/go-kratos/kratos/v2/internal/testdata/binding"
@ -9,88 +8,131 @@ import (
 )

 func TestProtoPath(t *testing.T) {
-	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.naming}", nil, false)
-	fmt.Println(url)
-	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.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)
-	fmt.Println(url)
-	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}",
-		&binding.HelloRequest{
-			Name: "test", Sub: &binding.Sub{Name: "hello"},
-			TestRepeated: []string{"123", "456"},
-		},
-		false,
-	)
-	fmt.Println(url)
-	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)
-	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)
-	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/sub` {
-		t.Fatalf("proto path not expected!actual: %s ", url)
-	}
-
-	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/` {
-		t.Fatalf("proto path not expected!actual: %s ", url)
-	}
-
-	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/` {
-		t.Fatalf("proto path not expected!actual: %s ", url)
-	}
-
-	url = EncodeURL("http://helloworld.Greeter/helloworld/{name}/sub", &binding.HelloRequest{
-		Name: "go",
-		Sub:  &binding.Sub{Name: "kratos"},
-	}, true)
-	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/go/sub?sub.naming=kratos` {
-		t.Fatalf("proto path not expected!actual: %s ", url)
+	tests := []struct {
+		pathTemplate string
+		request      *binding.HelloRequest
+		needQuery    bool
+		want         string
+	}{
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!!"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/test/sub/2233!!!!",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}",
+			request:      nil,
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{}/sub/{sub.naming}",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{}/sub/hello",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{}/sub/{sub.naming}",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{}/sub/hello",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{}/sub/{sub.name.cc}",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{}/sub/",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{}/sub/{test_repeated}",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "hello"}, TestRepeated: []string{"123", "456"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{}/sub/123",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub/{sub.naming}",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "5566!!!"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/test/sub/5566!!!",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/sub",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "2233!!!"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/sub",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}",
+			request:      &binding.HelloRequest{Name: "test"},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/test/sub/",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub/{sub.name}",
+			request:      &binding.HelloRequest{Name: "test"},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/test/sub/",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{name}/sub",
+			request:      &binding.HelloRequest{Name: "go", Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    true,
+			want:         "http://helloworld.Greeter/helloworld/go/sub?sub.naming=kratos",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/sub/{sub.naming}",
+			request:      &binding.HelloRequest{Sub: &binding.Sub{Name: "kratos"}, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.naming"}}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.naming",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/sub/[{sub.naming}]",
+			request:      &binding.HelloRequest{Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/sub/[kratos]",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/[{name}]/sub/[{sub.naming}]",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/[test]/sub/[kratos]",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/[{}]/sub/[{sub.naming}]",
+			request:      &binding.HelloRequest{Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/[{}]/sub/[kratos]",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/[{}]/sub/[{sub.naming}]/{[]}",
+			request:      &binding.HelloRequest{Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/[{}]/sub/[kratos]/{[]}",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{[sub]}/[{sub.naming}]",
+			request:      &binding.HelloRequest{Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{[sub]}/[kratos]",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{[name]}/[{sub.naming}]",
+			request:      &binding.HelloRequest{Name: "test", Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{[name]}/[kratos]",
+		},
+		{
+			pathTemplate: "http://helloworld.Greeter/helloworld/{}/[]/[{sub.naming}]",
+			request:      &binding.HelloRequest{Sub: &binding.Sub{Name: "kratos"}},
+			needQuery:    false,
+			want:         "http://helloworld.Greeter/helloworld/{}/[]/[kratos]",
+		},
 	}

-	url = EncodeURL("http://helloworld.Greeter/helloworld/sub/{sub.naming}", &binding.HelloRequest{
-		Sub:        &binding.Sub{Name: "kratos"},
-		UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.naming"}},
-	}, false)
-	fmt.Println(url)
-	if url != `http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.naming` {
-		t.Fatalf("proto path not expected!actual: %s ", url)
+	for _, test := range tests {
+		if EncodeURL(test.pathTemplate, test.request, test.needQuery) != test.want {
+			t.Fatalf("want: %s, got: %s", test.want, EncodeURL(test.pathTemplate, test.request, test.needQuery))
+		}
 	}
 }