mark classes as final and methods and properties as private

php-http · Dec 2, 2018 · 4af4acf · 4af4acf
1 parent 14a2a39
commit 4af4acf
Show file tree

Hide file tree

Showing 16 changed files with 114 additions and 141 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Changed
 - Abstract method `HttpClientPool::chooseHttpClient()` has now an explicit return type (`Http\Client\Common\HttpClientPoolItem`)
 - Interface method `Plugin::handleRequest(...)` has now an explicit return type (`Http\Promise\Promise`)
+- Made all classes final as they are not intended to be extended.
 
 ### Removed
 - Deprecated option `debug_plugins` has been removed from `PluginClient`

diff --git a/spec/HttpClientPoolItemSpec.php → ...ClientPool/HttpClientPoolItemImplSpec.php b/spec/HttpClientPoolItemSpec.php → ...ClientPool/HttpClientPoolItemImplSpec.php
@@ -1,6 +1,6 @@
 <?php
 
-namespace spec\Http\Client\Common;
+namespace spec\Http\Client\Common\HttpClientPool;
 
 use Http\Client\Exception;
 use Http\Client\Exception\TransferException;
@@ -14,7 +14,7 @@
 use Psr\Http\Message\ResponseInterface;
 use Http\Client\Exception\RequestException;
 
-class HttpClientPoolItemSpec extends ObjectBehavior
+class HttpClientPoolItemImplSpec extends ObjectBehavior
 {
     public function let(HttpClient $httpClient)
     {

diff --git a/spec/HttpClientPool/LeastUsedClientPoolSpec.php b/spec/HttpClientPool/LeastUsedClientPoolSpec.php
@@ -2,7 +2,8 @@
 
 namespace spec\Http\Client\Common\HttpClientPool;
 
-use Http\Client\Common\HttpClientPoolItem;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItem;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItemImpl;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;
@@ -65,7 +66,7 @@ public function it_throw_exception_if_no_more_enable_client(HttpClient $client,
 
     public function it_reenable_client(HttpClient $client, RequestInterface $request)
     {
-        $this->addHttpClient(new HttpClientPoolItem($client->getWrappedObject(), 0));
+        $this->addHttpClient(new HttpClientPoolItemImpl($client->getWrappedObject(), 0));
         $client->sendRequest($request)->willThrow(HttpException::class);
 
         $this->shouldThrow(HttpException::class)->duringSendRequest($request);

diff --git a/spec/HttpClientPool/RandomClientPoolSpec.php b/spec/HttpClientPool/RandomClientPoolSpec.php
@@ -2,7 +2,7 @@
 
 namespace spec\Http\Client\Common\HttpClientPool;
 
-use Http\Client\Common\HttpClientPoolItem;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItemImpl;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;
@@ -65,7 +65,7 @@ public function it_throw_exception_if_no_more_enable_client(HttpClient $client,
 
     public function it_reenable_client(HttpClient $client, RequestInterface $request)
     {
-        $this->addHttpClient(new HttpClientPoolItem($client->getWrappedObject(), 0));
+        $this->addHttpClient(new HttpClientPoolItemImpl($client->getWrappedObject(), 0));
         $client->sendRequest($request)->willThrow(HttpException::class);
 
         $this->shouldThrow(HttpException::class)->duringSendRequest($request);

diff --git a/spec/HttpClientPool/RoundRobinClientPoolSpec.php b/spec/HttpClientPool/RoundRobinClientPoolSpec.php
@@ -2,7 +2,7 @@
 
 namespace spec\Http\Client\Common\HttpClientPool;
 
-use Http\Client\Common\HttpClientPoolItem;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItemImpl;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;
@@ -65,7 +65,7 @@ public function it_throw_exception_if_no_more_enable_client(HttpClient $client,
 
     public function it_reenable_client(HttpClient $client, RequestInterface $request)
     {
-        $this->addHttpClient(new HttpClientPoolItem($client->getWrappedObject(), 0));
+        $this->addHttpClient(new HttpClientPoolItemImpl($client->getWrappedObject(), 0));
         $client->sendRequest($request)->willThrow(HttpException::class);
 
         $this->shouldThrow(HttpException::class)->duringSendRequest($request);

diff --git a/spec/HttpMethodsClientSpec.php b/spec/HttpMethodsClientSpec.php
@@ -5,132 +5,88 @@
 use GuzzleHttp\Psr7\Response;
 use Http\Client\Common\HttpMethodsClient;
 use Http\Client\HttpClient;
-use Http\Message\MessageFactory;
+use Http\Message\RequestFactory;
 use PhpSpec\ObjectBehavior;
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
 
 class HttpMethodsClientSpec extends ObjectBehavior
 {
-    public function let(HttpClient $client, MessageFactory $messageFactory)
+    private static $requestData = [
+        'uri' => '/uri',
+        'headers' => [
+            'Content-Type' => 'text/plain',
+        ],
+        'body' => 'body',
+    ];
+
+    public function let(HttpClient $client, RequestFactory $requestFactory)
     {
         $this->beAnInstanceOf(
-            HttpMethodsClientStub::class, [
+            HttpMethodsClient::class, [
                 $client,
-                $messageFactory,
+                $requestFactory,
             ]
         );
     }
 
-    public function it_sends_a_get_request()
+    public function it_sends_a_get_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->get($data['uri'], $data['headers'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'get');
     }
 
-    public function it_sends_a_head_request()
-    {
-        $data = HttpMethodsClientStub::$requestData;
 
-        $this->head($data['uri'], $data['headers'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+    public function it_sends_a_head_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
+    {
+        $this->assert($client, $requestFactory, $request, $response, 'head');
     }
 
-    public function it_sends_a_trace_request()
+    public function it_sends_a_trace_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->trace($data['uri'], $data['headers'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'trace');
     }
 
-    public function it_sends_a_post_request()
+    public function it_sends_a_post_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->post($data['uri'], $data['headers'], $data['body'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'post', self::$requestData['body']);
     }
 
-    public function it_sends_a_put_request()
+    public function it_sends_a_put_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->put($data['uri'], $data['headers'], $data['body'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'put', self::$requestData['body']);
     }
 
-    public function it_sends_a_patch_request()
+    public function it_sends_a_patch_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->patch($data['uri'], $data['headers'], $data['body'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'patch', self::$requestData['body']);
     }
 
-    public function it_sends_a_delete_request()
+    public function it_sends_a_delete_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->delete($data['uri'], $data['headers'], $data['body'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'delete', self::$requestData['body']);
     }
 
-    public function it_sends_a_options_request()
+    public function it_sends_an_options_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->options($data['uri'], $data['headers'], $data['body'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $this->assert($client, $requestFactory, $request, $response, 'options', self::$requestData['body']);
     }
 
-    public function it_sends_request_with_underlying_client(HttpClient $client, MessageFactory $messageFactory, RequestInterface $request, ResponseInterface $response)
+    /**
+     * Run the actual test.
+     *
+     * As there is no data provider in phpspec, we keep separate methods to get new mocks for each test.
+     */
+    private function assert(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response, string $method, string $body = null)
     {
         $client->sendRequest($request)->shouldBeCalled()->willReturn($response);
+        $this->mockFactory($requestFactory, $request, strtoupper($method), $body);
 
-        $this->beConstructedWith($client, $messageFactory);
-        $this->sendRequest($request)->shouldReturn($response);
-    }
-}
+        $this->$method(self::$requestData['uri'], self::$requestData['headers'], self::$requestData['body'])->shouldReturnAnInstanceOf(ResponseInterface::class);
 
-class HttpMethodsClientStub extends HttpMethodsClient
-{
-    public static $requestData = [
-        'uri' => '/uri',
-        'headers' => [
-            'Content-Type' => 'text/plain',
-        ],
-        'body' => 'body',
-    ];
+    }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function send($method, $uri, array $headers = [], $body = null): ResponseInterface
+    private function mockFactory(RequestFactory $requestFactory, RequestInterface $request, string $method, string $body = null)
     {
-        if ($uri !== self::$requestData['uri']) {
-            throw new \InvalidArgumentException('Invalid URI: '.$uri);
-        }
-
-        if ($headers !== self::$requestData['headers']) {
-            throw new \InvalidArgumentException('Invalid headers: '.print_r($headers, true));
-        }
-
-        switch ($method) {
-            case 'GET':
-            case 'HEAD':
-            case 'TRACE':
-                if (null !== $body) {
-                    throw new \InvalidArgumentException('Non-empty body');
-                }
-
-                return new Response();
-            case 'POST':
-            case 'PUT':
-            case 'PATCH':
-            case 'DELETE':
-            case 'OPTIONS':
-                if ($body !== self::$requestData['body']) {
-                    throw new \InvalidArgumentException('Invalid body: '.print_r($body, true));
-                }
-
-                return new Response();
-            default:
-                throw new \InvalidArgumentException('Invalid method: '.$method);
-        }
+        $requestFactory->createRequest($method, self::$requestData['uri'], self::$requestData['headers'], $body)->willReturn($request);
     }
 }
diff --git a/src/BatchClient.php b/src/BatchClient.php
@@ -15,7 +15,7 @@
  *
  * @author Joel Wurtz <jwurtz@jolicode.com>
  */
-class BatchClient implements HttpClient
+final class BatchClient implements HttpClient
 {
     /**
      * @var HttpClient

diff --git a/src/Deferred.php b/src/Deferred.php
@@ -9,7 +9,7 @@
 /**
  * A deferred allow to return a promise which has not been resolved yet.
  */
-class Deferred implements Promise
+final class Deferred implements Promise
 {
     private $value;
 

diff --git a/src/HttpClientPool.php b/src/HttpClientPool.php
@@ -3,6 +3,8 @@
 namespace Http\Client\Common;
 
 use Http\Client\Common\Exception\HttpClientNotFoundException;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItem;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItemImpl;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Psr\Http\Message\RequestInterface;
@@ -27,7 +29,7 @@ abstract class HttpClientPool implements HttpAsyncClient, HttpClient
     public function addHttpClient($client)
     {
         if (!$client instanceof HttpClientPoolItem) {
-            $client = new HttpClientPoolItem($client);
+            $client = new HttpClientPoolItemImpl($client);
         }
 
         $this->clientPool[] = $client;

diff --git a/src/HttpClientPool/HttpClientPoolItem.php b/src/HttpClientPool/HttpClientPoolItem.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace Http\Client\Common\HttpClientPool;
+
+use Http\Client\HttpAsyncClient;
+use Http\Client\HttpClient;
+
+/**
+ * A HttpClientPoolItem represent a HttpClient inside a Pool.
+ *
+ * It is disabled when a request failed and can be reenabled after a certain number of seconds.
+ * It also keep tracks of the current number of open requests the client is currently being sending
+ * (only usable for async method).
+ *
+ * @author Joel Wurtz <joel.wurtz@gmail.com>
+ */
+interface HttpClientPoolItem extends HttpClient, HttpAsyncClient
+{
+    /**
+     * Whether this client is disabled or not.
+     *
+     * Will also reactivate this client if possible
+     *
+     * @internal
+     *
+     * @return bool
+     */
+    public function isDisabled();
+
+    /**
+     * Get current number of request that are currently being sent by the underlying HTTP client.
+     *
+     * @internal
+     *
+     * @return int
+     */
+    public function getSendingRequestCount();
+}