Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for "==" to the PHP C extension #7883

Merged
merged 1 commit into from Sep 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
139 changes: 139 additions & 0 deletions php/ext/google/protobuf/message.c
Expand Up @@ -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()
*
Expand Down Expand Up @@ -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;
Expand Down
142 changes: 142 additions & 0 deletions php/tests/GeneratedClassTest.php
Expand Up @@ -1529,6 +1529,148 @@ public function testValueIsReference()
$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?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question, is equal only recommend for test? If so, I think we don't need to worry about it.
Otherwise, I think not comparing equal ensures backward compatibility.

}

#########################################################
# Test no segfault when error happens
#########################################################
Expand Down