Oneof accessor should return the field name that is actually set. (#2631)
diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
index 16e397f..f5a9499 100644
--- a/php/ext/google/protobuf/message.c
+++ b/php/ext/google/protobuf/message.c
@@ -41,6 +41,7 @@
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 @@
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);
+}
diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h
index 8022a9a..bd01005 100644
--- a/php/ext/google/protobuf/protobuf.h
+++ b/php/ext/google/protobuf/protobuf.h
@@ -237,10 +237,13 @@
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);
// -----------------------------------------------------------------------------
diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c
index 8a2b3a2..42bcce5 100644
--- a/php/ext/google/protobuf/storage.c
+++ b/php/ext/google/protobuf/storage.c
@@ -527,7 +527,7 @@
}
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 @@
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);
+}
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index 3d1f159..031c82a 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -163,6 +163,17 @@
$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
*/
diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php
index 27912ec..3587092 100644
--- a/php/tests/generated_class_test.php
+++ b/php/tests/generated_class_test.php
@@ -573,23 +573,28 @@
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 @@
$this->assertSame(0.0, $m->getOneofFloat());
$this->assertSame('', $m->getOneofString());
$this->assertSame(1, $m->getOneofMessage()->getA());
+ $this->assertSame("oneof_message", $m->getMyOneof());
}
#########################################################
diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc
index be2739f..36f00ac 100644
--- a/src/google/protobuf/compiler/php/php_generator.cc
+++ b/src/google/protobuf/compiler/php/php_generator.cc
@@ -864,7 +864,7 @@
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());
