Merge pull request #39 from clue-labs/socket

Replace deprecated SocketClient with new Socket component and improve forward compatibility with upcoming ReactPHP components

Author: clue

README.md

@@ -91,7 +91,7 @@ $factory = new Factory($loop);
 ```
 
 If you need custom DNS or proxy settings, you can explicitly pass a
-custom instance of the [`ConnectorInterface`](https://github.com/reactphp/socket-client#connectorinterface):
+custom instance of the [`ConnectorInterface`](https://github.com/reactphp/socket#connectorinterface):
 
 ```php
 $factory = new Factory($loop, $connector);

composer.json

@@ -12,12 +12,13 @@
     ],
     "require": {
         "php": ">=5.3",
-        "evenement/evenement": "~1.0|~2.0",
-        "react/promise": "~1.0|~2.0",
-        "react/socket-client": "^0.5 || ^0.4 || ^0.3",
-        "react/event-loop": "0.3.*|0.4.*"
+        "evenement/evenement": "^3.0 || ^2.0 || ^1.0",
+        "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3",
+        "react/promise": "^2.0 || ^1.0",
+        "react/socket": "^1.0 || ^0.8.2"
     },
     "require-dev": {
+        "clue/block-react": "^1.2",
         "phpunit/phpunit": "^5.0 || ^4.8"
     },
     "autoload": {

src/Client.php

@@ -2,17 +2,17 @@
 
 namespace Clue\React\Ami;
 
-use Clue\React\Ami\Protocol\Event;
 use Clue\React\Ami\Protocol\Action;
-use Evenement\EventEmitter;
-use React\Stream\Stream;
+use Clue\React\Ami\Protocol\ErrorException;
+use Clue\React\Ami\Protocol\Event;
+use Clue\React\Ami\Protocol\Message;
 use Clue\React\Ami\Protocol\Parser;
+use Clue\React\Ami\Protocol\UnexpectedMessageException;
+use Evenement\EventEmitter;
 use React\Promise\Deferred;
+use React\Socket\ConnectionInterface;
 use Exception;
 use UnexpectedValueException;
-use Clue\React\Ami\Protocol\Message;
-use Clue\React\Ami\Protocol\ErrorException;
-use Clue\React\Ami\Protocol\UnexpectedMessageException;
 
 class Client extends EventEmitter
 {
@@ -23,7 +23,7 @@ class Client extends EventEmitter
 
     private $actionId = 0;
 
-    public function __construct(Stream $stream, Parser $parser = null)
+    public function __construct(ConnectionInterface $stream, Parser $parser = null)
     {
         if ($parser === null) {
             $parser = new Parser();
@@ -47,8 +47,6 @@ public function __construct(Stream $stream, Parser $parser = null)
         $this->on('error', array($that, 'close'));
 
         $this->stream->on('close', array ($that, 'close'));
-
-        $this->stream->resume();
     }
 
     public function request(Action $message)

src/Factory.php

@@ -3,47 +3,40 @@
 namespace Clue\React\Ami;
 
 use React\EventLoop\LoopInterface;
-use React\SocketClient\ConnectorInterface;
-use React\SocketClient\Connector;
-use React\SocketClient\SecureConnector;
-use React\Dns\Resolver\Factory as ResolverFactory;
-use React\Stream\Stream;
+use React\Socket\ConnectionInterface;
+use React\Socket\Connector;
+use React\Socket\ConnectorInterface;
 use InvalidArgumentException;
 
 class Factory
 {
     private $loop;
     private $connector;
-    private $secureConnector;
 
-    public function __construct(LoopInterface $loop, ConnectorInterface $connector = null, ConnectorInterface $secureConnector = null)
+    public function __construct(LoopInterface $loop, ConnectorInterface $connector = null)
     {
         if ($connector === null) {
-            $resolverFactory = new ResolverFactory();
-            $connector = new Connector($loop, $resolverFactory->create('8.8.8.8', $loop));
-        }
-        if ($secureConnector === null) {
-            $secureConnector = new SecureConnector($connector, $loop);
+            $connector = new Connector($loop);
         }
 
         $this->loop = $loop;
         $this->connector = $connector;
-        $this->secureConnector = $secureConnector;
     }
 
     public function createClient($address = null)
     {
         $parts = $this->parseUrl($address);
 
-        $secure = (isset($parts['scheme']) && $parts['scheme'] !== 'tcp');
-        $connector = $secure ? $this->secureConnector : $this->connector;
+        if (isset($parts['scheme']) && $parts['scheme'] !== 'tcp') {
+            $parts['host'] = 'tls://' . $parts['host'];
+        }
 
-        $promise = $connector->create($parts['host'], $parts['port'])->then(function (Stream $stream) {
+        $promise = $this->connector->connect($parts['host'] . ':' . $parts['port'])->then(function (ConnectionInterface $stream) {
             return new Client($stream);
         });
 
         if (isset($parts['user'])) {
-            $promise = $promise->then(function (Client $client) use ($parts, $secure) {
+            $promise = $promise->then(function (Client $client) use ($parts) {
                 $sender = new ActionSender($client);
 
                 return $sender->login($parts['user'], $parts['pass'])->then(
@@ -79,10 +72,6 @@ private function parseUrl($target)
             $parts['port'] = '5038';
         }
 
-        if ($parts['host'] === 'localhost') {
-            $parts['host'] = '127.0.0.1';
-        }
-
         return $parts;
     }
 }

tests/ClientTest.php

@@ -16,19 +16,19 @@ public function testClosingStreamClosesClient()
 
         $client->on('close', $this->expectCallableOnce());
 
-        $stream->close();
-        //$stream->emit('close', array($this));
+        $stream->emit('close');
     }
 
     public function testParserExceptionForwardsErrorAndClosesClient()
     {
         $stream = $this->createStreamMock();
+        $stream->expects($this->once())->method('close');
+
         $parser = new Parser();
 
         $client = new Client($stream, $parser);
 
         $client->on('error', $this->expectCallableOnce());
-        $client->on('close', $this->expectCallableOnce());
 
         $stream->emit('data', array("invalid chunk\r\n\r\ninvalid chunk\r\n\r\n"));
     }
@@ -45,6 +45,6 @@ public function testUnexpectedResponseEmitsErrorAndClosesClient()
 
     private function createStreamMock()
     {
-        return new Stream(fopen('php://memory', 'r+'), $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock());
+        return $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('write', 'close'))->getMock();
     }
 }

tests/CollectorTest.php

@@ -51,7 +51,7 @@ public function testCollectingSIPEvents()
 
     private function createClientMock()
     {
-        $stream = $this->getMockBuilder('React\Stream\Stream')->disableOriginalConstructor()->getMock();
+        $stream = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->getMock();
 
         $client = $this->getMockBuilder('Clue\React\Ami\Client')->setMethods(array('createAction'))->setConstructorArgs(array($stream))->getMock();
 

tests/FactoryTest.php

@@ -6,16 +6,14 @@ class FactoryTest extends TestCase
 {
     private $loop;
     private $tcp;
-    private $tls;
     private $factory;
 
     public function setUp()
     {
         $this->loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
-        $this->tcp = $this->getMockBuilder('React\SocketClient\ConnectorInterface')->getMock();
-        $this->tls = $this->getMockBuilder('React\SocketClient\ConnectorInterface')->getMock();
+        $this->tcp = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
 
-        $this->factory = new Factory($this->loop, $this->tcp, $this->tls);
+        $this->factory = new Factory($this->loop, $this->tcp);
     }
 
     public function testDefaultCtor()
@@ -26,23 +24,23 @@ public function testDefaultCtor()
     public function testCreateClientUsesTcpConnectorWithDefaultLocation()
     {
         $promise = new Promise(function () { });
-        $this->tcp->expects($this->once())->method('create')->with('127.0.0.1', 5038)->willReturn($promise);
+        $this->tcp->expects($this->once())->method('connect')->with('127.0.0.1:5038')->willReturn($promise);
 
         $this->factory->createClient();
     }
 
-    public function testCreateClientUsesTcpConnectorWithLocalhostLocation()
+    public function testCreateClientUsesDefaultPortForTcpConnection()
     {
         $promise = new Promise(function () { });
-        $this->tcp->expects($this->once())->method('create')->with('127.0.0.1', 5038)->willReturn($promise);
+        $this->tcp->expects($this->once())->method('connect')->with('localhost:5038')->willReturn($promise);
 
         $this->factory->createClient('localhost');
     }
 
     public function testCreateClientUsesTlsConnectorWithTlsLocation()
     {
         $promise = new Promise(function () { });
-        $this->tls->expects($this->once())->method('create')->with('ami.local', 1234)->willReturn($promise);
+        $this->tcp->expects($this->once())->method('connect')->with('tls://ami.local:1234')->willReturn($promise);
 
         $this->factory->createClient('tls://ami.local:1234');
     }

tests/FunctionalTest.php

@@ -1,10 +1,10 @@
 <?php
 
 use Clue\React\Ami\Factory;
-use React\Promise\PromiseInterface;
 use Clue\React\Ami\Client;
 use Clue\React\Ami\ActionSender;
-use Clue\React\Ami\Protocol\Response;
+use Clue\React\Block;
+use React\Promise\PromiseInterface;
 
 class FunctionalTest extends TestCase
 {
@@ -90,23 +90,6 @@ public function testSendRejectedAfterClose(Client $client)
 
     private function waitFor(PromiseInterface $promise)
     {
-        $resolved = null;
-        $exception = null;
-
-        $promise->then(function ($c) use (&$resolved) {
-            $resolved = $c;
-        }, function($error) use (&$exception) {
-            $exception = $error;
-        });
-
-        while ($resolved === null && $exception === null) {
-            self::$loop->tick();
-        }
-
-        if ($exception !== null) {
-            throw $exception;
-        }
-
-        return $resolved;
+        return Block\await($promise, self::$loop, 5.0);
     }
 }