Replace deprecated spl_object_hash() with spl_object_id() in ConnectionErrorProof (#1738)

php 8.6 deprecates spl_object_hash(). ConnectionErrorProof only uses it as an array key for per-connection exceptions, so spl_object_id() is a drop-in replacement. added a dedicated test for the pipeline.

CHANGELOG.md

@@ -12,6 +12,7 @@
 - Fixed cluster CAS transactions returning transaction objects for reads and sending `MULTI` twice
 - Fixed `MultiExec::multi()` being ignored as the first call of a CAS transaction
 - Fixed cleanup of empty transactions and transaction callbacks that throw PHP errors
+- Fixed `spl_object_hash()` deprecation on PHP 8.6 in `ConnectionErrorProof` pipeline
 
 ### Security
 - Fixed aborted cluster transactions leaving node connections in `MULTI`

src/Pipeline/ConnectionErrorProof.php

@@ -101,10 +101,10 @@ class ConnectionErrorProof extends Pipeline
             $command = $commands->dequeue();
 
             $cmdConnection = $connection->getConnectionByCommand($command);
-            $connectionHash = spl_object_hash($cmdConnection);
+            $connectionId = spl_object_id($cmdConnection);
 
-            if (isset($exceptions[$connectionHash])) {
+            if (isset($exceptions[$connectionId])) {
-                $responses[$i] = $exceptions[$connectionHash];
+                $responses[$i] = $exceptions[$connectionId];
                 continue;
             }
 
@@ -112,7 +112,7 @@ class ConnectionErrorProof extends Pipeline
                 $responses[$i] = $cmdConnection->readResponse($command);
             } catch (CommunicationException $exception) {
                 $responses[$i] = $exception;
-                $exceptions[$connectionHash] = $exception;
+                $exceptions[$connectionId] = $exception;
             }
         }
 

tests/Predis/Pipeline/ConnectionErrorProofTest.php

@@ -0,0 +1,160 @@
+<?php
+
+/*
+ * This file is part of the Predis package.
+ *
+ * (c) 2009-2020 Daniele Alessandri
+ * (c) 2021-2026 Till Krüss
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Predis\Pipeline;
+
+use Predis\Client;
+use Predis\Command\CommandInterface;
+use Predis\Command\Redis\PING;
+use Predis\Connection\ConnectionException;
+use Predis\NotSupportedException;
+use PredisTestCase;
+
+class ConnectionErrorProofTest extends PredisTestCase
+{
+    /**
+     * @group disconnected
+     */
+    public function testReturnsExceptionForEachCommandOnWriteFailureWithSingleConnection(): void
+    {
+        $buffer = (new PING())->serializeCommand() . (new PING())->serializeCommand() . (new PING())->serializeCommand();
+
+        $connection = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $exception = new ConnectionException($connection, 'Connection failed');
+
+        $connection
+            ->expects($this->once())
+            ->method('write')
+            ->with($buffer)
+            ->willThrowException($exception);
+        $connection
+            ->expects($this->never())
+            ->method('readResponse');
+
+        $pipeline = new ConnectionErrorProof(new Client($connection));
+
+        $pipeline->ping();
+        $pipeline->ping();
+        $pipeline->ping();
+
+        $this->assertSame([$exception, $exception, $exception], $pipeline->execute());
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testReturnsExceptionOfEachFailedConnectionWithClusterConnection(): void
+    {
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $exception1 = new ConnectionException($connection1, 'Connection to node 1 failed');
+        $connection1
+            ->expects($this->exactly(2))
+            ->method('write');
+        $connection1
+            ->expects($this->once())
+            ->method('readResponse')
+            ->willThrowException($exception1);
+
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
+        $exception2 = new ConnectionException($connection2, 'Connection to node 2 failed');
+        $connection2
+            ->expects($this->exactly(2))
+            ->method('write');
+        $connection2
+            ->expects($this->once())
+            ->method('readResponse')
+            ->willThrowException($exception2);
+
+        $cluster = $this->getMockClusterConnection(['foo' => $connection1, 'bar' => $connection2]);
+
+        $pipeline = new ConnectionErrorProof(new Client($cluster));
+
+        $pipeline->get('foo');
+        $pipeline->get('bar');
+        $pipeline->get('foo');
+        $pipeline->get('bar');
+
+        $this->assertSame([$exception1, $exception2, $exception1, $exception2], $pipeline->execute());
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testReadsResponsesFromHealthyConnectionsWhenOneFailsWithClusterConnection(): void
+    {
+        $connection1 = $this->getMockConnection('tcp://127.0.0.1:7001');
+        $exception = new ConnectionException($connection1, 'Connection to node 1 failed');
+        $connection1
+            ->expects($this->exactly(2))
+            ->method('write');
+        $connection1
+            ->expects($this->once())
+            ->method('readResponse')
+            ->willThrowException($exception);
+
+        $connection2 = $this->getMockConnection('tcp://127.0.0.1:7002');
+        $connection2
+            ->expects($this->exactly(2))
+            ->method('write');
+        $connection2
+            ->expects($this->exactly(2))
+            ->method('readResponse')
+            ->willReturnOnConsecutiveCalls('value1', 'value2');
+
+        $cluster = $this->getMockClusterConnection(['foo' => $connection1, 'bar' => $connection2]);
+
+        $pipeline = new ConnectionErrorProof(new Client($cluster));
+
+        $pipeline->get('foo');
+        $pipeline->get('bar');
+        $pipeline->get('foo');
+        $pipeline->get('bar');
+
+        $this->assertSame([$exception, 'value1', $exception, 'value2'], $pipeline->execute());
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testThrowsExceptionOnUnsupportedConnection(): void
+    {
+        $this->expectException(NotSupportedException::class);
+        $this->expectExceptionMessageMatches('/^The connection class \'.+\' is not supported\.$/');
+
+        $connection = $this->getMockBuilder('Predis\Connection\Replication\ReplicationInterface')->getMock();
+
+        $pipeline = new ConnectionErrorProof(new Client($connection));
+
+        $pipeline->ping();
+        $pipeline->execute();
+    }
+
+    /**
+     * Returns a mocked cluster connection routing commands by their first key.
+     *
+     * @param array $connectionsByKey Node connections indexed by key
+     *
+     * @return \PHPUnit\Framework\MockObject\MockObject|\Predis\Connection\Cluster\ClusterInterface
+     */
+    private function getMockClusterConnection(array $connectionsByKey)
+    {
+        $cluster = $this->getMockBuilder('Predis\Connection\Cluster\ClusterInterface')->getMock();
+        $cluster
+            ->expects($this->atLeastOnce())
+            ->method('getConnectionByCommand')
+            ->willReturnCallback(static function (CommandInterface $command) use ($connectionsByKey) {
+                return $connectionsByKey[$command->getArgument(0)];
+            });
+
+        return $cluster;
+    }
+}