Added proto3 presence support for PHP (#7724)
* WIP.
* Added proto3 presence support for PHP.
* Added compatibility code for old generated code.
diff --git a/Makefile.am b/Makefile.am
index 631df6f..15b5a42 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -954,6 +954,7 @@
php/tests/test_base.php \
php/tests/test_util.php \
php/tests/undefined_test.php \
+ php/tests/valgrind.supp \
php/tests/well_known_test.php \
php/tests/wrapper_type_setters_test.php
diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
index 20dd37a..78f37ed 100644
--- a/php/ext/google/protobuf/message.c
+++ b/php/ext/google/protobuf/message.c
@@ -109,6 +109,76 @@
}
/**
+ * Message_has_property()
+ *
+ * Object handler for testing whether a property exists. Called when PHP code
+ * does any of:
+ *
+ * isset($message->foobar);
+ * property_exists($message->foobar);
+ *
+ * Note that all properties of generated messages are private, so this should
+ * only be possible to invoke from generated code, which has accessors like this
+ * (if the field has presence):
+ *
+ * public function hasOptionalInt32()
+ * {
+ * return isset($this->optional_int32);
+ * }
+ */
+static int Message_has_property(zval *obj, zval *member, int has_set_exists,
+ void **cache_slot) {
+ Message* intern = (Message*)Z_OBJ_P(obj);
+ const upb_fielddef *f = get_field(intern, member);
+
+ if (!f) return 0;
+
+ if (!upb_fielddef_haspresence(f)) {
+ zend_throw_exception_ex(
+ NULL, 0,
+ "Cannot call isset() on field %s which does not have presence.",
+ ZSTR_VAL(intern->desc->class_entry->name));
+ return 0;
+ }
+
+ return upb_msg_has(intern->msg, f);
+}
+
+/**
+ * Message_unset_property()
+ *
+ * Object handler for unsetting a property. Called when PHP code calls:
+ * does any of:
+ *
+ * unset($message->foobar);
+ *
+ * Note that all properties of generated messages are private, so this should
+ * only be possible to invoke from generated code, which has accessors like this
+ * (if the field has presence):
+ *
+ * public function clearOptionalInt32()
+ * {
+ * unset($this->optional_int32);
+ * }
+ */
+static void Message_unset_property(zval *obj, zval *member, void **cache_slot) {
+ Message* intern = (Message*)Z_OBJ_P(obj);
+ const upb_fielddef *f = get_field(intern, member);
+
+ if (!f) return;
+
+ if (!upb_fielddef_haspresence(f)) {
+ zend_throw_exception_ex(
+ NULL, 0,
+ "Cannot call unset() on field %s which does not have presence.",
+ ZSTR_VAL(intern->desc->class_entry->name));
+ return;
+ }
+
+ upb_msg_clearfield(intern->msg, f);
+}
+
+/**
* Message_read_property()
*
* Object handler for reading a property in PHP. Called when PHP code does:
@@ -836,6 +906,8 @@
h->dtor_obj = Message_dtor;
h->read_property = Message_read_property;
h->write_property = Message_write_property;
+ h->has_property = Message_has_property;
+ h->unset_property = Message_unset_property;
h->get_properties = Message_get_properties;
h->get_property_ptr_ptr = Message_get_property_ptr_ptr;
}
diff --git a/php/src/GPBMetadata/Google/Protobuf/Struct.php b/php/src/GPBMetadata/Google/Protobuf/Struct.php
index 96b42af..8e6191d 100644
--- a/php/src/GPBMetadata/Google/Protobuf/Struct.php
+++ b/php/src/GPBMetadata/Google/Protobuf/Struct.php
@@ -15,29 +15,8 @@
return;
}
$pool->internalAddGeneratedFile(hex2bin(
- "0a81050a1c676f6f676c652f70726f746f6275662f7374727563742e7072" .
- "6f746f120f676f6f676c652e70726f746f6275662284010a065374727563" .
- "7412330a066669656c647318012003280b32232e676f6f676c652e70726f" .
- "746f6275662e5374727563742e4669656c6473456e7472791a450a0b4669" .
- "656c6473456e747279120b0a036b657918012001280912250a0576616c75" .
- "6518022001280b32162e676f6f676c652e70726f746f6275662e56616c75" .
- "653a02380122ea010a0556616c756512300a0a6e756c6c5f76616c756518" .
- "012001280e321a2e676f6f676c652e70726f746f6275662e4e756c6c5661" .
- "6c7565480012160a0c6e756d6265725f76616c7565180220012801480012" .
- "160a0c737472696e675f76616c7565180320012809480012140a0a626f6f" .
- "6c5f76616c75651804200128084800122f0a0c7374727563745f76616c75" .
- "6518052001280b32172e676f6f676c652e70726f746f6275662e53747275" .
- "6374480012300a0a6c6973745f76616c756518062001280b321a2e676f6f" .
- "676c652e70726f746f6275662e4c69737456616c7565480042060a046b69" .
- "6e6422330a094c69737456616c756512260a0676616c7565731801200328" .
- "0b32162e676f6f676c652e70726f746f6275662e56616c75652a1b0a094e" .
- "756c6c56616c7565120e0a0a4e554c4c5f56414c554510004281010a1363" .
- "6f6d2e676f6f676c652e70726f746f627566420b53747275637450726f74" .
- "6f50015a316769746875622e636f6d2f676f6c616e672f70726f746f6275" .
- "662f7074797065732f7374727563743b7374727563747062f80101a20203" .
- "475042aa021e476f6f676c652e50726f746f6275662e57656c6c4b6e6f77" .
- "6e5479706573620670726f746f33"
- ));
+ "0a81050a1c676f6f676c652f70726f746f6275662f7374727563742e70726f746f120f676f6f676c652e70726f746f6275662284010a0653747275637412330a066669656c647318012003280b32232e676f6f676c652e70726f746f6275662e5374727563742e4669656c6473456e7472791a450a0b4669656c6473456e747279120b0a036b657918012001280912250a0576616c756518022001280b32162e676f6f676c652e70726f746f6275662e56616c75653a02380122ea010a0556616c756512300a0a6e756c6c5f76616c756518012001280e321a2e676f6f676c652e70726f746f6275662e4e756c6c56616c7565480012160a0c6e756d6265725f76616c7565180220012801480012160a0c737472696e675f76616c7565180320012809480012140a0a626f6f6c5f76616c75651804200128084800122f0a0c7374727563745f76616c756518052001280b32172e676f6f676c652e70726f746f6275662e537472756374480012300a0a6c6973745f76616c756518062001280b321a2e676f6f676c652e70726f746f6275662e4c69737456616c7565480042060a046b696e6422330a094c69737456616c756512260a0676616c75657318012003280b32162e676f6f676c652e70726f746f6275662e56616c75652a1b0a094e756c6c56616c7565120e0a0a4e554c4c5f56414c554510004281010a13636f6d2e676f6f676c652e70726f746f627566420b53747275637450726f746f50015a316769746875622e636f6d2f676f6c616e672f70726f746f6275662f7074797065732f7374727563743b7374727563747062f80101a20203475042aa021e476f6f676c652e50726f746f6275662e57656c6c4b6e6f776e5479706573620670726f746f33"
+ ), true);
static::$is_initialized = true;
}
diff --git a/php/src/Google/Protobuf/Descriptor.php b/php/src/Google/Protobuf/Descriptor.php
index 986b81e..36436e2 100644
--- a/php/src/Google/Protobuf/Descriptor.php
+++ b/php/src/Google/Protobuf/Descriptor.php
@@ -97,4 +97,12 @@
{
return count($this->internal_desc->getOneofDecl());
}
+
+ /**
+ * @return int Number of real oneofs in message
+ */
+ public function getRealOneofDeclCount()
+ {
+ return $this->internal_desc->getRealOneofDeclCount();
+ }
}
diff --git a/php/src/Google/Protobuf/FieldDescriptor.php b/php/src/Google/Protobuf/FieldDescriptor.php
index ac9271f..6d08cea 100644
--- a/php/src/Google/Protobuf/FieldDescriptor.php
+++ b/php/src/Google/Protobuf/FieldDescriptor.php
@@ -114,4 +114,12 @@
{
return $this->internal_desc->isMap();
}
+
+ /**
+ * @return boolean
+ */
+ public function hasOptionalKeyword()
+ {
+ return $this->internal_desc->hasOptionalKeyword();
+ }
}
diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php
index 77614bc..c02d2b4 100644
--- a/php/src/Google/Protobuf/Internal/Message.php
+++ b/php/src/Google/Protobuf/Internal/Message.php
@@ -225,6 +225,15 @@
}
}
+ protected function hasOneof($number)
+ {
+ $field = $this->desc->getFieldByNumber($number);
+ $oneof = $this->desc->getOneofDecl()[$field->getOneofIndex()];
+ $oneof_name = $oneof->getName();
+ $oneof_field = $this->$oneof_name;
+ return $number === $oneof_field->getNumber();
+ }
+
protected function writeOneof($number, $value)
{
$field = $this->desc->getFieldByNumber($number);
@@ -1559,14 +1568,19 @@
*/
private function existField($field)
{
- $oneof_index = $field->getOneofIndex();
- if ($oneof_index !== -1) {
- $oneof = $this->desc->getOneofDecl()[$oneof_index];
- $oneof_name = $oneof->getName();
- return $this->$oneof_name->getNumber() === $field->getNumber();
+ $getter = $field->getGetter();
+ $hazzer = "has" . substr($getter, 3);
+
+ if (method_exists($this, $hazzer)) {
+ return $this->$hazzer();
+ } else if ($field->getOneofIndex() !== -1) {
+ // For old generated code, which does not have hazzers for oneof
+ // fields.
+ $oneof = $this->desc->getOneofDecl()[$field->getOneofIndex()];
+ $oneof_name = $oneof->getName();
+ return $this->$oneof_name->getNumber() === $field->getNumber();
}
- $getter = $field->getGetter();
$values = $this->$getter();
if ($field->isMap()) {
return count($values) !== 0;
diff --git a/php/src/Google/Protobuf/NullValue.php b/php/src/Google/Protobuf/NullValue.php
index a72cbb2..61569f8 100644
--- a/php/src/Google/Protobuf/NullValue.php
+++ b/php/src/Google/Protobuf/NullValue.php
@@ -35,6 +35,7 @@
return self::$valueToName[$value];
}
+
public static function value($name)
{
$const = __CLASS__ . '::' . strtoupper($name);
diff --git a/php/src/Google/Protobuf/OneofDescriptor.php b/php/src/Google/Protobuf/OneofDescriptor.php
index d973663..92b4e27 100644
--- a/php/src/Google/Protobuf/OneofDescriptor.php
+++ b/php/src/Google/Protobuf/OneofDescriptor.php
@@ -72,4 +72,9 @@
{
return count($this->internal_desc->getFields());
}
+
+ public function isSynthetic()
+ {
+ return $this->internal_desc->isSynthetic();
+ }
}
diff --git a/php/src/Google/Protobuf/Value.php b/php/src/Google/Protobuf/Value.php
index 5c1e864..20db3cc 100644
--- a/php/src/Google/Protobuf/Value.php
+++ b/php/src/Google/Protobuf/Value.php
@@ -57,6 +57,11 @@
return $this->readOneof(1);
}
+ public function hasNullValue()
+ {
+ return $this->hasOneof(1);
+ }
+
/**
* Represents a null value.
*
@@ -83,6 +88,11 @@
return $this->readOneof(2);
}
+ public function hasNumberValue()
+ {
+ return $this->hasOneof(2);
+ }
+
/**
* Represents a double value.
*
@@ -109,6 +119,11 @@
return $this->readOneof(3);
}
+ public function hasStringValue()
+ {
+ return $this->hasOneof(3);
+ }
+
/**
* Represents a string value.
*
@@ -135,6 +150,11 @@
return $this->readOneof(4);
}
+ public function hasBoolValue()
+ {
+ return $this->hasOneof(4);
+ }
+
/**
* Represents a boolean value.
*
@@ -161,6 +181,11 @@
return $this->readOneof(5);
}
+ public function hasStructValue()
+ {
+ return $this->hasOneof(5);
+ }
+
/**
* Represents a structured value.
*
@@ -187,6 +212,11 @@
return $this->readOneof(6);
}
+ public function hasListValue()
+ {
+ return $this->hasOneof(6);
+ }
+
/**
* Represents a repeated `Value`.
*
diff --git a/php/tests/encode_decode_test.php b/php/tests/encode_decode_test.php
index 5442f50..ea8bd65 100644
--- a/php/tests/encode_decode_test.php
+++ b/php/tests/encode_decode_test.php
@@ -326,6 +326,24 @@
}
+ public function testEncodeDecodeOptional()
+ {
+ $m = new TestMessage();
+ $this->assertFalse($m->hasTrueOptionalInt32());
+ $data = $m->serializeToString();
+ $this->assertSame("", $data);
+
+ $m->setTrueOptionalInt32(0);
+ $this->assertTrue($m->hasTrueOptionalInt32());
+ $data = $m->serializeToString();
+ $this->assertNotSame("", $data);
+
+ $m2 = new TestMessage();
+ $m2->mergeFromString($data);
+ $this->assertTrue($m2->hasTrueOptionalInt32());
+ $this->assertSame(0, $m2->getTrueOptionalInt32());
+ }
+
public function testJsonEncodeDecodeOneof()
{
$m = new TestMessage();
diff --git a/php/tests/generate_protos.sh b/php/tests/generate_protos.sh
index 0c2a555..e83c3c1 100755
--- a/php/tests/generate_protos.sh
+++ b/php/tests/generate_protos.sh
@@ -7,10 +7,10 @@
rm -rf generated
mkdir -p generated
-find proto -type f -name "*.proto"| xargs ../../src/protoc --php_out=generated -I../../src -I.
+find proto -type f -name "*.proto"| xargs ../../src/protoc --experimental_allow_proto3_optional --php_out=generated -I../../src -I.
if [ "$1" = "--aggregate_metadata" ]; then
# Overwrite some of the files to use aggregation.
AGGREGATED_FILES="proto/test.proto proto/test_include.proto proto/test_import_descriptor_proto.proto"
- ../../src/protoc --php_out=aggregate_metadata=foo#bar:generated -I../../src -I. $AGGREGATED_FILES
+ ../../src/protoc --experimental_allow_proto3_optional --php_out=aggregate_metadata=foo#bar:generated -I../../src -I. $AGGREGATED_FILES
fi
diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php
index 053697d..f49c4e9 100644
--- a/php/tests/generated_class_test.php
+++ b/php/tests/generated_class_test.php
@@ -72,6 +72,28 @@
}
#########################################################
+ # Test optional int32 field.
+ #########################################################
+
+ public function testOptionalInt32Field()
+ {
+ $m = new TestMessage();
+
+ $this->assertFalse($m->hasTrueOptionalInt32());
+ $this->assertSame(0, $m->getTrueOptionalInt32());
+
+ // Set integer.
+ $m->setTrueOptionalInt32(MAX_INT32);
+ $this->assertTrue($m->hasTrueOptionalInt32());
+ $this->assertSame(MAX_INT32, $m->getTrueOptionalInt32());
+
+ // Clear integer.
+ $m->clearTrueOptionalInt32();
+ $this->assertFalse($m->hasTrueOptionalInt32());
+ $this->assertSame(0, $m->getTrueOptionalInt32());
+ }
+
+ #########################################################
# Test uint32 field.
#########################################################
diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto
index 9505709..368b19e 100644
--- a/php/tests/proto/test.proto
+++ b/php/tests/proto/test.proto
@@ -34,6 +34,27 @@
bar.TestInclude optional_included_message = 18;
TestMessage recursive = 19;
+ // True optional
+ optional int32 true_optional_int32 = 201;
+ optional int64 true_optional_int64 = 202;
+ optional uint32 true_optional_uint32 = 203;
+ optional uint64 true_optional_uint64 = 204;
+ optional sint32 true_optional_sint32 = 205;
+ optional sint64 true_optional_sint64 = 206;
+ optional fixed32 true_optional_fixed32 = 207;
+ optional fixed64 true_optional_fixed64 = 208;
+ optional sfixed32 true_optional_sfixed32 = 209;
+ optional sfixed64 true_optional_sfixed64 = 210;
+ optional float true_optional_float = 211;
+ optional double true_optional_double = 212;
+ optional bool true_optional_bool = 213;
+ optional string true_optional_string = 214;
+ optional bytes true_optional_bytes = 215;
+
+ optional TestEnum true_optional_enum = 216;
+ optional Sub true_optional_message = 217;
+ optional bar.TestInclude true_optional_included_message = 218;
+
// Repeated
repeated int32 repeated_int32 = 31;
repeated int64 repeated_int64 = 32;
diff --git a/php/tests/test.sh b/php/tests/test.sh
index b10b57f..4beeed5 100755
--- a/php/tests/test.sh
+++ b/php/tests/test.sh
@@ -51,8 +51,8 @@
export ZEND_DONT_UNLOAD_MODULES=1
export USE_ZEND_ALLOC=0
-valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php
-valgrind --leak-check=yes php -d protobuf.keep_descriptor_pool_after_request=1 -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php
+valgrind --suppressions=valgrind.supp --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php
+valgrind --suppressions=valgrind.supp --leak-check=yes php -d protobuf.keep_descriptor_pool_after_request=1 -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php
# TODO(teboring): Only for debug (phpunit has memory leak which blocks this beging used by
# regresssion test.)
diff --git a/php/tests/valgrind.supp b/php/tests/valgrind.supp
new file mode 100644
index 0000000..e83b0a3
--- /dev/null
+++ b/php/tests/valgrind.supp
@@ -0,0 +1,12 @@
+{
+ PHP_Equal_Val
+ Memcheck:Cond
+ fun:zend_string_equal_val
+}
+
+{
+ PHP_ScanDir_Tail
+ Memcheck:Cond
+ obj:/usr/bin/php7.3
+ fun:__scandir64_tail
+}
diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc
index 4a6206a..a4b7139 100644
--- a/src/google/protobuf/compiler/php/php_generator.cc
+++ b/src/google/protobuf/compiler/php/php_generator.cc
@@ -624,15 +624,17 @@
printer->Print(
"private $^name^;\n",
"name", field->name());
- } else if (field->containing_oneof()) {
+ } else if (field->real_containing_oneof()) {
// Oneof fields are handled by GenerateOneofField.
return;
} else {
+ std::string initial_value =
+ field->has_presence() ? "null" : DefaultForField(field);
GenerateFieldDocComment(printer, field, is_descriptor, kFieldProperty);
printer->Print(
- "protected $^name^ = ^default^;\n",
+ "protected $^name^ = ^initial_value^;\n",
"name", field->name(),
- "default", DefaultForField(field));
+ "initial_value", initial_value);
}
if (is_descriptor) {
@@ -652,20 +654,41 @@
void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor,
io::Printer* printer) {
- const OneofDescriptor* oneof = field->containing_oneof();
+ const OneofDescriptor* oneof = field->real_containing_oneof();
// Generate getter.
+ GenerateFieldDocComment(printer, field, is_descriptor, kFieldGetter);
+
if (oneof != NULL) {
- GenerateFieldDocComment(printer, field, is_descriptor, kFieldGetter);
printer->Print(
"public function get^camel_name^()\n"
"{\n"
" return $this->readOneof(^number^);\n"
+ "}\n\n"
+ "public function has^camel_name^()\n"
+ "{\n"
+ " return $this->hasOneof(^number^);\n"
"}\n\n",
"camel_name", UnderscoresToCamelCase(field->name(), true),
"number", IntToString(field->number()));
+ } else if (field->has_presence()) {
+ printer->Print(
+ "public function get^camel_name^()\n"
+ "{\n"
+ " return isset($this->^name^) ? $this->^name^ : ^default_value^;\n"
+ "}\n\n"
+ "public function has^camel_name^()\n"
+ "{\n"
+ " return isset($this->^name^);\n"
+ "}\n\n"
+ "public function clear^camel_name^()\n"
+ "{\n"
+ " unset($this->^name^);\n"
+ "}\n\n",
+ "camel_name", UnderscoresToCamelCase(field->name(), true),
+ "name", field->name(),
+ "default_value", DefaultForField(field));
} else {
- GenerateFieldDocComment(printer, field, is_descriptor, kFieldGetter);
printer->Print(
"public function get^camel_name^()\n"
"{\n"
@@ -878,7 +901,7 @@
"value", ToUpper(val->type_name()),
"number", StrCat(field->number()),
"other", EnumOrMessageSuffix(val, true));
- } else if (!field->containing_oneof()) {
+ } else if (!field->real_containing_oneof()) {
printer->Print(
"->^label^('^field^', "
"\\Google\\Protobuf\\Internal\\GPBType::^type^, ^number^^other^)\n",
@@ -891,7 +914,7 @@
}
// oneofs.
- for (int i = 0; i < message->oneof_decl_count(); i++) {
+ for (int i = 0; i < message->real_oneof_decl_count(); i++) {
const OneofDescriptor* oneof = message->oneof_decl(i);
printer->Print("->oneof(^name^)\n",
"name", oneof->name());
@@ -1414,7 +1437,7 @@
const FieldDescriptor* field = message->field(i);
GenerateField(field, &printer, is_descriptor);
}
- for (int i = 0; i < message->oneof_decl_count(); i++) {
+ for (int i = 0; i < message->real_oneof_decl_count(); i++) {
const OneofDescriptor* oneof = message->oneof_decl(i);
GenerateOneofField(oneof, &printer);
}
@@ -1443,7 +1466,7 @@
const FieldDescriptor* field = message->field(i);
GenerateFieldAccessor(field, is_descriptor, &printer);
}
- for (int i = 0; i < message->oneof_decl_count(); i++) {
+ for (int i = 0; i < message->real_oneof_decl_count(); i++) {
const OneofDescriptor* oneof = message->oneof_decl(i);
printer.Print(
"/**\n"
diff --git a/src/google/protobuf/compiler/php/php_generator.h b/src/google/protobuf/compiler/php/php_generator.h
index ca9d23a..f67bb40 100644
--- a/src/google/protobuf/compiler/php/php_generator.h
+++ b/src/google/protobuf/compiler/php/php_generator.h
@@ -55,6 +55,11 @@
const std::string& parameter,
GeneratorContext* generator_context,
std::string* error) const override;
+
+ uint64_t GetSupportedFeatures() const override {
+ return FEATURE_PROTO3_OPTIONAL;
+ }
+
private:
bool Generate(
const FileDescriptor* file,
