Merge pull request #123 from php-http/mark-as-final

mark classes as final and methods and properties as private
php-http · Dec 25, 2018 · 4d59cc4 · 4d59cc4
2 parents 5541efd + fadb51a
commit 4d59cc4
Show file tree

Hide file tree

Showing 23 changed files with 326 additions and 306 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@
 - RetryPlugin will no longer retry requests when the response failed with a HTTP code < 500.
 - 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  classes final that are not intended to be extended.
+  Added interfaces for BatchClient, HttpClientRouter and HttpMethodsClient.
+  (These interfaces use the `Interface` suffix to avoid name collisions.)
+- Added an interface for HttpClientPool and moved the abstract class to the HttpClientPool sub namespace.
 
 ### Removed
 - Deprecated option `debug_plugins` has been removed from `PluginClient`

diff --git a/spec/HttpClientPoolItemSpec.php → ...HttpClientPool/HttpClientPoolItemSpec.php b/spec/HttpClientPoolItemSpec.php → ...HttpClientPool/HttpClientPoolItemSpec.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;

diff --git a/spec/HttpClientPool/LeastUsedClientPoolSpec.php b/spec/HttpClientPool/LeastUsedClientPoolSpec.php
@@ -2,7 +2,7 @@
 
 namespace spec\Http\Client\Common\HttpClientPool;
 
-use Http\Client\Common\HttpClientPoolItem;
+use Http\Client\Common\HttpClientPool\HttpClientPoolItem;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;

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\HttpClientPoolItem;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;

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\HttpClientPoolItem;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;

diff --git a/spec/HttpClientRouterSpec.php b/spec/HttpClientRouterSpec.php
@@ -2,14 +2,15 @@
 
 namespace spec\Http\Client\Common;
 
+use Http\Client\Common\HttpClientRouter;
 use Http\Message\RequestMatcher;
 use Http\Client\HttpAsyncClient;
 use Http\Client\HttpClient;
 use Http\Promise\Promise;
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
 use PhpSpec\ObjectBehavior;
-use Http\Client\Common\HttpClientRouter;
+use Http\Client\Common\HttpClientRouterInterface;
 use Http\Client\Exception\RequestException;
 
 class HttpClientRouterSpec extends ObjectBehavior
@@ -19,6 +20,11 @@ public function it_is_initializable()
         $this->shouldHaveType(HttpClientRouter::class);
     }
 
+    public function it_is_an_http_client_router()
+    {
+        $this->shouldImplement(HttpClientRouterInterface::class);
+    }
+
     public function it_is_an_http_client()
     {
         $this->shouldImplement(HttpClient::class);

diff --git a/spec/HttpMethodsClientSpec.php b/spec/HttpMethodsClientSpec.php
@@ -2,135 +2,88 @@
 
 namespace spec\Http\Client\Common;
 
-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()
+    public function it_sends_a_head_request(HttpClient $client, RequestFactory $requestFactory, RequestInterface $request, ResponseInterface $response)
     {
-        $data = HttpMethodsClientStub::$requestData;
-
-        $this->head($data['uri'], $data['headers'])->shouldReturnAnInstanceOf(ResponseInterface::class);
+        $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/spec/Plugin/AddPathPluginSpec.php b/spec/Plugin/AddPathPluginSpec.php
@@ -46,7 +46,7 @@ public function it_adds_path(
         $this->handleRequest($request, PluginStub::next(), function () {});
     }
 
-    function it_removes_ending_slashes(
+    public function it_removes_ending_slashes(
         RequestInterface $request,
         UriInterface $host,
         UriInterface $host2,

diff --git a/src/BatchClient.php b/src/BatchClient.php
@@ -8,14 +8,7 @@
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
 
-/**
- * BatchClient allow to sends multiple request and retrieve a Batch Result.
- *
- * This implementation simply loops over the requests and uses sendRequest with each of them.
- *
- * @author Joel Wurtz <jwurtz@jolicode.com>
- */
-class BatchClient implements HttpClient
+final class BatchClient implements BatchClientInterface
 {
     /**
      * @var HttpClient
@@ -27,28 +20,11 @@ public function __construct(HttpClient $client)
         $this->client = $client;
     }
 
-    /**
-     * {@inheritdoc}
-     */
     public function sendRequest(RequestInterface $request): ResponseInterface
     {
         return $this->client->sendRequest($request);
     }
 
-    /**
-     * Send several requests.
-     *
-     * You may not assume that the requests are executed in a particular order. If the order matters
-     * for your application, use sendRequest sequentially.
-     *
-     * @param RequestInterface[] The requests to send
-     *
-     * @return BatchResult Containing one result per request
-     *
-     * @throws BatchException If one or more requests fails. The exception gives access to the
-     *                        BatchResult with a map of request to result for success, request to
-     *                        exception for failures
-     */
     public function sendRequests(array $requests): BatchResult
     {
         $batchResult = new BatchResult();

diff --git a/src/BatchClientInterface.php b/src/BatchClientInterface.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace Http\Client\Common;
+
+use Http\Client\Exception;
+use Http\Client\HttpClient;
+use Http\Client\Common\Exception\BatchException;
+use Psr\Http\Message\RequestInterface;
+
+/**
+ * BatchClient allow to sends multiple request and retrieve a Batch Result.
+ *
+ * This implementation simply loops over the requests and uses sendRequest with each of them.
+ *
+ * @author Joel Wurtz <jwurtz@jolicode.com>
+ */
+interface BatchClientInterface extends HttpClient
+{
+    /**
+     * Send several requests.
+     *
+     * You may not assume that the requests are executed in a particular order. If the order matters
+     * for your application, use sendRequest sequentially.
+     *
+     * @param RequestInterface[] The requests to send
+     *
+     * @return BatchResult Containing one result per request
+     *
+     * @throws BatchException If one or more requests fails. The exception gives access to the
+     *                        BatchResult with a map of request to result for success, request to
+     *                        exception for failures
+     */
+    public function sendRequests(array $requests): BatchResult;
+}
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/EmulatedHttpAsyncClient.php b/src/EmulatedHttpAsyncClient.php
@@ -6,13 +6,11 @@
 use Http\Client\HttpClient;
 
 /**
- * Emulates an async HTTP client.
- *
- * This should be replaced by an anonymous class in PHP 7.
+ * Emulates an async HTTP client with the help of a synchronous client.
  *
  * @author Márk Sági-Kazár <mark.sagikazar@gmail.com>
  */
-class EmulatedHttpAsyncClient implements HttpClient, HttpAsyncClient
+final class EmulatedHttpAsyncClient implements HttpClient, HttpAsyncClient
 {
     use HttpAsyncClientEmulator;
     use HttpClientDecorator;

diff --git a/src/EmulatedHttpClient.php b/src/EmulatedHttpClient.php
@@ -6,13 +6,11 @@
 use Http\Client\HttpClient;
 
 /**
- * Emulates an HTTP client.
- *
- * This should be replaced by an anonymous class in PHP 7.
+ * Emulates a synchronous HTTP client with the help of an asynchronous client.
  *
  * @author Márk Sági-Kazár <mark.sagikazar@gmail.com>
  */
-class EmulatedHttpClient implements HttpClient, HttpAsyncClient
+final class EmulatedHttpClient implements HttpClient, HttpAsyncClient
 {
     use HttpAsyncClientDecorator;
     use HttpClientEmulator;