From bcbaabe53a8d661f5a473d2a157a4278ad8bf579 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Wed, 1 Mar 2017 10:39:48 -0800 Subject: [PATCH] Add mergeFrom method on Message (#2766) This method merges the contents of the specified message into the current message. Singular fields that are set in the specified message overwrite the corresponding fields in the current message. Repeated fields are appended. Map fields key-value pairs are overritten. Singular/Oneof sub-messages are recursively merged. All overritten sub-messages are deep-copied. --- php/ext/google/protobuf/array.c | 1 + php/ext/google/protobuf/encode_decode.c | 2 +- php/ext/google/protobuf/map.c | 4 +- php/ext/google/protobuf/message.c | 28 +++ php/ext/google/protobuf/protobuf.h | 4 + php/ext/google/protobuf/storage.c | 216 ++++++++++++++++++- php/src/Google/Protobuf/Internal/Message.php | 68 ++++++ php/tests/generated_class_test.php | 109 ++++++++++ php/tests/memory_leak_test.php | 5 + php/tests/proto/test.proto | 1 + php/tests/test.sh | 8 +- 11 files changed, 439 insertions(+), 7 deletions(-) diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index e4a88c393d..63bb6d0a09 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -106,6 +106,7 @@ void repeated_field_init(TSRMLS_D) { repeated_field_handlers = PEMALLOC(zend_object_handlers); memcpy(repeated_field_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers)); + repeated_field_handlers->write_dimension = repeated_field_write_dimension; repeated_field_handlers->get_gc = repeated_field_get_gc; } diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 57fc81d3f1..78b12a06dd 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -434,7 +434,7 @@ static void map_slot_key(upb_fieldtype_t type, const void* from, } static void map_slot_value(upb_fieldtype_t type, const void* from, - upb_value* v) { + upb_value* v) { size_t len; void* to = upb_value_memory(v); #ifndef NDEBUG diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index 35747b05c8..ab98879d73 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -310,8 +310,8 @@ static bool map_field_write_dimension(zval *object, zval *key, mem = upb_value_memory(&v); memset(mem, 0, native_slot_size(intern->value_type)); - if (!native_slot_set(intern->value_type, intern->msg_ce, mem, value - TSRMLS_CC)) { + if (!native_slot_set(intern->value_type, intern->msg_ce, mem, + value TSRMLS_CC)) { return false; } #ifndef NDEBUG diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 46da9024d5..b35df3113a 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -43,6 +43,7 @@ static zend_function_entry message_methods[] = { PHP_ME(Message, decode, NULL, ZEND_ACC_PUBLIC) PHP_ME(Message, jsonEncode, NULL, ZEND_ACC_PUBLIC) PHP_ME(Message, jsonDecode, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Message, mergeFrom, 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) @@ -209,6 +210,13 @@ static zend_object_value message_create(zend_class_entry* ce TSRMLS_DC) { return return_value; } +void message_create_with_type(zend_class_entry* ce, zval** message TSRMLS_DC) { + MAKE_STD_ZVAL(*message); + Z_TYPE_PP(message) = IS_OBJECT; + Z_OBJVAL_PP(message) = ce->create_object(ce TSRMLS_CC); + Z_DELREF_PP(message); +} + void build_class_from_descriptor(zval* php_descriptor TSRMLS_DC) { Descriptor* desc = UNBOX(Descriptor, php_descriptor); @@ -260,6 +268,26 @@ PHP_METHOD(Message, clear) { msg->std.properties_table TSRMLS_CC); } +PHP_METHOD(Message, mergeFrom) { + zval* value; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "O", &value, + message_type) == FAILURE) { + return; + } + + MessageHeader* from = + (MessageHeader*)zend_object_store_get_object(value TSRMLS_CC); + MessageHeader* to = + (MessageHeader*)zend_object_store_get_object(getThis() TSRMLS_CC); + + if(from->descriptor != to->descriptor) { + zend_error(E_USER_ERROR, "Cannot merge messages with different class."); + return; + } + + layout_merge(from->descriptor->layout, from, to TSRMLS_CC); +} + PHP_METHOD(Message, readOneof) { long index; diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index d4737fb940..1562bbaf1b 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -151,6 +151,7 @@ extern zend_class_entry* enum_descriptor_type; // ----------------------------------------------------------------------------- void* message_data(void* msg); +void message_create_with_type(zend_class_entry* ce, zval** message TSRMLS_DC); // Build PHP class for given descriptor. Instead of building from scratch, this // function modifies existing class which has been partially defined in PHP @@ -240,11 +241,14 @@ zval* layout_get(MessageLayout* layout, const void* storage, const upb_fielddef* field, zval** cache TSRMLS_DC); void layout_set(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, zval* val TSRMLS_DC); +void layout_merge(MessageLayout* layout, MessageHeader* from, + MessageHeader* to 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, clear); +PHP_METHOD(Message, mergeFrom); PHP_METHOD(Message, readOneof); PHP_METHOD(Message, writeOneof); PHP_METHOD(Message, whichOneof); diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 5e05b935e8..1b239ee373 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -57,6 +57,31 @@ size_t native_slot_size(upb_fieldtype_t type) { } } +static bool native_slot_is_default(upb_fieldtype_t type, void* memory) { + switch (type) { +#define CASE_TYPE(upb_type, c_type) \ + case UPB_TYPE_##upb_type: { \ + return DEREF(memory, c_type) == 0; \ + } + CASE_TYPE(INT32, int32_t ) + CASE_TYPE(UINT32, uint32_t) + CASE_TYPE(ENUM, int32_t ) + CASE_TYPE(INT64, int64_t ) + CASE_TYPE(UINT64, uint64_t) + CASE_TYPE(FLOAT, float ) + CASE_TYPE(DOUBLE, double ) + CASE_TYPE(BOOL, int8_t ) + +#undef CASE_TYPE + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + return Z_STRLEN_PP(DEREF(memory, zval**)) == 0; + case UPB_TYPE_MESSAGE: + return Z_TYPE_PP(DEREF(memory, zval**)) == IS_NULL; + default: return false; + } +} + bool native_slot_set(upb_fieldtype_t type, const zend_class_entry* klass, void* memory, zval* value TSRMLS_DC) { switch (type) { @@ -499,7 +524,6 @@ void layout_init(MessageLayout* layout, void* storage, repeated_field_create_with_type(repeated_field_type, field, property_ptr TSRMLS_CC); DEREF(memory, zval**) = property_ptr; - property_ptr = NULL; } else { native_slot_init(upb_fielddef_type(field), memory, property_ptr); } @@ -601,6 +625,196 @@ void layout_set(MessageLayout* layout, MessageHeader* header, } } +void layout_merge(MessageLayout* layout, MessageHeader* from, + MessageHeader* to TSRMLS_DC) { + int i, j; + upb_msg_field_iter it; + + for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it); + upb_msg_field_next(&it), i++) { + const upb_fielddef* field = upb_msg_iter_field(&it); + + void* to_memory = slot_memory(layout, message_data(to), field); + void* from_memory = slot_memory(layout, message_data(from), field); + + if (upb_fielddef_containingoneof(field)) { + uint32_t oneof_case_offset = + layout->fields[upb_fielddef_index(field)].case_offset + + sizeof(MessageHeader); + // For a oneof, check that this field is actually present -- skip all the + // below if not. + if (DEREF(((uint8_t*)from + oneof_case_offset), uint32_t) != + upb_fielddef_number(field)) { + continue; + } + uint32_t* from_oneof_case = slot_oneof_case(layout, message_data(from), field); + uint32_t* to_oneof_case = slot_oneof_case(layout, message_data(to), field); + + // For non-singular fields, the related memory needs to point to the + // actual zval in properties table first. + switch (upb_fielddef_type(field)) { + case UPB_TYPE_MESSAGE: + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: { + int property_cache_index = + layout->fields[upb_fielddef_index(field)].cache_index; + DEREF(to_memory, zval**) = + &(to->std.properties_table)[property_cache_index]; + break; + } + default: + break; + } + + *to_oneof_case = *from_oneof_case; + + // Otherwise, fall through to the appropriate singular-field handler + // below. + } + + if (is_map_field(field)) { + int size, key_length, value_length; + MapIter map_it; + + zval* to_map_php = *DEREF(to_memory, zval**); + zval* from_map_php = *DEREF(from_memory, zval**); + Map* to_map = zend_object_store_get_object(to_map_php TSRMLS_CC); + Map* from_map = zend_object_store_get_object(from_map_php TSRMLS_CC); + + size = upb_strtable_count(&from_map->table); + if (size == 0) continue; + + for (map_begin(from_map_php, &map_it TSRMLS_CC); !map_done(&map_it); + map_next(&map_it)) { + const char* key = map_iter_key(&map_it, &key_length); + upb_value value = map_iter_value(&map_it, &value_length); + void* mem = upb_value_memory(&value); + switch (to_map->value_type) { + case UPB_TYPE_MESSAGE: { + zval* new_message; + message_create_with_type(to_map->msg_ce, &new_message TSRMLS_CC); + Z_ADDREF_P(new_message); + + zval* subdesc_php = get_ce_obj(to_map->msg_ce); + Descriptor* subdesc = + zend_object_store_get_object(subdesc_php TSRMLS_CC); + MessageHeader* sub_from = + (MessageHeader*)zend_object_store_get_object(DEREF(mem, zval*) + TSRMLS_CC); + MessageHeader* sub_to = + (MessageHeader*)zend_object_store_get_object( + new_message TSRMLS_CC); + layout_merge(subdesc->layout, sub_from, sub_to TSRMLS_CC); + DEREF(mem, zval*) = new_message; + break; + } + case UPB_TYPE_BYTES: + case UPB_TYPE_STRING: + Z_ADDREF_PP((zval**)mem); + break; + default: + break; + } + map_index_set(to_map, key, key_length, value); + } + + } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { + zval* to_array_php = *DEREF(to_memory, zval**); + zval* from_array_php = *DEREF(from_memory, zval**); + RepeatedField* to_array = + zend_object_store_get_object(to_array_php TSRMLS_CC); + RepeatedField* from_array = + zend_object_store_get_object(from_array_php TSRMLS_CC); + + int size = zend_hash_num_elements(HASH_OF(from_array->array)); + if (size > 0) { + for (j = 0; j < size; j++) { + void* memory = NULL; + zend_hash_index_find(HASH_OF(from_array->array), j, (void**)&memory); + switch (to_array->type) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: { + zval* str; + MAKE_STD_ZVAL(str); + ZVAL_STRINGL(str, Z_STRVAL_PP((zval**)memory), + Z_STRLEN_PP((zval**)memory), 1); + memory = &str; + break; + } + case UPB_TYPE_MESSAGE: { + zval* new_message; + message_create_with_type(from_array->msg_ce, &new_message TSRMLS_CC); + Z_ADDREF_P(new_message); + + zval* subdesc_php = get_ce_obj(from_array->msg_ce); + Descriptor* subdesc = + zend_object_store_get_object(subdesc_php TSRMLS_CC); + MessageHeader* sub_from = + (MessageHeader*)zend_object_store_get_object( + DEREF(memory, zval*) TSRMLS_CC); + MessageHeader* sub_to = + (MessageHeader*)zend_object_store_get_object( + new_message TSRMLS_CC); + layout_merge(subdesc->layout, sub_from, sub_to TSRMLS_CC); + + memory = &new_message; + } + default: + break; + } + repeated_field_push_native(to_array, memory TSRMLS_CC); + } + } + } else { + upb_fieldtype_t type = upb_fielddef_type(field); + zend_class_entry *ce = NULL; + if (!native_slot_is_default(type, from_memory)) { + switch (type) { +#define CASE_TYPE(upb_type, c_type) \ + case UPB_TYPE_##upb_type: { \ + DEREF(to_memory, c_type) = DEREF(from_memory, c_type); \ + break; \ + } + CASE_TYPE(INT32, int32_t) + CASE_TYPE(UINT32, uint32_t) + CASE_TYPE(ENUM, int32_t) + CASE_TYPE(INT64, int64_t) + CASE_TYPE(UINT64, uint64_t) + CASE_TYPE(FLOAT, float) + CASE_TYPE(DOUBLE, double) + CASE_TYPE(BOOL, int8_t) + +#undef CASE_TYPE + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + native_slot_set(type, NULL, value_memory(field, to_memory), + *DEREF(from_memory, zval**) TSRMLS_CC); + break; + case UPB_TYPE_MESSAGE: { + const upb_msgdef* msg = upb_fielddef_msgsubdef(field); + zval* desc_php = get_def_obj(msg); + Descriptor* desc = zend_object_store_get_object(desc_php TSRMLS_CC); + ce = desc->klass; + if (native_slot_is_default(type, to_memory)) { + zval* new_message = NULL; + message_create_with_type(ce, &new_message TSRMLS_CC); + native_slot_set(type, ce, value_memory(field, to_memory), + new_message TSRMLS_CC); + } + MessageHeader* sub_from = + (MessageHeader*)zend_object_store_get_object( + *DEREF(from_memory, zval**) TSRMLS_CC); + MessageHeader* sub_to = + (MessageHeader*)zend_object_store_get_object( + *DEREF(to_memory, zval**) TSRMLS_CC); + layout_merge(desc->layout, sub_from, sub_to TSRMLS_CC); + } + } + } + } + } +} + const char* layout_get_oneof_case(MessageLayout* layout, const void* storage, const upb_oneofdef* oneof TSRMLS_DC) { upb_oneof_iter i; diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index 9e162a225a..17ef8536ac 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -490,6 +490,74 @@ class Message } } + /** + * Merges the contents of the specified message into current message. + * + * This method merges the contents of the specified message into the + * current message. Singular fields that are set in the specified message + * overwrite the corresponding fields in the current message. Repeated + * fields are appended. Map fields key-value pairs are overritten. + * Singular/Oneof sub-messages are recursively merged. All overritten + * sub-messages are deep-copied. + * + * @param object $msg Protobuf message to be merged from. + * @return null. + */ + public function mergeFrom($msg) + { + if (get_class($this) !== get_class($msg)) { + user_error("Cannot merge messages with different class."); + return; + } + + foreach ($this->desc->getField() as $field) { + $setter = $field->getSetter(); + $getter = $field->getGetter(); + if ($field->isMap()) { + if (count($msg->$getter()) != 0) { + $value_field = $field->getMessageType()->getFieldByNumber(2); + foreach ($msg->$getter() as $key => $value) { + if ($value_field->getType() == GPBType::MESSAGE) { + $klass = $value_field->getMessageType()->getClass(); + $copy = new $klass; + $copy->mergeFrom($value); + $this->$getter()[$key] = $copy; + } else { + $this->$getter()[$key] = $value; + } + } + } + } else if ($field->getLabel() === GPBLabel::REPEATED) { + if (count($msg->$getter()) != 0) { + foreach ($msg->$getter() as $tmp) { + if ($field->getType() == GPBType::MESSAGE) { + $klass = $field->getMessageType()->getClass(); + $copy = new $klass; + $copy->mergeFrom($tmp); + $this->$getter()[] = $copy; + } else { + $this->$getter()[] = $tmp; + } + } + } + } else if ($field->getLabel() === GPBLabel::OPTIONAL) { + if($msg->$getter() !== $this->defaultValue($field)) { + $tmp = $msg->$getter(); + if ($field->getType() == GPBType::MESSAGE) { + if (is_null($this->$getter())) { + $klass = $field->getMessageType()->getClass(); + $new_msg = new $klass; + $this->$setter($new_msg); + } + $this->$getter()->mergeFrom($tmp); + } else { + $this->$setter($tmp); + } + } + } + } + } + /** * Parses a protocol buffer contained in a string. * diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index 83ce113931..7f8567b80c 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php @@ -621,6 +621,115 @@ class GeneratedClassTest extends TestBase $this->expectEmptyFields($m); } + ######################################################### + # Test mergeFrom method. + ######################################################### + + public function testMessageMergeFrom() + { + $m = new TestMessage(); + $this->setFields($m); + $this->expectFields($m); + $arr = $m->getOptionalMessage()->getB(); + $arr[] = 1; + + $n = new TestMessage(); + + // Singular + $n->setOptionalInt32(100); + $sub1 = new TestMessage_Sub(); + $sub1->setA(101); + $sub1->getB()[] = 102; + $n->setOptionalMessage($sub1); + + // Repeated + $n->getRepeatedInt32()[] = 200; + $n->getRepeatedString()[] = 'abc'; + $sub2 = new TestMessage_Sub(); + $sub2->setA(201); + $n->getRepeatedMessage()[] = $sub2; + + // Map + $n->getMapInt32Int32()[1] = 300; + $n->getMapInt32Int32()[-62] = 301; + $n->getMapStringString()['def'] = 'def'; + $n->getMapInt32Message()[1] = new TestMessage_Sub(); + $n->getMapInt32Message()[1]->setA(302); + $n->getMapInt32Message()[2] = new TestMessage_Sub(); + $n->getMapInt32Message()[2]->setA(303); + + $m->mergeFrom($n); + + $this->assertSame(100, $m->getOptionalInt32()); + $this->assertSame(42, $m->getOptionalUint32()); + $this->assertSame(101, $m->getOptionalMessage()->getA()); + $this->assertSame(2, count($m->getOptionalMessage()->getB())); + $this->assertSame(1, $m->getOptionalMessage()->getB()[0]); + $this->assertSame(102, $m->getOptionalMessage()->getB()[1]); + + $this->assertSame(3, count($m->getRepeatedInt32())); + $this->assertSame(200, $m->getRepeatedInt32()[2]); + $this->assertSame(2, count($m->getRepeatedUint32())); + $this->assertSame(3, count($m->getRepeatedString())); + $this->assertSame('abc', $m->getRepeatedString()[2]); + $this->assertSame(3, count($m->getRepeatedMessage())); + $this->assertSame(201, $m->getRepeatedMessage()[2]->getA()); + + $this->assertSame(2, count($m->getMapInt32Int32())); + $this->assertSame(300, $m->getMapInt32Int32()[1]); + $this->assertSame(301, $m->getMapInt32Int32()[-62]); + $this->assertSame(1, count($m->getMapUint32Uint32())); + $this->assertSame(2, count($m->getMapStringString())); + $this->assertSame('def', $m->getMapStringString()['def']); + + $this->assertSame(2, count($m->getMapInt32Message())); + $this->assertSame(302, $m->getMapInt32Message()[1]->getA()); + $this->assertSame(303, $m->getMapInt32Message()[2]->getA()); + + $this->assertSame("", $m->getMyOneof()); + + // Check sub-messages are copied by value. + $n->getOptionalMessage()->setA(-101); + $this->assertSame(101, $m->getOptionalMessage()->getA()); + $n->getRepeatedMessage()[0]->setA(-201); + $this->assertSame(201, $m->getRepeatedMessage()[2]->getA()); + $n->getMapInt32Message()[1]->setA(-302); + $this->assertSame(302, $m->getMapInt32Message()[1]->getA()); + + // Test merge oneof. + $m = new TestMessage(); + + $n = new TestMessage(); + $n->setOneofInt32(1); + $m->mergeFrom($n); + $this->assertSame(1, $m->getOneofInt32()); + + $sub = new TestMessage_Sub(); + $n->setOneofMessage($sub); + $n->getOneofMessage()->setA(400); + $m->mergeFrom($n); + $this->assertSame(400, $m->getOneofMessage()->getA()); + $n->getOneofMessage()->setA(-400); + $this->assertSame(400, $m->getOneofMessage()->getA()); + + // Test all fields + $m = new TestMessage(); + $n = new TestMessage(); + $this->setFields($m); + $n->mergeFrom($m); + $this->expectFields($n); + } + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testMessageMergeFromInvalidTypeFail() + { + $m = new TestMessage(); + $n = new TestMessage_Sub(); + $m->mergeFrom($n); + } + ######################################################### # Test message/enum without namespace. ######################################################### diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php index af3272734f..d4776d6f56 100644 --- a/php/tests/memory_leak_test.php +++ b/php/tests/memory_leak_test.php @@ -79,3 +79,8 @@ $data = $m->encode(); $n = new TestMessage(); $n->decode($data); assert(1 === $n->getOneofMessage()->getA()); + +$from = new TestMessage(); +$to = new TestMessage(); +TestUtil::setTestMessage($from); +$to->mergeFrom($from); diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto index 594aee4dcd..c971df214e 100644 --- a/php/tests/proto/test.proto +++ b/php/tests/proto/test.proto @@ -90,6 +90,7 @@ message TestMessage { message Sub { int32 a = 1; + repeated int32 b = 2; } // Reserved for non-existing field test. diff --git a/php/tests/test.sh b/php/tests/test.sh index 3635d86c27..fc3f018650 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -19,7 +19,9 @@ do echo "" done -# Make sure to run the memory test in debug mode. -php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php +# # Make sure to run the memory test in debug mode. +# php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php -USE_ZEND_ALLOC=0 valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php +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