First bunch of review fixes

.travis.yml

@@ -7,6 +7,8 @@ language: php
 php:
   - 7.0
   - 7.1
+  - 7.2
+  - 7.3
   - nightly
 
 python:

README.md

@@ -156,8 +156,8 @@ Tarantool class instance
 $tnt = new Tarantool(); // -> new Tarantool('localhost', 3301);
 $tnt = new Tarantool('tcp://test:test@localhost');
 $tnt = new Tarantool('tcp://test:test@localhost:3301');
-$tnt = new Tarantool('test:test@unix/:/var/tmp/tarantool.sock);
-$tnt = new Tarantool('unix:///var/tmp/tarantool.sock); /* if no login is needed */
+$tnt = new Tarantool('test:test@unix/:/var/tmp/tarantool.sock');
+$tnt = new Tarantool('unix:///var/tmp/tarantool.sock'); /* if no login is needed */
 ```
 
 ## Manipulation connection

lib/tarantool_server.py

@@ -211,7 +211,6 @@ def find_exe(self):
         raise RuntimeError("Can't find server executable in " + os.environ["PATH"])
 
     def generate_configuration(self):
-        # print(self.args)
         os.putenv("PRIMARY_PORT", str(self.args['primary']))
         os.putenv("ADMIN_PORT", str(self.args['admin']))
 
@@ -261,7 +260,7 @@ def start(self):
         self.generate_configuration()
         if self.script:
             shutil.copy(self.script, self.script_dst)
-            os.chmod(self.script_dst, 511)
+            os.chmod(self.script_dst, 0o777)
         args = self.prepare_args()
         self.process = subprocess.Popen(args,
                 cwd = self.vardir,
@@ -284,4 +283,4 @@ def clean(self):
 
     def __del__(self):
         self.stop()
-        # self.clean()
+        self.clean()

src/php_tarantool.h

@@ -95,7 +95,11 @@ ZEND_EXTERN_MODULE_GLOBALS(tarantool);
 
 typedef struct tarantool_object {
 	struct tarantool_connection  {
+		/* physical url, that's used for connection in raw PHP obtained
+		 * from parsed url by `tarantool_url_write_php_format` */
 		char                    *url;
+		/* parsed url, after have user/password/...
+		 * obtained from user string by `tarantool_url_parse` */
 		struct tarantool_url    *url_parsed;
 		php_stream              *stream;
 		struct tarantool_schema *schema;

src/tarantool.c

@@ -154,7 +154,6 @@ pid_gen(struct tarantool_url *url, const char *user, const char *prefix,
 	const char *suffix, size_t suffix_len, size_t *olen) {
 
 	char *plist_id = NULL, *tmp = NULL;
-	/* if user is not defined, then user is 'guest' */
 	int len = 0;
 	if (url->type == TARANTOOL_URL_TCP) {
 		len = spprintf(&plist_id, 0, "tarantool-%s:id=%s:%d-%s",
@@ -265,8 +264,8 @@ static int __tarantool_connect(tarantool_object *t_obj) {
 			continue;
 		if (tntll_stream_read2(obj->stream, obj->greeting,
 				       GREETING_SIZE) != GREETING_SIZE) {
-			// int errno = php_stream_errno();
-			spprintf(&err, 0, "Fails to read greeting [%d]: %s", errno, strerror(errno));
+			spprintf(&err, 0, "Fails to read greeting [%d]: %s",
+				 errno, strerror(errno));
 			continue;
 		}
 		if (php_tp_verify_greetings(obj->greeting) == 0) {
@@ -278,7 +277,7 @@ static int __tarantool_connect(tarantool_object *t_obj) {
 	}
 	if (count == 0) {
 ioexception:
-		tarantool_throw_ioexception("%s ", err);
+		tarantool_throw_ioexception("%s", err);
 		efree(err);
 		return FAILURE;
 	}
@@ -294,7 +293,7 @@ inline static int __tarantool_reconnect(tarantool_object *t_obj) {
 	return __tarantool_connect(t_obj);
 }
 
-static void 
+static void
 tarantool_connection_free(tarantool_connection *obj, int is_persistent
 			  TSRMLS_DC) {
 	if (obj == NULL)
@@ -303,7 +302,7 @@ tarantool_connection_free(tarantool_connection *obj, int is_persistent
 		pefree(obj->greeting, is_persistent);
 		obj->greeting = NULL;
 	}
-	// tarantool_stream_close(obj);
+	tarantool_stream_close(obj);
 	if (obj->persistent_id) {
 		zend_string_release(obj->persistent_id);
 		obj->persistent_id = NULL;
@@ -626,10 +625,11 @@ int obtain_space_by_spaceno(tarantool_connection *obj, uint32_t space_no) {
 	size_t body_size = php_mp_unpack_package_size(pack_len);
 	smart_string_ensure(obj->value, body_size);
 	if (tarantool_stream_read(obj, obj->value->c,
-				body_size) == FAILURE)
+				  body_size) == FAILURE)
 		return FAILURE;
 
-	struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
+	struct tnt_response resp;
+	memset(&resp, 0, sizeof(struct tnt_response));
 	if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
 		tarantool_throw_parsingexception("query");
 		return FAILURE;
@@ -682,10 +682,11 @@ int get_spaceno_by_name(tarantool_connection *obj, zval *name) {
 	size_t body_size = php_mp_unpack_package_size(pack_len);
 	smart_string_ensure(obj->value, body_size);
 	if (tarantool_stream_read(obj, obj->value->c,
-				body_size) == FAILURE)
+				  body_size) == FAILURE)
 		return FAILURE;
 
-	struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
+	struct tnt_response resp;
+	memset(&resp, 0, sizeof(struct tnt_response));
 	if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
 		tarantool_throw_parsingexception("query");
 		return FAILURE;
@@ -749,7 +750,8 @@ int get_indexno_by_name(tarantool_connection *obj, int space_no,
 	if (tarantool_stream_read(obj, obj->value->c, body_size) == FAILURE)
 		return FAILURE;
 
-	struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
+	struct tnt_response resp;
+	memset(&resp, 0, sizeof(struct tnt_response));
 	if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
 		tarantool_throw_parsingexception("query");
 		return FAILURE;
@@ -804,7 +806,8 @@ int get_fieldno_by_name(tarantool_connection *obj, uint32_t space_no,
 	if (tarantool_stream_read(obj, obj->value->c, body_size) == FAILURE)
 		return FAILURE;
 
-	struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
+	struct tnt_response resp;
+	memset(&resp, 0, sizeof(struct tnt_response));
 	if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
 		tarantool_throw_parsingexception("query");
 		return FAILURE;
@@ -1119,9 +1122,9 @@ PHP_METHOD(Tarantool, __construct) {
 		suffix_or_port = &suffix_or_port_default;
 	}
 
-	if (strstr(host, "tcp://" ) != 0 ||
-	    strstr(host, "unix://") != 0 ||
-	    strstr(host, "unix/:" ) != 0 ||
+	if (strstr(host, "tcp://" ) != NULL ||
+	    strstr(host, "unix://") != NULL ||
+	    strstr(host, "unix/:" ) != NULL ||
 	    Z_TYPE_P(suffix_or_port) == IS_STRING) {
 		url = estrdup(host);
 		url_len = host_len;
@@ -1173,34 +1176,6 @@ PHP_METHOD(Tarantool, __construct) {
 	if (url_parsed == NULL) {
 		THROW_EXC("failed to parse url: '%s'", url);
 		RETURN_FALSE;
-	} else {
-		/* set default user
-		if (url_parsed->user == NULL) {
-			url_parsed->user = estrdup("guest");
-		} */
-		/* set default password
-		if (url_parsed->pass && strlen(url_parsed->pass) == 0) {
-			efree(url_parsed->pass);
-			url_parsed->pass = NULL;
-		} */
-		/* try to deduct scheme (based on host/path presence)
-		if (url_parsed->scheme == NULL) {
-			if (url_parsed->host != NULL) {
-				url_parsed->scheme = estrdup("tcp");
-			} else if (url_parsed->path != NULL) {
-				url_parsed->scheme = estrdup("unix");
-			} else {
-				THROW_EXC("Unknown url: %s", url);
-				RETURN_FALSE;
-			}
-		} */
-		/* check that protocik is supported
-		if (strcmp(url_parsed->scheme, "tcp") != 0 &&
-		    strcmp(url_parsed->scheme, "unix") != 0) { 
-			THROW_EXC("Unsupported protocol: %s",
-				  url_parsed->scheme);
-			RETURN_FALSE;
-		} */
 	}
 	efree(url);
 
@@ -1212,7 +1187,7 @@ PHP_METHOD(Tarantool, __construct) {
 
 		le = zend_hash_find_ptr(&EG(persistent_list), plist_id);
 		if (le != NULL) {
-			/* It's likely §*/
+			/* It's likely */
 			if (le->type == php_tarantool_list_entry()) {
 				obj = (tarantool_connection *) le->ptr;
 				plist_new_entry = false;
@@ -1245,7 +1220,6 @@ PHP_METHOD(Tarantool, __construct) {
 		char *url_s = pestrdup(obj->url, is_persistent);
 		efree(obj->url);
 		obj->url = url_s;
-		/* If pass == NULL, then authenticate without password */
 		if (is_persistent) {
 			obj->persistent_id = pid_pzsgen(obj->url_parsed,
 							obj->url_parsed->user,

src/tarantool_schema.c

@@ -14,20 +14,24 @@
 #define MUR_SEED 13
 #include "third_party/msgpuck.h"
 
-#define MH_DEFINE_CMPFUNC(NAME, TYPE)								\
-	int mh_##NAME##cmp_eq(const TYPE **lval, const TYPE **rval, void *arg) {		\
-		(void *) arg;									\
-		if ((*lval)->key.id_len != (*rval)->key.id_len)					\
-			return 0;								\
-		return !memcmp((*lval)->key.id, (*rval)->key.id, (*rval)->key.id_len);		\
-	}											\
-												\
-	int mh_##NAME##cmp_key_eq(const struct schema_key *key, const TYPE **val, void *arg) {	\
-		(void *) arg;									\
-		if (key->id_len != (*val)->key.id_len)						\
-			return 0;								\
-		return !memcmp(key->id, (*val)->key.id, key->id_len);				\
+#define MH_DEFINE_CMPFUNC(NAME, TYPE)					      \
+	int mh_##NAME##cmp_eq(const TYPE **lval, const TYPE **rval, void *arg)\
+	{								      \
+		(void *) arg;						      \
+		if ((*lval)->key.id_len != (*rval)->key.id_len)		      \
+			return 0;					      \
+		return !memcmp((*lval)->key.id, (*rval)->key.id,	      \
+				(*rval)->key.id_len);			      \
+	}								      \
+									      \
+	int mh_##NAME##cmp_key_eq(const struct schema_key *key,		      \
+				  const TYPE **val, void *arg) {	      \
+		(void *) arg;						      \
+		if (key->id_len != (*val)->key.id_len)			      \
+			return 0;					      \
+		return !memcmp(key->id, (*val)->key.id, key->id_len);	      \
 	}
+
 MH_DEFINE_CMPFUNC(field, struct schema_field_value);
 MH_DEFINE_CMPFUNC(index, struct schema_index_value);
 MH_DEFINE_CMPFUNC(space, struct schema_space_value);
@@ -37,7 +41,8 @@ MH_DEFINE_CMPFUNC(space, struct schema_space_value);
 
 #define mh_eq(a, b, arg)     mh_fieldcmp_eq(a, b, arg)
 #define mh_eq_key(a, b, arg) mh_fieldcmp_key_eq(a, b, arg)
-#define mh_hash(x, arg)      PMurHash32(MUR_SEED, (*x)->key.id, (*x)->key.id_len)
+#define mh_hash(x, arg)      PMurHash32(MUR_SEED, (*x)->key.id, \
+					(*x)->key.id_len)
 #define mh_hash_key(x, arg)  PMurHash32(MUR_SEED, (x)->id, (x)->id_len);
 
 #define mh_node_t struct schema_field_value *

src/tarantool_url.c

@@ -16,7 +16,8 @@ url_part_copy(const char *part, size_t part_len, int persistent) {
 
 struct tarantool_url *
 tarantool_url_parse(const char *url, int persistent) {
-	struct uri parsed; memset(&parsed, 0, sizeof(struct uri));
+	struct uri parsed;
+	memset(&parsed, 0, sizeof(struct uri));
 
 	if (uri_parse(&parsed, url) == -1)
 		return NULL;
@@ -67,8 +68,8 @@ tarantool_url_parse(const char *url, int persistent) {
 			host_len = parsed.host_len;
 		}
 		if (parsed.service == NULL) {
-			port     = "3303";
-			port_len = sizeof("3303") - 1;
+			port     = "3301";
+			port_len = sizeof("3301") - 1;
 		} else {
 			port     = parsed.service;
 			port_len = parsed.service_len;

src/tarantool_url.h

@@ -25,6 +25,9 @@ tarantool_url_parse(const char *url, int persistent);
 void
 tarantool_url_free(struct tarantool_url *url, int persistent);
 
+/*
+ * Construct url in PHP format (without user/password)
+ */
 const char *
 tarantool_url_write_php_format(struct tarantool_url *url, int persistent);
 

test-run.py

@@ -1,17 +1,14 @@
 #!/usr/bin/env python
+# pylint: disable=missing-docstring
+from __future__ import print_function
 
 import os
 import sys
-import shlex
 import shutil
 import subprocess
 
-import time
-
 from lib.tarantool_server import TarantoolServer
 
-from pprint import pprint
-
 PHPUNIT_PHAR = 'phpunit.phar'
 
 def read_popen(cmd):
@@ -25,9 +22,9 @@ def read_popen_config(cmd):
 
 def find_php_bin():
     path = read_popen('which php').strip().decode()
-    if (path.find('phpenv') != -1):
+    if path.find('phpenv') != -1:
         version = read_popen('phpenv global')
-        if (version.find('system') != -1):
+        if version.find('system') != -1:
             return '/usr/bin/php'
         return '~/.phpenv/versions/{0}/bin/php'.format(version.strip())
     return path
@@ -38,9 +35,9 @@ def prepare_env(php_ini):
     if not os.path.isdir('var') and not os.path.exists('var'):
         os.mkdir('var')
     shutil.copy('test/shared/phpunit.xml', 'var')
-    test_dir_path = os.path.abspath(os.path.join(os.getcwd(), 'test'))
-    test_lib_path = os.path.join(test_dir_path, PHPUNIT_PHAR)
-#    shutil.copy('test/shared/tarantool.ini', 'var')
+    # test_dir_path = os.path.abspath(os.path.join(os.getcwd(), 'test'))
+    # test_lib_path = os.path.join(test_dir_path, PHPUNIT_PHAR)
+    # shutil.copy('test/shared/tarantool.ini', 'var')
     shutil.copy(php_ini, 'var')
     shutil.copy('modules/tarantool.so', 'var')
 
@@ -60,9 +57,9 @@ def run_tests(vardir):
         print('Running against ' + version)
 
         for php_ini in [
-            'test/shared/tarantool-1.ini'
-            # 'test/shared/tarantool-2.ini',
-            # 'test/shared/tarantool-3.ini'
+                'test/shared/tarantool-1.ini',
+                'test/shared/tarantool-2.ini',
+                'test/shared/tarantool-3.ini'
         ]:
             cmd = ''
             shutil.copy(php_ini, os.path.join(test_cwd, 'tarantool.ini'))
@@ -98,20 +95,18 @@ def run_tests(vardir):
 
             print('Running "%s" with "%s"' % (cmd, php_ini))
             proc = subprocess.Popen(cmd, shell=True, cwd=test_cwd)
-            rv = proc.wait()
-            if rv != 0:
+            if proc.wait() != 0:
                 print('Error')
-                return -1
+                return
             if '--gdb' in sys.argv or '--lldb' in sys.argv:
-                return -1
+                return
 
     finally:
-        a = [
+        for elem in [
                 os.path.join(test_cwd, 'tarantool.ini'),
                 os.path.join(test_cwd, 'phpunit.xml'),
                 os.path.join(test_cwd, 'tarantool.so'),
-        ]
-        for elem in a:
+        ]:
             if os.path.exists(elem):
                 os.remove(elem)