commit f1aa92a5f5b2c0ebf4711c126877d5339fd55f4b
author Piotr Hałas <piotr@halas.net.pl> Mon Dec 02 09:03:38 2024 -0800
committer Copybara-Service <copybara-worker@google.com> Mon Dec 02 09:18:39 2024 -0800
tree b45aab1fdcde783160ac69a624f03e7159a9eba4
parent 5d0865cf1537772b8e0969563402654087b40d31

Handle optional fields correctly in Message_get method (#18982)

Refactor the Message_get method to accurately handle optional fields with presence checks by returning NULL if the field is not set. Also, add new unit tests in WellKnownTest.php to ensure correct behavior of reflection properties for optional fields with various data types.

should fix https://github.com/protocolbuffers/protobuf/issues/18966

Closes #18982

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/18982 from s2x:fix-property-reflection-values 9d7030bfb544fd90754f3828b2698d6d1eb20a9d
PiperOrigin-RevId: 701986724

php/ext/google/protobuf/message.c

diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c
index b00a115..277c8bf 100644
--- a/php/ext/google/protobuf/message.c
+++ b/php/ext/google/protobuf/message.c
@@ -312,7 +312,13 @@
   const upb_FieldDef* f = get_field(intern, member);
 
   if (!f) return &EG(uninitialized_zval);
-  Message_get(intern, f, rv);
+
+  if (upb_FieldDef_IsOptional(f) && upb_FieldDef_HasPresence(f) &&
+      Message_has_property(obj, member, 0, cache_slot) == false) {
+    ZVAL_NULL(rv);
+  } else {
+    Message_get(intern, f, rv);
+  }
   return rv;
 }
 

php/tests/WellKnownTest.php

diff --git a/php/tests/WellKnownTest.php b/php/tests/WellKnownTest.php
index 486c65f..13c2139 100644
--- a/php/tests/WellKnownTest.php
+++ b/php/tests/WellKnownTest.php
@@ -416,4 +416,44 @@
             ['\Google\Protobuf\Syntax'],
         ];
     }
+
+    /**
+     * @dataProvider optionalFieldsDataProvider
+     */
+    public function testReflectionProperty($property, $default)
+    {
+        $testMessage = new TestMessage();
+
+        $functionName = implode('', array_map('ucwords', explode('_', $property)));
+
+        $reflectionProperty = new \ReflectionProperty($testMessage, $property);
+
+        self::assertFalse(call_user_func([$testMessage, 'has'.$functionName]));
+        self::assertEquals($default, call_user_func([$testMessage, 'get'.$functionName]));
+        self::assertNull($reflectionProperty->getValue($testMessage));
+    }
+
+    public function optionalFieldsDataProvider()
+    {
+        return [
+            ['true_optional_int32', 0],
+            ['true_optional_int64', 0],
+            ['true_optional_uint32', 0],
+            ['true_optional_uint64', 0],
+            ['true_optional_sint32', 0],
+            ['true_optional_sint64', 0],
+            ['true_optional_fixed32', 0],
+            ['true_optional_fixed64', 0],
+            ['true_optional_sfixed32', 0],
+            ['true_optional_sfixed64', 0],
+            ['true_optional_float', 0.0],
+            ['true_optional_double', 0.0],
+            ['true_optional_bool', false],
+            ['true_optional_string', ''],
+            ['true_optional_bytes', ''],
+            ['true_optional_enum', null],
+            ['true_optional_message', null],
+            ['true_optional_included_message', null],
+        ];
+    }
 }