Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: plain map and slice #464

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ When a non-`nil` union value is encountered, a single key is en/decoded. The key
type name, or scheam full name in the case of a named schema (enum, fixed or record).
* ***T:** This is allowed in a "nullable" union. A nullable union is defined as a two schema union,
with one of the types being `null` (ie. `["null", "string"]` or `["string", "null"]`), in this case
a `*T` is allowed, with `T` matching the conversion table above. In the case of a slice, the slice can be used
a `*T` is allowed, with `T` matching the conversion table above. In the case of a slice or a map, the slice or the map can be used
directly.
* **any:** An `interface` can be provided and the type or name resolved. Primitive types
are pre-registered, but named types, maps and slices will need to be registered with the `Register` function.
Expand Down
6 changes: 6 additions & 0 deletions cmd/avrogen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ type config struct {
FullName bool
Encoders bool
FullSchema bool
PlainMap bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decoder will not work for map like it does for slice currently.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback, however I'm not sure what you are referring to here. What would need to change in the decoder for this to work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nrwiersma I'm not familiar with the whole codec logic yet but I had a look and tried to add support for nullable maps for encoding and decoding. Have a look at my latest commits and let me know what you think.

PlainSlice bool
StrictTypes bool
Initialisms string
}
Expand All @@ -45,6 +47,8 @@ func realMain(args []string, stdout, stderr io.Writer) int {
flgs.BoolVar(&cfg.Encoders, "encoders", false, "Generate encoders for the structs.")
flgs.BoolVar(&cfg.FullSchema, "fullschema", false, "Use the full schema in the generated encoders.")
flgs.BoolVar(&cfg.StrictTypes, "strict-types", false, "Use strict type sizes (e.g. int32) during generation.")
flgs.BoolVar(&cfg.PlainMap, "plain-map", false, "Use a plain map instead of a ptr for nullable map unions.")
flgs.BoolVar(&cfg.PlainSlice, "plain-slice", false, "Use a plain slice instead of a ptr for nullable slice unions.")
flgs.StringVar(&cfg.Initialisms, "initialisms", "", "Custom initialisms <VAL>[,...] for struct and field names.")
flgs.StringVar(&cfg.TemplateFileName, "template-filename", "", "Override output template with one loaded from file.")
flgs.Usage = func() {
Expand Down Expand Up @@ -85,6 +89,8 @@ func realMain(args []string, stdout, stderr io.Writer) int {
gen.WithEncoders(cfg.Encoders),
gen.WithInitialisms(initialisms),
gen.WithTemplate(string(template)),
gen.WithPlainMap(cfg.PlainMap),
gen.WithPlainSlice(cfg.PlainSlice),
gen.WithStrictTypes(cfg.StrictTypes),
gen.WithFullSchema(cfg.FullSchema),
}
Expand Down
27 changes: 27 additions & 0 deletions cmd/avrogen/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,33 @@ func TestAvroGen_GeneratesSchemaWithStrictTypes(t *testing.T) {
assert.Equal(t, want, got)
}

func TestAvroGen_GeneratePlain(t *testing.T) {
for _, opt := range []string{"map", "slice"} {
t.Run(opt, func(t *testing.T) {
t.Parallel()

path := t.TempDir()
file := filepath.Join(path, "test.go")

args := []string{"avrogen", "-pkg", "testpkg", "-o", file, "-plain-" + opt, "testdata/schema.avsc"}
gotCode := realMain(args, io.Discard, io.Discard)
require.Equal(t, 0, gotCode)

got, err := os.ReadFile(file)
require.NoError(t, err)

if *update {
err = os.WriteFile("testdata/golden_plain"+opt+".go", got, 0600)
require.NoError(t, err)
}

want, err := os.ReadFile("testdata/golden_plain" + opt + ".go")
require.NoError(t, err)
assert.Equal(t, want, got)
})
}
}

func TestParseTags(t *testing.T) {
tests := []struct {
name string
Expand Down
6 changes: 4 additions & 2 deletions cmd/avrogen/testdata/golden.go

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

8 changes: 5 additions & 3 deletions cmd/avrogen/testdata/golden_encoders.go

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

8 changes: 5 additions & 3 deletions cmd/avrogen/testdata/golden_encoders_fullschema.go

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

6 changes: 4 additions & 2 deletions cmd/avrogen/testdata/golden_fullname.go

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

11 changes: 11 additions & 0 deletions cmd/avrogen/testdata/golden_plainmap.go

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

11 changes: 11 additions & 0 deletions cmd/avrogen/testdata/golden_plainslice.go

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

6 changes: 4 additions & 2 deletions cmd/avrogen/testdata/golden_stricttypes.go

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

6 changes: 4 additions & 2 deletions cmd/avrogen/testdata/schema.avsc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"doc": "Test is a test struct",
"fields": [
{ "name": "someString", "type": "string", "doc": "SomeString is a string" },
{ "name": "someInt", "type": "int" }
{ "name": "someInt", "type": "int" },
{ "name": "someNullableMap", "type": ["null", {"type": "map", "values": "int"}]},
{ "name": "someNullableSlice", "type": ["null", {"type": "array", "items": "int"}]}
]
}
}
28 changes: 24 additions & 4 deletions codec_union.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@ import (
func createDecoderOfUnion(d *decoderContext, schema *UnionSchema, typ reflect2.Type) ValDecoder {
switch typ.Kind() {
case reflect.Map:
if typ.(reflect2.MapType).Key().Kind() != reflect.String ||
typ.(reflect2.MapType).Elem().Kind() != reflect.Interface {
if typ.(reflect2.MapType).Key().Kind() != reflect.String {
break
}

if typ.(reflect2.MapType).Elem().Kind() != reflect.Interface {
if !schema.Nullable() {
break
}
Comment on lines +20 to +23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an edge case here being:
["null", {"type": "map", "values": ["null", "string"]}]
or something like this. In the traditional struct layout this could be *map[string]any which is specifically excluded here. In the new layout being proposed, this would be map[string]any, and the en/decoder would not be able to tell the cases apart.

To deal with this case, IMO the en/decoder should always error towards the map union (existing functionality) and failing that, check the new non-pointer version. This would also need to be documented.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nrwiersma sorry my use case has changed and I no longer need this feature. Sadly I don't have time to fix it at the moment. Feel free to fix it, hand it over to another volunteer or close it.


return decoderOfNullableUnion(d, schema, typ)
}

return decoderOfMapUnion(d, schema, typ)
case reflect.Slice:
if !schema.Nullable() {
Expand Down Expand Up @@ -44,10 +52,18 @@ func createDecoderOfUnion(d *decoderContext, schema *UnionSchema, typ reflect2.T
func createEncoderOfUnion(e *encoderContext, schema *UnionSchema, typ reflect2.Type) ValEncoder {
switch typ.Kind() {
case reflect.Map:
if typ.(reflect2.MapType).Key().Kind() != reflect.String ||
typ.(reflect2.MapType).Elem().Kind() != reflect.Interface {
if typ.(reflect2.MapType).Key().Kind() != reflect.String {
break
}

if typ.(reflect2.MapType).Elem().Kind() != reflect.Interface {
if !schema.Nullable() {
break
}

return encoderOfNullableUnion(e, schema, typ)
}

return encoderOfMapUnion(e, schema, typ)
case reflect.Slice:
if !schema.Nullable() {
Expand Down Expand Up @@ -179,6 +195,8 @@ func decoderOfNullableUnion(d *decoderContext, schema Schema, typ reflect2.Type)
isPtr = true
case *reflect2.UnsafeSliceType:
baseTyp = v
case *reflect2.UnsafeMapType:
baseTyp = v
}
decoder := decoderOfType(d, union.Types()[typeIdx], baseTyp)

Expand Down Expand Up @@ -249,6 +267,8 @@ func encoderOfNullableUnion(e *encoderContext, schema Schema, typ reflect2.Type)
isPtr = true
case *reflect2.UnsafeSliceType:
baseTyp = v
case *reflect2.UnsafeMapType:
baseTyp = v
}
encoder := encoderOfType(e, union.Types()[typeIdx], baseTyp)

Expand Down
36 changes: 36 additions & 0 deletions decoder_union_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,42 @@ func TestDecoder_UnionPtrReversedNull(t *testing.T) {
assert.Nil(t, got)
}

func TestDecoder_UnionNullableMap(t *testing.T) {
tt := []struct {
name string
data []byte
want map[string]string
}{{
name: "WithData",
data: []byte{0x02, 0x01, 0x10, 0x06, 0x66, 0x6F, 0x6F, 0x06, 0x66, 0x6F, 0x6F, 0x00},
want: map[string]string{"foo": "foo"},
}, {
name: "Empty",
data: []byte{0x02, 0x00},
want: map[string]string{},
}, {
name: "Null",
data: []byte{0x00},
want: nil,
}}

schema := `["null", {"type":"map", "values": "string"}]`

for _, test := range tt {
t.Run(test.name, func(t *testing.T) {
defer ConfigTeardown()

dec, _ := avro.NewDecoder(schema, bytes.NewReader(test.data))

var got map[string]string
err := dec.Decode(&got)

require.NoError(t, err)
assert.Equal(t, test.want, got)
})
}
}

func TestDecoder_UnionNullableSlice(t *testing.T) {
defer ConfigTeardown()

Expand Down
37 changes: 37 additions & 0 deletions encoder_union_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,43 @@ func TestEncoder_UnionPtrNotNullable(t *testing.T) {
assert.Error(t, err)
}

func TestEncoder_UnionNullableMap(t *testing.T) {
tt := []struct {
name string
data map[string]string
want []byte
}{{
name: "WithData",
data: map[string]string{"foo": "foo"},
want: []byte{0x02, 0x01, 0x10, 0x06, 0x66, 0x6F, 0x6F, 0x06, 0x66, 0x6F, 0x6F, 0x00},
}, {
name: "Empty",
data: map[string]string{},
want: []byte{0x02, 0x00},
}, {
name: "Null",
data: nil,
want: []byte{0x00},
}}

schema := `["null", {"type":"map", "values": "string"}]`

for _, test := range tt {
t.Run(test.name, func(t *testing.T) {
defer ConfigTeardown()

buf := bytes.NewBuffer([]byte{})
enc, err := avro.NewEncoder(schema, buf)
require.NoError(t, err)

err = enc.Encode(test.data)

require.NoError(t, err)
assert.Equal(t, test.want, buf.Bytes())
})
}
}

func TestEncoder_UnionNullableSlice(t *testing.T) {
defer ConfigTeardown()

Expand Down
31 changes: 31 additions & 0 deletions gen/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ type Config struct {
FullName bool
Encoders bool
FullSchema bool
PlainMap bool
PlainSlice bool
StrictTypes bool
Initialisms []string
LogicalTypes []LogicalType
Expand All @@ -43,6 +45,9 @@ const (
Kebab TagStyle = "kebab"
// UpperCamel is a style like ImWrittenInUpperCamel.
UpperCamel TagStyle = "upper-camel"

prefixMap string = "map["
prefixSlice string = "[]"
)

//go:embed output_template.tmpl
Expand Down Expand Up @@ -85,6 +90,8 @@ func StructFromSchema(schema avro.Schema, w io.Writer, cfg Config) error {
WithInitialisms(cfg.Initialisms),
WithStrictTypes(cfg.StrictTypes),
WithFullSchema(cfg.FullSchema),
WithPlainMap(cfg.PlainMap),
WithPlainSlice(cfg.PlainSlice),
}
for _, opt := range cfg.LogicalTypes {
opts = append(opts, WithLogicalType(opt))
Expand Down Expand Up @@ -168,6 +175,20 @@ func WithFullSchema(b bool) OptsFunc {
}
}

// WithPlainMap configures the generator to emit a plain map and not a map ptr for nullable unions.
func WithPlainMap(b bool) OptsFunc {
return func(g *Generator) {
g.plainMap = b
}
}

// WithPlainSlice configures the generator to emit a plain map and not a map ptr for nullable unions.
func WithPlainSlice(b bool) OptsFunc {
return func(g *Generator) {
g.plainSlice = b
}
}

// LogicalType used when the name of the "LogicalType" field in the Avro schema matches the Name attribute.
type LogicalType struct {
// Name of the LogicalType
Expand Down Expand Up @@ -210,6 +231,8 @@ type Generator struct {
fullName bool
encoders bool
fullSchema bool
plainMap bool
plainSlice bool
strictTypes bool
initialisms []string
logicalTypes map[avro.LogicalType]LogicalType
Expand Down Expand Up @@ -356,6 +379,14 @@ func (g *Generator) resolveUnionTypes(s *avro.UnionSchema) string {
types = append(types, g.generate(elem))
}
if s.Nullable() {
if g.plainMap && strings.HasPrefix(types[0], prefixMap) {
return types[0]
}

if g.plainSlice && strings.HasPrefix(types[0], prefixSlice) {
return types[0]
}

return "*" + types[0]
}
return "any"
Expand Down
Loading
Loading