diff --git a/CHANGELOG b/CHANGELOG index 61fa83d..6b8ba7b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,8 @@ # UNRELEASED +- Feature / breaking change: add `preserve_non_string_maps` and change default behavior to turn all maps into string-keyed ones. + # 0.6.1 - 2024-02-27 - Fix: `collapse_fields` was only working on the first instance of the field, not when it was found inside a map etc. later on. diff --git a/README.md b/README.md index f5b295b..33ad8f5 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,58 @@ enum Category { } ``` +* `preserve_non_string_maps` - if set to true, will replace maps with non-string keys with records. E.g. if you have a map like: +```protobuf +message MyRecord { + map my_field = 1; +} +``` + +...with this option off, it will be translated into: + +```json +{ + "type": "record", + "name": "MyRecord", + "fields": [ + { + "name": "my_field", + "type": { + "type": "map", + "values": "string" + } + } + ] +} +``` + +...but with it on, it will be translated into: + +```json +{ + "type": "record", + "name": "MyRecord", + "fields": [ + { + "name": "my_field", + "type": { + "type": "record", + "fields": [ + { + "name": "key", + "type": "int" + }, + { + "name": "value", + "type": "string" + } + ] + } + } + ] +} +``` + --- To Do List: diff --git a/avro/record.go b/avro/record.go index 93703d3..85c75b7 100644 --- a/avro/record.go +++ b/avro/record.go @@ -42,17 +42,19 @@ func (t Record) ToJSON(types *TypeRepo) (any, error) { return jsonMap, nil } -func RecordFromProto(proto *descriptorpb.DescriptorProto, namespace string) []NamedType { +func RecordFromProto(proto *descriptorpb.DescriptorProto, namespace string, typeRepo *TypeRepo) []NamedType { // Protobuf `map` is actually a record with a `map_entry` boolean set, and two fields ("key" and "value"). // Avro only supports maps with string keys, but Protobuf supports maps with any key type. // If we detect a string key, we can turn this into an Avro map, but otherwise we have to leave it as a record. - if proto.Options.GetMapEntry() && proto.Field[0].GetType() == descriptorpb.FieldDescriptorProto_TYPE_STRING { - return []NamedType{ - Map{ - Namespace: namespace, - Name: proto.GetName(), - Values: FieldTypeFromProto(proto.Field[1]), - }, + if proto.Options.GetMapEntry() { + if !typeRepo.PreserveNonStringMaps || proto.Field[0].GetType() == descriptorpb.FieldDescriptorProto_TYPE_STRING { + return []NamedType{ + Map{ + Namespace: namespace, + Name: proto.GetName(), + Values: FieldTypeFromProto(proto.Field[1]), + }, + } } } @@ -61,7 +63,7 @@ func RecordFromProto(proto *descriptorpb.DescriptorProto, namespace string) []Na nested := make([]NamedType, len(proto.NestedType)) // enums := make([]Enum, len(proto.EnumType)) for i, field := range proto.NestedType { - nested[i] = RecordFromProto(field, fmt.Sprintf("%s.%s", namespace, proto.GetName()))[0] + nested[i] = RecordFromProto(field, fmt.Sprintf("%s.%s", namespace, proto.GetName()), typeRepo)[0] } //for i, field := range proto.EnumType { // enums[i] = EnumFromProto(field) diff --git a/avro/typeRepo.go b/avro/typeRepo.go index fbb4f21..19cb8e0 100644 --- a/avro/typeRepo.go +++ b/avro/typeRepo.go @@ -13,6 +13,7 @@ type TypeRepo struct { NamespaceMap map[string]string CollapseFields []string RemoveEnumPrefixes bool + PreserveNonStringMaps bool } func NewTypeRepo(params input.Params) *TypeRepo { @@ -21,6 +22,7 @@ func NewTypeRepo(params input.Params) *TypeRepo { NamespaceMap: params.NamespaceMap, CollapseFields: params.CollapseFields, RemoveEnumPrefixes: params.RemoveEnumPrefixes, + PreserveNonStringMaps: params.PreserveNonStringMaps, } } diff --git a/input/params.go b/input/params.go index e291c0f..23ec163 100644 --- a/input/params.go +++ b/input/params.go @@ -13,6 +13,7 @@ type Params struct { NamespaceMap map[string]string CollapseFields []string RemoveEnumPrefixes bool + PreserveNonStringMaps bool } func ReadRequest() (*pluginpb.CodeGeneratorRequest, error) { @@ -60,6 +61,8 @@ func ParseParams(req *pluginpb.CodeGeneratorRequest) Params { params.CollapseFields = strings.Split(v, ";") } else if k == "remove_enum_prefixes" { params.RemoveEnumPrefixes = v == "true" + } else if k == "preserve_non_string_maps" { + params.PreserveNonStringMaps = true } } return params diff --git a/main.go b/main.go index b3b6bfd..23fd7d0 100644 --- a/main.go +++ b/main.go @@ -17,7 +17,7 @@ var params input.Params var typeRepo *avro.TypeRepo func processMessage(proto *descriptorpb.DescriptorProto, protoPackage string) { - records := avro.RecordFromProto(proto, protoPackage) + records := avro.RecordFromProto(proto, protoPackage, typeRepo) for _, record := range records { typeRepo.AddType(record) } diff --git a/main_test.go b/main_test.go index 7fb2cda..59ee7e6 100644 --- a/main_test.go +++ b/main_test.go @@ -65,8 +65,13 @@ func runTest(t *testing.T, directory string, options map[string]string) { } protoc(t, args) - assertEqualFiles(t, workdir + "/testdata/" + directory, tmpdir) - + testDir := workdir + "/testdata/" + directory + if os.Getenv("UPDATE_SNAPSHOTS") != "" { + cmd := exec.Command("/bin/sh", "-c", fmt.Sprintf("cp %v/* %v", tmpdir, testDir)) + cmd.Run() + } else { + assertEqualFiles(t, testDir, tmpdir) + } } func Test_Base(t *testing.T) { @@ -85,6 +90,10 @@ func Test_NamespaceMap(t *testing.T) { runTest(t, "namespace_map", map[string]string{"namespace_map": "testdata:mynamespace"}) } +func Test_PreserveNonStringMaps(t *testing.T) { + runTest(t, "preserve_non_string_maps", map[string]string{"preserve_non_string_maps": "true"}) +} + func assertEqualFiles(t *testing.T, original, generated string) { names, err := fileNames(original, false) if err != nil { diff --git a/testdata/base/AOneOf.avsc b/testdata/base/AOneOf.avsc index 1f59e63..77270ee 100644 --- a/testdata/base/AOneOf.avsc +++ b/testdata/base/AOneOf.avsc @@ -4,7 +4,7 @@ "namespace": "testdata", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/base/Foobar.avsc b/testdata/base/Foobar.avsc index bfd8507..eb00c54 100644 --- a/testdata/base/Foobar.avsc +++ b/testdata/base/Foobar.avsc @@ -110,26 +110,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "testdata.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "testdata.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "testdata.Yowza" }, - "default": [] + "default": {} } ] } \ No newline at end of file diff --git a/testdata/base/Widget.avsc b/testdata/base/Widget.avsc index 5821529..e8892c1 100644 --- a/testdata/base/Widget.avsc +++ b/testdata/base/Widget.avsc @@ -130,26 +130,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "testdata.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "testdata.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "testdata.Yowza" }, - "default": [] + "default": {} } ] }, @@ -192,7 +176,7 @@ "namespace": "testdata", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/collapse_fields/AOneOf.avsc b/testdata/collapse_fields/AOneOf.avsc index 1f59e63..77270ee 100644 --- a/testdata/collapse_fields/AOneOf.avsc +++ b/testdata/collapse_fields/AOneOf.avsc @@ -4,7 +4,7 @@ "namespace": "testdata", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/collapse_fields/AYowzaMapEntry.avsc b/testdata/collapse_fields/AYowzaMapEntry.avsc deleted file mode 100644 index 8f576f3..0000000 --- a/testdata/collapse_fields/AYowzaMapEntry.avsc +++ /dev/null @@ -1,28 +0,0 @@ -{ - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "testdata.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": { - "type": "record", - "name": "Yowza", - "namespace": "testdata", - "fields": [ - { - "name": "hoo_boy", - "type": "float", - "default": 0 - } - ] - }, - "default": {} - } - ] -} \ No newline at end of file diff --git a/testdata/collapse_fields/Foobar.avsc b/testdata/collapse_fields/Foobar.avsc index bfd8507..eb00c54 100644 --- a/testdata/collapse_fields/Foobar.avsc +++ b/testdata/collapse_fields/Foobar.avsc @@ -110,26 +110,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "testdata.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "testdata.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "testdata.Yowza" }, - "default": [] + "default": {} } ] } \ No newline at end of file diff --git a/testdata/collapse_fields/Widget.avsc b/testdata/collapse_fields/Widget.avsc index 23c8b08..96b626f 100644 --- a/testdata/collapse_fields/Widget.avsc +++ b/testdata/collapse_fields/Widget.avsc @@ -130,26 +130,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "testdata.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "testdata.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "testdata.Yowza" }, - "default": [] + "default": {} } ] }, @@ -188,7 +172,7 @@ "namespace": "testdata", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/emit_only/Widget.avsc b/testdata/emit_only/Widget.avsc index 5821529..e8892c1 100644 --- a/testdata/emit_only/Widget.avsc +++ b/testdata/emit_only/Widget.avsc @@ -130,26 +130,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "testdata.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "testdata.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "testdata.Yowza" }, - "default": [] + "default": {} } ] }, @@ -192,7 +176,7 @@ "namespace": "testdata", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/namespace_map/AOneOf.avsc b/testdata/namespace_map/AOneOf.avsc index eec9b15..beed706 100644 --- a/testdata/namespace_map/AOneOf.avsc +++ b/testdata/namespace_map/AOneOf.avsc @@ -4,7 +4,7 @@ "namespace": "mynamespace", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/namespace_map/AYowzaMapEntry.avsc b/testdata/namespace_map/AYowzaMapEntry.avsc deleted file mode 100644 index fab228d..0000000 --- a/testdata/namespace_map/AYowzaMapEntry.avsc +++ /dev/null @@ -1,28 +0,0 @@ -{ - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "mynamespace.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": { - "type": "record", - "name": "Yowza", - "namespace": "mynamespace", - "fields": [ - { - "name": "hoo_boy", - "type": "float", - "default": 0 - } - ] - }, - "default": {} - } - ] -} \ No newline at end of file diff --git a/testdata/namespace_map/Foobar.avsc b/testdata/namespace_map/Foobar.avsc index 9ccd402..7a4f32b 100644 --- a/testdata/namespace_map/Foobar.avsc +++ b/testdata/namespace_map/Foobar.avsc @@ -110,26 +110,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "mynamespace.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "mynamespace.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "mynamespace.Yowza" }, - "default": [] + "default": {} } ] } \ No newline at end of file diff --git a/testdata/namespace_map/Widget.avsc b/testdata/namespace_map/Widget.avsc index 2d8b85f..b4829c7 100644 --- a/testdata/namespace_map/Widget.avsc +++ b/testdata/namespace_map/Widget.avsc @@ -130,26 +130,10 @@ { "name": "a_yowza_map", "type": { - "type": "array", - "items": { - "type": "record", - "name": "AYowzaMapEntry", - "namespace": "mynamespace.Foobar", - "fields": [ - { - "name": "key", - "type": "boolean", - "default": false - }, - { - "name": "value", - "type": "mynamespace.Yowza", - "default": "" - } - ] - } + "type": "map", + "values": "mynamespace.Yowza" }, - "default": [] + "default": {} } ] }, @@ -192,7 +176,7 @@ "namespace": "mynamespace", "fields": [ { - "name": "a_oneof", + "name": "oneof_types", "type": [ { "type": "record", diff --git a/testdata/preserve_non_string_maps/AOneOf.avsc b/testdata/preserve_non_string_maps/AOneOf.avsc new file mode 100644 index 0000000..77270ee --- /dev/null +++ b/testdata/preserve_non_string_maps/AOneOf.avsc @@ -0,0 +1,37 @@ +{ + "type": "record", + "name": "AOneOf", + "namespace": "testdata", + "fields": [ + { + "name": "oneof_types", + "type": [ + { + "type": "record", + "name": "TypeA", + "namespace": "testdata", + "fields": [ + { + "name": "foo", + "type": "string", + "default": "" + } + ] + }, + { + "type": "record", + "name": "TypeB", + "namespace": "testdata", + "fields": [ + { + "name": "bar", + "type": "string", + "default": "" + } + ] + } + ], + "default": null + } + ] +} \ No newline at end of file diff --git a/testdata/base/AYowzaMapEntry.avsc b/testdata/preserve_non_string_maps/AYowzaMapEntry.avsc similarity index 100% rename from testdata/base/AYowzaMapEntry.avsc rename to testdata/preserve_non_string_maps/AYowzaMapEntry.avsc diff --git a/testdata/preserve_non_string_maps/Foobar.avsc b/testdata/preserve_non_string_maps/Foobar.avsc new file mode 100644 index 0000000..bfd8507 --- /dev/null +++ b/testdata/preserve_non_string_maps/Foobar.avsc @@ -0,0 +1,135 @@ +{ + "type": "record", + "name": "Foobar", + "namespace": "testdata", + "fields": [ + { + "name": "name", + "type": "string", + "default": "" + }, + { + "name": "blarp", + "type": { + "type": "enum", + "name": "Blarp", + "symbols": [ + "BLARP_UNSPECIFIED", + "BLARP_ME", + "BLARP_YOU" + ], + "default": "BLARP_UNSPECIFIED" + }, + "default": "BLARP_UNSPECIFIED" + }, + { + "name": "yowza", + "type": { + "type": "record", + "name": "Yowza", + "namespace": "testdata", + "fields": [ + { + "name": "hoo_boy", + "type": "float", + "default": 0 + } + ] + }, + "default": {} + }, + { + "name": "blarps", + "type": { + "type": "array", + "items": "testdata.Blarp" + }, + "default": [] + }, + { + "name": "yowzas", + "type": { + "type": "array", + "items": "testdata.Yowza" + }, + "default": [] + }, + { + "name": "names", + "type": { + "type": "array", + "items": "string" + }, + "default": [] + }, + { + "name": "optional_name", + "type": [ + "null", + "string" + ], + "default": null + }, + { + "name": "optional_blarp", + "type": [ + "null", + "testdata.Blarp" + ], + "default": null + }, + { + "name": "optional_yowza", + "type": [ + "null", + "testdata.Yowza" + ], + "default": null + }, + { + "name": "a_num", + "type": "int", + "default": 0 + }, + { + "name": "a_string_map", + "type": { + "type": "map", + "values": "string" + }, + "default": {} + }, + { + "name": "a_blarp_map", + "type": { + "type": "map", + "values": "testdata.Blarp" + }, + "default": {} + }, + { + "name": "a_yowza_map", + "type": { + "type": "array", + "items": { + "type": "record", + "name": "AYowzaMapEntry", + "namespace": "testdata.Foobar", + "fields": [ + { + "name": "key", + "type": "boolean", + "default": false + }, + { + "name": "value", + "type": "testdata.Yowza", + "default": "" + } + ] + } + }, + "default": [] + } + ] +} \ No newline at end of file diff --git a/testdata/preserve_non_string_maps/StringList.avsc b/testdata/preserve_non_string_maps/StringList.avsc new file mode 100644 index 0000000..fec2273 --- /dev/null +++ b/testdata/preserve_non_string_maps/StringList.avsc @@ -0,0 +1,15 @@ +{ + "type": "record", + "name": "StringList", + "namespace": "testdata", + "fields": [ + { + "name": "data", + "type": { + "type": "array", + "items": "string" + }, + "default": [] + } + ] +} \ No newline at end of file diff --git a/testdata/preserve_non_string_maps/TypeA.avsc b/testdata/preserve_non_string_maps/TypeA.avsc new file mode 100644 index 0000000..a0de7f4 --- /dev/null +++ b/testdata/preserve_non_string_maps/TypeA.avsc @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "TypeA", + "namespace": "testdata", + "fields": [ + { + "name": "foo", + "type": "string", + "default": "" + } + ] +} \ No newline at end of file diff --git a/testdata/preserve_non_string_maps/TypeB.avsc b/testdata/preserve_non_string_maps/TypeB.avsc new file mode 100644 index 0000000..e3e6c51 --- /dev/null +++ b/testdata/preserve_non_string_maps/TypeB.avsc @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "TypeB", + "namespace": "testdata", + "fields": [ + { + "name": "bar", + "type": "string", + "default": "" + } + ] +} \ No newline at end of file diff --git a/testdata/preserve_non_string_maps/Widget.avsc b/testdata/preserve_non_string_maps/Widget.avsc new file mode 100644 index 0000000..55556a8 --- /dev/null +++ b/testdata/preserve_non_string_maps/Widget.avsc @@ -0,0 +1,230 @@ +{ + "type": "record", + "name": "Widget", + "namespace": "testdata", + "fields": [ + { + "name": "from_date", + "type": [ + "null", + "long" + ], + "default": null + }, + { + "name": "to_date", + "type": "long", + "default": 0 + }, + { + "name": "foobar", + "type": { + "type": "record", + "name": "Foobar", + "namespace": "testdata", + "fields": [ + { + "name": "name", + "type": "string", + "default": "" + }, + { + "name": "blarp", + "type": { + "type": "enum", + "name": "Blarp", + "symbols": [ + "BLARP_UNSPECIFIED", + "BLARP_ME", + "BLARP_YOU" + ], + "default": "BLARP_UNSPECIFIED" + }, + "default": "BLARP_UNSPECIFIED" + }, + { + "name": "yowza", + "type": { + "type": "record", + "name": "Yowza", + "namespace": "testdata", + "fields": [ + { + "name": "hoo_boy", + "type": "float", + "default": 0 + } + ] + }, + "default": {} + }, + { + "name": "blarps", + "type": { + "type": "array", + "items": "testdata.Blarp" + }, + "default": [] + }, + { + "name": "yowzas", + "type": { + "type": "array", + "items": "testdata.Yowza" + }, + "default": [] + }, + { + "name": "names", + "type": { + "type": "array", + "items": "string" + }, + "default": [] + }, + { + "name": "optional_name", + "type": [ + "null", + "string" + ], + "default": null + }, + { + "name": "optional_blarp", + "type": [ + "null", + "testdata.Blarp" + ], + "default": null + }, + { + "name": "optional_yowza", + "type": [ + "null", + "testdata.Yowza" + ], + "default": null + }, + { + "name": "a_num", + "type": "int", + "default": 0 + }, + { + "name": "a_string_map", + "type": { + "type": "map", + "values": "string" + }, + "default": {} + }, + { + "name": "a_blarp_map", + "type": { + "type": "map", + "values": "testdata.Blarp" + }, + "default": {} + }, + { + "name": "a_yowza_map", + "type": { + "type": "array", + "items": { + "type": "record", + "name": "AYowzaMapEntry", + "namespace": "testdata.Foobar", + "fields": [ + { + "name": "key", + "type": "boolean", + "default": false + }, + { + "name": "value", + "type": "testdata.Yowza", + "default": "" + } + ] + } + }, + "default": [] + } + ] + }, + "default": {} + }, + { + "name": "strings", + "type": { + "type": "record", + "name": "StringList", + "namespace": "testdata", + "fields": [ + { + "name": "data", + "type": { + "type": "array", + "items": "string" + }, + "default": [] + } + ] + }, + "default": {} + }, + { + "name": "string_map", + "type": { + "type": "map", + "values": "testdata.StringList" + }, + "default": {} + }, + { + "name": "a_one_of", + "type": [ + "null", + { + "type": "record", + "name": "AOneOf", + "namespace": "testdata", + "fields": [ + { + "name": "oneof_types", + "type": [ + { + "type": "record", + "name": "TypeA", + "namespace": "testdata", + "fields": [ + { + "name": "foo", + "type": "string", + "default": "" + } + ] + }, + { + "type": "record", + "name": "TypeB", + "namespace": "testdata", + "fields": [ + { + "name": "bar", + "type": "string", + "default": "" + } + ] + } + ], + "default": null + } + ] + } + ], + "default": null + } + ] +} \ No newline at end of file diff --git a/testdata/preserve_non_string_maps/Yowza.avsc b/testdata/preserve_non_string_maps/Yowza.avsc new file mode 100644 index 0000000..ab10b59 --- /dev/null +++ b/testdata/preserve_non_string_maps/Yowza.avsc @@ -0,0 +1,12 @@ +{ + "type": "record", + "name": "Yowza", + "namespace": "testdata", + "fields": [ + { + "name": "hoo_boy", + "type": "float", + "default": 0 + } + ] +} \ No newline at end of file diff --git a/testdata/widget.proto b/testdata/widget.proto index 98787df..0f9778f 100644 --- a/testdata/widget.proto +++ b/testdata/widget.proto @@ -18,7 +18,7 @@ message TypeB { } message AOneOf { - oneof a_oneof { + oneof oneof_types { TypeA a = 1; TypeB b = 2; }