Skip to content
This repository has been archived by the owner on Sep 29, 2024. It is now read-only.

Commit

Permalink
Ignore conflicting portable names within a single codec.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmalloc committed Jun 21, 2024
1 parent bb23ebe commit 41a4476
Show file tree
Hide file tree
Showing 8 changed files with 248 additions and 30 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions codec/doc.go
Original file line number Diff line number Diff line change
@@ -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
156 changes: 156 additions & 0 deletions codec/internal/fixtures/conflicting/protobuf.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions codec/internal/fixtures/conflicting/protobuf.proto
Original file line number Diff line number Diff line change
@@ -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; }
2 changes: 1 addition & 1 deletion codec/internal/fixtures/protobuf.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 2 additions & 4 deletions codec/internal/fixtures/protobuf.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
63 changes: 41 additions & 22 deletions codec/marshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand All @@ -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
Expand Down
32 changes: 29 additions & 3 deletions codec/marshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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: "<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{}),
Expand All @@ -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",
),
))
})
Expand Down

0 comments on commit 41a4476

Please sign in to comment.