Merge pull request #12775 from thinkerou/fix_todo

PHP: complete some todo

src/php/ext/grpc/php7_wrapper.h

@@ -30,6 +30,8 @@
   add_property_string(arg, name, context, b)
 #define php_grpc_add_property_stringl(res, name, str, len, b) \
   add_property_stringl(res, name, str, len, b)
+#define php_grpc_add_property_zval(res, name, val) \
+  add_property_zval(res, name, val)
 #define php_grpc_add_next_index_stringl(data, str, len, b) \
   add_next_index_stringl(data, str, len, b)
 
@@ -154,6 +156,11 @@ static inline int php_grpc_zend_hash_find(HashTable *ht, char *key, int len,
   add_property_string(arg, name, context)
 #define php_grpc_add_property_stringl(res, name, str, len, b) \
   add_property_stringl(res, name, str, len)
+#define php_grpc_add_property_zval(res, name, val) do { \
+  zval tmp; \
+  tmp = *val; \
+  add_property_zval(res, name, &tmp); \
+  } while(0)
 #define php_grpc_add_next_index_stringl(data, str, len, b) \
   add_next_index_stringl(data, str, len)
 

src/php/ext/grpc/server.c

@@ -89,8 +89,6 @@ PHP_METHOD(Server, __construct) {
   if (args_array == NULL) {
     server->wrapped = grpc_server_create(NULL, NULL);
   } else {
-    //TODO(thinkerou): deal it if key of array is long, crash now on php7
-    // and update unit test case
     php_grpc_read_args_array(args_array, &args TSRMLS_CC);
     server->wrapped = grpc_server_create(&args, NULL);
     efree(args.args);
@@ -140,25 +138,12 @@ PHP_METHOD(Server, requestCall) {
   php_grpc_add_property_string(result, "host", host_text, true);
   gpr_free(method_text);
   gpr_free(host_text);
-#if PHP_MAJOR_VERSION < 7
+  php_grpc_add_property_zval(result, "call",
-  add_property_zval(result, "call", grpc_php_wrap_call(call, true TSRMLS_CC));
+                             grpc_php_wrap_call(call, true TSRMLS_CC));
-  add_property_zval(result, "absolute_deadline",
+  php_grpc_add_property_zval(result, "absolute_deadline",
                              grpc_php_wrap_timeval(details.deadline TSRMLS_CC));
-  add_property_zval(result, "metadata", grpc_parse_metadata_array(&metadata
+  php_grpc_add_property_zval(result, "metadata",
-                                                                  TSRMLS_CC));
+                             grpc_parse_metadata_array(&metadata TSRMLS_CC));
-#else
-  zval zv_call;
-  zval zv_timeval;
-  zval zv_md;
-  //TODO(thinkerou): why use zval* to unit test error?
-  zv_call = *grpc_php_wrap_call(call, true);
-  zv_timeval = *grpc_php_wrap_timeval(details.deadline);
-  zv_md = *grpc_parse_metadata_array(&metadata);
-
-  add_property_zval(result, "call", &zv_call);
-  add_property_zval(result, "absolute_deadline", &zv_timeval);
-  add_property_zval(result, "metadata", &zv_md);
-#endif
 
  cleanup:
   grpc_call_details_destroy(&details);
@@ -202,9 +187,9 @@ PHP_METHOD(Server, addSecureHttp2Port) {
   if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sO", &addr, &addr_len,
                             &creds_obj, grpc_ce_server_credentials) ==
       FAILURE) {
-    zend_throw_exception(
+    zend_throw_exception(spl_ce_InvalidArgumentException,
-        spl_ce_InvalidArgumentException,
+                         "add_http2_port expects a string and a "
-        "add_http2_port expects a string and a ServerCredentials", 1 TSRMLS_CC);
+                         "ServerCredentials", 1 TSRMLS_CC);
     return;
   }
   wrapped_grpc_server_credentials *creds =

src/php/tests/unit_tests/ServerTest.php

@@ -96,6 +96,24 @@ class ServerTest extends PHPUnit_Framework_TestCase
         $this->assertNull($this->server);
     }
 
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testInvalidConstructorWithNumKeyOfArray()
+    {
+        $this->server = new Grpc\Server([10 => '127.0.0.1',
+                                         20 => '8080', ]);
+        $this->assertNull($this->server);
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testInvalidConstructorWithList()
+    {
+        $this->server = new Grpc\Server(['127.0.0.1', '8080']);
+        $this->assertNull($this->server);
+    }
     /**
      * @expectedException InvalidArgumentException
      */