From a0e624c0b8601fe2649af4b2b4038b1ce1bdf3f6 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Sun, 19 Jun 2022 20:16:58 +0800 Subject: [PATCH] fix(binding): fix bind field mask (#2112) * fix bind field mask Co-authored-by: chenzhihui --- encoding/form/form_test.go | 12 ++++++ encoding/form/proto_encode.go | 30 ++++++++++++-- encoding/form/well_known_types.go | 2 + internal/testdata/binding/test.pb.go | 59 +++++++++++++++++---------- internal/testdata/binding/test.proto | 3 ++ transport/http/binding/encode.go | 3 ++ transport/http/binding/encode_test.go | 12 +++++- 7 files changed, 95 insertions(+), 26 deletions(-) diff --git a/encoding/form/form_test.go b/encoding/form/form_test.go index 384869c52..11b45fe13 100644 --- a/encoding/form/form_test.go +++ b/encoding/form/form_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/protobuf/types/known/wrapperspb" "github.com/go-kratos/kratos/v2/encoding" + bdtest "github.com/go-kratos/kratos/v2/internal/testdata/binding" "github.com/go-kratos/kratos/v2/internal/testdata/complex" ectest "github.com/go-kratos/kratos/v2/internal/testdata/encoding" ) @@ -192,3 +193,14 @@ func TestDecodeBytesValuePb(t *testing.T) { t.Errorf("except %v, got %v", val, string(in2.Bytes.Value)) } } + +func TestEncodeFieldMask(t *testing.T) { + req := &bdtest.HelloRequest{ + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"foo", "bar"}}, + } + if v := EncodeFieldMask(req.ProtoReflect()); v != "updateMask=foo,bar" { + t.Errorf("got %s", v) + } else { + t.Log(v) + } +} diff --git a/encoding/form/proto_encode.go b/encoding/form/proto_encode.go index 781b1ccf8..797d7982d 100644 --- a/encoding/form/proto_encode.go +++ b/encoding/form/proto_encode.go @@ -8,9 +8,9 @@ import ( "strconv" "strings" - "google.golang.org/genproto/protobuf/field_mask" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/known/fieldmaskpb" ) // EncodeValues encode a message into url values. @@ -160,9 +160,9 @@ func encodeMessage(msgDescriptor protoreflect.MessageDescriptor, value protorefl fd := msgDescriptor.Fields() v := value.Message().Get(fd.ByName(protoreflect.Name("value"))) return fmt.Sprintf("%v", v.Interface()), nil - case "google.protobuf.FieldMask": - m, ok := value.Message().Interface().(*field_mask.FieldMask) - if !ok { + case fieldMaskFullName: + m, ok := value.Message().Interface().(*fieldmaskpb.FieldMask) + if !ok || m == nil { return "", nil } for i, v := range m.Paths { @@ -174,6 +174,28 @@ func encodeMessage(msgDescriptor protoreflect.MessageDescriptor, value protorefl } } +// EncodeFieldMask return field mask name=paths +func EncodeFieldMask(m protoreflect.Message) (query string) { + m.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool { + if fd.Kind() == protoreflect.MessageKind { + if msg := fd.Message(); msg.FullName() == fieldMaskFullName { + value, err := encodeMessage(msg, v) + if err != nil { + return false + } + if fd.HasJSONName() { + query = fd.JSONName() + "=" + value + } else { + query = fd.TextName() + "=" + value + } + return false + } + } + return true + }) + return +} + // JSONCamelCase converts a snake_case identifier to a camelCase identifier, // according to the protobuf JSON specification. // references: https://github.com/protocolbuffers/protobuf-go/blob/master/encoding/protojson/well_known_types.go#L842 diff --git a/encoding/form/well_known_types.go b/encoding/form/well_known_types.go index f87ca6403..8b0894dbf 100644 --- a/encoding/form/well_known_types.go +++ b/encoding/form/well_known_types.go @@ -31,6 +31,8 @@ const ( // google.protobuf.Struct. structMessageFullname protoreflect.FullName = "google.protobuf.Struct" structFieldsFieldNumber protoreflect.FieldNumber = 1 + + fieldMaskFullName protoreflect.FullName = "google.protobuf.FieldMask" ) func marshalTimestamp(m protoreflect.Message) (string, error) { diff --git a/internal/testdata/binding/test.pb.go b/internal/testdata/binding/test.pb.go index 5f2318c7a..52d157848 100644 --- a/internal/testdata/binding/test.pb.go +++ b/internal/testdata/binding/test.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.27.1 -// protoc v3.17.3 +// protoc-gen-go v1.28.0 +// protoc v3.20.0 // source: test.proto package binding @@ -9,6 +9,7 @@ package binding import ( protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" + fieldmaskpb "google.golang.org/protobuf/types/known/fieldmaskpb" reflect "reflect" sync "sync" ) @@ -26,8 +27,9 @@ type HelloRequest struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` - Sub *Sub `protobuf:"bytes,2,opt,name=sub,proto3" json:"sub,omitempty"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` + Sub *Sub `protobuf:"bytes,2,opt,name=sub,proto3" json:"sub,omitempty"` + UpdateMask *fieldmaskpb.FieldMask `protobuf:"bytes,3,opt,name=update_mask,json=updateMask,proto3" json:"update_mask,omitempty"` } func (x *HelloRequest) Reset() { @@ -76,6 +78,13 @@ func (x *HelloRequest) GetSub() *Sub { return nil } +func (x *HelloRequest) GetUpdateMask() *fieldmaskpb.FieldMask { + if x != nil { + return x.UpdateMask + } + return nil +} + type Sub struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -127,16 +136,22 @@ var File_test_proto protoreflect.FileDescriptor var file_test_proto_rawDesc = []byte{ 0x0a, 0x0a, 0x74, 0x65, 0x73, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x07, 0x62, 0x69, - 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x22, 0x42, 0x0a, 0x0c, 0x48, 0x65, 0x6c, 0x6c, 0x6f, 0x52, 0x65, - 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x1e, 0x0a, 0x03, 0x73, 0x75, 0x62, - 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x0c, 0x2e, 0x62, 0x69, 0x6e, 0x64, 0x69, 0x6e, 0x67, - 0x2e, 0x53, 0x75, 0x62, 0x52, 0x03, 0x73, 0x75, 0x62, 0x22, 0x1b, 0x0a, 0x03, 0x53, 0x75, 0x62, - 0x12, 0x14, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, - 0x6e, 0x61, 0x6d, 0x69, 0x6e, 0x67, 0x42, 0x2f, 0x5a, 0x2d, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, - 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x6f, 0x2d, 0x6b, 0x72, 0x61, 0x74, 0x6f, 0x73, 0x2f, 0x6b, - 0x72, 0x61, 0x74, 0x6f, 0x73, 0x2f, 0x74, 0x72, 0x61, 0x6e, 0x73, 0x70, 0x6f, 0x72, 0x74, 0x2f, - 0x62, 0x69, 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x1a, 0x20, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2f, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2f, 0x66, 0x69, 0x65, 0x6c, 0x64, 0x5f, 0x6d, 0x61, 0x73, + 0x6b, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0x7f, 0x0a, 0x0c, 0x48, 0x65, 0x6c, 0x6c, 0x6f, + 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, + 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x1e, 0x0a, 0x03, 0x73, + 0x75, 0x62, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x0c, 0x2e, 0x62, 0x69, 0x6e, 0x64, 0x69, + 0x6e, 0x67, 0x2e, 0x53, 0x75, 0x62, 0x52, 0x03, 0x73, 0x75, 0x62, 0x12, 0x3b, 0x0a, 0x0b, 0x75, + 0x70, 0x64, 0x61, 0x74, 0x65, 0x5f, 0x6d, 0x61, 0x73, 0x6b, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, + 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, + 0x75, 0x66, 0x2e, 0x46, 0x69, 0x65, 0x6c, 0x64, 0x4d, 0x61, 0x73, 0x6b, 0x52, 0x0a, 0x75, 0x70, + 0x64, 0x61, 0x74, 0x65, 0x4d, 0x61, 0x73, 0x6b, 0x22, 0x1b, 0x0a, 0x03, 0x53, 0x75, 0x62, 0x12, + 0x14, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x06, 0x6e, + 0x61, 0x6d, 0x69, 0x6e, 0x67, 0x42, 0x2f, 0x5a, 0x2d, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, + 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x6f, 0x2d, 0x6b, 0x72, 0x61, 0x74, 0x6f, 0x73, 0x2f, 0x6b, 0x72, + 0x61, 0x74, 0x6f, 0x73, 0x2f, 0x74, 0x72, 0x61, 0x6e, 0x73, 0x70, 0x6f, 0x72, 0x74, 0x2f, 0x62, + 0x69, 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -153,16 +168,18 @@ func file_test_proto_rawDescGZIP() []byte { var file_test_proto_msgTypes = make([]protoimpl.MessageInfo, 2) var file_test_proto_goTypes = []interface{}{ - (*HelloRequest)(nil), // 0: binding.HelloRequest - (*Sub)(nil), // 1: binding.Sub + (*HelloRequest)(nil), // 0: binding.HelloRequest + (*Sub)(nil), // 1: binding.Sub + (*fieldmaskpb.FieldMask)(nil), // 2: google.protobuf.FieldMask } var file_test_proto_depIdxs = []int32{ 1, // 0: binding.HelloRequest.sub:type_name -> binding.Sub - 1, // [1:1] is the sub-list for method output_type - 1, // [1:1] is the sub-list for method input_type - 1, // [1:1] is the sub-list for extension type_name - 1, // [1:1] is the sub-list for extension extendee - 0, // [0:1] is the sub-list for field type_name + 2, // 1: binding.HelloRequest.update_mask:type_name -> google.protobuf.FieldMask + 2, // [2:2] is the sub-list for method output_type + 2, // [2:2] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name } func init() { file_test_proto_init() } diff --git a/internal/testdata/binding/test.proto b/internal/testdata/binding/test.proto index a2d8b0d3a..06bd2f994 100644 --- a/internal/testdata/binding/test.proto +++ b/internal/testdata/binding/test.proto @@ -2,12 +2,15 @@ syntax = "proto3"; package binding; +import "google/protobuf/field_mask.proto"; + option go_package = "github.com/go-kratos/kratos/transport/binding"; // The request message containing the user's name. message HelloRequest { string name = 1; Sub sub = 2; + google.protobuf.FieldMask update_mask = 3; } message Sub{ diff --git a/transport/http/binding/encode.go b/transport/http/binding/encode.go index 190e65e59..14faf2e92 100644 --- a/transport/http/binding/encode.go +++ b/transport/http/binding/encode.go @@ -36,6 +36,9 @@ func EncodeURL(pathTemplate string, msg proto.Message, needQuery bool) string { return "/" + value }) if !needQuery { + if query := form.EncodeFieldMask(msg.ProtoReflect()); query != "" { + return path + "?" + query + } return path } u, err := form.EncodeValues(msg) diff --git a/transport/http/binding/encode_test.go b/transport/http/binding/encode_test.go index 5cf574364..5e0618f0d 100644 --- a/transport/http/binding/encode_test.go +++ b/transport/http/binding/encode_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/go-kratos/kratos/v2/internal/testdata/binding" + "google.golang.org/protobuf/types/known/fieldmaskpb" ) func TestProtoPath(t *testing.T) { @@ -43,7 +44,16 @@ func TestProtoPath(t *testing.T) { Sub: &binding.Sub{Name: "kratos"}, }, true) fmt.Println(url) - if url != `http://helloworld.Greeter/helloworld/go/sub?sub.naming=kratos` { + if url != `http://helloworld.Greeter/helloworld/go/sub?sub.naming=kratos&updateMask=` { + t.Fatalf("proto path not expected!actual: %s ", url) + } + + url = EncodeURL("http://helloworld.Greeter/helloworld/sub/{sub.name}", &binding.HelloRequest{ + Sub: &binding.Sub{Name: "kratos"}, + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"name", "sub.name"}}, + }, false) + fmt.Println(url) + if url != `http://helloworld.Greeter/helloworld/sub/kratos?updateMask=name,sub.name` { t.Fatalf("proto path not expected!actual: %s ", url) } }