Skip to content

Commit

Permalink
Fix TypeError on decoding enum map values in Ruby (#6262)
Browse files Browse the repository at this point in the history
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 <her@sorah.jp>
  • Loading branch information
sorah authored and TeBoring committed Jun 16, 2019
1 parent e5f246d commit 997bd35
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
3 changes: 3 additions & 0 deletions ruby/ext/google/protobuf_c/encode_decode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
32 changes: 19 additions & 13 deletions ruby/tests/basic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,12 @@ def test_map_field
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
Expand Down Expand Up @@ -206,8 +208,9 @@ def test_map_inspect
m = MapMessage.new(
:map_string_int32 => {"a" => 1, "b" => 2},
:map_string_msg => {"a" => TestMessage2.new(:foo => 1),
"b" => TestMessage2.new(:foo => 2)})
expected = "<BasicTest::MapMessage: map_string_int32: {\"b\"=>2, \"a\"=>1}, map_string_msg: {\"b\"=><BasicTest::TestMessage2: foo: 2>, \"a\"=><BasicTest::TestMessage2: foo: 1>}>"
"b" => TestMessage2.new(:foo => 2)},
:map_string_enum => {"a" => :A, "b" => :B})
expected = "<BasicTest::MapMessage: map_string_int32: {\"b\"=>2, \"a\"=>1}, map_string_msg: {\"b\"=><BasicTest::TestMessage2: foo: 2>, \"a\"=><BasicTest::TestMessage2: foo: 1>}, map_string_enum: {\"b\"=>:B, \"a\"=>:A}>"
assert_equal expected, m.inspect
end

Expand Down Expand Up @@ -237,7 +240,8 @@ def test_map_encode_decode
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

Expand Down Expand Up @@ -298,10 +302,12 @@ def test_to_h
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
Expand All @@ -311,26 +317,26 @@ def test_json_maps
# 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
Expand Down
1 change: 1 addition & 0 deletions ruby/tests/basic_test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ message Recursive2 {
message MapMessage {
map<string, int32> map_string_int32 = 1;
map<string, TestMessage2> map_string_msg = 2;
map<string, TestEnum> map_string_enum = 3;
}

message MapMessageWireEquiv {
Expand Down

0 comments on commit 997bd35

Please sign in to comment.