From 3cf802d3d657ff836409a4fb1d7ed51d7ebd9044 Mon Sep 17 00:00:00 2001 From: Tyler Schade Date: Fri, 21 Jun 2024 19:36:07 -0400 Subject: [PATCH] hashing repeated fields --- Makefile | 1 + templates/hash/file.go | 1 + templates/hash/messages.go | 8 ++- tests/api/hello.pb.clone.go | 29 ++++++++++ tests/api/hello.pb.equal.go | 46 +++++++++++++++ tests/api/hello.pb.go | 110 +++++++++++++++++++++++++++++------- tests/api/hello.pb.hash.go | 75 +++++++++++++++++++++++- tests/api/hello.pb.merge.go | 12 ++++ tests/api/hello.proto | 5 ++ tests/hash_test.go | 22 +++++++- 10 files changed, 285 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index d8d26cd..cc31cff 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,7 @@ GO_IMPORT:=$(subst $(space),,$(GO_IMPORT_SPACES)) .PHONY: generated-code generated-code: + rm -rf tests/api/*.go PATH=$(DEPSGOBIN):$$PATH $(DEPSGOBIN)/protoc -I=. -I=./external --go_out="${EXT_IMPORT}:." extproto/ext.proto PATH=$(DEPSGOBIN):$$PATH cp -r ${PACKAGE}/extproto/* extproto PATH=$(DEPSGOBIN):$$PATH $(DEPSGOBIN)/protoc -I=. -I=./extproto -I=./external --go_out="." --ext_out="." tests/api/hello.proto diff --git a/templates/hash/file.go b/templates/hash/file.go index 508e046..ae605c3 100644 --- a/templates/hash/file.go +++ b/templates/hash/file.go @@ -11,6 +11,7 @@ import ( "fmt" "hash" "hash/fnv" + "strconv" "github.com/solo-io/protoc-gen-ext/pkg/hasher/hashstructure" safe_hasher "github.com/solo-io/protoc-gen-ext/pkg/hasher" diff --git a/templates/hash/messages.go b/templates/hash/messages.go index bcbb4b1..ced7032 100644 --- a/templates/hash/messages.go +++ b/templates/hash/messages.go @@ -69,7 +69,13 @@ const mapTpl = ` ` const repeatedTpl = ` - for _, v := range {{ .FieldAccessor }} { + if _, err = hasher.Write([]byte("{{ .FieldName }}")); err != nil { + return 0, err + } + for i, v := range {{ .FieldAccessor }} { + if _, err = hasher.Write([]byte(strconv.Itoa(i))); err != nil { + return 0, err + } {{ .InnerTemplates.Value }} } ` diff --git a/tests/api/hello.pb.clone.go b/tests/api/hello.pb.clone.go index 6c64f2e..904ae2b 100644 --- a/tests/api/hello.pb.clone.go +++ b/tests/api/hello.pb.clone.go @@ -367,3 +367,32 @@ func (m *MultipleStrings) Clone() proto.Message { return target } + +// Clone function +func (m *Repeated) Clone() proto.Message { + var target *Repeated + if m == nil { + return target + } + target = &Repeated{} + + if m.GetFirst() != nil { + target.First = make([]string, len(m.GetFirst())) + for idx, v := range m.GetFirst() { + + target.First[idx] = v + + } + } + + if m.GetSecond() != nil { + target.Second = make([]string, len(m.GetSecond())) + for idx, v := range m.GetSecond() { + + target.Second[idx] = v + + } + } + + return target +} diff --git a/tests/api/hello.pb.equal.go b/tests/api/hello.pb.equal.go index 4a09e5c..71d75f6 100644 --- a/tests/api/hello.pb.equal.go +++ b/tests/api/hello.pb.equal.go @@ -618,3 +618,49 @@ func (m *MultipleStrings) Equal(that interface{}) bool { return true } + +// Equal function +func (m *Repeated) Equal(that interface{}) bool { + if that == nil { + return m == nil + } + + target, ok := that.(*Repeated) + if !ok { + that2, ok := that.(Repeated) + if ok { + target = &that2 + } else { + return false + } + } + if target == nil { + return m == nil + } else if m == nil { + return false + } + + if len(m.GetFirst()) != len(target.GetFirst()) { + return false + } + for idx, v := range m.GetFirst() { + + if strings.Compare(v, target.GetFirst()[idx]) != 0 { + return false + } + + } + + if len(m.GetSecond()) != len(target.GetSecond()) { + return false + } + for idx, v := range m.GetSecond() { + + if strings.Compare(v, target.GetSecond()[idx]) != 0 { + return false + } + + } + + return true +} diff --git a/tests/api/hello.pb.go b/tests/api/hello.pb.go index 2c1e07a..2383f96 100644 --- a/tests/api/hello.pb.go +++ b/tests/api/hello.pb.go @@ -728,6 +728,61 @@ func (x *MultipleStrings) GetS2() string { return "" } +type Repeated struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + First []string `protobuf:"bytes,1,rep,name=first,proto3" json:"first,omitempty"` + Second []string `protobuf:"bytes,2,rep,name=second,proto3" json:"second,omitempty"` +} + +func (x *Repeated) Reset() { + *x = Repeated{} + if protoimpl.UnsafeEnabled { + mi := &file_tests_api_hello_proto_msgTypes[5] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *Repeated) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Repeated) ProtoMessage() {} + +func (x *Repeated) ProtoReflect() protoreflect.Message { + mi := &file_tests_api_hello_proto_msgTypes[5] + 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 Repeated.ProtoReflect.Descriptor instead. +func (*Repeated) Descriptor() ([]byte, []int) { + return file_tests_api_hello_proto_rawDescGZIP(), []int{5} +} + +func (x *Repeated) GetFirst() []string { + if x != nil { + return x.First + } + return nil +} + +func (x *Repeated) GetSecond() []string { + if x != nil { + return x.Second + } + return nil +} + var File_tests_api_hello_proto protoreflect.FileDescriptor var file_tests_api_hello_proto_rawDesc = []byte{ @@ -924,13 +979,17 @@ var file_tests_api_hello_proto_rawDesc = []byte{ 0x22, 0x31, 0x0a, 0x0f, 0x4d, 0x75, 0x6c, 0x74, 0x69, 0x70, 0x6c, 0x65, 0x53, 0x74, 0x72, 0x69, 0x6e, 0x67, 0x73, 0x12, 0x0e, 0x0a, 0x02, 0x73, 0x31, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x73, 0x31, 0x12, 0x0e, 0x0a, 0x02, 0x73, 0x32, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, - 0x02, 0x73, 0x32, 0x2a, 0x1c, 0x0a, 0x04, 0x54, 0x65, 0x73, 0x74, 0x12, 0x09, 0x0a, 0x05, 0x48, - 0x45, 0x4c, 0x4c, 0x4f, 0x10, 0x00, 0x12, 0x09, 0x0a, 0x05, 0x57, 0x4f, 0x52, 0x4c, 0x44, 0x10, - 0x01, 0x42, 0x3d, 0x5a, 0x2b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, - 0x73, 0x6f, 0x6c, 0x6f, 0x2d, 0x69, 0x6f, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x63, 0x2d, 0x67, - 0x65, 0x6e, 0x2d, 0x65, 0x78, 0x74, 0x2f, 0x74, 0x65, 0x73, 0x74, 0x73, 0x2f, 0x61, 0x70, 0x69, - 0xb8, 0xf5, 0x04, 0x01, 0xc8, 0xf5, 0x04, 0x01, 0xc0, 0xf5, 0x04, 0x01, 0xd0, 0xf5, 0x04, 0x01, - 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x02, 0x73, 0x32, 0x22, 0x38, 0x0a, 0x08, 0x52, 0x65, 0x70, 0x65, 0x61, 0x74, 0x65, 0x64, 0x12, + 0x14, 0x0a, 0x05, 0x66, 0x69, 0x72, 0x73, 0x74, 0x18, 0x01, 0x20, 0x03, 0x28, 0x09, 0x52, 0x05, + 0x66, 0x69, 0x72, 0x73, 0x74, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x65, 0x63, 0x6f, 0x6e, 0x64, 0x18, + 0x02, 0x20, 0x03, 0x28, 0x09, 0x52, 0x06, 0x73, 0x65, 0x63, 0x6f, 0x6e, 0x64, 0x2a, 0x1c, 0x0a, + 0x04, 0x54, 0x65, 0x73, 0x74, 0x12, 0x09, 0x0a, 0x05, 0x48, 0x45, 0x4c, 0x4c, 0x4f, 0x10, 0x00, + 0x12, 0x09, 0x0a, 0x05, 0x57, 0x4f, 0x52, 0x4c, 0x44, 0x10, 0x01, 0x42, 0x3d, 0x5a, 0x2b, 0x67, + 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x73, 0x6f, 0x6c, 0x6f, 0x2d, 0x69, + 0x6f, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x63, 0x2d, 0x67, 0x65, 0x6e, 0x2d, 0x65, 0x78, 0x74, + 0x2f, 0x74, 0x65, 0x73, 0x74, 0x73, 0x2f, 0x61, 0x70, 0x69, 0xb8, 0xf5, 0x04, 0x01, 0xc8, 0xf5, + 0x04, 0x01, 0xc0, 0xf5, 0x04, 0x01, 0xd0, 0xf5, 0x04, 0x01, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, + 0x6f, 0x33, } var ( @@ -946,7 +1005,7 @@ func file_tests_api_hello_proto_rawDescGZIP() []byte { } var file_tests_api_hello_proto_enumTypes = make([]protoimpl.EnumInfo, 1) -var file_tests_api_hello_proto_msgTypes = make([]protoimpl.MessageInfo, 8) +var file_tests_api_hello_proto_msgTypes = make([]protoimpl.MessageInfo, 9) var file_tests_api_hello_proto_goTypes = []interface{}{ (Test)(0), // 0: envoy.type.Test (*Simple)(nil), // 1: envoy.type.Simple @@ -954,28 +1013,29 @@ var file_tests_api_hello_proto_goTypes = []interface{}{ (*Empty)(nil), // 3: envoy.type.Empty (*NestedEmpty)(nil), // 4: envoy.type.NestedEmpty (*MultipleStrings)(nil), // 5: envoy.type.MultipleStrings - nil, // 6: envoy.type.Nested.InitialEntry - nil, // 7: envoy.type.Nested.SimpleMapEntry - nil, // 8: envoy.type.Nested.MapExternalEntry - (*structpb.Struct)(nil), // 9: google.protobuf.Struct + (*Repeated)(nil), // 6: envoy.type.Repeated + nil, // 7: envoy.type.Nested.InitialEntry + nil, // 8: envoy.type.Nested.SimpleMapEntry + nil, // 9: envoy.type.Nested.MapExternalEntry + (*structpb.Struct)(nil), // 10: google.protobuf.Struct } var file_tests_api_hello_proto_depIdxs = []int32{ 1, // 0: envoy.type.Nested.simple:type_name -> envoy.type.Simple 1, // 1: envoy.type.Nested.other_simple:type_name -> envoy.type.Simple 0, // 2: envoy.type.Nested.test:type_name -> envoy.type.Test 3, // 3: envoy.type.Nested.empty:type_name -> envoy.type.Empty - 9, // 4: envoy.type.Nested.details:type_name -> google.protobuf.Struct + 10, // 4: envoy.type.Nested.details:type_name -> google.protobuf.Struct 1, // 5: envoy.type.Nested.skipper:type_name -> envoy.type.Simple 1, // 6: envoy.type.Nested.x:type_name -> envoy.type.Simple - 6, // 7: envoy.type.Nested.initial:type_name -> envoy.type.Nested.InitialEntry - 7, // 8: envoy.type.Nested.simple_map:type_name -> envoy.type.Nested.SimpleMapEntry + 7, // 7: envoy.type.Nested.initial:type_name -> envoy.type.Nested.InitialEntry + 8, // 8: envoy.type.Nested.simple_map:type_name -> envoy.type.Nested.SimpleMapEntry 3, // 9: envoy.type.Nested.empty_one_of:type_name -> envoy.type.Empty 4, // 10: envoy.type.Nested.nested_one_of:type_name -> envoy.type.NestedEmpty - 9, // 11: envoy.type.Nested.repeated_external:type_name -> google.protobuf.Struct - 8, // 12: envoy.type.Nested.map_external:type_name -> envoy.type.Nested.MapExternalEntry + 10, // 11: envoy.type.Nested.repeated_external:type_name -> google.protobuf.Struct + 9, // 12: envoy.type.Nested.map_external:type_name -> envoy.type.Nested.MapExternalEntry 2, // 13: envoy.type.NestedEmpty.nested:type_name -> envoy.type.Nested 1, // 14: envoy.type.Nested.InitialEntry.value:type_name -> envoy.type.Simple - 9, // 15: envoy.type.Nested.MapExternalEntry.value:type_name -> google.protobuf.Struct + 10, // 15: envoy.type.Nested.MapExternalEntry.value:type_name -> google.protobuf.Struct 16, // [16:16] is the sub-list for method output_type 16, // [16:16] is the sub-list for method input_type 16, // [16:16] is the sub-list for extension type_name @@ -1049,6 +1109,18 @@ func file_tests_api_hello_proto_init() { return nil } } + file_tests_api_hello_proto_msgTypes[5].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*Repeated); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } } file_tests_api_hello_proto_msgTypes[0].OneofWrappers = []interface{}{} file_tests_api_hello_proto_msgTypes[1].OneofWrappers = []interface{}{ @@ -1063,7 +1135,7 @@ func file_tests_api_hello_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_tests_api_hello_proto_rawDesc, NumEnums: 1, - NumMessages: 8, + NumMessages: 9, NumExtensions: 0, NumServices: 0, }, diff --git a/tests/api/hello.pb.hash.go b/tests/api/hello.pb.hash.go index 6b8bccf..d813ab5 100644 --- a/tests/api/hello.pb.hash.go +++ b/tests/api/hello.pb.hash.go @@ -9,6 +9,7 @@ import ( "fmt" "hash" "hash/fnv" + "strconv" safe_hasher "github.com/solo-io/protoc-gen-ext/pkg/hasher" "github.com/solo-io/protoc-gen-ext/pkg/hasher/hashstructure" @@ -351,7 +352,13 @@ func (m *Nested) Hash(hasher hash.Hash64) (uint64, error) { } } - for _, v := range m.GetHello() { + if _, err = hasher.Write([]byte("Hello")); err != nil { + return 0, err + } + for i, v := range m.GetHello() { + if _, err = hasher.Write([]byte(strconv.Itoa(i))); err != nil { + return 0, err + } if _, err = hasher.Write([]byte("v")); err != nil { return 0, err @@ -382,7 +389,13 @@ func (m *Nested) Hash(hasher hash.Hash64) (uint64, error) { } } - for _, v := range m.GetX() { + if _, err = hasher.Write([]byte("X")); err != nil { + return 0, err + } + for i, v := range m.GetX() { + if _, err = hasher.Write([]byte(strconv.Itoa(i))); err != nil { + return 0, err + } if h, ok := interface{}(v).(safe_hasher.SafeHasher); ok { if _, err = hasher.Write([]byte("v")); err != nil { @@ -485,7 +498,13 @@ func (m *Nested) Hash(hasher hash.Hash64) (uint64, error) { return 0, err } - for _, v := range m.GetRepeatedExternal() { + if _, err = hasher.Write([]byte("RepeatedExternal")); err != nil { + return 0, err + } + for i, v := range m.GetRepeatedExternal() { + if _, err = hasher.Write([]byte(strconv.Itoa(i))); err != nil { + return 0, err + } if h, ok := interface{}(v).(safe_hasher.SafeHasher); ok { if _, err = hasher.Write([]byte("v")); err != nil { @@ -701,3 +720,53 @@ func (m *MultipleStrings) Hash(hasher hash.Hash64) (uint64, error) { return hasher.Sum64(), nil } + +// Hash function +func (m *Repeated) Hash(hasher hash.Hash64) (uint64, error) { + if m == nil { + return 0, nil + } + if hasher == nil { + hasher = fnv.New64() + } + var err error + if _, err = hasher.Write([]byte("envoy.type.github.com/solo-io/protoc-gen-ext/tests/api.Repeated")); err != nil { + return 0, err + } + + if _, err = hasher.Write([]byte("First")); err != nil { + return 0, err + } + for i, v := range m.GetFirst() { + if _, err = hasher.Write([]byte(strconv.Itoa(i))); err != nil { + return 0, err + } + + if _, err = hasher.Write([]byte("v")); err != nil { + return 0, err + } + if _, err = hasher.Write([]byte(v)); err != nil { + return 0, err + } + + } + + if _, err = hasher.Write([]byte("Second")); err != nil { + return 0, err + } + for i, v := range m.GetSecond() { + if _, err = hasher.Write([]byte(strconv.Itoa(i))); err != nil { + return 0, err + } + + if _, err = hasher.Write([]byte("v")); err != nil { + return 0, err + } + if _, err = hasher.Write([]byte(v)); err != nil { + return 0, err + } + + } + + return hasher.Sum64(), nil +} diff --git a/tests/api/hello.pb.merge.go b/tests/api/hello.pb.merge.go index 9b038c9..a902573 100644 --- a/tests/api/hello.pb.merge.go +++ b/tests/api/hello.pb.merge.go @@ -214,3 +214,15 @@ func (m *MultipleStrings) Merge(overrides *MultipleStrings) { m.S2 = overrides.S2 } + +// Merge non-nil fields from overrides into m +func (m *Repeated) Merge(overrides *Repeated) { + if m == nil || overrides == nil { + return + } + + m.First = overrides.First + + m.Second = overrides.Second + +} diff --git a/tests/api/hello.proto b/tests/api/hello.proto index a912134..7ca929f 100644 --- a/tests/api/hello.proto +++ b/tests/api/hello.proto @@ -105,4 +105,9 @@ message NestedEmpty { message MultipleStrings { string s1 = 1; string s2 = 2; +} + +message Repeated { + repeated string first = 1; + repeated string second = 2; } \ No newline at end of file diff --git a/tests/hash_test.go b/tests/hash_test.go index 10c47dd..34bd255 100644 --- a/tests/hash_test.go +++ b/tests/hash_test.go @@ -245,7 +245,7 @@ var _ = Describe("hash", func() { }) }) - Context("Will include field names in the hash", func() { + Context("Hashing object values of same type", func() { When("multiple fields of same type have the same value", func() { It("should produce different hash values", func() { object1 := &api.MultipleStrings{ @@ -305,6 +305,26 @@ var _ = Describe("hash", func() { Expect(hash1).To(Equal(hash2)) }) }) + + When("two objects have different repeated fields of the same type with the same values", func() { + It("should produce different hash values", func() { + object1 := &api.Repeated{ + First: []string{"hello", "world"}, + } + + object2 := &api.Repeated{ + Second: []string{"hello", "world"}, + } + + hash1, err := object1.Hash(nil) + Expect(err).NotTo(HaveOccurred()) + + hash2, err := object2.Hash(nil) + Expect(err).NotTo(HaveOccurred()) + + Expect(hash1).NotTo(Equal(hash2)) + }) + }) }) })