From 5ca9f921ae2a9b5d8b9111465c447c202e2944cb Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 3 Feb 2015 11:21:11 -0800 Subject: [PATCH] Changed PHP metadata representation to associative array of arrays of strings --- src/php/ext/grpc/call.c | 107 +++++++++++--------------- src/php/ext/grpc/call.h | 1 + src/php/tests/unit_tests/CallTest.php | 4 +- 3 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/php/ext/grpc/call.c b/src/php/ext/grpc/call.c index bd3490b3627..6e0ae590617 100644 --- a/src/php/ext/grpc/call.c +++ b/src/php/ext/grpc/call.c @@ -85,73 +85,25 @@ zval *grpc_call_create_metadata_array(int count, grpc_metadata *elements) { memcpy(str_val, elem->value, elem->value_length); if (zend_hash_find(array_hash, str_key, key_len, (void **)data) == SUCCESS) { - switch (Z_TYPE_P(*data)) { - case IS_STRING: - MAKE_STD_ZVAL(inner_array); - array_init(inner_array); - add_next_index_zval(inner_array, *data); - add_assoc_zval(array, str_key, inner_array); - break; - case IS_ARRAY: - inner_array = *data; - break; - default: - zend_throw_exception(zend_exception_get_default(), - "Metadata hash somehow contains wrong types.", - 1 TSRMLS_CC); + if (Z_TYPE_P(*data) != IS_ARRAY) { + zend_throw_exception(zend_exception_get_default(), + "Metadata hash somehow contains wrong types.", + 1 TSRMLS_CC); efree(str_key); efree(str_val); return NULL; } - add_next_index_stringl(inner_array, str_val, elem->value_length, false); + add_next_index_stringl(*data, str_val, elem->value_length, false); } else { - add_assoc_stringl(array, str_key, str_val, elem->value_length, false); + MAKE_STD_ZVAL(inner_array); + array_init(inner_array); + add_next_index_stringl(inner_array, str_val, elem->value_length, false); + add_assoc_zval(array, str_key, inner_array); } } return array; } -int php_grpc_call_add_metadata_array_walk(void *elem TSRMLS_DC, int num_args, - va_list args, - zend_hash_key *hash_key) { - grpc_call_error error_code; - zval **data = (zval **)elem; - grpc_metadata metadata; - grpc_call *call = va_arg(args, grpc_call *); - gpr_uint32 flags = va_arg(args, gpr_uint32); - const char *key; - HashTable *inner_hash; - /* We assume that either two args were passed, and we are in the recursive - case (and the second argument is the key), or one arg was passed and - hash_key is the string key. */ - if (num_args > 2) { - key = va_arg(args, const char *); - } else { - /* TODO(mlumish): If possible, check that hash_key is a string */ - key = hash_key->arKey; - } - switch (Z_TYPE_P(*data)) { - case IS_STRING: - metadata.key = (char *)key; - metadata.value = Z_STRVAL_P(*data); - metadata.value_length = Z_STRLEN_P(*data); - error_code = grpc_call_add_metadata_old(call, &metadata, 0u); - MAYBE_THROW_CALL_ERROR(add_metadata, error_code); - break; - case IS_ARRAY: - inner_hash = Z_ARRVAL_P(*data); - zend_hash_apply_with_arguments(inner_hash TSRMLS_CC, - php_grpc_call_add_metadata_array_walk, 3, - call, flags, key); - break; - default: - zend_throw_exception(zend_exception_get_default(), - "Metadata hash somehow contains wrong types.", - 1 TSRMLS_CC); - } - return ZEND_HASH_APPLY_KEEP; -} - /** * Constructs a new instance of the Call class. * @param Channel $channel The channel to associate the call with. Must not be @@ -204,8 +156,18 @@ PHP_METHOD(Call, __construct) { PHP_METHOD(Call, add_metadata) { wrapped_grpc_call *call = (wrapped_grpc_call *)zend_object_store_get_object(getThis() TSRMLS_CC); + grpc_metadata metadata; + grpc_call_error error_code; zval *array; + zval **inner_array; + zval **value; HashTable *array_hash; + HashPosition array_pointer; + HashTable *inner_array_hash; + HashPosition inner_array_pointer; + char *key; + uint key_len; + ulong index; long flags = 0; /* "a|l" == 1 array, 1 optional long */ if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "a|l", &array, &flags) == @@ -216,9 +178,34 @@ PHP_METHOD(Call, add_metadata) { return; } array_hash = Z_ARRVAL_P(array); - zend_hash_apply_with_arguments(array_hash TSRMLS_CC, - php_grpc_call_add_metadata_array_walk, 2, - call->wrapped, (gpr_uint32)flags); + for (zend_hash_internal_pointer_reset_ex(array_hash, &array_pointer); + zend_hash_get_current_data_ex(array_hash, (void**)&inner_array, + &array_pointer) == SUCCESS; + zend_hash_move_forward_ex(array_hash, &array_pointer)) { + if (zend_hash_get_current_key_ex(array_hash, &key, &key_len, &index, 0, + &array_pointer) != HASH_KEY_IS_STRING) { + zend_throw_exception(spl_ce_InvalidArgumentException, + "metadata keys must be strings", 1 TSRMLS_CC); + return; + } + inner_array_hash = Z_ARRVAL_P(array); + for (zend_hash_internal_pointer_reset_ex(inner_array_hash, + &inner_array_pointer); + zend_hash_get_current_data_ex(inner_array_hash, (void**)&value, + &inner_array_pointer) == SUCCESS; + zend_hash_move_forward_ex(inner_array_hash, &inner_array_pointer)) { + if (Z_TYPE_P(*value) != IS_STRING) { + zend_throw_exception(spl_ce_InvalidArgumentException, + "metadata values must be arrays of strings", + 1 TSRMLS_CC); + } + metadata.key = key; + metadata.value = Z_STRVAL_P(*value); + metadata.value_length = Z_STRLEN_P(*value); + error_code = grpc_call_add_metadata_old(call->wrapped, &metadata, 0u); + MAYBE_THROW_CALL_ERROR(add_metadata, error_code); + } + } } /** diff --git a/src/php/ext/grpc/call.h b/src/php/ext/grpc/call.h index 232c5d7cf29..ba1f1e797fb 100644 --- a/src/php/ext/grpc/call.h +++ b/src/php/ext/grpc/call.h @@ -19,6 +19,7 @@ zend_throw_exception(spl_ce_LogicException, \ #func_name " was called incorrectly", \ (long)error_code TSRMLS_CC); \ + return; \ } \ } while (0) diff --git a/src/php/tests/unit_tests/CallTest.php b/src/php/tests/unit_tests/CallTest.php index 795831cb65c..b6ce974e6e9 100755 --- a/src/php/tests/unit_tests/CallTest.php +++ b/src/php/tests/unit_tests/CallTest.php @@ -46,7 +46,7 @@ class CallTest extends PHPUnit_Framework_TestCase{ } public function testAddSingleMetadata() { - $this->call->add_metadata(['key' => 'value'], 0); + $this->call->add_metadata(['key' => ['value']], 0); /* Dummy assert: Checks that the previous call completed without error */ $this->assertTrue(true); } @@ -59,7 +59,7 @@ class CallTest extends PHPUnit_Framework_TestCase{ public function testAddSingleAndMultiValueMetadata() { $this->call->add_metadata( - ['key1' => 'value1', + ['key1' => ['value1'], 'key2' => ['value2', 'value3']], 0); /* Dummy assert: Checks that the previous call completed without error */ $this->assertTrue(true);