Upgrade HTTPlug from Legacy to PSR 17/18 Discovery Factories

composer.json

@@ -12,20 +12,20 @@
     ],
     "require": {
         "php": "^7.2 || ^8.0",
-        "psr/http-message": "^1.0",
-        "php-http/httplug": "^1.1 || ^2.0",
+        "ext-json": "*",
+        "composer-runtime-api": "^2.0",
+        "php-http/client-common": "^1.9 || ^2.0",
         "php-http/discovery": "^1.0",
         "psr/http-client-implementation": "^1.0",
-        "php-http/client-common": "^1.9 || ^2.0",
         "php-http/message-factory": "^1.0",
-        "composer-runtime-api": "^2.0"
+        "psr/http-message-implementation": "^1.0"
     },
     "require-dev": {
-        "phpunit/phpunit": "^8.0 || ^9.0",
-        "guzzlehttp/guzzle": "^7",
-        "php-http/mock-client": "^1.0",
         "friendsofphp/php-cs-fixer": "^3.0",
-        "phpstan/phpstan": "^1.2"
+        "guzzlehttp/guzzle": "^7.0",
+        "php-http/mock-client": "^1.0",
+        "phpstan/phpstan": "^1.2",
+        "phpunit/phpunit": "^8.0 || ^9.0"
     },
     "autoload": {
         "psr-4": { "PrivatePackagist\\ApiClient\\": "src/" }

src/Client.php

@@ -10,7 +10,7 @@
 namespace PrivatePackagist\ApiClient;
 
 use Http\Client\Common\Plugin;
-use Http\Discovery\UriFactoryDiscovery;
+use Http\Discovery\Psr17FactoryDiscovery;
 use PrivatePackagist\ApiClient\HttpClient\HttpPluginClientBuilder;
 use PrivatePackagist\ApiClient\HttpClient\Message\ResponseMediator;
 use PrivatePackagist\ApiClient\HttpClient\Plugin\ExceptionThrower;
@@ -31,7 +31,7 @@ public function __construct(HttpPluginClientBuilder $httpClientBuilder = null, $
         $privatePackagistUrl = $privatePackagistUrl ? : 'https://packagist.com';
         $this->responseMediator = $responseMediator ? : new ResponseMediator();
 
-        $builder->addPlugin(new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri($privatePackagistUrl)));
+        $builder->addPlugin(new Plugin\AddHostPlugin(Psr17FactoryDiscovery::findUriFactory()->createUri($privatePackagistUrl)));
         $builder->addPlugin(new PathPrepend('/api'));
         $builder->addPlugin(new Plugin\RedirectPlugin());
         $headers = [

src/HttpClient/HttpPluginClientBuilder.php

@@ -12,26 +12,59 @@
 use Http\Client\Common\HttpMethodsClient;
 use Http\Client\Common\Plugin;
 use Http\Client\Common\PluginClient;
-use Http\Discovery\HttpClientDiscovery;
-use Http\Discovery\MessageFactoryDiscovery;
+use Http\Discovery\Psr17FactoryDiscovery;
+use Http\Discovery\Psr18ClientDiscovery;
 use Http\Message\RequestFactory;
 use Psr\Http\Client\ClientInterface;
+use Psr\Http\Message\RequestFactoryInterface;
+use Psr\Http\Message\StreamFactoryInterface;
 
 class HttpPluginClientBuilder
 {
     /** @var ClientInterface */
     private $httpClient;
     /** @var HttpMethodsClient|null */
     private $pluginClient;
-    /** @var RequestFactory */
+    /** @var RequestFactory|RequestFactoryInterface */
     private $requestFactory;
+    /** @var StreamFactoryInterface */
+    private $streamFactory;
     /** @var Plugin[] */
     private $plugins = [];
 
-    public function __construct(ClientInterface $httpClient = null, RequestFactory $requestFactory = null)
-    {
-        $this->httpClient = $httpClient ?: HttpClientDiscovery::find();
-        $this->requestFactory = $requestFactory ?: MessageFactoryDiscovery::find();
+    /**
+     * @param RequestFactory|RequestFactoryInterface|null $requestFactory
+     * @param StreamFactoryInterface|null $streamFactory
+     */
+    public function __construct(
+        ?ClientInterface $httpClient = null,
+        $requestFactory = null,
+        ?StreamFactoryInterface $streamFactory= null
+    ) {
+        $requestFactory = $requestFactory ?? Psr17FactoryDiscovery::findRequestFactory();
+        if ($requestFactory instanceof RequestFactory) {
+            // Use same format as symfony/deprecation-contracts.
+            @trigger_error(sprintf(
+                'Since %s %s: %s is deprecated, use %s instead.',
+                'private-packagist/api-client',
+                '1.36.0',
+                RequestFactory::class,
+                RequestFactoryInterface::class
+            ), \E_USER_DEPRECATED);
+        } elseif (!$requestFactory instanceof RequestFactoryInterface) {
+            /** @var mixed $requestFactory value unknown; set to mixed, prevent PHPStan complaining about guard clauses */
+            throw new \TypeError(sprintf(
+                '%s::__construct(): Argument #2 ($requestFactory) must be of type %s|%s, %s given',
+                self::class,
+                RequestFactory::class,
+                RequestFactoryInterface::class,
+                is_object($requestFactory) ? get_class($requestFactory) : gettype($requestFactory)
+            ));
+        }
+
+        $this->httpClient = $httpClient ?? Psr18ClientDiscovery::find();
+        $this->requestFactory = $requestFactory;
+        $this->streamFactory = $streamFactory ?? Psr17FactoryDiscovery::findStreamFactory();
     }
 
     public function addPlugin(Plugin $plugin)
@@ -41,7 +74,7 @@ public function addPlugin(Plugin $plugin)
     }
 
     /**
-     * @param string $pluginClass
+     * @param class-string $pluginClass
      */
     public function removePlugin($pluginClass)
     {
@@ -58,7 +91,8 @@ public function getHttpClient()
         if (!$this->pluginClient) {
             $this->pluginClient = new HttpMethodsClient(
                 new PluginClient($this->httpClient, $this->plugins),
-                $this->requestFactory
+                $this->requestFactory,
+                $this->streamFactory
             );
         }
 

tests/HttpClient/HttpPluginClientBuilderTest.php

@@ -0,0 +1,81 @@
+<?php
+
+/*
+ * (c) Packagist Conductors GmbH <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace PrivatePackagist\ApiClient\HttpClient\Plugin;
+
+use GuzzleHttp\Psr7\HttpFactory;
+use GuzzleHttp\Psr7\Response;
+use Http\Client\Common\HttpMethodsClientInterface;
+use Http\Message\MessageFactory\GuzzleMessageFactory;
+use Http\Message\RequestMatcher as RequestMatcherInterface;
+use Http\Mock\Client as MockClient;
+use PHPUnit\Framework\TestCase;
+use PrivatePackagist\ApiClient\HttpClient\HttpPluginClientBuilder;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+
+class HttpPluginClientBuilderTest extends TestCase
+{
+    public function testInvalidRequestFactory(): void
+    {
+        $this->expectException(\TypeError::class);
+        $definitelyNotARequestFactory = new \stdClass;
+        /** @phpstan-ignore-next-line We are passing in an invalid type on purpose. */
+        new HttpPluginClientBuilder(new MockClient, $definitelyNotARequestFactory);
+    }
+
+    /** @dataProvider provideRequestFactories */
+    public function testRequestFactory(?object $factory): void
+    {
+        $mockHttp = new MockClient;
+        $mockHttp->setDefaultException(new \Exception('Mock HTTP client did not match request.'));
+        $mockHttp->on($this->matchRequestIncludingHeaders(), new Response(307, ['Location' => '/kittens.jpg']));
+
+        $builder = new HttpPluginClientBuilder($mockHttp, $factory);
+        // Make sure that the RequestFactory passed is acceptable for the client.
+        $client = $builder->getHttpClient();
+        $this->assertInstanceOf(HttpMethodsClientInterface::class, $client);
+
+        // Ensure that the Request Factory correctly generates a request object (including headers
+        // as RequestFactory and RequestFactoryInterface set headers differently).
+        $response = $client->get('https://example.com/puppies.jpg', ['Accept' => 'image/vnd.cute+jpeg']);
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertSame(307, $response->getStatusCode());
+        $locationHeaders = $response->getHeader('Location');
+        $this->assertCount(1, $locationHeaders);
+        $this->assertSame('/kittens.jpg', reset($locationHeaders));
+    }
+
+    /**
+     * The concrete implementation of the RequestMatcher interface does not allow matching on
+     * headers, which we need to test to ensure both legacy and PSR17 implementations work.
+     */
+    private function matchRequestIncludingHeaders(): RequestMatcherInterface
+    {
+        return new class implements RequestMatcherInterface {
+            public function matches(RequestInterface $request): bool
+            {
+                $acceptHeaders = $request->getHeader('Accept');
+                return $request->getUri()->getPath() === '/puppies.jpg'
+                    && count($acceptHeaders) === 1
+                    && reset($acceptHeaders) === 'image/vnd.cute+jpeg';
+            }
+        };
+    }
+
+    /** @return iterable{object|null} */
+    public static function provideRequestFactories(): iterable
+    {
+        yield [null];
+        // Http\Message\RequestFactory
+        yield [new GuzzleMessageFactory];
+        // Psr\Http\Message\RequestFactoryInterface
+        yield [new HttpFactory];
+    }
+}