Oneof accessor should return the field name that is actually set. (#2631)

php/ext/google/protobuf/message.c

@@ -41,6 +41,7 @@ static  zend_function_entry message_methods[] = {
   PHP_ME(Message, decode, NULL, ZEND_ACC_PUBLIC)
   PHP_ME(Message, readOneof, NULL, ZEND_ACC_PROTECTED)
   PHP_ME(Message, writeOneof, NULL, ZEND_ACC_PROTECTED)
+  PHP_ME(Message, whichOneof, NULL, ZEND_ACC_PROTECTED)
   PHP_ME(Message, __construct, NULL, ZEND_ACC_PROTECTED)
   {NULL, NULL, NULL}
 };
@@ -258,3 +259,22 @@ PHP_METHOD(Message, writeOneof) {
 
   layout_set(msg->descriptor->layout, msg, field, value TSRMLS_CC);
 }
+
+PHP_METHOD(Message, whichOneof) {
+  char* oneof_name;
+  int length;
+
+  if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &oneof_name,
+                            &length) == FAILURE) {
+    return;
+  }
+
+  MessageHeader* msg =
+      (MessageHeader*)zend_object_store_get_object(getThis() TSRMLS_CC);
+
+  const upb_oneofdef* oneof =
+      upb_msgdef_ntoo(msg->descriptor->msgdef, oneof_name, length);
+  const char* oneof_case_name = layout_get_oneof_case(
+      msg->descriptor->layout, message_data(msg), oneof TSRMLS_CC);
+  RETURN_STRING(oneof_case_name, 1);
+}

php/ext/google/protobuf/protobuf.h

@@ -237,10 +237,13 @@ zval* layout_get(MessageLayout* layout, const void* storage,
                  const upb_fielddef* field, zval** cache TSRMLS_DC);
 void layout_set(MessageLayout* layout, MessageHeader* header,
                 const upb_fielddef* field, zval* val TSRMLS_DC);
+const char* layout_get_oneof_case(MessageLayout* layout, const void* storage,
+                                  const upb_oneofdef* oneof TSRMLS_DC);
 void free_layout(MessageLayout* layout);
 
 PHP_METHOD(Message, readOneof);
 PHP_METHOD(Message, writeOneof);
+PHP_METHOD(Message, whichOneof);
 PHP_METHOD(Message, __construct);
 
 // -----------------------------------------------------------------------------

php/ext/google/protobuf/storage.c

@@ -527,7 +527,7 @@ zval* layout_get(MessageLayout* layout, const void* storage,
 }
 
 void layout_set(MessageLayout* layout, MessageHeader* header,
-		const upb_fielddef* field, zval* val TSRMLS_DC) {
+                const upb_fielddef* field, zval* val TSRMLS_DC) {
   void* storage = message_data(header);
   void* memory = slot_memory(layout, storage, field);
   uint32_t* oneof_case = slot_oneof_case(layout, storage, field);
@@ -582,3 +582,22 @@ void layout_set(MessageLayout* layout, MessageHeader* header,
     native_slot_set(type, ce, value_memory(field, memory), val TSRMLS_CC);
   }
 }
+
+const char* layout_get_oneof_case(MessageLayout* layout, const void* storage,
+                                  const upb_oneofdef* oneof TSRMLS_DC) {
+  upb_oneof_iter i;
+  const upb_fielddef* first_field;
+
+  // Oneof is guaranteed to have at least one field. Get the first field.
+  for(upb_oneof_begin(&i, oneof); !upb_oneof_done(&i); upb_oneof_next(&i)) {
+    first_field = upb_oneof_iter_field(&i);
+    break;
+  }
+
+  uint32_t* oneof_case = slot_oneof_case(layout, storage, first_field);
+  if (*oneof_case == 0) {
+    return "";
+  }
+  const upb_fielddef* field = upb_oneofdef_itof(oneof, *oneof_case);
+  return upb_fielddef_name(field);
+}

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

@@ -163,6 +163,17 @@ class Message
         $oneof_field->setNumber($number);
     }
 
+    protected function whichOneof($oneof_name)
+    {
+        $oneof_field = $this->$oneof_name;
+        $number = $oneof_field->getNumber();
+        if ($number == 0) {
+          return "";
+        }
+        $field = $this->desc->getFieldByNumber($number);
+        return $field->getName();
+    }
+
     /**
      * @ignore
      */

php/tests/generated_class_test.php

@@ -573,23 +573,28 @@ class GeneratedClassTest extends PHPUnit_Framework_TestCase
     public function testOneofField() {
         $m = new TestMessage();
 
+        $this->assertSame("", $m->getMyOneof());
+
         $m->setOneofInt32(1);
         $this->assertSame(1, $m->getOneofInt32());
         $this->assertSame(0.0, $m->getOneofFloat());
         $this->assertSame('', $m->getOneofString());
         $this->assertSame(NULL, $m->getOneofMessage());
+        $this->assertSame("oneof_int32", $m->getMyOneof());
 
         $m->setOneofFloat(2.0);
         $this->assertSame(0, $m->getOneofInt32());
         $this->assertSame(2.0, $m->getOneofFloat());
         $this->assertSame('', $m->getOneofString());
         $this->assertSame(NULL, $m->getOneofMessage());
+        $this->assertSame("oneof_float", $m->getMyOneof());
 
         $m->setOneofString('abc');
         $this->assertSame(0, $m->getOneofInt32());
         $this->assertSame(0.0, $m->getOneofFloat());
         $this->assertSame('abc', $m->getOneofString());
         $this->assertSame(NULL, $m->getOneofMessage());
+        $this->assertSame("oneof_string", $m->getMyOneof());
 
         $sub_m = new TestMessage_Sub();
         $sub_m->setA(1);
@@ -598,6 +603,7 @@ class GeneratedClassTest extends PHPUnit_Framework_TestCase
         $this->assertSame(0.0, $m->getOneofFloat());
         $this->assertSame('', $m->getOneofString());
         $this->assertSame(1, $m->getOneofMessage()->getA());
+        $this->assertSame("oneof_message", $m->getMyOneof());
     }
 
     #########################################################

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

@@ -864,7 +864,7 @@ void GenerateMessageFile(const FileDescriptor* file, const Descriptor* message,
     printer.Print(
       "public function get^camel_name^()\n"
       "{\n"
-      "    return $this->^name^;\n"
+      "    return $this->whichOneof(\"^name^\");\n"
       "}\n\n",
       "camel_name", UnderscoresToCamelCase(oneof->name(), true), "name",
       oneof->name());