Skip to content

Commit 2fa51cf

Browse files
glaubinixglaubinix
authored
Merge pull request #67 from packagist/z/httplug-upgrade
Upgrade HTTPlug from Legacy to PSR 17/18 Discovery Factories
2 parents 98aa7dd + 9427e98 commit 2fa51cf

File tree

4 files changed

+134
-19
lines changed

4 files changed

+134
-19
lines changed

‎composer.json‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,20 @@
1212
],
1313
"require": {
1414
"php": "^7.2 || ^8.0",
15-
"psr/http-message": "^1.0",
16-
"php-http/httplug": "^1.1 || ^2.0",
15+
"ext-json": "*",
16+
"composer-runtime-api": "^2.0",
17+
"php-http/client-common": "^1.9 || ^2.0",
1718
"php-http/discovery": "^1.0",
1819
"psr/http-client-implementation": "^1.0",
19-
"php-http/client-common": "^1.9 || ^2.0",
2020
"php-http/message-factory": "^1.0",
21-
"composer-runtime-api": "^2.0"
21+
"psr/http-message-implementation": "^1.0"
2222
},
2323
"require-dev": {
24-
"phpunit/phpunit": "^8.0 || ^9.0",
25-
"guzzlehttp/guzzle": "^7",
26-
"php-http/mock-client": "^1.0",
2724
"friendsofphp/php-cs-fixer": "^3.0",
28-
"phpstan/phpstan": "^1.2"
25+
"guzzlehttp/guzzle": "^7.0",
26+
"php-http/mock-client": "^1.0",
27+
"phpstan/phpstan": "^1.2",
28+
"phpunit/phpunit": "^8.0 || ^9.0"
2929
},
3030
"autoload": {
3131
"psr-4": { "PrivatePackagist\\ApiClient\\": "src/" }

‎src/Client.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
namespace PrivatePackagist\ApiClient;
1111

1212
use Http\Client\Common\Plugin;
13-
use Http\Discovery\UriFactoryDiscovery;
13+
use Http\Discovery\Psr17FactoryDiscovery;
1414
use PrivatePackagist\ApiClient\HttpClient\HttpPluginClientBuilder;
1515
use PrivatePackagist\ApiClient\HttpClient\Message\ResponseMediator;
1616
use PrivatePackagist\ApiClient\HttpClient\Plugin\ExceptionThrower;
@@ -31,7 +31,7 @@ public function __construct(HttpPluginClientBuilder $httpClientBuilder = null, $
3131
$privatePackagistUrl = $privatePackagistUrl ? : 'https://packagist.com';
3232
$this->responseMediator = $responseMediator ? : new ResponseMediator();
3333

34-
$builder->addPlugin(new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri($privatePackagistUrl)));
34+
$builder->addPlugin(new Plugin\AddHostPlugin(Psr17FactoryDiscovery::findUriFactory()->createUri($privatePackagistUrl)));
3535
$builder->addPlugin(new PathPrepend('/api'));
3636
$builder->addPlugin(new Plugin\RedirectPlugin());
3737
$headers = [

‎src/HttpClient/HttpPluginClientBuilder.php‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,59 @@
1212
use Http\Client\Common\HttpMethodsClient;
1313
use Http\Client\Common\Plugin;
1414
use Http\Client\Common\PluginClient;
15-
use Http\Discovery\HttpClientDiscovery;
16-
use Http\Discovery\MessageFactoryDiscovery;
15+
use Http\Discovery\Psr17FactoryDiscovery;
16+
use Http\Discovery\Psr18ClientDiscovery;
1717
use Http\Message\RequestFactory;
1818
use Psr\Http\Client\ClientInterface;
19+
use Psr\Http\Message\RequestFactoryInterface;
20+
use Psr\Http\Message\StreamFactoryInterface;
1921

2022
class HttpPluginClientBuilder
2123
{
2224
/** @var ClientInterface */
2325
private $httpClient;
2426
/** @var HttpMethodsClient|null */
2527
private $pluginClient;
26-
/** @var RequestFactory */
28+
/** @var RequestFactory|RequestFactoryInterface */
2729
private $requestFactory;
30+
/** @var StreamFactoryInterface */
31+
private $streamFactory;
2832
/** @var Plugin[] */
2933
private $plugins = [];
3034

31-
public function __construct(ClientInterface $httpClient = null, RequestFactory $requestFactory = null)
32-
{
33-
$this->httpClient = $httpClient ?: HttpClientDiscovery::find();
34-
$this->requestFactory = $requestFactory ?: MessageFactoryDiscovery::find();
35+
/**
36+
* @param RequestFactory|RequestFactoryInterface|null $requestFactory
37+
* @param StreamFactoryInterface|null $streamFactory
38+
*/
39+
public function __construct(
40+
?ClientInterface $httpClient = null,
41+
$requestFactory = null,
42+
?StreamFactoryInterface $streamFactory= null
43+
) {
44+
$requestFactory = $requestFactory ?? Psr17FactoryDiscovery::findRequestFactory();
45+
if ($requestFactory instanceof RequestFactory) {
46+
// Use same format as symfony/deprecation-contracts.
47+
@trigger_error(sprintf(
48+
'Since %s %s: %s is deprecated, use %s instead.',
49+
'private-packagist/api-client',
50+
'1.36.0',
51+
RequestFactory::class,
52+
RequestFactoryInterface::class
53+
), \E_USER_DEPRECATED);
54+
} elseif (!$requestFactory instanceof RequestFactoryInterface) {
55+
/** @var mixed $requestFactory value unknown; set to mixed, prevent PHPStan complaining about guard clauses */
56+
throw new \TypeError(sprintf(
57+
'%s::__construct(): Argument #2 ($requestFactory) must be of type %s|%s, %s given',
58+
self::class,
59+
RequestFactory::class,
60+
RequestFactoryInterface::class,
61+
is_object($requestFactory) ? get_class($requestFactory) : gettype($requestFactory)
62+
));
63+
}
64+
65+
$this->httpClient = $httpClient ?? Psr18ClientDiscovery::find();
66+
$this->requestFactory = $requestFactory;
67+
$this->streamFactory = $streamFactory ?? Psr17FactoryDiscovery::findStreamFactory();
3568
}
3669

3770
public function addPlugin(Plugin $plugin)
@@ -41,7 +74,7 @@ public function addPlugin(Plugin $plugin)
4174
}
4275

4376
/**
44-
* @param string $pluginClass
77+
* @param class-string $pluginClass
4578
*/
4679
public function removePlugin($pluginClass)
4780
{
@@ -58,7 +91,8 @@ public function getHttpClient()
5891
if (!$this->pluginClient) {
5992
$this->pluginClient = new HttpMethodsClient(
6093
new PluginClient($this->httpClient, $this->plugins),
61-
$this->requestFactory
94+
$this->requestFactory,
95+
$this->streamFactory
6296
);
6397
}
6498

‎tests/HttpClient/HttpPluginClientBuilderTest.php‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
3+
/*
4+
* (c) Packagist Conductors GmbH <[email protected]>
5+
*
6+
* For the full copyright and license information, please view the LICENSE
7+
* file that was distributed with this source code.
8+
*/
9+
10+
namespace PrivatePackagist\ApiClient\HttpClient\Plugin;
11+
12+
use GuzzleHttp\Psr7\HttpFactory;
13+
use GuzzleHttp\Psr7\Response;
14+
use Http\Client\Common\HttpMethodsClientInterface;
15+
use Http\Message\MessageFactory\GuzzleMessageFactory;
16+
use Http\Message\RequestMatcher as RequestMatcherInterface;
17+
use Http\Mock\Client as MockClient;
18+
use PHPUnit\Framework\TestCase;
19+
use PrivatePackagist\ApiClient\HttpClient\HttpPluginClientBuilder;
20+
use Psr\Http\Message\RequestInterface;
21+
use Psr\Http\Message\ResponseInterface;
22+
23+
class HttpPluginClientBuilderTest extends TestCase
24+
{
25+
public function testInvalidRequestFactory(): void
26+
{
27+
$this->expectException(\TypeError::class);
28+
$definitelyNotARequestFactory = new \stdClass;
29+
/** @phpstan-ignore-next-line We are passing in an invalid type on purpose. */
30+
new HttpPluginClientBuilder(new MockClient, $definitelyNotARequestFactory);
31+
}
32+
33+
/** @dataProvider provideRequestFactories */
34+
public function testRequestFactory(?object $factory): void
35+
{
36+
$mockHttp = new MockClient;
37+
$mockHttp->setDefaultException(new \Exception('Mock HTTP client did not match request.'));
38+
$mockHttp->on($this->matchRequestIncludingHeaders(), new Response(307, ['Location' => '/kittens.jpg']));
39+
40+
$builder = new HttpPluginClientBuilder($mockHttp, $factory);
41+
// Make sure that the RequestFactory passed is acceptable for the client.
42+
$client = $builder->getHttpClient();
43+
$this->assertInstanceOf(HttpMethodsClientInterface::class, $client);
44+
45+
// Ensure that the Request Factory correctly generates a request object (including headers
46+
// as RequestFactory and RequestFactoryInterface set headers differently).
47+
$response = $client->get('https://example.com/puppies.jpg', ['Accept' => 'image/vnd.cute+jpeg']);
48+
$this->assertInstanceOf(ResponseInterface::class, $response);
49+
$this->assertSame(307, $response->getStatusCode());
50+
$locationHeaders = $response->getHeader('Location');
51+
$this->assertCount(1, $locationHeaders);
52+
$this->assertSame('/kittens.jpg', reset($locationHeaders));
53+
}
54+
55+
/**
56+
* The concrete implementation of the RequestMatcher interface does not allow matching on
57+
* headers, which we need to test to ensure both legacy and PSR17 implementations work.
58+
*/
59+
private function matchRequestIncludingHeaders(): RequestMatcherInterface
60+
{
61+
return new class implements RequestMatcherInterface {
62+
public function matches(RequestInterface $request): bool
63+
{
64+
$acceptHeaders = $request->getHeader('Accept');
65+
return $request->getUri()->getPath() === '/puppies.jpg'
66+
&& count($acceptHeaders) === 1
67+
&& reset($acceptHeaders) === 'image/vnd.cute+jpeg';
68+
}
69+
};
70+
}
71+
72+
/** @return iterable{object|null} */
73+
public static function provideRequestFactories(): iterable
74+
{
75+
yield [null];
76+
// Http\Message\RequestFactory
77+
yield [new GuzzleMessageFactory];
78+
// Psr\Http\Message\RequestFactoryInterface
79+
yield [new HttpFactory];
80+
}
81+
}

0 commit comments

Comments
 (0)