Lazily Create Singular Wrapper Message (#6833)

* Register additional handlers from wrappers

* Return zval instead of parse frame

* Use parse frame

* Update upb

* Lazily create wrapper messages

* Fix a segment fault

Need check type of field before getting submsg def

* Avoid expanding during serialization and direct access

* Fix a bug that getXXXUnwrapped returns null for string

* Implement writeWrapperUnwrapped

* Add more tests

* Fix oneof wrapper parsing

* Fix get oneof field

* Avoid expansion for oneof wrappers

* Fix bug

* Fix a bug that in php7 variable is defined out of scope

* Fix broken tests
 * Update upb to fix Timestamp conformance tests
 * Fix segmentation fault for oneof wrapper fields

* Fix encoding/decoding top level wrapper values

* Add type checking for write wrapper value in php7

* Fix zts build

* Fix the bug that readWrapperValue uses parent message's layout to access wrapper value

* Fix wrapper in map

php/ext/google/protobuf/message.c

@@ -55,6 +55,8 @@ static  zend_function_entry message_methods[] = {
   PHP_ME(Message, serializeToJsonString, NULL, ZEND_ACC_PUBLIC)
   PHP_ME(Message, mergeFromJsonString, NULL, ZEND_ACC_PUBLIC)
   PHP_ME(Message, mergeFrom, NULL, ZEND_ACC_PUBLIC)
+  PHP_ME(Message, readWrapperValue, NULL, ZEND_ACC_PROTECTED)
+  PHP_ME(Message, writeWrapperValue, NULL, ZEND_ACC_PROTECTED)
   PHP_ME(Message, readOneof, NULL, ZEND_ACC_PROTECTED)
   PHP_ME(Message, writeOneof, NULL, ZEND_ACC_PROTECTED)
   PHP_ME(Message, whichOneof, NULL, ZEND_ACC_PROTECTED)
@@ -295,12 +297,6 @@ void build_class_from_descriptor(
 // PHP Methods
 // -----------------------------------------------------------------------------
 
-static bool is_wrapper_msg(const upb_msgdef* m) {
-  upb_wellknowntype_t type = upb_msgdef_wellknowntype(m);
-  return type >= UPB_WELLKNOWN_DOUBLEVALUE &&
-         type <= UPB_WELLKNOWN_BOOLVALUE;
-}
-
 static void append_wrapper_message(
     zend_class_entry* subklass, RepeatedField* intern, zval* value TSRMLS_DC) {
   MessageHeader* submsg;
@@ -561,6 +557,133 @@ PHP_METHOD(Message, mergeFrom) {
   layout_merge(from->descriptor->layout, from, to TSRMLS_CC);
 }
 
+PHP_METHOD(Message, readWrapperValue) {
+  char* member;
+  PHP_PROTO_SIZE length;
+  if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &member,
+                            &length) == FAILURE) {
+    return;
+  }
+
+  MessageHeader* msg = UNBOX(MessageHeader, getThis());
+  const upb_fielddef* field =
+      upb_msgdef_ntofz(msg->descriptor->msgdef, member);
+
+  if (upb_fielddef_containingoneof(field)) {
+    uint32_t* oneof_case =
+        slot_oneof_case(msg->descriptor->layout, message_data(msg), field);
+    if (*oneof_case != upb_fielddef_number(field)) {
+      RETURN_NULL();
+    }
+  }
+
+  zval* cached_zval =
+      CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, field));
+
+  if (Z_TYPE_P(cached_zval) == IS_NULL) {
+    RETURN_NULL();
+  }
+
+  if (Z_TYPE_P(cached_zval) == IS_OBJECT) {
+    const upb_msgdef* submsgdef = upb_fielddef_msgsubdef(field);
+    const upb_fielddef* value_field = upb_msgdef_itof(submsgdef, 1);
+    MessageHeader* submsg = UNBOX(MessageHeader, cached_zval);
+    CACHED_VALUE* cached_value = find_zval_property(submsg, value_field);
+    layout_get(submsg->descriptor->layout, submsg, value_field,
+               cached_value TSRMLS_CC);
+    RETURN_ZVAL(CACHED_PTR_TO_ZVAL_PTR(cached_value), 1, 0);
+  } else {
+    RETURN_ZVAL(cached_zval, 1, 0);
+  }
+}
+
+PHP_METHOD(Message, writeWrapperValue) {
+  char* member;
+  PHP_PROTO_SIZE length;
+  zval* value;
+  if (zend_parse_parameters(
+      ZEND_NUM_ARGS() TSRMLS_CC, "sz", &member, &length, &value) ==
+      FAILURE) {
+    return;
+  }
+
+  MessageHeader* msg = UNBOX(MessageHeader, getThis());
+  const upb_fielddef* field = upb_msgdef_ntofz(msg->descriptor->msgdef, member);
+
+  zval* cached_zval =
+      CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, field));
+
+  if (Z_TYPE_P(value) == IS_NULL) {
+    MessageHeader* msg = UNBOX(MessageHeader, getThis());
+    layout_set(msg->descriptor->layout, msg,
+               field, value TSRMLS_CC);
+    return;
+  }
+
+  {
+    // Type Checking
+    const upb_msgdef* submsgdef = upb_fielddef_msgsubdef(field);
+    const upb_fielddef* value_field = upb_msgdef_itof(submsgdef, 1);
+    upb_fieldtype_t type = upb_fielddef_type(value_field);
+    switch(type) {
+      case UPB_TYPE_STRING:
+      case UPB_TYPE_BYTES: {
+        if (!protobuf_convert_to_string(value)) {
+          return;
+        }
+        if (type == UPB_TYPE_STRING &&
+            !is_structurally_valid_utf8(Z_STRVAL_P(value), Z_STRLEN_P(value))) {
+          zend_error(E_USER_ERROR, "Given string is not UTF8 encoded.");
+          return;
+        }
+      }
+      break;
+#define CASE_TYPE(upb_type, type, c_type)                    \
+  case UPB_TYPE_##upb_type: {                                \
+    c_type type##_value;                                     \
+    if (!protobuf_convert_to_##type(value, &type##_value)) { \
+      return;                                                \
+    }                                                        \
+    break;                                                   \
+  }
+      CASE_TYPE(INT32,  int32,  int32_t)
+      CASE_TYPE(UINT32, uint32, uint32_t)
+      CASE_TYPE(ENUM,   int32,  int32_t)
+      CASE_TYPE(INT64,  int64,  int64_t)
+      CASE_TYPE(UINT64, uint64, uint64_t)
+      CASE_TYPE(FLOAT,  float,  float)
+      CASE_TYPE(DOUBLE, double, double)
+      CASE_TYPE(BOOL,   bool,   int8_t)
+
+#undef CASE_TYPE
+    }
+  }
+
+  if (upb_fielddef_containingoneof(field)) {
+    uint32_t* oneof_case =
+        slot_oneof_case(msg->descriptor->layout, message_data(msg), field);
+    if (*oneof_case != upb_fielddef_number(field)) {
+      zval null_value;
+      ZVAL_NULL(&null_value);
+      layout_set(msg->descriptor->layout, msg, field, &null_value TSRMLS_CC);
+      cached_zval = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, field));
+      ZVAL_ZVAL(cached_zval, value, 1, 0);
+      return;
+    }
+  }
+
+  if (Z_TYPE_P(cached_zval) == IS_OBJECT) {
+    const upb_msgdef* submsgdef = upb_fielddef_msgsubdef(field);
+    const upb_fielddef* value_field = upb_msgdef_itof(submsgdef, 1);
+    MessageHeader* submsg = UNBOX(MessageHeader, cached_zval);
+    CACHED_VALUE* cached_value = find_zval_property(submsg, value_field);
+    layout_set(submsg->descriptor->layout, submsg,
+               value_field, value TSRMLS_CC);
+  } else {
+    ZVAL_ZVAL(cached_zval, value, 1, 0);
+  }
+}
+
 PHP_METHOD(Message, readOneof) {
   PHP_PROTO_LONG index;
 

php/ext/google/protobuf/protobuf.h

@@ -965,6 +965,8 @@ void* slot_memory(MessageLayout* layout, const void* storage,
 
 PHP_METHOD(Message, clear);
 PHP_METHOD(Message, mergeFrom);
+PHP_METHOD(Message, readWrapperValue);
+PHP_METHOD(Message, writeWrapperValue);
 PHP_METHOD(Message, readOneof);
 PHP_METHOD(Message, writeOneof);
 PHP_METHOD(Message, whichOneof);
@@ -1525,4 +1527,7 @@ static inline zval* php_proto_message_read_property(
 bool is_reserved_name(const char* name);
 bool is_valid_constant_name(const char* name);
 
+// For lazy wrapper
+bool is_wrapper_msg(const upb_msgdef* m);
+
 #endif  // __GOOGLE_PROTOBUF_PHP_PROTOBUF_H__

php/ext/google/protobuf/storage.c

@@ -116,17 +116,17 @@ bool native_slot_set(upb_fieldtype_t type, const zend_class_entry* klass,
         return false;
       }
 
+#if PHP_MAJOR_VERSION < 7
+      REPLACE_ZVAL_VALUE((CACHED_VALUE*)memory, value, 1);
+#else
       zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR((CACHED_VALUE*)memory);
       if (EXPECTED(property_ptr != value)) {
         php_proto_zval_ptr_dtor(property_ptr);
       }
 
-#if PHP_MAJOR_VERSION < 7
-      DEREF(memory, zval*) = value;
-      Z_ADDREF_P(value);
-#else
       ZVAL_ZVAL(property_ptr, value, 1, 0);
 #endif
+
       break;
     }
 
@@ -797,25 +797,62 @@ zval* layout_get(MessageLayout* layout, MessageHeader* header,
   if (upb_fielddef_containingoneof(field)) {
     if (*oneof_case != upb_fielddef_number(field)) {
       native_slot_get_default(upb_fielddef_type(field), cache TSRMLS_CC);
-    } else {
-      upb_fieldtype_t type = upb_fielddef_type(field);
-      CACHED_VALUE* stored_cache = find_zval_property(header, field);
-      native_slot_get(
-          type, value_memory(type, memory, stored_cache), cache TSRMLS_CC);
+      return CACHED_PTR_TO_ZVAL_PTR(cache);
     }
-    return CACHED_PTR_TO_ZVAL_PTR(cache);
+    // Intentional fall through to be handled as a signuarl field.
   } else if (is_map_field(field)) {
     map_field_ensure_created(field, cache PHP_PROTO_TSRMLS_CC);
     return CACHED_PTR_TO_ZVAL_PTR(cache);
   } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) {
     repeated_field_ensure_created(field, cache PHP_PROTO_TSRMLS_CC);
     return CACHED_PTR_TO_ZVAL_PTR(cache);
+  }
+
+  CACHED_VALUE* stored_cache = find_zval_property(header, field);
+
+  if (upb_fielddef_type(field) == UPB_TYPE_MESSAGE &&
+      is_wrapper_msg(upb_fielddef_msgsubdef(field))) {
+    zval * cached_zval = CACHED_PTR_TO_ZVAL_PTR(stored_cache);
+#if PHP_MAJOR_VERSION >= 7
+    zend_object* obj;
+#endif
+    if (Z_TYPE_P(cached_zval) != IS_OBJECT &&
+        Z_TYPE_P(cached_zval) != IS_NULL) {
+      // Needs to expand value to wrapper.
+      const upb_msgdef* submsgdef = upb_fielddef_msgsubdef(field);
+      const upb_fielddef* value_field = upb_msgdef_itof(submsgdef, 1);
+      MessageHeader* submsg;
+      Descriptor* subdesc =
+          UNBOX_HASHTABLE_VALUE(
+              Descriptor, get_def_obj((void*)submsgdef));
+      zend_class_entry* subklass = subdesc->klass;
+#if PHP_MAJOR_VERSION < 7
+      zval* val = NULL;
+      MAKE_STD_ZVAL(val);
+      ZVAL_OBJ(val, subklass->create_object(subklass TSRMLS_CC));
+      submsg = UNBOX(MessageHeader, val);
+#else
+      obj = subklass->create_object(subklass TSRMLS_CC);
+      submsg = (MessageHeader*)((char*)obj - XtOffsetOf(MessageHeader, std));
+#endif
+      custom_data_init(subklass, submsg PHP_PROTO_TSRMLS_CC);
+
+      layout_set(subdesc->layout, submsg, value_field, cached_zval TSRMLS_CC);
+#if PHP_MAJOR_VERSION < 7
+      ZVAL_ZVAL(cached_zval, val, 1, 1);
+#else
+      ZVAL_OBJ(cached_zval, obj);
+#endif
+    }
+    if (stored_cache != cache) {
+      ZVAL_ZVAL(CACHED_PTR_TO_ZVAL_PTR(cache), cached_zval, 1, 0);
+    }
   } else {
     upb_fieldtype_t type = upb_fielddef_type(field);
-    native_slot_get(type, value_memory(type, memory, cache),
+    native_slot_get(type, value_memory(type, memory, stored_cache),
                     cache TSRMLS_CC);
-    return CACHED_PTR_TO_ZVAL_PTR(cache);
   }
+  return CACHED_PTR_TO_ZVAL_PTR(cache);
 }
 
 void layout_set(MessageLayout* layout, MessageHeader* header,
@@ -825,33 +862,6 @@ void layout_set(MessageLayout* layout, MessageHeader* header,
   uint32_t* oneof_case = slot_oneof_case(layout, storage, field);
 
   if (upb_fielddef_containingoneof(field)) {
-    upb_fieldtype_t type = upb_fielddef_type(field);
-    zend_class_entry *ce = NULL;
-
-    // For non-singular fields, the related memory needs to point to the actual
-    // zval in properties table first.
-    switch (type) {
-      case UPB_TYPE_MESSAGE: {
-        const upb_msgdef* msg = upb_fielddef_msgsubdef(field);
-        Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg));
-        ce = desc->klass;
-        // Intentionally fall through.
-      }
-      case UPB_TYPE_STRING:
-      case UPB_TYPE_BYTES: {
-        int property_cache_index =
-            header->descriptor->layout->fields[upb_fielddef_index(field)]
-                .cache_index;
-        DEREF(memory, CACHED_VALUE*) =
-            OBJ_PROP(&header->std, property_cache_index);
-        memory = DEREF(memory, CACHED_VALUE*);
-        break;
-      }
-      default:
-        break;
-    }
-
-    native_slot_set(type, ce, memory, val TSRMLS_CC);
     *oneof_case = upb_fielddef_number(field);
   } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) {
     // Works for both repeated and map fields
@@ -895,19 +905,20 @@ void layout_set(MessageLayout* layout, MessageHeader* header,
 #endif
       zval_dtor(&converted_value);
     }
-  } else {
-    upb_fieldtype_t type = upb_fielddef_type(field);
-    zend_class_entry *ce = NULL;
-    if (type == UPB_TYPE_MESSAGE) {
-      const upb_msgdef* msg = upb_fielddef_msgsubdef(field);
-      Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg));
-      ce = desc->klass;
-    }
-    CACHED_VALUE* cache = find_zval_property(header, field);
-    native_slot_set(
-        type, ce, value_memory(upb_fielddef_type(field), memory, cache),
-        val TSRMLS_CC);
+    return;
+  }
+
+  upb_fieldtype_t type = upb_fielddef_type(field);
+  zend_class_entry *ce = NULL;
+  if (type == UPB_TYPE_MESSAGE) {
+    const upb_msgdef* msg = upb_fielddef_msgsubdef(field);
+    Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg));
+    ce = desc->klass;
   }
+  CACHED_VALUE* cache = find_zval_property(header, field);
+  native_slot_set(
+      type, ce, value_memory(upb_fielddef_type(field), memory, cache),
+      val TSRMLS_CC);
 }
 
 static void native_slot_merge(

php/ext/google/protobuf/upb.h

@@ -122,7 +122,7 @@ int msvc_vsnprintf(char* s, size_t n, const char* format, va_list arg);
 #ifdef __cplusplus
 #if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) || \
     (defined(_MSC_VER) && _MSC_VER >= 1900)
-// C++11 is present
+/* C++11 is present */
 #else
 #error upb requires C++11 for C++ support
 #endif
@@ -5700,15 +5700,16 @@ UPB_INLINE bool upb_sink_startsubmsg(upb_sink s, upb_selector_t sel,
   return sub->closure ? true : false;
 }
 
-UPB_INLINE bool upb_sink_endsubmsg(upb_sink s, upb_selector_t sel) {
+UPB_INLINE bool upb_sink_endsubmsg(upb_sink s, upb_sink sub,
+                                   upb_selector_t sel) {
   typedef upb_endfield_handlerfunc func;
   func *endsubmsg;
   const void *hd;
   if (!s.handlers) return true;
   endsubmsg = (func*)upb_handlers_gethandler(s.handlers, sel, &hd);
 
-  if (!endsubmsg) return s.closure;
-  return endsubmsg(s.closure, hd);
+  if (!endsubmsg) return true;
+  return endsubmsg(sub.closure, hd);
 }
 
 #ifdef __cplusplus
@@ -5874,8 +5875,8 @@ class upb::Sink {
     return ret;
   }
 
-  bool EndSubMessage(HandlersPtr::Selector s) {
-    return upb_sink_endsubmsg(sink_, s);
+  bool EndSubMessage(HandlersPtr::Selector s, Sink sub) {
+    return upb_sink_endsubmsg(sink_, sub.sink_, s);
   }
 
   /* For repeated fields of any type, the sequence of values must be wrapped in

php/src/Google/Protobuf/Internal/Message.php

@@ -175,6 +175,42 @@ class Message
         }
     }
 
+    protected function readWrapperValue($member)
+    {
+        $field = $this->desc->getFieldByName($member);
+        $oneof_index = $field->getOneofIndex();
+        if ($oneof_index === -1) {
+            $wrapper = $this->$member;
+        } else {
+            $wrapper = $this->readOneof($field->getNumber());
+        }
+
+        if (is_null($wrapper)) {
+            return NULL;
+        } else {
+            return $wrapper->getValue();
+        }
+    }
+
+    protected function writeWrapperValue($member, $value)
+    {
+        $field = $this->desc->getFieldByName($member);
+        $wrapped_value = $value;
+        if (!is_null($value)) {
+            $desc = $field->getMessageType();
+            $klass = $desc->getClass();
+            $wrapped_value = new $klass;
+            $wrapped_value->setValue($value);
+        }
+
+        $oneof_index = $field->getOneofIndex();
+        if ($oneof_index === -1) {
+            $this->$member = $wrapped_value;
+        } else {
+            $this->writeOneof($field->getNumber(), $wrapped_value);
+        }
+    }
+
     protected function readOneof($number)
     {
         $field = $this->desc->getFieldByNumber($number);

php/tests/encode_decode_test.php

@@ -5,7 +5,13 @@ require_once('test_util.php');
 
 use Google\Protobuf\RepeatedField;
 use Google\Protobuf\GPBType;
+use Foo\TestInt32Value;
+use Foo\TestInt64Value;
+use Foo\TestUInt32Value;
+use Foo\TestUInt64Value;
+use Foo\TestBoolValue;
 use Foo\TestStringValue;
+use Foo\TestBytesValue;
 use Foo\TestAny;
 use Foo\TestEnum;
 use Foo\TestMessage;
@@ -98,6 +104,13 @@ class EncodeDecodeTest extends TestBase
         $this->assertSame("1", $m->serializeToJsonString());
     }
 
+    public function testDecodeRepeatedInt32Value()
+    {
+        $m = new TestInt32Value();
+        $m->mergeFromJsonString("{\"repeated_field\":[12345]}");
+        $this->assertSame(12345, $m->getRepeatedField()[0]->getValue());
+    }
+
     public function testDecodeTopLevelUInt32Value()
     {
         $m = new UInt32Value();
@@ -168,12 +181,18 @@ class EncodeDecodeTest extends TestBase
         $this->assertSame("\"a\"", $m->serializeToJsonString());
     }
 
-    public function testEncodeStringValue()
+    public function testDecodeRepeatedStringValue()
+    {
+        $m = new TestStringValue();
+        $m->mergeFromJsonString("{\"repeated_field\":[\"a\"]}");
+        $this->assertSame("a", $m->getRepeatedField()[0]->getValue());
+    }
+
+    public function testDecodeMapStringValue()
     {
-        $m = new TestStringValue(['field' => new StringValue(['value' => ''])]);
-        var_dump($m->getField());
-        var_dump($m->serializeToJsonString());
-        $this->assertSame("{\"field\":\"\"}", $m->serializeToJsonString());
+        $m = new TestStringValue();
+        $m->mergeFromJsonString("{\"map_field\":{\"1\": \"a\"}}");
+        $this->assertSame("a", $m->getMapField()[1]->getValue());
     }
 
     public function testDecodeTopLevelBytesValue()
@@ -1230,4 +1249,195 @@ class EncodeDecodeTest extends TestBase
         $this->assertTrue(true);
     }
 
+    /**
+     * @dataProvider wrappersDataProvider
+     */
+    public function testWrapperJsonDecodeAndGet(
+        $class,
+        $nonDefaultValue,
+        $nonDefaultValueData,
+        $defaultValue,
+        $defaultValueData
+    )
+    {
+        // Singular with non-default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"field\":" . $nonDefaultValueData . "}");
+        $wrapper = $m->getField();
+        $this->assertEquals($nonDefaultValue, $wrapper->getValue());
+
+        // Singular with default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"field\":" . $defaultValueData . "}");
+        $wrapper = $m->getField();
+        $this->assertEquals($defaultValue, $wrapper->getValue());
+
+        // Repeated with empty
+        $m = new $class();
+        $m->mergeFromJsonString("{\"repeated_field\":[]}");
+        $repeatedWrapper = $m->getRepeatedField();
+        $this->assertSame(0, count($repeatedWrapper));
+
+        // Repeated with non-default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"repeated_field\":[" . $defaultValueData . "]}");
+        $repeatedWrapper = $m->getRepeatedField();
+        $this->assertSame(1, count($repeatedWrapper));
+        $this->assertEquals($defaultValue, $repeatedWrapper[0]->getValue());
+
+        // Repeated with default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"repeated_field\":[" . $defaultValueData . "]}");
+        $repeatedWrapper = $m->getRepeatedField();
+        $this->assertSame(1, count($repeatedWrapper));
+        $this->assertEquals($defaultValue, $repeatedWrapper[0]->getValue());
+
+        // Oneof with non-default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"oneof_field\":" . $nonDefaultValueData . "}");
+        $wrapper = $m->getOneofField();
+        $this->assertEquals($nonDefaultValue, $wrapper->getValue());
+        $this->assertEquals("oneof_field", $m->getOneofFields());
+        $this->assertEquals(0, $m->getInt32Field());
+
+        // Oneof with default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"oneof_field\":" . $defaultValueData . "}");
+        $wrapper = $m->getOneofField();
+        $this->assertEquals($defaultValue, $wrapper->getValue());
+        $this->assertEquals("oneof_field", $m->getOneofFields());
+        $this->assertEquals(0, $m->getInt32Field());
+    }
+
+    /**
+     * @dataProvider wrappersDataProvider
+     */
+    public function testWrapperJsonDecodeAndGetUnwrapped(
+        $class,
+        $nonDefaultValue,
+        $nonDefaultValueData,
+        $defaultValue,
+        $defaultValueData
+    )
+    {
+        // Singular with non-default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"field\":" . $nonDefaultValueData . "}");
+        $this->assertEquals($nonDefaultValue, $m->getFieldUnwrapped());
+
+        // Singular with default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"field\":" . $defaultValueData . "}");
+        $this->assertEquals($defaultValue, $m->getFieldUnwrapped());
+
+        // Oneof with non-default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"oneof_field\":" . $nonDefaultValueData . "}");
+        $this->assertEquals($nonDefaultValue, $m->getOneofFieldUnwrapped());
+        $this->assertEquals("oneof_field", $m->getOneofFields());
+        $this->assertEquals(0, $m->getInt32Field());
+
+        // Oneof with default
+        $m = new $class();
+        $m->mergeFromJsonString("{\"oneof_field\":" . $defaultValueData . "}");
+        $this->assertEquals($defaultValue, $m->getOneofFieldUnwrapped());
+        $this->assertEquals("oneof_field", $m->getOneofFields());
+        $this->assertEquals(0, $m->getInt32Field());
+    }
+
+    /**
+     * @dataProvider wrappersDataProvider
+     */
+    public function testWrapperJsonDecodeEncode(
+        $class,
+        $nonDefaultValue,
+        $nonDefaultValueData,
+        $defaultValue,
+        $defaultValueData
+    )
+    {
+        // Singular with non-default
+        $from = new $class();
+        $to = new $class();
+        $from->mergeFromJsonString("{\"field\":" . $nonDefaultValueData . "}");
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($nonDefaultValue, $to->getFieldUnwrapped());
+
+        // Singular with default
+        $from = new $class();
+        $to = new $class();
+        $from->mergeFromJsonString("{\"field\":" . $defaultValueData . "}");
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($defaultValue, $to->getFieldUnwrapped());
+
+        // Oneof with non-default
+        $from = new $class();
+        $to = new $class();
+        $from->mergeFromJsonString("{\"oneof_field\":" . $nonDefaultValueData . "}");
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($nonDefaultValue, $to->getOneofFieldUnwrapped());
+
+        // Oneof with default
+        $from = new $class();
+        $to = new $class();
+        $from->mergeFromJsonString("{\"oneof_field\":" . $defaultValueData . "}");
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($defaultValue, $to->getOneofFieldUnwrapped());
+    }
+
+    /**
+     * @dataProvider wrappersDataProvider
+     */
+    public function testWrapperSetUnwrappedJsonEncode(
+        $class,
+        $nonDefaultValue,
+        $nonDefaultValueData,
+        $defaultValue,
+        $defaultValueData
+    )
+    {
+        // Singular with non-default
+        $from = new $class();
+        $to = new $class();
+        $from->setFieldUnwrapped($nonDefaultValue);
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($nonDefaultValue, $to->getFieldUnwrapped());
+
+        // Singular with default
+        $from = new $class();
+        $to = new $class();
+        $from->setFieldUnwrapped($defaultValue);
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($defaultValue, $to->getFieldUnwrapped());
+
+        // Oneof with non-default
+        $from = new $class();
+        $to = new $class();
+        $from->setOneofFieldUnwrapped($nonDefaultValue);
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($nonDefaultValue, $to->getOneofFieldUnwrapped());
+
+        // Oneof with default
+        $from = new $class();
+        $to = new $class();
+        $from->setOneofFieldUnwrapped($defaultValue);
+        $data = $from->serializeToJsonString();
+        $to->mergeFromJsonString($data);
+        $this->assertEquals($defaultValue, $to->getOneofFieldUnwrapped());
+    }
+
+    public function wrappersDataProvider()
+    {
+        return [
+            [TestInt32Value::class, 1, "1", 0, "0"],
+            [TestStringValue::class, "a", "\"a\"", "", "\"\""],
+        ];
+    }
 }

php/tests/memory_leak_test.php

@@ -20,9 +20,13 @@ require_once('generated/Bar/TestLegacyMessage/NestedMessage.php');
 require_once('generated/Foo/PBARRAY.php');
 require_once('generated/Foo/PBEmpty.php');
 require_once('generated/Foo/TestAny.php');
+require_once('generated/Foo/TestBoolValue.php');
+require_once('generated/Foo/TestBytesValue.php');
 require_once('generated/Foo/TestEnum.php');
 require_once('generated/Foo/TestIncludeNamespaceMessage.php');
 require_once('generated/Foo/TestIncludePrefixMessage.php');
+require_once('generated/Foo/TestInt32Value.php');
+require_once('generated/Foo/TestInt64Value.php');
 require_once('generated/Foo/TestMessage.php');
 require_once('generated/Foo/TestMessage/PBEmpty.php');
 require_once('generated/Foo/TestMessage/NestedEnum.php');
@@ -32,6 +36,8 @@ require_once('generated/Foo/TestPhpDoc.php');
 require_once('generated/Foo/TestRandomFieldOrder.php');
 require_once('generated/Foo/TestReverseFieldOrder.php');
 require_once('generated/Foo/TestStringValue.php');
+require_once('generated/Foo/TestUInt32Value.php');
+require_once('generated/Foo/TestUInt64Value.php');
 require_once('generated/Foo/TestUnpackedMessage.php');
 require_once('generated/Foo/testLowerCaseMessage.php');
 require_once('generated/Foo/testLowerCaseEnum.php');

php/tests/proto/test.proto

@@ -215,6 +215,66 @@ message TestAny {
   google.protobuf.Any any = 1;
 }
 
+message TestInt32Value {
+  google.protobuf.Int32Value field = 1;
+  repeated google.protobuf.Int32Value repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.Int32Value oneof_field = 3;
+    int32 int32_field = 4;
+  }
+}
+
+message TestInt64Value {
+  google.protobuf.Int64Value field = 1;
+  repeated google.protobuf.Int64Value repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.Int64Value oneof_field = 3;
+    int32 int32_field = 4;
+  }
+}
+
+message TestUInt32Value {
+  google.protobuf.UInt32Value field = 1;
+  repeated google.protobuf.UInt32Value repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.UInt32Value oneof_field = 3;
+    int32 int32_field = 4;
+  }
+}
+
+message TestUInt64Value {
+  google.protobuf.UInt64Value field = 1;
+  repeated google.protobuf.UInt64Value repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.UInt64Value oneof_field = 3;
+    int32 int32_field = 4;
+  }
+}
+
+message TestBoolValue {
+  google.protobuf.BoolValue field = 1;
+  repeated google.protobuf.BoolValue repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.BoolValue oneof_field = 3;
+    int32 int32_field = 4;
+  }
+}
+
 message TestStringValue {
   google.protobuf.StringValue field = 1;
+  repeated google.protobuf.StringValue repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.StringValue oneof_field = 3;
+    int32 int32_field = 4;
+  }
+  map<int32, google.protobuf.StringValue> map_field = 5;
+}
+
+message TestBytesValue {
+  google.protobuf.BytesValue field = 1;
+  repeated google.protobuf.BytesValue repeated_field = 2;
+  oneof oneof_fields {
+    google.protobuf.BytesValue oneof_field = 3;
+    int32 int32_field = 4;
+  }
 }

php/tests/wrapper_type_setters_test.php

@@ -132,7 +132,8 @@ class WrapperTypeSettersTest extends TestBase
                 [2.2, new DoubleValue(["value" => 2.2])],
                 [null, null],
                 [0, new DoubleValue()],
-            ]],[TestWrapperSetters::class, StringValue::class, "setStringValueOneof", "setStringValueOneofUnwrapped", "getStringValueOneof", "getStringValueOneofUnwrapped", [
+            ]],
+            [TestWrapperSetters::class, StringValue::class, "setStringValueOneof", "setStringValueOneofUnwrapped", "getStringValueOneof", "getStringValueOneofUnwrapped", [
                 ["asdf", new StringValue(["value" => "asdf"])],
                 ["", new StringValue(["value" => ""])],
                 [null, null],

src/google/protobuf/compiler/php/php_generator.cc

@@ -628,7 +628,7 @@ void GenerateField(const FieldDescriptor* field, io::Printer* printer,
   } else {
     GenerateFieldDocComment(printer, field, is_descriptor, kFieldProperty);
     printer->Print(
-        "private $^name^ = ^default^;\n",
+        "protected $^name^ = ^default^;\n",
         "name", field->name(),
         "default", DefaultForField(field));
   }
@@ -682,10 +682,10 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
     printer->Print(
         "public function get^camel_name^Unwrapped()\n"
         "{\n"
-        "    $wrapper = $this->get^camel_name^();\n"
-        "    return is_null($wrapper) ? null : $wrapper->getValue();\n"
+        "    return $this->readWrapperValue(\"^field_name^\");\n"
         "}\n\n",
-        "camel_name", UnderscoresToCamelCase(field->name(), true));
+        "camel_name", UnderscoresToCamelCase(field->name(), true),
+        "field_name", field->name());
   }
 
   // Generate setter.
@@ -795,11 +795,11 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
     printer->Print(
         "public function set^camel_name^Unwrapped($var)\n"
         "{\n"
-        "    $wrappedVar = is_null($var) ? null : new \\^wrapper_type^(['value' => $var]);\n"
-        "    return $this->set^camel_name^($wrappedVar);\n"
+        "    $this->writeWrapperValue(\"^field_name^\", $var);\n"
+        "    return $this;"
         "}\n\n",
         "camel_name", UnderscoresToCamelCase(field->name(), true),
-        "wrapper_type", LegacyFullClassName(field->message_type(), is_descriptor));
+        "field_name", field->name());
   }
 
   // Generate has method for proto2 only.