Merge pull request swooletw#302 from swooletw/develop

1. improve onHandShake callback
2. make pid file compatible with older versions

Author: albertcht

config/swoole_websocket.php

@@ -37,8 +37,7 @@
 
     /*
     |--------------------------------------------------------------------------
-    | Websocket handler for onHandShake callback
-    | If enable, onOpen would not be triggered
+    | Websocket handler for customized onHandShake callback
     |--------------------------------------------------------------------------
     */
     'handshake' => [

src/Commands/HttpServerCommand.php

@@ -212,8 +212,9 @@ protected function showInfos()
         $isWebsocket = Arr::get($this->config, 'websocket.enabled');
         $hasTaskWorker = $isWebsocket || Arr::get($this->config, 'queue.default') === 'swoole';
         $logFile = Arr::get($this->config, 'server.options.log_file');
-        $pidManager = $this->laravel->make(PidManager::class);
-        [$masterPid, $managerPid] = $pidManager->read();
+        $pids = $this->laravel->make(PidManager::class)->read();
+        $masterPid = $pids['masterPid'] ?? null;
+        $managerPid = $pids['managerPid'] ?? null;
 
         $table = [
             ['PHP Version', 'Version' => phpversion()],

src/Concerns/InteractsWithWebsocket.php

@@ -53,73 +53,54 @@ trait InteractsWithWebsocket
     protected $wsEvents = ['open', 'message', 'close'];
 
     /**
-     * "onOpen" listener.
-     *
-     * @param \Swoole\Websocket\Server $server
+     * "onHandShake" listener.
      * @param \Swoole\Http\Request $swooleRequest
+     * @param \Swoole\Http\Response $response
      */
-    public function onOpen($server, $swooleRequest)
+    public function onHandShake($swooleRequest, $response)
     {
-        $illuminateRequest = Request::make($swooleRequest)->toIlluminate();
-        $websocket = $this->app->make(Websocket::class);
-        $sandbox = $this->app->make(Sandbox::class);
-
-        try {
-            $websocket->reset(true)->setSender($swooleRequest->fd);
-            // set currnt request to sandbox
-            $sandbox->setRequest($illuminateRequest);
-            // enable sandbox
-            $sandbox->enable();
-            // check if socket.io connection established
-            if (! $this->websocketHandler->onOpen($swooleRequest->fd, $illuminateRequest)) {
-                return;
-            }
-            // trigger 'connect' websocket event
-            if ($websocket->eventExists('connect')) {
-                // set sandbox container to websocket pipeline
-                $websocket->setContainer($sandbox->getApplication());
-                $websocket->call('connect', $illuminateRequest);
-            }
-        } catch (Throwable $e) {
-            $this->logServerError($e);
-        } finally {
-            // disable and recycle sandbox resource
-            $sandbox->disable();
-        }
+        $this->onOpen(
+            $this->app->make(Server::class),
+            $swooleRequest,
+            $response
+        );
     }
 
     /**
-     * @param \Swoole\Http\Request $swooleRequest
-     * @param \Swoole\Http\Response $response
+     * "onOpen" listener.
      *
-     * @return bool
-     * @throws \Illuminate\Contracts\Container\BindingResolutionException
+     * @param \Swoole\Websocket\Server $server
+     * @param \Swoole\Http\Request $swooleRequest
+     * @param \Swoole\Http\Response $response (optional)
      */
-    public function onHandShake($swooleRequest, $response)
+    public function onOpen($server, $swooleRequest, $response = null)
     {
         $illuminateRequest = Request::make($swooleRequest)->toIlluminate();
         $websocket = $this->app->make(Websocket::class);
         $sandbox = $this->app->make(Sandbox::class);
-        $handler = $this->container->make('config')->get('swoole_websocket.handshake.handler');
+        $handshakeHandler = $this->app->make('config')
+            ->get('swoole_websocket.handshake.handler');
 
         try {
             $websocket->reset(true)->setSender($swooleRequest->fd);
             // set currnt request to sandbox
             $sandbox->setRequest($illuminateRequest);
             // enable sandbox
             $sandbox->enable();
-
-            if (! $this->app->make($handler)->handle($swooleRequest, $response)) {
-                return false;
+            // call customized handshake handler
+            if ($response && ! $this->app->make($handshakeHandler)->handle($swooleRequest, $response)) {
+                return;
+            }
+            // check if socket.io connection established
+            if (! $this->websocketHandler->onOpen($swooleRequest->fd, $illuminateRequest)) {
+                return;
             }
             // trigger 'connect' websocket event
             if ($websocket->eventExists('connect')) {
                 // set sandbox container to websocket pipeline
                 $websocket->setContainer($sandbox->getApplication());
                 $websocket->call('connect', $illuminateRequest);
             }
-
-            return true;
         } catch (Throwable $e) {
             $this->logServerError($e);
         } finally {
@@ -258,17 +239,19 @@ public function getPayloadParser()
     protected function prepareWebsocket()
     {
         $config = $this->container->make('config');
-        $isWebsocket = $config->get('swoole_http.websocket.enabled');
         $parser = $config->get('swoole_websocket.parser');
 
-        if ($isWebsocket) {
-            $handshake = $config->get('swoole_websocket.handshake.enabled');
+        if (! $this->isServerWebsocket = $config->get('swoole_http.websocket.enabled')) {
+            return;
+        }
 
-            $this->events = array_merge($this->events ?? [], array_merge($this->wsEvents, $handshake ? ['handshake'] : []));
-            $this->isServerWebsocket = true;
-            $this->prepareWebsocketRoom();
-            $this->setPayloadParser(new $parser);
+        if ($config->get('swoole_websocket.handshake.enabled')) {
+            $this->wsEvents = array_merge($this->wsEvents, ['handshake']);
         }
+
+        $this->events = array_merge($this->events ?? [], $this->wsEvents);
+        $this->prepareWebsocketRoom();
+        $this->setPayloadParser(new $parser);
     }
 
     /**

src/Server/PidManager.php

@@ -58,7 +58,10 @@ public function read(): array
             $pids = explode(',', $content);
         }
 
-        return $pids;
+        return [
+            'masterPid' => $pids[0] ?? null,
+            'managerPid' => $pids[1] ?? null
+        ];
     }
 
     /**

tests/Server/ManagerTest.php

@@ -471,8 +471,14 @@ public function testOnHandShake()
 
         $container->alias(Sandbox::class, 'swoole.sandbox');
 
+        $handler = m::mock(HandlerContract::class);
+        $handler->shouldReceive('onOpen')
+                ->with(1, m::type('Illuminate\Http\Request'))
+                ->andReturn(true);
+
         $manager = $this->getWebsocketManager();
         $manager->setApplication($container);
+        $manager->setWebsocketHandler($handler);
         $manager->onHandShake($request, $response);
     }
 

tests/Server/PidManagerTest.php

@@ -67,7 +67,7 @@ public function testRead()
         });
 
         $this->assertEquals(
-            ['1', '2'],
+            ['masterPid' => '1', 'managerPid' => '2'],
             $pidManager->read()
         );
     }