Fix keep alive when not use pool (#38)

Fix keep alive when not use the pool

Author: ihipop

.gitignore

@@ -10,3 +10,4 @@ temp/
 *.lock
 /tools
 /debug
+.phpunit.*

src/Request.php

@@ -115,7 +115,7 @@ public function isWaiting(): bool
         return $this->_status === self::STATUS_WAITING;
     }
 
-    protected function getConnectionTarget(): array
+    public function getConnectionTarget(): array
     {
         $host = $this->uri->getHost();
         if (empty($host)) {
@@ -128,6 +128,12 @@ protected function getConnectionTarget(): array
         return ['host' => $host, 'port' => $port, 'ssl' => $ssl];
     }
 
+    public function shouldRecycleClient($client)
+    {
+        $connectionInfo = $this->getConnectionTarget();
+
+        return (!$client || ($client->host !== $connectionInfo['host'] || $client->port !== $connectionInfo['port']));
+    }
     /** @return null|bool */
     public function getPool()
     {
@@ -498,24 +504,20 @@ public function exec()
             return $ret;
         }
 
-        /** get connection info */
-        list($host, $port, $ssl) = array_values($this->getConnectionTarget());
-        if ($this->client && ($this->client->host !== $host || $this->client->port !== $port)) {
+
+        if ($this->client && ($this->shouldRecycleClient($this->client))) {
             // target maybe changed
             $this->tryToRevertClientToPool();
         }
         if (!$this->client) {
+            /** get connection info */
+            $connectionInfo = $this->getConnectionTarget();
             /** create a new coroutine client */
             $client_pool = ClientPool::getInstance();
-            if ($this->use_pool && $client = $client_pool->getEx($host, $port)) {
+            if ($this->use_pool && $client = $client_pool->getEx($connectionInfo['host'], $connectionInfo['port'])) {
                 $this->client = $client;
             } else {
-                $options = [
-                    'host' => $host,
-                    'port' => $port,
-                    'ssl' => $ssl
-                ];
-                $this->client = $client_pool->createEx($options, !$this->use_pool);
+                $this->client = $client_pool->createEx($connectionInfo, !$this->use_pool);
             }
         }
 
@@ -748,7 +750,9 @@ public function recv()
         }
         $this->client->body = '';
 
-        $this->tryToRevertClientToPool();
+        if($this->use_pool){
+            $this->tryToRevertClientToPool();
+        }
 
         return $response;
     }

src/Saber.php

@@ -147,6 +147,8 @@ private function __construct(array $options = [])
         );
     }
 
+    /** @var \Swoole\Coroutine\Http\Client  a temp client when not use pool */
+    protected $lastTempClient;
     /**
      * @param array $options
      * @return Request|Response
@@ -156,6 +158,15 @@ public function request(array $options)
         $request = clone $this->raw;
         $this->setOptions($options, $request);
 
+        if (!$request->getPool()) {
+            $lastTempClient =& $this->lastTempClient;
+            if ($request->shouldRecycleClient($lastTempClient)) {
+                $lastTempClient = $request->client = \Swlib\Saber\ClientPool::getInstance()->createEx($request->getConnectionTarget(), true);
+                //This Temp Client will Recyle by https://github.com/swlib/saber/blob/1188d0a67d18430d5c1a11f8dcdc135852fc1e31/src/Request.php#L502-L506
+            } else {
+                $request->client = $lastTempClient;
+            }
+        }
         /** Psr style */
         if ($options['psr'] ?? false) {
             return $request;

tests/SaberTest.php

@@ -16,6 +16,7 @@
 use Swlib\Http\Exception\ServerException;
 use Swlib\Http\Exception\TooManyRedirectsException;
 use Swlib\Http\SwUploadFile;
+use Swlib\Http\Uri;
 use Swlib\Saber;
 use Swlib\SaberGM;
 use Swoole\Coroutine;
@@ -317,4 +318,30 @@ public function testFinalClear()
         $this->assertTrue(saber_pool_release());
     }
 
+    public function testKeepAliveAmongSameHostAndPortWithOutUsePool()
+    {
+        global $server_list;
+        list($ip, $port) = array_values($server_list['httpd']);
+        $saber = Saber::create([
+            'base_uri' => "http://$ip:$port",
+            //'base_uri' => "http://127.0.0.1:8081",
+            'use_pool' => false,
+            'exception_report' => HttpExceptionMask::E_ALL
+        ]);
+
+        $ReqWithSaber         = $saber->get('/anything?dump_info=$ReqWithSaber')->getParsedJsonArray();
+        $ReqWithSaber2        = $saber->get('/anything?dump_info=$ReqWithSaber2')->getParsedJsonArray();
+        $ReqWithSaberPSR      = $saber->request(['psr' => 1])->withMethod('GET')->withUri(new Uri("http://$ip:$port/anything?dump_info=ReqWithSaberPSR"))->exec()->recv()->getParsedJsonArray();
+        $ReqWithSaberPSR2     = $saber->request(['psr' => 1])->withMethod('GET')->withUri(new Uri("http://$ip:$port/anything?dump_info=ReqWithSaberPSR2"))->exec()->recv()->getParsedJsonArray();
+        $ReqAfterAnotherPort        = $saber->get('http://httpbin.org/anything?dump_info=$ReqWithSaber2')->getParsedJsonArray();
+        $ReqAfterAnotherPort        = $saber->get('/anything?dump_info=$ReqWithSaber2')->getParsedJsonArray();
+
+        $this->assertTrue($ReqWithSaber['server']['remote_port'] === $ReqWithSaber2['server']['remote_port']);
+        $this->assertTrue($ReqWithSaberPSR['server']['remote_port'] === $ReqWithSaberPSR2['server']['remote_port']);
+        $this->assertTrue($ReqWithSaber2['server']['remote_port'] === $ReqWithSaberPSR2['server']['remote_port']);
+        $this->assertFalse($ReqWithSaber2['server']['remote_port'] === $ReqAfterAnotherPort['server']['remote_port']);
+        $this->assertTrue($ReqWithSaber2['header']['connection'] === 'keep-alive');
+        $this->assertTrue($ReqWithSaberPSR2['header']['connection'] === 'keep-alive');
+    }
+
 }

tests/servers/httpd.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * Copyright: Swlib
+ * Author: Twosee <[email protected]>
+ * Maintainer: ihipop <[email protected]>
+ * Date: 2019年04月23日17:57:10
+ */
+
+$http = new Swoole\Http\Server($argv[1], $argv[2]);
+$http->set(['worker_num' => 1, 'log_file' => '/dev/null']);
+$http->on('request', function ($request, $response) {
+    $response->end(json_encode($request,JSON_UNESCAPED_UNICODE|JSON_UNESCAPED_SLASHES));
+});
+$http->start();

tests/servers/manager.php

@@ -43,6 +43,7 @@ function run_server(string $file): array
     }
 
     return [
-        'mixed' => run_server(__DIR__ . '/mixed.php')
+        'mixed' => run_server(__DIR__ . '/mixed.php'),
+        'httpd' => run_server(__DIR__ . '/httpd.php')
     ];
 })();