Fix for wrappers with a zero value

ruby/ext/google/protobuf_c/encode_decode.c

@@ -44,6 +44,23 @@ VALUE noleak_rb_str_cat(VALUE rb_str, const char *str, long len) {
   return rb_str;
 }
 
+bool is_wrapper(const upb_msgdef* m) {
+  switch (upb_msgdef_wellknowntype(m)) {
+    case UPB_WELLKNOWN_DOUBLEVALUE:
+    case UPB_WELLKNOWN_FLOATVALUE:
+    case UPB_WELLKNOWN_INT64VALUE:
+    case UPB_WELLKNOWN_UINT64VALUE:
+    case UPB_WELLKNOWN_INT32VALUE:
+    case UPB_WELLKNOWN_UINT32VALUE:
+    case UPB_WELLKNOWN_STRINGVALUE:
+    case UPB_WELLKNOWN_BYTESVALUE:
+    case UPB_WELLKNOWN_BOOLVALUE:
+      return true;
+    default:
+      return false;
+  }
+}
+
 // The code below also comes from upb's prototype Ruby binding, developed by
 // haberman@.
 
@@ -117,19 +134,26 @@ static const void* newhandlerdata(upb_handlers* h, uint32_t ofs, int32_t hasbit)
 typedef struct {
   size_t ofs;
   int32_t hasbit;
+  upb_fieldtype_t wrapped_type;  // Only for wrappers.
   VALUE subklass;
 } submsg_handlerdata_t;
 
 // Creates a handlerdata that contains offset and submessage type information.
 static const void *newsubmsghandlerdata(upb_handlers* h,
+                                        const upb_fielddef *f,
                                         uint32_t ofs,
                                         int32_t hasbit,
                                         VALUE subklass) {
   submsg_handlerdata_t *hd = ALLOC(submsg_handlerdata_t);
+  const upb_msgdef *subm = upb_fielddef_msgsubdef(f);
   hd->ofs = ofs;
   hd->hasbit = hasbit;
   hd->subklass = subklass;
   upb_handlers_addcleanup(h, hd, xfree);
+  if (is_wrapper(subm)) {
+    const upb_fielddef *value_f = upb_msgdef_itof(subm, 1);
+    hd->wrapped_type = upb_fielddef_type(value_f);
+  }
   return hd;
 }
 
@@ -310,12 +334,39 @@ static void *submsg_handler(void *closure, const void *hd) {
 }
 
 static void* startwrapper(void* closure, const void* hd) {
-  char* msg = closure;
   const submsg_handlerdata_t* submsgdata = hd;
+  char* msg = closure;
+  VALUE* field = (VALUE*)(msg + submsgdata->ofs);
 
   set_hasbit(closure, submsgdata->hasbit);
 
-  return msg + submsgdata->ofs;
+  switch (submsgdata->wrapped_type) {
+    case UPB_TYPE_FLOAT:
+    case UPB_TYPE_DOUBLE:
+      *field = DBL2NUM(0);
+      break;
+    case UPB_TYPE_BOOL:
+      *field = Qfalse;
+      break;
+    case UPB_TYPE_STRING:
+      *field = get_frozen_string(NULL, 0, false);
+      break;
+    case UPB_TYPE_BYTES:
+      *field = get_frozen_string(NULL, 0, true);
+      break;
+    case UPB_TYPE_ENUM:
+    case UPB_TYPE_INT32:
+    case UPB_TYPE_INT64:
+    case UPB_TYPE_UINT32:
+    case UPB_TYPE_UINT64:
+      *field = INT2NUM(0);
+      break;
+    case UPB_TYPE_MESSAGE:
+      rb_raise(rb_eRuntimeError,
+               "Internal logic error with well-known types.");
+  }
+
+  return field;
 }
 
 // Handler data for startmap/endmap handlers.
@@ -522,23 +573,6 @@ static void* oneof_startwrapper(void* closure, const void* hd) {
   return msg + oneofdata->ofs;
 }
 
-bool is_wrapper(const upb_msgdef* m) {
-  switch (upb_msgdef_wellknowntype(m)) {
-    case UPB_WELLKNOWN_DOUBLEVALUE:
-    case UPB_WELLKNOWN_FLOATVALUE:
-    case UPB_WELLKNOWN_INT64VALUE:
-    case UPB_WELLKNOWN_UINT64VALUE:
-    case UPB_WELLKNOWN_INT32VALUE:
-    case UPB_WELLKNOWN_UINT32VALUE:
-    case UPB_WELLKNOWN_STRINGVALUE:
-    case UPB_WELLKNOWN_BYTESVALUE:
-    case UPB_WELLKNOWN_BOOLVALUE:
-      return true;
-    default:
-      return false;
-  }
-}
-
 // Set up handlers for a repeated field.
 static void add_handlers_for_repeated_field(upb_handlers *h,
                                             const Descriptor* desc,
@@ -579,7 +613,7 @@ static void add_handlers_for_repeated_field(upb_handlers *h,
     case UPB_TYPE_MESSAGE: {
       VALUE subklass = field_type_class(desc->layout, f);
       upb_handlerattr attr = UPB_HANDLERATTR_INIT;
-      attr.handler_data = newsubmsghandlerdata(h, 0, -1, subklass);
+      attr.handler_data = newsubmsghandlerdata(h, f, 0, -1, subklass);
       if (is_wrapper(upb_fielddef_msgsubdef(f))) {
         upb_handlers_setstartsubmsg(h, f, appendwrapper_handler, &attr);
       } else {
@@ -708,7 +742,7 @@ static void add_handlers_for_singular_field(const Descriptor* desc,
     case UPB_TYPE_MESSAGE: {
       upb_handlerattr attr = UPB_HANDLERATTR_INIT;
       attr.handler_data = newsubmsghandlerdata(
-          h, offset, hasbit, field_type_class(desc->layout, f));
+          h, f, offset, hasbit, field_type_class(desc->layout, f));
       if (is_wrapper(upb_fielddef_msgsubdef(f))) {
         upb_handlers_setstartsubmsg(h, f, startwrapper, &attr);
       } else {

ruby/tests/basic.rb

@@ -276,6 +276,86 @@ def test_map_wrappers
       assert_equal m5, m
     end
 
+    def test_map_wrappers_with_default_values
+      run_asserts = ->(m) {
+        assert_equal 0.0, m.map_double[0].value
+        assert_equal 0.0, m.map_float[0].value
+        assert_equal 0, m.map_int32[0].value
+        assert_equal 0, m.map_int64[0].value
+        assert_equal 0, m.map_uint32[0].value
+        assert_equal 0, m.map_uint64[0].value
+        assert_equal false, m.map_bool[0].value
+        assert_equal '', m.map_string[0].value
+        assert_equal '', m.map_bytes[0].value
+      }
+
+      m = proto_module::Wrapper.new(
+        map_double: {0 => Google::Protobuf::DoubleValue.new(value: 0.0)},
+        map_float: {0 => Google::Protobuf::FloatValue.new(value: 0.0)},
+        map_int32: {0 => Google::Protobuf::Int32Value.new(value: 0)},
+        map_int64: {0 => Google::Protobuf::Int64Value.new(value: 0)},
+        map_uint32: {0 => Google::Protobuf::UInt32Value.new(value: 0)},
+        map_uint64: {0 => Google::Protobuf::UInt64Value.new(value: 0)},
+        map_bool: {0 => Google::Protobuf::BoolValue.new(value: false)},
+        map_string: {0 => Google::Protobuf::StringValue.new(value: '')},
+        map_bytes: {0 => Google::Protobuf::BytesValue.new(value: '')},
+      )
+
+      run_asserts.call(m)
+      serialized = proto_module::Wrapper::encode(m)
+      m2 = proto_module::Wrapper::decode(serialized)
+      run_asserts.call(m2)
+
+      # Test the case where we are serializing directly from the parsed form
+      # (before anything lazy is materialized).
+      m3 = proto_module::Wrapper::decode(serialized)
+      serialized2 = proto_module::Wrapper::encode(m3)
+      m4 = proto_module::Wrapper::decode(serialized2)
+      run_asserts.call(m4)
+
+      # Test that the lazy form compares equal to the expanded form.
+      m5 = proto_module::Wrapper::decode(serialized2)
+      assert_equal m5, m
+    end
+
+    def test_map_wrappers_with_no_value
+      run_asserts = ->(m) {
+        assert_equal 0.0, m.map_double[0].value
+        assert_equal 0.0, m.map_float[0].value
+        assert_equal 0, m.map_int32[0].value
+        assert_equal 0, m.map_int64[0].value
+        assert_equal 0, m.map_uint32[0].value
+        assert_equal 0, m.map_uint64[0].value
+        assert_equal false, m.map_bool[0].value
+        assert_equal '', m.map_string[0].value
+        assert_equal '', m.map_bytes[0].value
+      }
+
+      m = proto_module::Wrapper.new(
+        map_double: {0 => Google::Protobuf::DoubleValue.new()},
+        map_float: {0 => Google::Protobuf::FloatValue.new()},
+        map_int32: {0 => Google::Protobuf::Int32Value.new()},
+        map_int64: {0 => Google::Protobuf::Int64Value.new()},
+        map_uint32: {0 => Google::Protobuf::UInt32Value.new()},
+        map_uint64: {0 => Google::Protobuf::UInt64Value.new()},
+        map_bool: {0 => Google::Protobuf::BoolValue.new()},
+        map_string: {0 => Google::Protobuf::StringValue.new()},
+        map_bytes: {0 => Google::Protobuf::BytesValue.new()},
+      )
+      run_asserts.call(m)
+
+      serialized = proto_module::Wrapper::encode(m)
+      m2 = proto_module::Wrapper::decode(serialized)
+      run_asserts.call(m2)
+
+      # Test the case where we are serializing directly from the parsed form
+      # (before anything lazy is materialized).
+      m3 = proto_module::Wrapper::decode(serialized)
+      serialized2 = proto_module::Wrapper::encode(m3)
+      m4 = proto_module::Wrapper::decode(serialized2)
+      run_asserts.call(m4)
+    end
+
     def test_concurrent_decoding
       o = Outer.new
       o.items[0] = Inner.new

ruby/tests/common_tests.rb

@@ -1265,6 +1265,36 @@ def test_comparison_with_arbitrary_object
     assert proto_module::TestMessage.new != nil
   end
 
+  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)
+  end
+
   def test_wrapper_getters
     run_asserts = ->(m) {
       assert_equal 2.0, m.double_as_value