Fix/curl connection handling (#214)

* Updated the handling of curl connections * Updated the handling of curl connections * Updated tests, fixed codestyle * Fixed static analysis and codestyle * Fixed static analysis * Fixed static analysis and tests * Updated the handling of curl connections * Fixed codestyle, static analysis and tests for php 7.4 --------- Co-authored-by: nick <[email protected]>
manticoresoftware · Jun 25, 2024 · 0477b5c · 0477b5c
1 parent d323be1
commit 0477b5c
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 33 deletions.
diff --git a/src/Manticoresearch/Client.php b/src/Manticoresearch/Client.php
@@ -35,11 +35,6 @@ class Client
 	 */
 	const VERSION = '1.0.0';
 
-	/**
-	 * @var string
-	 */
-	protected $id;
-
 	/**
 	 * @var array
 	 */
@@ -66,7 +61,6 @@ class Client
 	 * $config['connectionStrategy'] = class name of pool strategy
 	 */
 	public function __construct($config = [], LoggerInterface $logger = null) {
-		$this->id = uniqid('', true);
 		$this->setConfig($config);
 		$this->logger = $logger ?? new NullLogger();
 		$this->initConnections();
@@ -371,7 +365,7 @@ public function explainQuery(array $params = []) {
 	public function request(Request $request, array $params = []): Response {
 		try {
 			$connection = $this->connectionPool->getConnection();
-			$this->lastResponse = $connection->getTransportHandler($this->logger, [$this->id])
+			$this->lastResponse = $connection->getTransportHandler($this->logger)
 				->execute($request, $params);
 		} catch (NoMoreNodesException $e) {
 			$e->setRequest($request);

diff --git a/src/Manticoresearch/Connection.php b/src/Manticoresearch/Connection.php
@@ -24,7 +24,13 @@ class Connection
 	 * @var bool
 	 */
 	protected $alive = true;
-/*
+
+	/**
+	 * @var resource
+	 */
+	protected $curl = null;
+
+	/*
  * $params['transport']  = transport class name
  * $params['host']       = hostname
  * $params['path']       = path
@@ -60,6 +66,10 @@ public function __construct(array $params) {
 		];
 		$this->config = array_merge($this->config, $params);
 		$this->alive = true;
+		if (!$this->config['persistent']) {
+			return;
+		}
+		$this->curl = curl_init();
 	}
 
 	/**
@@ -234,4 +244,13 @@ public function isAlive(): bool {
 	public function mark(bool $state) {
 		$this->alive = $state;
 	}
+
+	/**
+	 * @return resource|null
+	 *
+	 */
+	public function getCurl() {
+		return $this->curl ?? curl_init();
+	}
+
 }
diff --git a/src/Manticoresearch/Transport/Http.php b/src/Manticoresearch/Transport/Http.php
@@ -13,6 +13,7 @@
 use Manticoresearch\Request;
 use Manticoresearch\Response;
 use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 
 /**
  * Class Http
@@ -26,31 +27,22 @@ class Http extends \Manticoresearch\Transport implements TransportInterface
 	 */
 	protected $scheme = 'http';
 
-	/**
-	 * @var string
-	 */
-	protected $clientId;
-
-	protected static $curl = null;
-
 	/**
 	 * HTTP Transport constructor.
 	 * @param Connection|null $connection
 	 * @param LoggerInterface|null $logger
-	 * @param string|null $clientId
 	 */
 	public function __construct(
 		Connection $connection = null,
-		LoggerInterface $logger = null,
-		string $clientId = null
+		LoggerInterface $logger = null
 	) {
-		parent::__construct($connection, $logger);
-		$this->clientId = $clientId;
+		$this->connection = $connection;
+		$this->logger = $logger ?? new NullLogger();
 	}
 
 	public function execute(Request $request, $params = []) {
 		$connection = $this->getConnection();
-		$conn = $this->getCurlConnection($connection->getConfig('persistent'));
+		$conn = $connection->getCurl();
 		$url = $this->scheme . '://' . $connection->getHost() . ':' . $connection->getPort() . $connection->getPath();
 		$endpoint = $request->getPath();
 		$url .= $endpoint;
@@ -125,8 +117,6 @@ public function execute(Request $request, $params = []) {
 		//hard error
 		if ($errorno > 0) {
 			$error = curl_error($conn);
-
-			static::$curl = null;
 			throw new ConnectionException($error, $request);
 		}
 
@@ -159,14 +149,10 @@ public function execute(Request $request, $params = []) {
 		return $response;
 	}
 
-	protected function getCurlConnection(bool $persistent = true) {
-		if (!$persistent || ($this->clientId === null) || (static::$curl === null)
-			|| !isset(static::$curl[$this->clientId])) {
-			if (static::$curl === null) {
-				static::$curl = [];
-			}
-			static::$curl[$this->clientId] = curl_init();
-		}
-		return static::$curl[$this->clientId];
+	/**
+	 * @return Connection|null
+	 */
+	public function getConnection() {
+		return $this->connection;
 	}
 }
diff --git a/test/Manticoresearch/ConnectionTest.php b/test/Manticoresearch/ConnectionTest.php
@@ -112,7 +112,8 @@ public function testStaticCreateSelf() {
 
 	public function testStaticCreateEmptyParams() {
 		$newConnection = Connection::create([]);
-		$this->assertEquals($this->connection, $newConnection);
+		// Fix for php 7.4 since it treats Connection objects with different curl instances as not equal
+		$this->assertEquals($this->connection->getConfig(), $newConnection->getConfig());
 	}
 
 	public function testStaticCreateInvalidParams() {