From 636bf67d6cce1cef5d7e4211a286b84c791919ce Mon Sep 17 00:00:00 2001 From: freek Date: Mon, 26 Nov 2018 23:52:01 +0100 Subject: [PATCH 1/6] nitpick --- config/websockets.php | 1 - src/LaravelEcho/Http/Controllers/FetchChannel.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/config/websockets.php b/config/websockets.php index 2d52a54..75365fc 100644 --- a/config/websockets.php +++ b/config/websockets.php @@ -1,6 +1,5 @@ channelManager->find($request->appId, $request->channelName); if (is_null($channel)) { - throw new HttpException(404, 'Unknown channel "'.$request->channelName.'"'); + throw new HttpException(404, "Unknown channel `{$request->channelName}`."); } return $channel->toArray(); From 17109ec8db903c5c99c8481297657c1d74c2f0fa Mon Sep 17 00:00:00 2001 From: freek Date: Mon, 26 Nov 2018 23:58:28 +0100 Subject: [PATCH 2/6] nitpick --- tests/ConnectionTest.php | 67 +++++++++++++++------------------------- 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index 142e521..a4ad910 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -10,26 +10,30 @@ use BeyondCode\LaravelWebSockets\Tests\Mocks\Message; class ConnectionTest extends TestCase { + /** @var \BeyondCode\LaravelWebSockets\LaravelEcho\WebSocket\PusherServer */ + protected $pusherServer; + + public function setUp() + { + parent::setUp(); + + $this->pusherServer = app(PusherServer::class); + } + /** @test */ public function unknown_app_keys_can_not_connect() { $this->expectException(UnknownAppKeyException::class); - /** @var PusherServer $server */ - $server = app(PusherServer::class); - - $server->onOpen($this->getWebSocketConnection('/?appKey=test')); + $this->pusherServer->onOpen($this->getWebSocketConnection('/?appKey=test')); } /** @test */ public function known_app_keys_can_connect() { - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); $connection->assertSentEvent('pusher:connection_established'); } @@ -37,12 +41,9 @@ class ConnectionTest extends TestCase /** @test */ public function successful_connections_have_the_client_attached() { - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); $this->assertInstanceOf(Client::class, $connection->client); $this->assertSame(1234, $connection->client->appId); @@ -54,16 +55,13 @@ class ConnectionTest extends TestCase /** @test */ public function ping_returns_pong() { - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); $message = new Message('{"event": "pusher:ping"}'); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); - $server->onMessage($connection, $message); + $this->pusherServer->onMessage($connection, $message); $connection->assertSentEvent('pusher:pong'); } @@ -71,9 +69,6 @@ class ConnectionTest extends TestCase /** @test */ public function clients_can_subscribe_to_basic_channels() { - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); $message = new Message(json_encode([ @@ -83,9 +78,9 @@ class ConnectionTest extends TestCase ], ])); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); - $server->onMessage($connection, $message); + $this->pusherServer->onMessage($connection, $message); $connection->assertSentEvent('pusher_internal:subscription_succeeded', [ 'channel' => 'basic-channel' @@ -97,9 +92,6 @@ class ConnectionTest extends TestCase { $this->expectException(InvalidSignatureException::class); - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); $message = new Message(json_encode([ @@ -110,20 +102,17 @@ class ConnectionTest extends TestCase ], ])); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); - $server->onMessage($connection, $message); + $this->pusherServer->onMessage($connection, $message); } /** @test */ public function clients_can_subscribe_to_private_channels() { - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); $signature = "{$connection->socketId}:private-channel"; @@ -135,7 +124,7 @@ class ConnectionTest extends TestCase ], ])); - $server->onMessage($connection, $message); + $this->pusherServer->onMessage($connection, $message); $connection->assertSentEvent('pusher_internal:subscription_succeeded', [ 'channel' => 'private-channel' @@ -147,9 +136,6 @@ class ConnectionTest extends TestCase { $this->expectException(InvalidSignatureException::class); - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); $message = new Message(json_encode([ @@ -160,20 +146,17 @@ class ConnectionTest extends TestCase ], ])); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); - $server->onMessage($connection, $message); + $this->pusherServer->onMessage($connection, $message); } /** @test */ public function clients_can_subscribe_to_presence_channels() { - /** @var PusherServer $server */ - $server = app(PusherServer::class); - $connection = $this->getWebSocketConnection(); - $server->onOpen($connection); + $this->pusherServer->onOpen($connection); $channelData = [ 'user_id' => 1, @@ -193,7 +176,7 @@ class ConnectionTest extends TestCase ], ])); - $server->onMessage($connection, $message); + $this->pusherServer->onMessage($connection, $message); $connection->assertSentEvent('pusher_internal:subscription_succeeded', [ 'channel' => 'presence-channel', From 9fc7747b1d98e95d2e436512454c7906dde153b1 Mon Sep 17 00:00:00 2001 From: freek Date: Tue, 27 Nov 2018 00:00:26 +0100 Subject: [PATCH 3/6] implement close on mock --- tests/Mocks/Connection.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/Mocks/Connection.php b/tests/Mocks/Connection.php index e004d9a..992f29e 100644 --- a/tests/Mocks/Connection.php +++ b/tests/Mocks/Connection.php @@ -13,6 +13,8 @@ class Connection implements ConnectionInterface public $sentData = []; + public $closed = false; + function send($data) { $this->sentData[] = json_decode($data, true); @@ -20,7 +22,7 @@ class Connection implements ConnectionInterface function close() { - // TODO: Implement close() method. + $this->closed = true; } public function assertSentEvent(string $name, array $additionalParameters = []) @@ -35,4 +37,9 @@ class Connection implements ConnectionInterface PHPUnit::assertSame($event[$parameter], $value); } } + + public function assertClosed() + { + PHPUnit::assertTrue($this->closed); + } } \ No newline at end of file From e8fb5294abce90102d1ec136b8506197a39f039e Mon Sep 17 00:00:00 2001 From: freek Date: Tue, 27 Nov 2018 00:03:51 +0100 Subject: [PATCH 4/6] commit --- src/LaravelEcho/Http/Controllers/EchoController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/LaravelEcho/Http/Controllers/EchoController.php b/src/LaravelEcho/Http/Controllers/EchoController.php index 3221ebc..287f9c6 100644 --- a/src/LaravelEcho/Http/Controllers/EchoController.php +++ b/src/LaravelEcho/Http/Controllers/EchoController.php @@ -13,7 +13,6 @@ use Illuminate\Http\JsonResponse; use GuzzleHttp\Psr7\ServerRequest; use Ratchet\Http\HttpServerInterface; use Psr\Http\Message\RequestInterface; -use SebastianBergmann\ObjectEnumerator\Fixtures\ExceptionThrower; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory; use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Channels\ChannelManager; From ee0ab6b4630778bc1ce003587aa6ed34a43f94b4 Mon Sep 17 00:00:00 2001 From: freek Date: Tue, 27 Nov 2018 00:07:35 +0100 Subject: [PATCH 5/6] nitpicks --- src/LaravelEcho/Pusher/Channels/Channel.php | 1 - src/LaravelEcho/Pusher/Channels/ChannelManager.php | 2 +- src/LaravelEcho/Pusher/Channels/PresenceChannel.php | 3 ++- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/LaravelEcho/Pusher/Channels/Channel.php b/src/LaravelEcho/Pusher/Channels/Channel.php index f14f04c..3b23406 100644 --- a/src/LaravelEcho/Pusher/Channels/Channel.php +++ b/src/LaravelEcho/Pusher/Channels/Channel.php @@ -5,7 +5,6 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Channels; use BeyondCode\LaravelWebSockets\Events\ChannelOccupied; use BeyondCode\LaravelWebSockets\Events\ChannelVacated; use BeyondCode\LaravelWebSockets\Events\SubscribedToChannel; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Dashboard; use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\InvalidSignatureException; use Illuminate\Support\Collection; use Ratchet\ConnectionInterface; diff --git a/src/LaravelEcho/Pusher/Channels/ChannelManager.php b/src/LaravelEcho/Pusher/Channels/ChannelManager.php index 2b2c46b..bd8dd8a 100644 --- a/src/LaravelEcho/Pusher/Channels/ChannelManager.php +++ b/src/LaravelEcho/Pusher/Channels/ChannelManager.php @@ -28,7 +28,7 @@ class ChannelManager return $this->channels[$appId][$channelId] ?? null; } - protected function detectChannelClass($channelId): string + protected function detectChannelClass(string $channelId): string { if (starts_with($channelId, 'private-')) { return PrivateChannel::class; diff --git a/src/LaravelEcho/Pusher/Channels/PresenceChannel.php b/src/LaravelEcho/Pusher/Channels/PresenceChannel.php index d7002db..9cfc9e2 100644 --- a/src/LaravelEcho/Pusher/Channels/PresenceChannel.php +++ b/src/LaravelEcho/Pusher/Channels/PresenceChannel.php @@ -3,6 +3,7 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Channels; use Ratchet\ConnectionInterface; +use stdClass; class PresenceChannel extends Channel { @@ -16,7 +17,7 @@ class PresenceChannel extends Channel /* * @link https://pusher.com/docs/pusher_protocol#presence-channel-events */ - public function subscribe(ConnectionInterface $connection, $payload) + public function subscribe(ConnectionInterface $connection, stdClass $payload) { $this->verifySignature($connection, $payload); From 6895961c74ff18bb6a55c497ee3b5af36cef6125 Mon Sep 17 00:00:00 2001 From: freek Date: Tue, 27 Nov 2018 00:13:22 +0100 Subject: [PATCH 6/6] nitpicks --- src/LaravelEcho/Pusher/Channels/Channel.php | 4 +-- ...ionException.php => InvalidConnection.php} | 2 +- ...tureException.php => InvalidSignature.php} | 2 +- ...nAppKeyException.php => UnknownAppKey.php} | 2 +- src/LaravelEcho/WebSocket/Message.php | 15 ++++---- src/LaravelEcho/WebSocket/PusherServer.php | 36 ++++++++++--------- tests/ConnectionTest.php | 10 +++--- 7 files changed, 38 insertions(+), 33 deletions(-) rename src/LaravelEcho/Pusher/Exceptions/{InvalidConnectionException.php => InvalidConnection.php} (77%) rename src/LaravelEcho/Pusher/Exceptions/{InvalidSignatureException.php => InvalidSignature.php} (77%) rename src/LaravelEcho/Pusher/Exceptions/{UnknownAppKeyException.php => UnknownAppKey.php} (81%) diff --git a/src/LaravelEcho/Pusher/Channels/Channel.php b/src/LaravelEcho/Pusher/Channels/Channel.php index 3b23406..a54a157 100644 --- a/src/LaravelEcho/Pusher/Channels/Channel.php +++ b/src/LaravelEcho/Pusher/Channels/Channel.php @@ -5,7 +5,7 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Channels; use BeyondCode\LaravelWebSockets\Events\ChannelOccupied; use BeyondCode\LaravelWebSockets\Events\ChannelVacated; use BeyondCode\LaravelWebSockets\Events\SubscribedToChannel; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\InvalidSignatureException; +use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\InvalidSignature; use Illuminate\Support\Collection; use Ratchet\ConnectionInterface; use stdClass; @@ -39,7 +39,7 @@ class Channel } if (str_after($auth, ':') !== hash_hmac('sha256', $signature, $connection->client->appSecret)) { - throw new InvalidSignatureException(); + throw new InvalidSignature(); } } diff --git a/src/LaravelEcho/Pusher/Exceptions/InvalidConnectionException.php b/src/LaravelEcho/Pusher/Exceptions/InvalidConnection.php similarity index 77% rename from src/LaravelEcho/Pusher/Exceptions/InvalidConnectionException.php rename to src/LaravelEcho/Pusher/Exceptions/InvalidConnection.php index a996bc4..30a465a 100644 --- a/src/LaravelEcho/Pusher/Exceptions/InvalidConnectionException.php +++ b/src/LaravelEcho/Pusher/Exceptions/InvalidConnection.php @@ -2,7 +2,7 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions; -class InvalidConnectionException extends PusherException +class InvalidConnection extends PusherException { public function __construct() { diff --git a/src/LaravelEcho/Pusher/Exceptions/InvalidSignatureException.php b/src/LaravelEcho/Pusher/Exceptions/InvalidSignature.php similarity index 77% rename from src/LaravelEcho/Pusher/Exceptions/InvalidSignatureException.php rename to src/LaravelEcho/Pusher/Exceptions/InvalidSignature.php index ad71243..94ef279 100644 --- a/src/LaravelEcho/Pusher/Exceptions/InvalidSignatureException.php +++ b/src/LaravelEcho/Pusher/Exceptions/InvalidSignature.php @@ -2,7 +2,7 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions; -class InvalidSignatureException extends PusherException +class InvalidSignature extends PusherException { public function __construct() { diff --git a/src/LaravelEcho/Pusher/Exceptions/UnknownAppKeyException.php b/src/LaravelEcho/Pusher/Exceptions/UnknownAppKey.php similarity index 81% rename from src/LaravelEcho/Pusher/Exceptions/UnknownAppKeyException.php rename to src/LaravelEcho/Pusher/Exceptions/UnknownAppKey.php index c154d27..76e856f 100644 --- a/src/LaravelEcho/Pusher/Exceptions/UnknownAppKeyException.php +++ b/src/LaravelEcho/Pusher/Exceptions/UnknownAppKey.php @@ -2,7 +2,7 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions; -class UnknownAppKeyException extends PusherException +class UnknownAppKey extends PusherException { public function __construct(string $appKey) { diff --git a/src/LaravelEcho/WebSocket/Message.php b/src/LaravelEcho/WebSocket/Message.php index 72197eb..d746d6a 100644 --- a/src/LaravelEcho/WebSocket/Message.php +++ b/src/LaravelEcho/WebSocket/Message.php @@ -4,7 +4,6 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\WebSocket; use BeyondCode\LaravelWebSockets\Events\ClientMessageSent; use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Channels\ChannelManager; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Dashboard; use Ratchet\ConnectionInterface; use stdClass; @@ -30,12 +29,14 @@ class Message implements RespondableMessage public function respond() { - if (starts_with($this->payload->event, 'client-')) { - event(new ClientMessageSent($this->connection, $this->payload)); - - $channel = $this->channelManager->find($this->connection->client->appId, $this->payload->channel); - - optional($channel)->broadcast($this->payload); + if (!starts_with($this->payload->event, 'client-')) { + return; } + + event(new ClientMessageSent($this->connection, $this->payload)); + + $channel = $this->channelManager->find($this->connection->client->appId, $this->payload->channel); + + optional($channel)->broadcast($this->payload); } } \ No newline at end of file diff --git a/src/LaravelEcho/WebSocket/PusherServer.php b/src/LaravelEcho/WebSocket/PusherServer.php index 9a3de9d..aa3e644 100644 --- a/src/LaravelEcho/WebSocket/PusherServer.php +++ b/src/LaravelEcho/WebSocket/PusherServer.php @@ -3,7 +3,6 @@ namespace BeyondCode\LaravelWebSockets\LaravelEcho\WebSocket; use BeyondCode\LaravelWebSockets\Events\ConnectionEstablished; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Dashboard; use BeyondCode\LaravelWebSockets\QueryParameters; use Exception; use Ratchet\ConnectionInterface; @@ -12,7 +11,7 @@ use BeyondCode\LaravelWebSockets\WebSocketController; use BeyondCode\LaravelWebSockets\ClientProviders\Client; use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Channels\ChannelManager; use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\PusherException; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\UnknownAppKeyException; +use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\UnknownAppKey; class PusherServer extends WebSocketController { @@ -26,11 +25,10 @@ class PusherServer extends WebSocketController function onOpen(ConnectionInterface $connection) { - $this->generateSocketId($connection); - - $this->verifyConnection($connection); - - $this->establishConnection($connection); + $this + ->generateSocketId($connection) + ->verifyConnection($connection) + ->establishConnection($connection); } public function onMessage(ConnectionInterface $connection, MessageInterface $message) @@ -45,7 +43,7 @@ class PusherServer extends WebSocketController $this->channelManager->removeFromAllChannels($connection); } - function onError(ConnectionInterface $connection, Exception $exception) + public function onError(ConnectionInterface $connection, Exception $exception) { if ($exception instanceof PusherException) { $connection->send(json_encode( @@ -54,15 +52,26 @@ class PusherServer extends WebSocketController } } + protected function generateSocketId(ConnectionInterface $connection) + { + $socketId = sprintf("%d.%d", getmypid(), random_int(1, 100000000)); + + $connection->socketId = $socketId; + + return $this; + } + protected function verifyConnection(ConnectionInterface $connection) { $appKey = QueryParameters::create($connection->httpRequest)->get('appKey'); - if (! $client = Client::findByAppKey($appKey)) { - throw new UnknownAppKeyException($appKey); + if (!$client = Client::findByAppKey($appKey)) { + throw new UnknownAppKey($appKey); } $connection->client = $client; + + return $this; } protected function establishConnection(ConnectionInterface $connection) @@ -76,12 +85,7 @@ class PusherServer extends WebSocketController ])); event(new ConnectionEstablished($connection)); - } - protected function generateSocketId(ConnectionInterface $connection) - { - $socketId = sprintf("%d.%d", getmypid(), random_int(1, 100000000)); - - $connection->socketId = $socketId; + return $this; } } \ No newline at end of file diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index a4ad910..ea34ac7 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -3,8 +3,8 @@ namespace BeyondCode\LaravelWebSockets\Tests; use BeyondCode\LaravelWebSockets\ClientProviders\Client; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\InvalidSignatureException; -use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\UnknownAppKeyException; +use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\InvalidSignature; +use BeyondCode\LaravelWebSockets\LaravelEcho\Pusher\Exceptions\UnknownAppKey; use BeyondCode\LaravelWebSockets\LaravelEcho\WebSocket\PusherServer; use BeyondCode\LaravelWebSockets\Tests\Mocks\Message; @@ -23,7 +23,7 @@ class ConnectionTest extends TestCase /** @test */ public function unknown_app_keys_can_not_connect() { - $this->expectException(UnknownAppKeyException::class); + $this->expectException(UnknownAppKey::class); $this->pusherServer->onOpen($this->getWebSocketConnection('/?appKey=test')); } @@ -90,7 +90,7 @@ class ConnectionTest extends TestCase /** @test */ public function clients_need_valid_auth_signatures_for_private_channels() { - $this->expectException(InvalidSignatureException::class); + $this->expectException(InvalidSignature::class); $connection = $this->getWebSocketConnection(); @@ -134,7 +134,7 @@ class ConnectionTest extends TestCase /** @test */ public function clients_need_valid_auth_signatures_for_presence_channels() { - $this->expectException(InvalidSignatureException::class); + $this->expectException(InvalidSignature::class); $connection = $this->getWebSocketConnection();