Skip to content

Commit

Permalink
Remove optional $loop constructor argument, always use default loop
Browse files Browse the repository at this point in the history
  • Loading branch information
clue committed Feb 11, 2024
1 parent e9de03f commit 26c2d48
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 87 deletions.
6 changes: 0 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,6 @@ $connector = new React\Socket\Connector([
$redis = new Clue\React\Redis\RedisClient('localhost', $connector);
```

This class takes an optional `LoopInterface|null $loop` parameter that can be used to
pass the event loop instance to use for this object. You can use a `null` value
here in order to use the [default loop](https://github.com/reactphp/event-loop#loop).
This value SHOULD NOT be given unless you're sure you want to explicitly use a
given event loop instance.

#### __call()

The `__call(string $name, string[] $args): PromiseInterface<mixed>` method can be used to
Expand Down
16 changes: 5 additions & 11 deletions src/Io/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use Clue\Redis\Protocol\Factory as ProtocolFactory;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\Deferred;
use React\Promise\Promise;
use React\Promise\PromiseInterface;
Expand All @@ -18,24 +17,19 @@
*/
class Factory
{
/** @var LoopInterface */
private $loop;

/** @var ConnectorInterface */
private $connector;

/** @var ProtocolFactory */
private $protocol;

/**
* @param ?LoopInterface $loop
* @param ?ConnectorInterface $connector
* @param ?ProtocolFactory $protocol
*/
public function __construct(LoopInterface $loop = null, ConnectorInterface $connector = null, ProtocolFactory $protocol = null)
public function __construct(ConnectorInterface $connector = null, ProtocolFactory $protocol = null)
{
$this->loop = $loop ?: Loop::get();
$this->connector = $connector ?: new Connector([], $this->loop);
$this->connector = $connector ?: new Connector();
$this->protocol = $protocol ?: new ProtocolFactory();
}

Expand Down Expand Up @@ -182,13 +176,13 @@ function (\Exception $e) use ($redis, $uri) {
$timer = null;
$promise = $promise->then(function (StreamingClient $v) use (&$timer, $resolve): void {
if ($timer) {
$this->loop->cancelTimer($timer);
Loop::cancelTimer($timer);
}
$timer = false;
$resolve($v);
}, function (\Throwable $e) use (&$timer, $reject): void {
if ($timer) {
$this->loop->cancelTimer($timer);
Loop::cancelTimer($timer);
}
$timer = false;
$reject($e);
Expand All @@ -200,7 +194,7 @@ function (\Exception $e) use ($redis, $uri) {
}

// start timeout timer which will cancel the pending promise
$timer = $this->loop->addTimer($timeout, function () use ($timeout, &$promise, $reject, $uri): void {
$timer = Loop::addTimer($timeout, function () use ($timeout, &$promise, $reject, $uri): void {
$reject(new \RuntimeException(
'Connection to ' . $uri . ' timed out after ' . $timeout . ' seconds (ETIMEDOUT)',
\defined('SOCKET_ETIMEDOUT') ? \SOCKET_ETIMEDOUT : 110
Expand Down
22 changes: 6 additions & 16 deletions src/RedisClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use Clue\React\Redis\Io\StreamingClient;
use Evenement\EventEmitter;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\PromiseInterface;
use React\Socket\ConnectorInterface;
use React\Stream\Util;
Expand Down Expand Up @@ -40,9 +39,6 @@ class RedisClient extends EventEmitter
/** @var ?PromiseInterface<StreamingClient> */
private $promise = null;

/** @var LoopInterface */
private $loop;

/** @var float */
private $idlePeriod = 0.001;

Expand All @@ -58,12 +54,7 @@ class RedisClient extends EventEmitter
/** @var array<string,bool> */
private $psubscribed = [];

/**
* @param string $url
* @param ?ConnectorInterface $connector
* @param ?LoopInterface $loop
*/
public function __construct($url, ConnectorInterface $connector = null, LoopInterface $loop = null)
public function __construct(string $url, ConnectorInterface $connector = null)
{
$args = [];
\parse_str((string) \parse_url($url, \PHP_URL_QUERY), $args);
Expand All @@ -72,8 +63,7 @@ public function __construct($url, ConnectorInterface $connector = null, LoopInte
}

$this->target = $url;
$this->loop = $loop ?: Loop::get();
$this->factory = new Factory($this->loop, $connector);
$this->factory = new Factory($connector);
}

/**
Expand Down Expand Up @@ -102,7 +92,7 @@ private function client(): PromiseInterface
$this->subscribed = $this->psubscribed = [];

if ($this->idleTimer !== null) {
$this->loop->cancelTimer($this->idleTimer);
Loop::cancelTimer($this->idleTimer);
$this->idleTimer = null;
}
});
Expand Down Expand Up @@ -235,7 +225,7 @@ public function close(): void
}

if ($this->idleTimer !== null) {
$this->loop->cancelTimer($this->idleTimer);
Loop::cancelTimer($this->idleTimer);
$this->idleTimer = null;
}

Expand All @@ -248,7 +238,7 @@ private function awake(): void
++$this->pending;

if ($this->idleTimer !== null) {
$this->loop->cancelTimer($this->idleTimer);
Loop::cancelTimer($this->idleTimer);
$this->idleTimer = null;
}
}
Expand All @@ -258,7 +248,7 @@ private function idle(): void
--$this->pending;

if ($this->pending < 1 && $this->idlePeriod >= 0 && !$this->subscribed && !$this->psubscribed && $this->promise !== null) {
$this->idleTimer = $this->loop->addTimer($this->idlePeriod, function () {
$this->idleTimer = Loop::addTimer($this->idlePeriod, function () {
assert($this->promise instanceof PromiseInterface);
$this->promise->then(function (StreamingClient $redis) {
$redis->close();
Expand Down
1 change: 0 additions & 1 deletion tests/FunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

class FunctionalTest extends TestCase
{

/** @var string */
private $uri;

Expand Down
99 changes: 74 additions & 25 deletions tests/Io/FactoryStreamingClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Clue\React\Redis\Io\StreamingClient;
use Clue\Tests\React\Redis\TestCase;
use PHPUnit\Framework\MockObject\MockObject;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\EventLoop\TimerInterface;
use React\Promise\Deferred;
Expand All @@ -16,43 +17,39 @@

class FactoryStreamingClientTest extends TestCase
{
/** @var MockObject */
private $loop;

/** @var MockObject */
private $connector;

/** @var Factory */
private $factory;

public function setUp(): void
{
$this->loop = $this->createMock(LoopInterface::class);
$this->connector = $this->createMock(ConnectorInterface::class);
/** @var LoopInterface */
public static $loop;

assert($this->loop instanceof LoopInterface);
assert($this->connector instanceof ConnectorInterface);
$this->factory = new Factory($this->loop, $this->connector);
public static function setUpBeforeClass(): void
{
self::$loop = Loop::get();
}

public function testConstructWithoutLoopAssignsLoopAutomatically(): void
public static function tearDownAfterClass(): void
{
$factory = new Factory();
Loop::set(self::$loop);
}

$ref = new \ReflectionProperty($factory, 'loop');
$ref->setAccessible(true);
$loop = $ref->getValue($factory);
public function setUp(): void
{
$this->connector = $this->createMock(ConnectorInterface::class);

$this->assertInstanceOf(LoopInterface::class, $loop);
assert($this->connector instanceof ConnectorInterface);
$this->factory = new Factory($this->connector);
}

/**
* @doesNotPerformAssertions
*/
public function testCtor(): void
{
assert($this->loop instanceof LoopInterface);
$this->factory = new Factory($this->loop);
$this->factory = new Factory();
}

public function testWillConnectWithDefaultPort(): void
Expand Down Expand Up @@ -87,6 +84,10 @@ public function testWillWriteSelectCommandIfTargetContainsPath(): void
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$6\r\nselect\r\n$4\r\ndemo\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 89 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->willReturn(resolve($stream));
$this->factory->createClient('redis://127.0.0.1/demo');
}
Expand All @@ -96,6 +97,10 @@ public function testWillWriteSelectCommandIfTargetContainsDbQueryParameter(): vo
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$6\r\nselect\r\n$1\r\n4\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 102 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->willReturn(resolve($stream));
$this->factory->createClient('redis://127.0.0.1?db=4');
}
Expand All @@ -105,6 +110,10 @@ public function testWillWriteAuthCommandIfRedisUriContainsUserInfo(): void
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nworld\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 115 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('example.com:6379')->willReturn(resolve($stream));
$this->factory->createClient('redis://hello:world@example.com');
}
Expand All @@ -114,6 +123,10 @@ public function testWillWriteAuthCommandIfRedisUriContainsEncodedUserInfo(): voi
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nh@llo\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 128 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('example.com:6379')->willReturn(resolve($stream));
$this->factory->createClient('redis://:h%40llo@example.com');
}
Expand All @@ -123,6 +136,10 @@ public function testWillWriteAuthCommandIfTargetContainsPasswordQueryParameter()
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$6\r\nsecret\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 141 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('example.com:6379')->willReturn(resolve($stream));
$this->factory->createClient('redis://example.com?password=secret');
}
Expand All @@ -132,6 +149,10 @@ public function testWillWriteAuthCommandIfTargetContainsEncodedPasswordQueryPara
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nh@llo\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 154 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('example.com:6379')->willReturn(resolve($stream));
$this->factory->createClient('redis://example.com?password=h%40llo');
}
Expand All @@ -141,6 +162,10 @@ public function testWillWriteAuthCommandIfRedissUriContainsUserInfo(): void
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nworld\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 167 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('tls://example.com:6379')->willReturn(resolve($stream));
$this->factory->createClient('rediss://hello:world@example.com');
}
Expand All @@ -150,6 +175,10 @@ public function testWillWriteAuthCommandIfRedisUnixUriContainsPasswordQueryParam
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nworld\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 180 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('unix:///tmp/redis.sock')->willReturn(resolve($stream));
$this->factory->createClient('redis+unix:///tmp/redis.sock?password=world');
}
Expand All @@ -168,6 +197,10 @@ public function testWillWriteAuthCommandIfRedisUnixUriContainsUserInfo(): void
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$4\r\nauth\r\n$5\r\nworld\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 202 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('unix:///tmp/redis.sock')->willReturn(resolve($stream));
$this->factory->createClient('redis+unix://hello:world@/tmp/redis.sock');
}
Expand Down Expand Up @@ -275,6 +308,10 @@ public function testWillWriteSelectCommandIfRedisUnixUriContainsDbQueryParameter
$stream = $this->createMock(ConnectionInterface::class);
$stream->expects($this->once())->method('write')->with("*2\r\n$6\r\nselect\r\n$4\r\ndemo\r\n");

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer');
Loop::set($loop);

Check failure on line 313 in tests/Io/FactoryStreamingClientTest.php

View workflow job for this annotation

GitHub Actions / PHPStan (PHP 7.2)

Parameter #1 $loop of static method React\EventLoop\Loop::set() expects React\EventLoop\LoopInterface, PHPUnit\Framework\MockObject\MockObject given.

$this->connector->expects($this->once())->method('connect')->with('unix:///tmp/redis.sock')->willReturn(resolve($stream));
$this->factory->createClient('redis+unix:///tmp/redis.sock?db=demo');
}
Expand Down Expand Up @@ -584,8 +621,11 @@ public function testCancelWillCloseConnectionWhenConnectionWaitsForSelect(): voi

public function testCreateClientWithTimeoutParameterWillStartTimerAndRejectOnExplicitTimeout(): void
{
$loop = $this->createMock(LoopInterface::class);
Loop::set($loop);

$timeout = null;
$this->loop->expects($this->once())->method('addTimer')->with(0, $this->callback(function ($cb) use (&$timeout) {
$loop->expects($this->once())->method('addTimer')->with(0, $this->callback(function ($cb) use (&$timeout) {
$timeout = $cb;
return true;
}));
Expand Down Expand Up @@ -613,7 +653,10 @@ public function testCreateClientWithTimeoutParameterWillStartTimerAndRejectOnExp

public function testCreateClientWithNegativeTimeoutParameterWillNotStartTimer(): void
{
$this->loop->expects($this->never())->method('addTimer');
$loop = $this->createMock(LoopInterface::class);
Loop::set($loop);

$loop->expects($this->never())->method('addTimer');

$deferred = new Deferred();
$this->connector->expects($this->once())->method('connect')->with('127.0.0.1:2')->willReturn($deferred->promise());
Expand All @@ -623,7 +666,9 @@ public function testCreateClientWithNegativeTimeoutParameterWillNotStartTimer():

public function testCreateClientWithoutTimeoutParameterWillStartTimerWithDefaultTimeoutFromIni(): void
{
$this->loop->expects($this->once())->method('addTimer')->with(42, $this->anything());
$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('addTimer')->with(42, $this->anything());
Loop::set($loop);

$deferred = new Deferred();
$this->connector->expects($this->once())->method('connect')->with('127.0.0.1:2')->willReturn($deferred->promise());
Expand All @@ -637,9 +682,11 @@ public function testCreateClientWithoutTimeoutParameterWillStartTimerWithDefault

public function testCreateClientWillCancelTimerWhenConnectionResolves(): void
{
$loop = $this->createMock(LoopInterface::class);
$timer = $this->createMock(TimerInterface::class);
$this->loop->expects($this->once())->method('addTimer')->willReturn($timer);
$this->loop->expects($this->once())->method('cancelTimer')->with($timer);
$loop->expects($this->once())->method('addTimer')->willReturn($timer);
$loop->expects($this->once())->method('cancelTimer')->with($timer);
Loop::set($loop);

$deferred = new Deferred();
$this->connector->expects($this->once())->method('connect')->with('127.0.0.1:6379')->willReturn($deferred->promise());
Expand All @@ -652,9 +699,11 @@ public function testCreateClientWillCancelTimerWhenConnectionResolves(): void

public function testCreateClientWillCancelTimerWhenConnectionRejects(): void
{
$loop = $this->createMock(LoopInterface::class);
$timer = $this->createMock(TimerInterface::class);
$this->loop->expects($this->once())->method('addTimer')->willReturn($timer);
$this->loop->expects($this->once())->method('cancelTimer')->with($timer);
$loop->expects($this->once())->method('addTimer')->willReturn($timer);
$loop->expects($this->once())->method('cancelTimer')->with($timer);
Loop::set($loop);

$deferred = new Deferred();
$this->connector->expects($this->once())->method('connect')->with('127.0.0.1:6379')->willReturn($deferred->promise());
Expand Down
Loading

0 comments on commit 26c2d48

Please sign in to comment.