diff --git a/CHANGELOG.md b/CHANGELOG.md index daa3e04..57e41fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,13 @@ The format is based on [Keep a Changelog], and this project adheres to `dogma.StatelessProcessRoot` values without assuming that any other codec is available. +### Changed + +- `codec.NewMarshaler()` no longer returns an error when two (or more) types + have a conflicting portable name so long as at least one codec has a unique + portable name for that type. Any single codec that produces the same portable + name for multiple types is not used for those types. + ## [0.7.3] - 2023-03-27 ### Fixed diff --git a/codec/doc.go b/codec/doc.go new file mode 100644 index 0000000..a5801e9 --- /dev/null +++ b/codec/doc.go @@ -0,0 +1,3 @@ +// Package codec provides an implementation of [marshalkit.Marshaler] that uses +// a set of priority-ordered codecs to marshal and unmarshal types and values. +package codec diff --git a/codec/internal/fixtures/conflicting/protobuf.pb.go b/codec/internal/fixtures/conflicting/protobuf.pb.go new file mode 100644 index 0000000..cfd3f6f --- /dev/null +++ b/codec/internal/fixtures/conflicting/protobuf.pb.go @@ -0,0 +1,156 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.34.2 +// protoc v5.27.1 +// source: github.com/dogmatiq/marshalkit/codec/internal/fixtures/conflicting/protobuf.proto + +package conflicting + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// ProtoMessage is a protocol buffers message with the same name as +// [fixtures.ProtoMessage] but a different protocol buffers package name. +type ProtoMessage struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + Value string `protobuf:"bytes,1,opt,name=value,proto3" json:"value,omitempty"` +} + +func (x *ProtoMessage) Reset() { + *x = ProtoMessage{} + if protoimpl.UnsafeEnabled { + mi := &file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ProtoMessage) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ProtoMessage) ProtoMessage() {} + +func (x *ProtoMessage) ProtoReflect() protoreflect.Message { + mi := &file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_msgTypes[0] + 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 ProtoMessage.ProtoReflect.Descriptor instead. +func (*ProtoMessage) Descriptor() ([]byte, []int) { + return file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescGZIP(), []int{0} +} + +func (x *ProtoMessage) GetValue() string { + if x != nil { + return x.Value + } + return "" +} + +var File_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto protoreflect.FileDescriptor + +var file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDesc = []byte{ + 0x0a, 0x51, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x64, 0x6f, 0x67, + 0x6d, 0x61, 0x74, 0x69, 0x71, 0x2f, 0x6d, 0x61, 0x72, 0x73, 0x68, 0x61, 0x6c, 0x6b, 0x69, 0x74, + 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x63, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, + 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x2f, 0x63, 0x6f, 0x6e, 0x66, 0x6c, 0x69, 0x63, + 0x74, 0x69, 0x6e, 0x67, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x12, 0x28, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, 0x69, 0x71, 0x2e, 0x6d, 0x61, + 0x72, 0x73, 0x68, 0x61, 0x6c, 0x6b, 0x69, 0x74, 0x2e, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, + 0x73, 0x2e, 0x63, 0x6f, 0x6e, 0x66, 0x6c, 0x69, 0x63, 0x74, 0x69, 0x6e, 0x67, 0x22, 0x24, 0x0a, + 0x0c, 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x14, 0x0a, + 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, + 0x6c, 0x75, 0x65, 0x42, 0x44, 0x5a, 0x42, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, + 0x6d, 0x2f, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, 0x69, 0x71, 0x2f, 0x6d, 0x61, 0x72, 0x73, 0x68, + 0x61, 0x6c, 0x6b, 0x69, 0x74, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x63, 0x2f, 0x69, 0x6e, 0x74, 0x65, + 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x2f, 0x63, 0x6f, + 0x6e, 0x66, 0x6c, 0x69, 0x63, 0x74, 0x69, 0x6e, 0x67, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, + 0x33, +} + +var ( + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescOnce sync.Once + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescData = file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDesc +) + +func file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescGZIP() []byte { + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescOnce.Do(func() { + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescData = protoimpl.X.CompressGZIP(file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescData) + }) + return file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDescData +} + +var file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_msgTypes = make([]protoimpl.MessageInfo, 1) +var file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_goTypes = []any{ + (*ProtoMessage)(nil), // 0: dogmatiq.marshalkit.fixtures.conflicting.ProtoMessage +} +var file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_depIdxs = []int32{ + 0, // [0:0] is the sub-list for method output_type + 0, // [0:0] is the sub-list for method input_type + 0, // [0:0] is the sub-list for extension type_name + 0, // [0:0] is the sub-list for extension extendee + 0, // [0:0] is the sub-list for field type_name +} + +func init() { + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_init() +} +func file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_init() { + if File_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto != nil { + return + } + if !protoimpl.UnsafeEnabled { + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_msgTypes[0].Exporter = func(v any, i int) any { + switch v := v.(*ProtoMessage); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDesc, + NumEnums: 0, + NumMessages: 1, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_goTypes, + DependencyIndexes: file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_depIdxs, + MessageInfos: file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_msgTypes, + }.Build() + File_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto = out.File + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_rawDesc = nil + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_goTypes = nil + file_github_com_dogmatiq_marshalkit_codec_internal_fixtures_conflicting_protobuf_proto_depIdxs = nil +} diff --git a/codec/internal/fixtures/conflicting/protobuf.proto b/codec/internal/fixtures/conflicting/protobuf.proto new file mode 100644 index 0000000..b0ef9bd --- /dev/null +++ b/codec/internal/fixtures/conflicting/protobuf.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package dogmatiq.marshalkit.fixtures.conflicting; + +option go_package = "github.com/dogmatiq/marshalkit/codec/internal/fixtures/conflicting"; + +// ProtoMessage is a protocol buffers message with the same name as +// [fixtures.ProtoMessage] but a different protocol buffers package name. +message ProtoMessage { string value = 1; } diff --git a/codec/internal/fixtures/protobuf.pb.go b/codec/internal/fixtures/protobuf.pb.go index 63d0da7..b8a00f7 100644 --- a/codec/internal/fixtures/protobuf.pb.go +++ b/codec/internal/fixtures/protobuf.pb.go @@ -20,7 +20,7 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) -// ProtoMessage is a Dogma message implemented as a Protocol Buffers message. +// ProtoMessage is a generic protocol buffers message. type ProtoMessage struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache diff --git a/codec/internal/fixtures/protobuf.proto b/codec/internal/fixtures/protobuf.proto index a435156..b508325 100644 --- a/codec/internal/fixtures/protobuf.proto +++ b/codec/internal/fixtures/protobuf.proto @@ -4,7 +4,5 @@ package dogmatiq.marshalkit.fixtures; option go_package = "github.com/dogmatiq/marshalkit/codec/internal/fixtures"; -// ProtoMessage is a Dogma message implemented as a Protocol Buffers message. -message ProtoMessage { - string value = 1; -} +// ProtoMessage is a generic protocol buffers message. +message ProtoMessage { string value = 1; } diff --git a/codec/marshaler.go b/codec/marshaler.go index 62fc511..645709e 100644 --- a/codec/marshaler.go +++ b/codec/marshaler.go @@ -38,18 +38,46 @@ func NewMarshaler( typeByPortableName: map[string]reflect.Type{}, } - // build a list of all of the "unsupported" types - unsupported := map[reflect.Type]struct{}{} + // Build a list of all types that have not yet been assigned to at least one + // codec. + unassigned := map[reflect.Type]struct{}{} for _, rt := range types { - unsupported[rt] = struct{}{} + unassigned[rt] = struct{}{} } - // scan the codecs in order of preference + // Maintain a list of types that have conflicting portable names with other + // types within a single codec. + conflicts := map[reflect.Type]struct{}{} + + // Scan the codecs in order of preference. for _, c := range codecs { - caps := c.Query(types) + if _, ok := m.codecByBasicMediaType[c.BasicMediaType()]; ok { + return nil, fmt.Errorf( + "multiple codecs use the '%s' media-type", + c.BasicMediaType(), + ) + } + m.codecByBasicMediaType[c.BasicMediaType()] = c + + typesByName := map[string][]reflect.Type{} + for rt, n := range c.Query(types).Types { + typesByName[n] = append(typesByName[n], rt) + } + + for n, types := range typesByName { + for _, rt := range types { + if len(types) > 1 { + // Ignore any types with portable names that conflict WITHIN + // this codec. This keeps the algorithm deterministic + // without having to "rank" types. The expectation is that + // the types will have non-conflicting names under some + // other codec. + conflicts[rt] = struct{}{} + continue + } - if len(caps.Types) > 0 { - for rt, n := range caps.Types { + // Any portable names that conflict with other types ACROSS + // codecs result in an error. if x, ok := m.typeByPortableName[n]; ok && x != rt { return nil, fmt.Errorf( "the type name '%s' is used by both '%s' and '%s'", @@ -75,25 +103,16 @@ func NewMarshaler( m.types[rt] = bt - delete(unsupported, rt) + delete(unassigned, rt) } - - if _, ok := m.codecByBasicMediaType[c.BasicMediaType()]; ok { - return nil, fmt.Errorf( - "multiple codecs use the '%s' media-type", - c.BasicMediaType(), - ) - } - - m.codecByBasicMediaType[c.BasicMediaType()] = c } } - for rt := range unsupported { - return nil, fmt.Errorf( - "no codecs support the '%s' type", - rt, - ) + for rt := range unassigned { + if _, ok := conflicts[rt]; ok { + return nil, fmt.Errorf("naming conflicts occurred within all of the codecs that support the '%s' type", rt) + } + return nil, fmt.Errorf("no codecs support the '%s' type", rt) } return m, nil diff --git a/codec/marshaler_test.go b/codec/marshaler_test.go index 4eeaed0..e93a067 100644 --- a/codec/marshaler_test.go +++ b/codec/marshaler_test.go @@ -7,6 +7,7 @@ import ( "github.com/dogmatiq/marshalkit" . "github.com/dogmatiq/marshalkit/codec" . "github.com/dogmatiq/marshalkit/codec/internal/fixtures" + "github.com/dogmatiq/marshalkit/codec/internal/fixtures/conflicting" "github.com/dogmatiq/marshalkit/codec/json" "github.com/dogmatiq/marshalkit/codec/protobuf" . "github.com/jmalloc/gomegax" @@ -52,7 +53,32 @@ var _ = Describe("type Marshaler", func() { )) }) - It("returns an error if there conflicting portable type names", func() { + It("excludes types with conflicting portable type names", func() { + marshaler, err := NewMarshaler( + []reflect.Type{ + reflect.TypeOf(&ProtoMessage{}), + reflect.TypeOf(&conflicting.ProtoMessage{}), + }, + []Codec{ + &json.Codec{}, + &protobuf.DefaultNativeCodec, + }, + ) + Expect(err).ShouldNot(HaveOccurred()) + + p, err := marshaler.Marshal( + &ProtoMessage{ + Value: "", + }, + ) + Expect(err).ShouldNot(HaveOccurred()) + + // We expect the protocol buffers codec to be selected, because the + // names of the type messages conflict under the JSON codec. + Expect(p.MediaType).To(Equal("application/vnd.google.protobuf; type=dogmatiq.marshalkit.fixtures.ProtoMessage")) + }) + + It("returns an error if types with conflicting portable type names are excluded by all codecs", func() { _, err := NewMarshaler( []reflect.Type{ reflect.TypeOf(MessageA{}), @@ -64,10 +90,10 @@ var _ = Describe("type Marshaler", func() { ) Expect(err).To(Or( MatchError( - "the type name 'MessageA' is used by both 'fixtures.MessageA' and '*fixtures.MessageA'", + "naming conflicts occurred within all of the codecs that support the 'fixtures.MessageA' type", ), MatchError( - "the type name 'MessageA' is used by both '*fixtures.MessageA' and 'fixtures.MessageA'", + "naming conflicts occurred within all of the codecs that support the '*fixtures.MessageA' type", ), )) })