Fixed abstract action pushing message before checking connection.

kanata-php · Apr 9, 2024 · 4638e28 · 4638e28
1 parent 87ebf84
commit 4638e28
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 228 deletions.
diff --git a/composer.json b/composer.json
@@ -16,7 +16,6 @@
         "symfony/var-dumper": "^6.1",
         "phpunit/phpunit": "^9.2",
         "mockery/mockery": "^1.4",
-        "codedungeon/phpunit-result-printer": "^0.30.0",
         "openswoole/ide-helper": "^22.0",
         "phpro/grumphp": "^2.3",
         "phpstan/phpstan": "^1.10",

diff --git a/composer.lock b/composer.lock
diff --git a/phpunit.xml b/phpunit.xml
@@ -1,7 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<phpunit
-    bootstrap="vendor/autoload.php"
-    printerClass="Codedungeon\PHPUnitPrettyResultPrinter\Printer">
+<phpunit bootstrap="vendor/autoload.php">
   <testsuites>
     <testsuite name="Unit">
       <file>tests/Unit/MessageRouterTest.php</file>

diff --git a/src/Actions/Abstractions/AbstractAction.php b/src/Actions/Abstractions/AbstractAction.php
@@ -323,7 +323,9 @@ public function push(int $fd, string $data): void
             $this->server,
         );
 
-        $this->server->push($fd, $data);
+        if ($this->server->isEstablished($fd)) {
+            $this->server->push($fd, $data);
+        }
 
         $this->checkAcknowledgment($fd, $data);
     }

diff --git a/src/Actions/Traits/HasAcknowledgment.php b/src/Actions/Traits/HasAcknowledgment.php
@@ -4,6 +4,7 @@
 
 use Conveyor\Actions\AcknowledgeAction;
 use Conveyor\Constants;
+use Conveyor\Helpers\Arr;
 
 trait HasAcknowledgment
 {
@@ -43,7 +44,7 @@ public function checkAcknowledgment(int $fd, string $data): void
 
         $parsedData = json_decode($data, true);
         if (
-            $parsedData['action'] !== AcknowledgeAction::NAME
+            Arr::get($parsedData, 'action') !== AcknowledgeAction::NAME
             || !isset($parsedData['id'])
         ) {
             return;
@@ -64,7 +65,9 @@ public function checkAcknowledgment(int $fd, string $data): void
                 function () use ($fd, $data, $messageHash, &$timers) {
                     if ($this->messageAcknowledmentPersistence->has($messageHash)) {
                         $this->messageAcknowledmentPersistence->subtract($messageHash);
-                        $this->server->push($fd, $data);
+                        if ($this->server->isEstablished($fd)) {
+                            $this->server->push($fd, $data);
+                        }
                         return;
                     }
 

diff --git a/tests/Unit/MessageRouterTest.php b/tests/Unit/MessageRouterTest.php
@@ -47,6 +47,7 @@ public function testCanSendBaseAction()
         ]);
 
         $server = Mockery::mock(Server::class);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push')
             ->andReturnUsing(function ($fd, $data) use ($expectedResponse) {
                 $this->assertEquals($expectedResponse, $data);
@@ -252,6 +253,7 @@ public function testCantSendPlainText()
         $expectedTrue = false;
 
         $server = Mockery::mock(Server::class);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push')
             ->andReturnUsing(function ($fd, $data) use (&$expectedTrue, $expectedFd, $expectedMessage) {
                 $this->assertEquals($expectedFd, $fd);
@@ -287,7 +289,7 @@ public function testCanCallRefreshPersistence()
         Conveyor::refresh([$channelPersistence]);
 
         $server = Mockery::mock(Server::class);
-        $server->shouldReceive('isEstablished')->andReturn(true);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push');
 
         Conveyor::init()
@@ -325,6 +327,7 @@ function (string $data, int $fd, Server $server) {
         ]);
 
         $server = Mockery::mock(Server::class);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push')
             ->andReturnUsing(function ($fd, $data) use ($expectedResponse) {
                 $this->assertEquals($expectedResponse, $data);
@@ -365,12 +368,12 @@ public function testChannelAcknowledgmentAlone()
         ]);
 
         $server = Mockery::mock(Server::class);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push')
             ->andReturnUsing(function ($fd, $data) use ($expectedResponse) {
                 $this->assertEquals($expectedResponse, $data);
                 return true;
             });
-        $server->shouldReceive('isEstablished')->andReturnTrue();
 
         $clearVerification = false;
 
@@ -401,18 +404,17 @@ public function testChannelPresenceAlone()
         ]);
 
         $server = Mockery::mock(Server::class);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push')
             ->andReturnUsing(function ($fd, $data) {
                 // presence assertion (after acknowledgment)
                 $parsedData = json_decode($data, true);
                 $parsedMessage = json_decode($parsedData['data'], true);
                 $this->assertEquals(ChannelConnectAction::NAME, $parsedData['action']);
-                $this->assertTrue(isset($parsedData['id']));
                 $this->assertEquals(Constants::ACTION_EVENT_CHANNEL_PRESENCE, $parsedMessage['event']);
                 $this->assertCount(1, $parsedMessage['fds']);
                 return true;
             });
-        $server->shouldReceive('isEstablished')->andReturnTrue();
 
         $clearVerification = false;
 
@@ -448,6 +450,7 @@ public function testChannelAcknowledgmentAndPresence()
 
         $counter = 0;
         $server = Mockery::mock(Server::class);
+        $server->shouldReceive('isEstablished')->andReturnTrue();
         $server->shouldReceive('push')
             ->andReturnUsing(function ($fd, $data) use ($expectedResponse, &$counter) {
                 if ($counter === 0) {
@@ -465,7 +468,6 @@ public function testChannelAcknowledgmentAndPresence()
                 $counter++;
                 return true;
             });
-        $server->shouldReceive('isEstablished')->andReturnTrue();
 
         $clearVerification = false;