Fix for JSON serialization of 0/empty-valued wrapper types (#7198)

* Fixed Ruby JSON serialization of 0/empty wrapper fields. * Removed newly-passing conformance tests from the failure list.
protocolbuffers · Feb 12, 2020 · 910220b · 910220b
1 parent 537c5aa
commit 910220b
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 20 deletions.
diff --git a/conformance/failure_list_ruby.txt b/conformance/failure_list_ruby.txt
@@ -94,25 +94,7 @@ Required.Proto3.JsonInput.IgnoreUnknownJsonObject.ProtobufOutput
 Required.Proto3.JsonInput.IgnoreUnknownJsonString.ProtobufOutput
 Required.Proto3.JsonInput.IgnoreUnknownJsonTrue.ProtobufOutput
 Required.Proto3.JsonInput.OneofFieldDuplicate
-Required.Proto3.JsonInput.OptionalBoolWrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalBytesWrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalDoubleWrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalFloatWrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalInt32Wrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalInt64Wrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalStringWrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalUint32Wrapper.JsonOutput
-Required.Proto3.JsonInput.OptionalUint64Wrapper.JsonOutput
 Required.Proto3.JsonInput.RejectTopLevelNull
-Required.Proto3.JsonInput.RepeatedBoolWrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedBytesWrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedDoubleWrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedFloatWrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedInt32Wrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedInt64Wrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedStringWrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedUint32Wrapper.JsonOutput
-Required.Proto3.JsonInput.RepeatedUint64Wrapper.JsonOutput
 Required.Proto3.JsonInput.StringFieldSurrogatePair.JsonOutput
 Required.Proto3.JsonInput.StringFieldSurrogatePair.ProtobufOutput
 Required.Proto3.ProtobufInput.DoubleFieldNormalizeQuietNan.JsonOutput

diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c
@@ -1432,6 +1432,7 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc,
   MessageHeader* msg;
   upb_msg_field_iter i;
   upb_status status;
+  bool json_wrapper = is_wrapper(desc->msgdef) && is_json;
 
   if (is_json &&
       upb_msgdef_wellknowntype(desc->msgdef) == UPB_WELLKNOWN_ANY) {
@@ -1508,7 +1509,7 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc,
         is_default = RSTRING_LEN(str) == 0;
       }
 
-      if (is_matching_oneof || emit_defaults || !is_default) {
+      if (is_matching_oneof || emit_defaults || !is_default || json_wrapper) {
         putstr(str, f, sink);
       }
     } else if (upb_fielddef_issubmsg(f)) {
@@ -1528,7 +1529,7 @@ static void putmsg(VALUE msg_rb, const Descriptor* desc,
     } else if (upb_msgdef_syntax(desc->msgdef) == UPB_SYNTAX_PROTO3) {       \
       is_default = default_value == value;                                   \
     }                                                                        \
-    if (is_matching_oneof || emit_defaults || !is_default) {                 \
+    if (is_matching_oneof || emit_defaults || !is_default || json_wrapper) { \
       upb_sink_put##upbtype(sink, sel, value);                               \
     }                                                                        \
   } break;

diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb
@@ -1265,6 +1265,40 @@ def test_comparison_with_arbitrary_object
     assert proto_module::TestMessage.new != nil
   end
 
+<<<<<<< HEAD
+=======
+  def test_wrappers_set_to_default
+    run_asserts = ->(m) {
+      assert_equal 0.0, m.double.value
+      assert_equal 0.0, m.float.value
+      assert_equal 0, m.int32.value
+      assert_equal 0, m.int64.value
+      assert_equal 0, m.uint32.value
+      assert_equal 0, m.uint64.value
+      assert_equal false, m.bool.value
+      assert_equal '', m.string.value
+      assert_equal '', m.bytes.value
+    }
+
+    m = proto_module::Wrapper.new(
+      double: Google::Protobuf::DoubleValue.new(value: 0.0),
+      float: Google::Protobuf::FloatValue.new(value: 0.0),
+      int32: Google::Protobuf::Int32Value.new(value: 0),
+      int64: Google::Protobuf::Int64Value.new(value: 0),
+      uint32: Google::Protobuf::UInt32Value.new(value: 0),
+      uint64: Google::Protobuf::UInt64Value.new(value: 0),
+      bool: Google::Protobuf::BoolValue.new(value: false),
+      string: Google::Protobuf::StringValue.new(value: ""),
+      bytes: Google::Protobuf::BytesValue.new(value: ''),
+    )
+
+    run_asserts.call(m)
+    m2 = proto_module::Wrapper.decode(m.to_proto)
+    run_asserts.call(m2)
+    m3 = proto_module::Wrapper.decode_json(m.to_json)
+  end
+
+>>>>>>> f2c5ee511... Fix for JSON serialization of 0/empty-valued wrapper types (#7198)
   def test_wrapper_getters
     run_asserts = ->(m) {
       assert_equal 2.0, m.double_as_value