From 997bd354d55de6a8462ac3db0b002aa73b240cff Mon Sep 17 00:00:00 2001 From: Sorah Fukumori Date: Mon, 17 Jun 2019 07:20:18 +0900 Subject: [PATCH] Fix TypeError on decoding enum map values in Ruby (#6262) value_field_typeclass should be a enum module, not EnumDescriptor object. Also expanding tests for enum valued maps. Fixes #4580 Signed-off-by: Sorah Fukumori --- ruby/ext/google/protobuf_c/encode_decode.c | 3 ++ ruby/tests/basic.rb | 32 +++++++++++++--------- ruby/tests/basic_test.proto | 1 + 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c index 4b79368552..40952c6b1b 100644 --- a/ruby/ext/google/protobuf_c/encode_decode.c +++ b/ruby/ext/google/protobuf_c/encode_decode.c @@ -389,6 +389,9 @@ static bool endmap_handler(void *closure, const void *hd, upb_status* s) { if (mapdata->value_field_type == UPB_TYPE_MESSAGE || mapdata->value_field_type == UPB_TYPE_ENUM) { value_field_typeclass = get_def_obj(mapdata->value_field_subdef); + if (mapdata->value_field_type == UPB_TYPE_ENUM) { + value_field_typeclass = EnumDescriptor_enummodule(value_field_typeclass); + } } value = native_slot_get( diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 591a1c3a6c..9ec738ba61 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -170,10 +170,12 @@ module BasicTest m = MapMessage.new( :map_string_int32 => {"a" => 1, "b" => 2}, :map_string_msg => {"a" => TestMessage2.new(:foo => 1), - "b" => TestMessage2.new(:foo => 2)}) + "b" => TestMessage2.new(:foo => 2)}, + :map_string_enum => {"a" => :A, "b" => :B}) assert m.map_string_int32.keys.sort == ["a", "b"] assert m.map_string_int32["a"] == 1 assert m.map_string_msg["b"].foo == 2 + assert m.map_string_enum["a"] == :A m.map_string_int32["c"] = 3 assert m.map_string_int32["c"] == 3 @@ -206,8 +208,9 @@ module BasicTest m = MapMessage.new( :map_string_int32 => {"a" => 1, "b" => 2}, :map_string_msg => {"a" => TestMessage2.new(:foo => 1), - "b" => TestMessage2.new(:foo => 2)}) - expected = "2, \"a\"=>1}, map_string_msg: {\"b\"=>, \"a\"=>}>" + "b" => TestMessage2.new(:foo => 2)}, + :map_string_enum => {"a" => :A, "b" => :B}) + expected = "2, \"a\"=>1}, map_string_msg: {\"b\"=>, \"a\"=>}, map_string_enum: {\"b\"=>:B, \"a\"=>:A}>" assert_equal expected, m.inspect end @@ -237,7 +240,8 @@ module BasicTest m = MapMessage.new( :map_string_int32 => {"a" => 1, "b" => 2}, :map_string_msg => {"a" => TestMessage2.new(:foo => 1), - "b" => TestMessage2.new(:foo => 2)}) + "b" => TestMessage2.new(:foo => 2)}, + :map_string_enum => {"a" => :A, "b" => :B}) m2 = MapMessage.decode(MapMessage.encode(m)) assert m == m2 @@ -298,10 +302,12 @@ module BasicTest m = MapMessage.new( :map_string_int32 => {"a" => 1, "b" => 2}, :map_string_msg => {"a" => TestMessage2.new(:foo => 1), - "b" => TestMessage2.new(:foo => 2)}) + "b" => TestMessage2.new(:foo => 2)}, + :map_string_enum => {"a" => :A, "b" => :B}) expected_result = { :map_string_int32 => {"a" => 1, "b" => 2}, - :map_string_msg => {"a" => {:foo => 1}, "b" => {:foo => 2}} + :map_string_msg => {"a" => {:foo => 1}, "b" => {:foo => 2}}, + :map_string_enum => {"a" => :A, "b" => :B} } assert_equal expected_result, m.to_h end @@ -311,26 +317,26 @@ module BasicTest # TODO: Fix JSON in JRuby version. return if RUBY_PLATFORM == "java" m = MapMessage.new(:map_string_int32 => {"a" => 1}) - expected = {mapStringInt32: {a: 1}, mapStringMsg: {}} - expected_preserve = {map_string_int32: {a: 1}, map_string_msg: {}} - assert JSON.parse(MapMessage.encode_json(m), :symbolize_names => true) == expected + expected = {mapStringInt32: {a: 1}, mapStringMsg: {}, mapStringEnum: {}} + expected_preserve = {map_string_int32: {a: 1}, map_string_msg: {}, map_string_enum: {}} + assert_equal JSON.parse(MapMessage.encode_json(m), :symbolize_names => true), expected json = MapMessage.encode_json(m, :preserve_proto_fieldnames => true) - assert JSON.parse(json, :symbolize_names => true) == expected_preserve + assert_equal JSON.parse(json, :symbolize_names => true), expected_preserve m2 = MapMessage.decode_json(MapMessage.encode_json(m)) - assert m == m2 + assert_equal m, m2 end def test_json_maps_emit_defaults_submsg # TODO: Fix JSON in JRuby version. return if RUBY_PLATFORM == "java" m = MapMessage.new(:map_string_msg => {"a" => TestMessage2.new}) - expected = {mapStringInt32: {}, mapStringMsg: {a: {foo: 0}}} + expected = {mapStringInt32: {}, mapStringMsg: {a: {foo: 0}}, mapStringEnum: {}} actual = MapMessage.encode_json(m, :emit_defaults => true) - assert JSON.parse(actual, :symbolize_names => true) == expected + assert_equal JSON.parse(actual, :symbolize_names => true), expected end def test_respond_to diff --git a/ruby/tests/basic_test.proto b/ruby/tests/basic_test.proto index 3a5a53284c..e5811dc881 100644 --- a/ruby/tests/basic_test.proto +++ b/ruby/tests/basic_test.proto @@ -80,6 +80,7 @@ message Recursive2 { message MapMessage { map map_string_int32 = 1; map map_string_msg = 2; + map map_string_enum = 3; } message MapMessageWireEquiv {