From f0942fb0a21e984a535c9ade53dd4642425c3aca Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Mon, 14 Sep 2020 13:40:07 -0700 Subject: [PATCH] Equals passes tests in both pure-PHP and the C extension. (#7883) --- php/ext/google/protobuf/message.c | 139 +++++++++++++++++++++++++++++ php/tests/GeneratedClassTest.php | 142 ++++++++++++++++++++++++++++++ 2 files changed, 281 insertions(+) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 63d2b084a..e913379cf 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -108,6 +108,144 @@ static const upb_fielddef *get_field(Message *msg, PROTO_STR *member) { return f; } +static bool MessageEq(const upb_msg *m1, const upb_msg *m2, const upb_msgdef *m); + +/** + * ValueEq()() + */ +static bool ValueEq(upb_msgval val1, upb_msgval val2, upb_fieldtype_t type, + const upb_msgdef *m) { + switch (type) { + case UPB_TYPE_BOOL: + return val1.bool_val == val2.bool_val; + case UPB_TYPE_INT32: + case UPB_TYPE_UINT32: + case UPB_TYPE_ENUM: + return val1.int32_val == val2.int32_val; + case UPB_TYPE_INT64: + case UPB_TYPE_UINT64: + return val1.int64_val == val2.int64_val; + case UPB_TYPE_FLOAT: + return val1.float_val == val2.float_val; + case UPB_TYPE_DOUBLE: + return val1.double_val == val2.double_val; + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + return val1.str_val.size == val2.str_val.size && + memcmp(val1.str_val.data, val2.str_val.data, val1.str_val.size) == 0; + case UPB_TYPE_MESSAGE: + return MessageEq(val1.msg_val, val2.msg_val, m); + default: + return false; + } +} + +/** + * MapEq() + */ +static bool MapEq(const upb_map *m1, const upb_map *m2, + upb_fieldtype_t key_type, upb_fieldtype_t val_type, + const upb_msgdef *m) { + size_t iter = UPB_MAP_BEGIN; + + if ((m1 == NULL) != (m2 == NULL)) return false; + if (m1 == NULL) return true; + if (upb_map_size(m1) != upb_map_size(m2)) return false; + + while (upb_mapiter_next(m1, &iter)) { + upb_msgval key = upb_mapiter_key(m1, iter); + upb_msgval val1 = upb_mapiter_value(m1, iter); + upb_msgval val2; + + if (!upb_map_get(m2, key, &val2)) return false; + if (!ValueEq(val1, val2, val_type, m)) return false; + } + + return true; +} + +/** + * ArrayEq() + */ +static bool ArrayEq(const upb_array *a1, const upb_array *a2, + upb_fieldtype_t type, const upb_msgdef *m) { + size_t i; + size_t n; + + if ((a1 == NULL) != (a2 == NULL)) return false; + if (a1 == NULL) return true; + + n = upb_array_size(a1); + if (n != upb_array_size(a2)) return false; + + for (i = 0; i < n; i++) { + upb_msgval val1 = upb_array_get(a1, i); + upb_msgval val2 = upb_array_get(a2, i); + if (!ValueEq(val1, val2, type, m)) return false; + } + + return true; +} + +/** + * MessageEq() + */ +static bool MessageEq(const upb_msg *m1, const upb_msg *m2, const upb_msgdef *m) { + upb_msg_field_iter i; + + for(upb_msg_field_begin(&i, m); + !upb_msg_field_done(&i); + upb_msg_field_next(&i)) { + const upb_fielddef *f = upb_msg_iter_field(&i); + upb_msgval val1 = upb_msg_get(m1, f); + upb_msgval val2 = upb_msg_get(m2, f); + upb_fieldtype_t type = upb_fielddef_type(f); + const upb_msgdef *sub_m = upb_fielddef_msgsubdef(f); + + if (upb_fielddef_haspresence(f)) { + if (upb_msg_has(m1, f) != upb_msg_has(m2, f)) { + return false; + } + if (!upb_msg_has(m1, f)) continue; + } + + if (upb_fielddef_ismap(f)) { + const upb_fielddef *key_f = upb_msgdef_itof(sub_m, 1); + const upb_fielddef *val_f = upb_msgdef_itof(sub_m, 2); + upb_fieldtype_t key_type = upb_fielddef_type(key_f); + upb_fieldtype_t val_type = upb_fielddef_type(val_f); + const upb_msgdef *val_m = upb_fielddef_msgsubdef(val_f); + if (!MapEq(val1.map_val, val2.map_val, key_type, val_type, val_m)) { + return false; + } + } else if (upb_fielddef_isseq(f)) { + if (!ArrayEq(val1.array_val, val2.array_val, type, sub_m)) return false; + } else { + if (!ValueEq(val1, val2, type, sub_m)) return false; + } + } + + return true; +} + +/** + * Message_compare_objects() + * + * Object handler for comparing two message objects. Called whenever PHP code + * does: + * + * $m1 == $m2 + */ +static int Message_compare_objects(zval *m1, zval *m2) { + Message* intern1 = (Message*)Z_OBJ_P(m1); + Message* intern2 = (Message*)Z_OBJ_P(m2); + const upb_msgdef *m = intern1->desc->msgdef; + + if (intern2->desc->msgdef != m) return 1; + + return MessageEq(intern1->msg, intern2->msg, m) ? 0 : 1; +} + /** * Message_has_property() * @@ -935,6 +1073,7 @@ void Message_ModuleInit() { memcpy(h, &std_object_handlers, sizeof(zend_object_handlers)); h->dtor_obj = Message_dtor; + h->compare_objects = Message_compare_objects; h->read_property = Message_read_property; h->write_property = Message_write_property; h->has_property = Message_has_property; diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php index f49c4e970..90c1069ed 100644 --- a/php/tests/GeneratedClassTest.php +++ b/php/tests/GeneratedClassTest.php @@ -1529,6 +1529,148 @@ class GeneratedClassTest extends TestBase $m->setOptionalString($values[0]); } + ######################################################### + # Test equality + ######################################################### + + public function testShallowEquality() + { + $m1 = new TestMessage([ + 'optional_int32' => -42, + 'optional_int64' => -43, + 'optional_uint32' => 42, + 'optional_uint64' => 43, + 'optional_sint32' => -44, + 'optional_sint64' => -45, + 'optional_fixed32' => 46, + 'optional_fixed64' => 47, + 'optional_sfixed32' => -46, + 'optional_sfixed64' => -47, + 'optional_float' => 1.5, + 'optional_double' => 1.6, + 'optional_bool' => true, + 'optional_string' => 'a', + 'optional_bytes' => 'bbbb', + 'optional_enum' => TestEnum::ONE, + ]); + $data = $m1->serializeToString(); + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $this->assertTrue($m1 == $m2); + + $m1->setOptionalInt32(1234); + $this->assertTrue($m1 != $m2); + } + + public function testDeepEquality() + { + $m1 = new TestMessage([ + 'optional_int32' => -42, + 'optional_int64' => -43, + 'optional_uint32' => 42, + 'optional_uint64' => 43, + 'optional_sint32' => -44, + 'optional_sint64' => -45, + 'optional_fixed32' => 46, + 'optional_fixed64' => 47, + 'optional_sfixed32' => -46, + 'optional_sfixed64' => -47, + 'optional_float' => 1.5, + 'optional_double' => 1.6, + 'optional_bool' => true, + 'optional_string' => 'a', + 'optional_bytes' => 'bbbb', + 'optional_enum' => TestEnum::ONE, + 'optional_message' => new Sub([ + 'a' => 33 + ]), + 'repeated_int32' => [-42, -52], + 'repeated_int64' => [-43, -53], + 'repeated_uint32' => [42, 52], + 'repeated_uint64' => [43, 53], + 'repeated_sint32' => [-44, -54], + 'repeated_sint64' => [-45, -55], + 'repeated_fixed32' => [46, 56], + 'repeated_fixed64' => [47, 57], + 'repeated_sfixed32' => [-46, -56], + 'repeated_sfixed64' => [-47, -57], + 'repeated_float' => [1.5, 2.5], + 'repeated_double' => [1.6, 2.6], + 'repeated_bool' => [true, false], + 'repeated_string' => ['a', 'c'], + 'repeated_bytes' => ['bbbb', 'dddd'], + 'repeated_enum' => [TestEnum::ZERO, TestEnum::ONE], + 'repeated_message' => [new Sub(['a' => 34]), + new Sub(['a' => 35])], + 'map_int32_int32' => [-62 => -62], + 'map_int64_int64' => [-63 => -63], + 'map_uint32_uint32' => [62 => 62], + 'map_uint64_uint64' => [63 => 63], + 'map_sint32_sint32' => [-64 => -64], + 'map_sint64_sint64' => [-65 => -65], + 'map_fixed32_fixed32' => [66 => 66], + 'map_fixed64_fixed64' => [67 => 67], + 'map_sfixed32_sfixed32' => [-68 => -68], + 'map_sfixed64_sfixed64' => [-69 => -69], + 'map_int32_float' => [1 => 3.5], + 'map_int32_double' => [1 => 3.6], + 'map_bool_bool' => [true => true], + 'map_string_string' => ['e' => 'e'], + 'map_int32_bytes' => [1 => 'ffff'], + 'map_int32_enum' => [1 => TestEnum::ONE], + 'map_int32_message' => [1 => new Sub(['a' => 36])], + ]); + $data = $m1->serializeToString(); + + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $this->assertTrue($m1 == $m2); + + # Nested sub-message is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getOptionalMessage()->setA(1234); + $this->assertTrue($m1 != $m2); + + # Repeated field element is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getRepeatedInt32()[0] = 1234; + $this->assertTrue($m1 != $m2); + + # Repeated field length is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getRepeatedInt32()[] = 1234; + $this->assertTrue($m1 != $m2); + + # SubMessage inside repeated field is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getRepeatedMessage()[0]->setA(1234); + $this->assertTrue($m1 != $m2); + + # Map value is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getMapInt32Int32()[-62] = 1234; + $this->assertTrue($m1 != $m2); + + # Map size is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getMapInt32Int32()[1234] = 1234; + $this->assertTrue($m1 != $m2); + + # SubMessage inside map field is checked. + $m2 = new TestMessage(); + $m2->mergeFromString($data); + $m2->getMapInt32Message()[1]->setA(1234); + $this->assertTrue($m1 != $m2); + + # TODO: what about unknown fields? + } + ######################################################### # Test no segfault when error happens #########################################################