Browse Source

Use "persistent" with non-bool strings to open different persistent connections.

stream_socket_client() has the undocumented ability to open different persistent
streams by providing a path in the $address string. Previously we supported this
behaviour with a combination of "persistent" and "path" (see #139) but this can
be confusing, especially now that we support the redis:// scheme which uses the
path part of an URI string to specify a database number.

After this change, instead of using an URI string such as:

  $parameters = 'tcp://127.0.0.1/first?persistent=1&database=5';

You should use the following ones:

  $parameters = 'tcp://127.0.0.1?persistent=first&database=5';
  $parameters = 'redis://127.0.0.1/5?persistent=first';

Avoiding "path" makes even more sense when using array connection parameters:

  $parameters = [
    'host'       => '127.0.0.1',
    'database'   => 5,
    'persistent' => 'first',
  ]

This feature is not supported when using UNIX domain sockets because the path
trick of stream_socket_client() does not play well with the actual path of the
socket file. The client will throw an InvalidArgumentException exception to
notify the user.

NOTE: unfortunately we have to disable the tests for persistent connections when
running under HHVM due to a bug in their implementation of get_resource_type()
preventing us to recognize a persistent stream from userland code.
Daniele Alessandri 9 năm trước cách đây
mục cha
commit
8277afc7a8

+ 7 - 0
CHANGELOG.md

@@ -6,6 +6,13 @@ v1.1.0 (2015-xx-xx)
   `RENAMENX`, `HSET`, `HSETNX`, `HEXISTS`, `SISMEMBER`, `SMOVE`. The original
   integer value is returned instead.
 
+- Non-boolean string values passed to the `persistent` connection parameter can
+  be used to create different persistent connections. Note that this feature was
+  already present in Predis, but required both `persistent` and `path` to be set
+  as illustrated by [#139](https://github.com/nrk/predis/pull/139). This change
+  is needed to prevent confusion with how `path` is used to select a database
+  with the `redis` scheme.
+
 
 v1.0.1 (2015-01-02)
 ================================================================================

+ 19 - 7
src/Connection/StreamConnection.php

@@ -132,9 +132,14 @@ class StreamConnection extends AbstractConnection
             $flags |= STREAM_CLIENT_ASYNC_CONNECT;
         }
 
-        if (isset($parameters->persistent) && $parameters->persistent) {
-            $flags |= STREAM_CLIENT_PERSISTENT;
-            $address .= strpos($path = $parameters->path, '/') === 0 ? $path : "/$path";
+        if (isset($parameters->persistent)) {
+            if (false !== $persistent = filter_var($parameters->persistent, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE)) {
+                $flags |= STREAM_CLIENT_PERSISTENT;
+
+                if ($persistent === null) {
+                    $address = "{$address}/{$parameters->persistent}";
+                }
+            }
         }
 
         $resource = $this->createStreamSocket($parameters, $address, $flags);
@@ -155,14 +160,21 @@ class StreamConnection extends AbstractConnection
             throw new \InvalidArgumentException('Missing UNIX domain socket path.');
         }
 
-        $address = "unix://{$parameters->path}";
         $flags = STREAM_CLIENT_CONNECT;
 
-        if (isset($parameters->persistent) && $parameters->persistent) {
-            $flags |= STREAM_CLIENT_PERSISTENT;
+        if (isset($parameters->persistent)) {
+            if (false !== $persistent = filter_var($parameters->persistent, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE)) {
+                $flags |= STREAM_CLIENT_PERSISTENT;
+
+                if ($persistent === null) {
+                    throw new \InvalidArgumentException(
+                        'Persistent connection IDs are not supported when using UNIX domain sockets.'
+                    );
+                }
+            }
         }
 
-        $resource = $this->createStreamSocket($parameters, $address, $flags);
+        $resource = $this->createStreamSocket($parameters, "unix://{$parameters->path}", $flags);
 
         return $resource;
     }

+ 26 - 0
tests/PHPUnit/PredisConnectionTestCase.php

@@ -334,6 +334,32 @@ abstract class PredisConnectionTestCase extends PredisTestCase
         );
     }
 
+    /**
+     * Asserts that the connection is using a persistent resource stream.
+     *
+     * This assertion will trigger a connect() operation if the connection has
+     * not been open yet.
+     *
+     * @param NodeConnectionInterface $connection Connection instance.
+     */
+    protected function assertPersistentConnection(NodeConnectionInterface $connection)
+    {
+        $this->assertSame('persistent stream', get_resource_type($connection->getResource()));
+    }
+
+    /**
+     * Asserts that the connection is not using a persistent resource stream.
+     *
+     * This assertion will trigger a connect() operation if the connection has
+     * not been open yet.
+     *
+     * @param NodeConnectionInterface $connection Connection instance.
+     */
+    protected function assertNonPersistentConnection(NodeConnectionInterface $connection)
+    {
+        $this->assertSame('stream', get_resource_type($connection->getResource()));
+    }
+
     /**
      * Returns a new instance of a connection instance.
      *

+ 10 - 0
tests/PHPUnit/PredisTestCase.php

@@ -31,6 +31,16 @@ abstract class PredisTestCase extends \PHPUnit_Framework_TestCase
         usleep($seconds * 1000000);
     }
 
+    /**
+     * Returns if the current runtime is HHVM.
+     *
+     * @return bool
+     */
+    protected function isHHVM()
+    {
+        return defined('HHVM_VERSION');
+    }
+
     /**
      * Verifies that a Redis command is a valid Predis\Command\CommandInterface
      * instance with the specified ID and command arguments.

+ 84 - 0
tests/Predis/Connection/CompositeStreamConnectionTest.php

@@ -113,6 +113,90 @@ class CompositeStreamConnectionTest extends PredisConnectionTestCase
         $this->assertSame(array('foo', 'hoge', 'lol'), iterator_to_array($iterator));
     }
 
+    /**
+     * @group connected
+     */
+    public function testPersistentParameterWithFalseLikeValues()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new CompositeStreamConnection($this->getParameters(array('persistent' => 0)));
+        $this->assertNonPersistentConnection($connection1);
+
+        $connection2 = new CompositeStreamConnection($this->getParameters(array('persistent' => false)));
+        $this->assertNonPersistentConnection($connection2);
+
+        $connection3 = new CompositeStreamConnection($this->getParameters(array('persistent' => '0')));
+        $this->assertNonPersistentConnection($connection3);
+
+        $connection4 = new CompositeStreamConnection($this->getParameters(array('persistent' => 'false')));
+        $this->assertNonPersistentConnection($connection4);
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentParameterWithTrueLikeValues()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new CompositeStreamConnection($this->getParameters(array('persistent' => 1)));
+        $this->assertPersistentConnection($connection1);
+
+        $connection2 = new CompositeStreamConnection($this->getParameters(array('persistent' => true)));
+        $this->assertPersistentConnection($connection2);
+
+        $connection3 = new CompositeStreamConnection($this->getParameters(array('persistent' => '1')));
+        $this->assertPersistentConnection($connection3);
+
+        $connection4 = new CompositeStreamConnection($this->getParameters(array('persistent' => 'true')));
+        $this->assertPersistentConnection($connection4);
+
+        $connection1->disconnect();
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentConnectionsToSameNodeShareResource()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new CompositeStreamConnection($this->getParameters(array('persistent' => true)));
+        $connection2 = new CompositeStreamConnection($this->getParameters(array('persistent' => true)));
+
+        $this->assertPersistentConnection($connection1);
+        $this->assertPersistentConnection($connection2);
+
+        $this->assertSame($connection1->getResource(), $connection2->getResource());
+
+        $connection1->disconnect();
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentConnectionsToSameNodeDoNotShareResourceUsingDifferentPersistentID()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new CompositeStreamConnection($this->getParameters(array('persistent' => 'conn1')));
+        $connection2 = new CompositeStreamConnection($this->getParameters(array('persistent' => 'conn2')));
+
+        $this->assertPersistentConnection($connection1);
+        $this->assertPersistentConnection($connection2);
+
+        $this->assertNotSame($connection1->getResource(), $connection2->getResource());
+    }
+
     /**
      * @group connected
      * @expectedException \Predis\Protocol\ProtocolException

+ 84 - 0
tests/Predis/Connection/PhpiredisStreamConnectionTest.php

@@ -137,6 +137,90 @@ class PhpiredisStreamConnectionTest extends PredisConnectionTestCase
         $this->assertSame(array('foo', 'hoge', 'lol'), $connection->executeCommand($cmdLrange));
     }
 
+    /**
+     * @group connected
+     */
+    public function testPersistentParameterWithFalseLikeValues()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => 0)));
+        $this->assertNonPersistentConnection($connection1);
+
+        $connection2 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => false)));
+        $this->assertNonPersistentConnection($connection2);
+
+        $connection3 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => '0')));
+        $this->assertNonPersistentConnection($connection3);
+
+        $connection4 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => 'false')));
+        $this->assertNonPersistentConnection($connection4);
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentParameterWithTrueLikeValues()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => 1)));
+        $this->assertPersistentConnection($connection1);
+
+        $connection2 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => true)));
+        $this->assertPersistentConnection($connection2);
+
+        $connection3 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => '1')));
+        $this->assertPersistentConnection($connection3);
+
+        $connection4 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => 'true')));
+        $this->assertPersistentConnection($connection4);
+
+        $connection1->disconnect();
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentConnectionsToSameNodeShareResource()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => true)));
+        $connection2 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => true)));
+
+        $this->assertPersistentConnection($connection1);
+        $this->assertPersistentConnection($connection2);
+
+        $this->assertSame($connection1->getResource(), $connection2->getResource());
+
+        $connection1->disconnect();
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentConnectionsToSameNodeDoNotShareResourceUsingDifferentPersistentID()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => 'conn1')));
+        $connection2 = new PhpiredisStreamConnection($this->getParameters(array('persistent' => 'conn2')));
+
+        $this->assertPersistentConnection($connection1);
+        $this->assertPersistentConnection($connection2);
+
+        $this->assertNotSame($connection1->getResource(), $connection2->getResource());
+    }
+
     /**
      * @medium
      * @group connected

+ 84 - 0
tests/Predis/Connection/StreamConnectionTest.php

@@ -116,6 +116,90 @@ class StreamConnectionTest extends PredisConnectionTestCase
         $this->assertTrue($connection->isConnected());
     }
 
+    /**
+     * @group connected
+     */
+    public function testPersistentParameterWithFalseLikeValues()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new StreamConnection($this->getParameters(array('persistent' => 0)));
+        $this->assertNonPersistentConnection($connection1);
+
+        $connection2 = new StreamConnection($this->getParameters(array('persistent' => false)));
+        $this->assertNonPersistentConnection($connection2);
+
+        $connection3 = new StreamConnection($this->getParameters(array('persistent' => '0')));
+        $this->assertNonPersistentConnection($connection3);
+
+        $connection4 = new StreamConnection($this->getParameters(array('persistent' => 'false')));
+        $this->assertNonPersistentConnection($connection4);
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentParameterWithTrueLikeValues()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new StreamConnection($this->getParameters(array('persistent' => 1)));
+        $this->assertPersistentConnection($connection1);
+
+        $connection2 = new StreamConnection($this->getParameters(array('persistent' => true)));
+        $this->assertPersistentConnection($connection2);
+
+        $connection3 = new StreamConnection($this->getParameters(array('persistent' => '1')));
+        $this->assertPersistentConnection($connection3);
+
+        $connection4 = new StreamConnection($this->getParameters(array('persistent' => 'true')));
+        $this->assertPersistentConnection($connection4);
+
+        $connection1->disconnect();
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentConnectionsToSameNodeShareResource()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new StreamConnection($this->getParameters(array('persistent' => true)));
+        $connection2 = new StreamConnection($this->getParameters(array('persistent' => true)));
+
+        $this->assertPersistentConnection($connection1);
+        $this->assertPersistentConnection($connection2);
+
+        $this->assertSame($connection1->getResource(), $connection2->getResource());
+
+        $connection1->disconnect();
+    }
+
+    /**
+     * @group connected
+     */
+    public function testPersistentConnectionsToSameNodeDoNotShareResourceUsingDifferentPersistentID()
+    {
+        if ($this->isHHVM()) {
+            $this->markTestSkipped('This test does not currently work on HHVM.');
+        }
+
+        $connection1 = new StreamConnection($this->getParameters(array('persistent' => 'conn1')));
+        $connection2 = new StreamConnection($this->getParameters(array('persistent' => 'conn2')));
+
+        $this->assertPersistentConnection($connection1);
+        $this->assertPersistentConnection($connection2);
+
+        $this->assertNotSame($connection1->getResource(), $connection2->getResource());
+    }
+
     /**
      * @group connected
      * @expectedException \Predis\Protocol\ProtocolException