From c3819a6e566d3fba329000767e3a9383729445e9 Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Fri, 30 Aug 2024 10:30:06 +0000 Subject: [PATCH 1/2] fix(spanner): add custom encoder decoder check for protos --- spanner/testdata/protos/custom_singer.go | 25 ++++ spanner/testdata/protos/descriptors.pb | Bin 345 -> 501 bytes spanner/testdata/protos/singer.pb.go | 146 +++++++++++++++++++++-- spanner/testdata/protos/singer.proto | 11 ++ spanner/value.go | 29 +++++ spanner/value_test.go | 5 + 6 files changed, 205 insertions(+), 11 deletions(-) create mode 100644 spanner/testdata/protos/custom_singer.go diff --git a/spanner/testdata/protos/custom_singer.go b/spanner/testdata/protos/custom_singer.go new file mode 100644 index 000000000000..5769a7255415 --- /dev/null +++ b/spanner/testdata/protos/custom_singer.go @@ -0,0 +1,25 @@ +package protos + +import ( + "errors" +) + +func (c *CustomSingerInfo) EncodeSpanner() (interface{}, error) { + if c == nil { + return nil, nil + } + return c.SingerName, nil +} + +func (c CustomGenre) EncodeSpanner() (interface{}, error) { + return c.String(), nil +} + +func (c *CustomSingerInfo) DecodeSpanner(input interface{}) error { + str, ok := input.(string) + if !ok { + return errors.New("the interface does not contain a string") + } + c.SingerName = &str + return nil +} diff --git a/spanner/testdata/protos/descriptors.pb b/spanner/testdata/protos/descriptors.pb index 10193075ede57d49885522496fbcce7ad0d64a21..9c577bbfda5b5e172055220b7860deaf27ad4aa1 100644 GIT binary patch delta 171 zcmcb~^p%;J>l5=vW?@E8B@ZqE=hEVm{M_Koy!6x}&%CsJAr&s}VlXd0FEKY&f>D7{ zgVTc{hzla<2NdgIWOU-=;)zEnn=Hy`=%W?D#SJmdJvFZ=Rfvy^%Q-YS#NRhQz&}8M fL5LqH4dHt^MnwrQA_=(p`*;g5Aqh-&V3Y&^vtTZe delta 20 ccmey$e3OZp>l)KWW?{z335*7lA2A9807cRUE&u=k diff --git a/spanner/testdata/protos/singer.pb.go b/spanner/testdata/protos/singer.pb.go index 080aa6b036fc..165f1562a138 100644 --- a/spanner/testdata/protos/singer.pb.go +++ b/spanner/testdata/protos/singer.pb.go @@ -88,6 +88,58 @@ func (Genre) EnumDescriptor() ([]byte, []int) { return file_singer_proto_rawDescGZIP(), []int{0} } +type CustomGenre int32 + +const ( + CustomGenre_CUSTOM_POP CustomGenre = 0 + CustomGenre_CUSTOM_JAZZ CustomGenre = 1 + CustomGenre_CUSTOM_FOLK CustomGenre = 2 + CustomGenre_CUSTOM_ROCK CustomGenre = 3 +) + +// Enum value maps for CustomGenre. +var ( + CustomGenre_name = map[int32]string{ + 0: "CUSTOM_POP", + 1: "CUSTOM_JAZZ", + 2: "CUSTOM_FOLK", + 3: "CUSTOM_ROCK", + } + CustomGenre_value = map[string]int32{ + "CUSTOM_POP": 0, + "CUSTOM_JAZZ": 1, + "CUSTOM_FOLK": 2, + "CUSTOM_ROCK": 3, + } +) + +func (x CustomGenre) Enum() *CustomGenre { + p := new(CustomGenre) + *p = x + return p +} + +func (x CustomGenre) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (CustomGenre) Descriptor() protoreflect.EnumDescriptor { + return file_singer_proto_enumTypes[1].Descriptor() +} + +func (CustomGenre) Type() protoreflect.EnumType { + return &file_singer_proto_enumTypes[1] +} + +func (x CustomGenre) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use CustomGenre.Descriptor instead. +func (CustomGenre) EnumDescriptor() ([]byte, []int) { + return file_singer_proto_rawDescGZIP(), []int{1} +} + type SingerInfo struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -159,6 +211,53 @@ func (x *SingerInfo) GetGenre() Genre { return Genre_POP } +type CustomSingerInfo struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + SingerName *string `protobuf:"bytes,1,opt,name=singer_name,json=singerName,proto3,oneof" json:"singer_name,omitempty"` +} + +func (x *CustomSingerInfo) Reset() { + *x = CustomSingerInfo{} + if protoimpl.UnsafeEnabled { + mi := &file_singer_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *CustomSingerInfo) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*CustomSingerInfo) ProtoMessage() {} + +func (x *CustomSingerInfo) ProtoReflect() protoreflect.Message { + mi := &file_singer_proto_msgTypes[1] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use CustomSingerInfo.ProtoReflect.Descriptor instead. +func (*CustomSingerInfo) Descriptor() ([]byte, []int) { + return file_singer_proto_rawDescGZIP(), []int{1} +} + +func (x *CustomSingerInfo) GetSingerName() string { + if x != nil && x.SingerName != nil { + return *x.SingerName + } + return "" +} + var File_singer_proto protoreflect.FileDescriptor var file_singer_proto_rawDesc = []byte{ @@ -179,11 +278,21 @@ var file_singer_proto_rawDesc = []byte{ 0x5f, 0x73, 0x69, 0x6e, 0x67, 0x65, 0x72, 0x5f, 0x69, 0x64, 0x42, 0x0d, 0x0a, 0x0b, 0x5f, 0x62, 0x69, 0x72, 0x74, 0x68, 0x5f, 0x64, 0x61, 0x74, 0x65, 0x42, 0x0e, 0x0a, 0x0c, 0x5f, 0x6e, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x69, 0x74, 0x79, 0x42, 0x08, 0x0a, 0x06, 0x5f, 0x67, 0x65, - 0x6e, 0x72, 0x65, 0x2a, 0x2e, 0x0a, 0x05, 0x47, 0x65, 0x6e, 0x72, 0x65, 0x12, 0x07, 0x0a, 0x03, - 0x50, 0x4f, 0x50, 0x10, 0x00, 0x12, 0x08, 0x0a, 0x04, 0x4a, 0x41, 0x5a, 0x5a, 0x10, 0x01, 0x12, - 0x08, 0x0a, 0x04, 0x46, 0x4f, 0x4c, 0x4b, 0x10, 0x02, 0x12, 0x08, 0x0a, 0x04, 0x52, 0x4f, 0x43, - 0x4b, 0x10, 0x03, 0x42, 0x09, 0x5a, 0x07, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2f, 0x62, 0x06, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x6e, 0x72, 0x65, 0x22, 0x48, 0x0a, 0x10, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x53, 0x69, 0x6e, + 0x67, 0x65, 0x72, 0x49, 0x6e, 0x66, 0x6f, 0x12, 0x24, 0x0a, 0x0b, 0x73, 0x69, 0x6e, 0x67, 0x65, + 0x72, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x48, 0x00, 0x52, 0x0a, + 0x73, 0x69, 0x6e, 0x67, 0x65, 0x72, 0x4e, 0x61, 0x6d, 0x65, 0x88, 0x01, 0x01, 0x42, 0x0e, 0x0a, + 0x0c, 0x5f, 0x73, 0x69, 0x6e, 0x67, 0x65, 0x72, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x2a, 0x2e, 0x0a, + 0x05, 0x47, 0x65, 0x6e, 0x72, 0x65, 0x12, 0x07, 0x0a, 0x03, 0x50, 0x4f, 0x50, 0x10, 0x00, 0x12, + 0x08, 0x0a, 0x04, 0x4a, 0x41, 0x5a, 0x5a, 0x10, 0x01, 0x12, 0x08, 0x0a, 0x04, 0x46, 0x4f, 0x4c, + 0x4b, 0x10, 0x02, 0x12, 0x08, 0x0a, 0x04, 0x52, 0x4f, 0x43, 0x4b, 0x10, 0x03, 0x2a, 0x50, 0x0a, + 0x0b, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x47, 0x65, 0x6e, 0x72, 0x65, 0x12, 0x0e, 0x0a, 0x0a, + 0x43, 0x55, 0x53, 0x54, 0x4f, 0x4d, 0x5f, 0x50, 0x4f, 0x50, 0x10, 0x00, 0x12, 0x0f, 0x0a, 0x0b, + 0x43, 0x55, 0x53, 0x54, 0x4f, 0x4d, 0x5f, 0x4a, 0x41, 0x5a, 0x5a, 0x10, 0x01, 0x12, 0x0f, 0x0a, + 0x0b, 0x43, 0x55, 0x53, 0x54, 0x4f, 0x4d, 0x5f, 0x46, 0x4f, 0x4c, 0x4b, 0x10, 0x02, 0x12, 0x0f, + 0x0a, 0x0b, 0x43, 0x55, 0x53, 0x54, 0x4f, 0x4d, 0x5f, 0x52, 0x4f, 0x43, 0x4b, 0x10, 0x03, 0x42, + 0x09, 0x5a, 0x07, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x2f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, + 0x6f, 0x33, } var ( @@ -198,11 +307,13 @@ func file_singer_proto_rawDescGZIP() []byte { return file_singer_proto_rawDescData } -var file_singer_proto_enumTypes = make([]protoimpl.EnumInfo, 1) -var file_singer_proto_msgTypes = make([]protoimpl.MessageInfo, 1) +var file_singer_proto_enumTypes = make([]protoimpl.EnumInfo, 2) +var file_singer_proto_msgTypes = make([]protoimpl.MessageInfo, 2) var file_singer_proto_goTypes = []interface{}{ - (Genre)(0), // 0: examples.spanner.music.Genre - (*SingerInfo)(nil), // 1: examples.spanner.music.SingerInfo + (Genre)(0), // 0: examples.spanner.music.Genre + (CustomGenre)(0), // 1: examples.spanner.music.CustomGenre + (*SingerInfo)(nil), // 2: examples.spanner.music.SingerInfo + (*CustomSingerInfo)(nil), // 3: examples.spanner.music.CustomSingerInfo } var file_singer_proto_depIdxs = []int32{ 0, // 0: examples.spanner.music.SingerInfo.genre:type_name -> examples.spanner.music.Genre @@ -231,15 +342,28 @@ func file_singer_proto_init() { return nil } } + file_singer_proto_msgTypes[1].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*CustomSingerInfo); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } } file_singer_proto_msgTypes[0].OneofWrappers = []interface{}{} + file_singer_proto_msgTypes[1].OneofWrappers = []interface{}{} type x struct{} out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_singer_proto_rawDesc, - NumEnums: 1, - NumMessages: 1, + NumEnums: 2, + NumMessages: 2, NumExtensions: 0, NumServices: 0, }, diff --git a/spanner/testdata/protos/singer.proto b/spanner/testdata/protos/singer.proto index bae04443d5c4..1320fc8b21a0 100644 --- a/spanner/testdata/protos/singer.proto +++ b/spanner/testdata/protos/singer.proto @@ -32,3 +32,14 @@ enum Genre { FOLK = 2; ROCK = 3; } + +message CustomSingerInfo { + optional string singer_name = 1; +} + +enum CustomGenre { + CUSTOM_POP = 0; + CUSTOM_JAZZ = 1; + CUSTOM_FOLK = 2; + CUSTOM_ROCK = 3; +} diff --git a/spanner/value.go b/spanner/value.go index a02af38957d7..704ce6cfdb5d 100644 --- a/spanner/value.go +++ b/spanner/value.go @@ -2279,6 +2279,15 @@ func decodeValue(v *proto3.Value, t *sppb.Type, ptr interface{}, opts ...DecodeO reflect.ValueOf(p.ProtoEnumVal).Elem().SetInt(y) p.Valid = true case proto.Message: + // Check if the pointer is a custom type that implements spanner.Decoder + // interface. + if decodedVal, ok := ptr.(Decoder); ok { + x, err := getGenericValue(t, v) + if err != nil { + return err + } + return decodedVal.DecodeSpanner(x) + } if p == nil { return errNilDst(p) } @@ -4429,6 +4438,16 @@ func encodeValue(v interface{}) (*proto3.Value, *sppb.Type, error) { case []GenericColumnValue: return nil, nil, errEncoderUnsupportedType(v) case protoreflect.Enum: + // Check if the value is of protoreflect.Enum type that implements spanner.Encoder + // interface. + if encodedVal, ok := v.(Encoder); ok { + nv, err := encodedVal.EncodeSpanner() + if err != nil { + return nil, nil, err + } + return encodeValue(nv) + } + if v != nil { var protoEnumfqn string rv := reflect.ValueOf(v) @@ -4447,6 +4466,16 @@ func encodeValue(v interface{}) (*proto3.Value, *sppb.Type, error) { } return nil, nil, errNotValidSrc(v) case proto.Message: + // Check if the value is of proto.Message type that implements spanner.Encoder + // interface. + if encodedVal, ok := v.(Encoder); ok { + nv, err := encodedVal.EncodeSpanner() + if err != nil { + return nil, nil, err + } + return encodeValue(nv) + } + if v != nil { if v.ProtoReflect().IsValid() { bytes, err := proto.Marshal(v) diff --git a/spanner/value_test.go b/spanner/value_test.go index b1b2e33e87f9..d4351ae41088 100644 --- a/spanner/value_test.go +++ b/spanner/value_test.go @@ -563,6 +563,9 @@ func TestEncodeValue(t *testing.T) { {[]*pb.Genre{nil, (*pb.Genre)(nil)}, listProto(nullProto(), nullProto()), listType(tProtoEnum), "Array of Proto Enum with nil values"}, {[]*pb.SingerInfo{singer1ProtoMsg, singer2ProtoMsg, nil, (*pb.SingerInfo)(nil)}, listProto(protoMessageProto(singer1ProtoMsg), protoMessageProto(singer2ProtoMsg), nullProto(), nullProto()), listType(tProtoMessage), "Array of Proto Message with non-nil and nil values"}, {[]*pb.Genre{&singer1ProtoEnum, &singer2ProtoEnum, nil, (*pb.Genre)(nil)}, listProto(protoEnumProto(singer1ProtoEnum), protoEnumProto(singer2ProtoEnum), nullProto(), nullProto()), listType(tProtoEnum), "Array of Proto Enum with non-nil and nil values"}, + // PROTO MESSAGE AND ENUM WITH CUSTOM ENCODER + {&pb.CustomSingerInfo{SingerName: &sValue}, stringProto("abc"), tString, "Proto message with encoder interface to string"}, + {pb.CustomGenre_CUSTOM_ROCK, stringProto("CUSTOM_ROCK"), tString, "Proto Enum with encoder interface to string"}, } { got, gotType, err := encodeValue(test.in) if err != nil { @@ -1996,6 +1999,8 @@ func TestDecodeValue(t *testing.T) { {desc: "decode all NULL elements in ARRAY> to []*pb.SingerInfo", proto: listProto(nullProto(), nullProto()), protoType: listType(protoMessageType(protoMessagefqn)), want: []*pb.SingerInfo{nil, nil}}, {desc: "decode ARRAY> to []*pb.Genre", proto: listProto(nullProto(), protoEnumProto(pb.Genre_ROCK), protoEnumProto(pb.Genre_FOLK)), protoType: listType(protoEnumType(protoEnumfqn)), want: []*pb.Genre{nil, &singerEnumValue, &singer2ProtoEnum}}, {desc: "decode all NULL elements in ARRAY> to []*pb.Genre", proto: listProto(nullProto(), nullProto()), protoType: listType(protoEnumType(protoEnumfqn)), want: []*pb.Genre{nil, nil}}, + // PROTO MESSAGE WITH CUSTOM DECODER + {desc: "decode STRING to Proto message", proto: stringProto("abc"), protoType: stringType(), want: pb.CustomSingerInfo{SingerName: proto.String("abc")}}, } { gotp := reflect.New(reflect.TypeOf(test.want)) v := gotp.Interface() From d177f598f2214793b540b87f68eb0647724a0442 Mon Sep 17 00:00:00 2001 From: Sri Harsha CH Date: Fri, 30 Aug 2024 10:36:41 +0000 Subject: [PATCH 2/2] fix(spanner): add header --- spanner/testdata/protos/custom_singer.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spanner/testdata/protos/custom_singer.go b/spanner/testdata/protos/custom_singer.go index 5769a7255415..50a9927efc6b 100644 --- a/spanner/testdata/protos/custom_singer.go +++ b/spanner/testdata/protos/custom_singer.go @@ -1,3 +1,19 @@ +/* +Copyright 2024 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package protos import (