Upgrade HTTPlug from Legacy to PSR 17/18 Discovery Factories

[zanbaldwin]

Note: Have yet to check lowest supported PHP version for this. Will just check result of CI.

Tested With

composer.json

{
    "name": "acme/sdk-test",
    "minimum-stability": "dev",
    "require": {
        "private-packagist/api-client": "*",
        "php-http/guzzle7-adapter": "^1.0"
    },
    "repositories": [
        {
            "type": "path",
            "url": "../packagist-client",
            "options": {
                "symlink": true
            }
        }
    ],
    "config": {
        "allow-plugins": {
            "php-http/discovery": true
        }
    }
}

index.php

<?php declare(strict_types=1);

require_once __DIR__ . '/vendor/autoload.php';

$client = new \PrivatePackagist\ApiClient\Client(null, 'http://packagist.com.lo');
$client->authenticate('7280a1da6c486c87a99d', '6469742...9be53aa5f7b');

$teamNames = array_map(fn (array $team): string => $team['name'] ?? '', $client->teams()->all());
var_dump($teamNames);

[glaubinix]

Looks like the install instructions aren't working aymore if php-http/guzzle7-adapter needs to be used. Can you adjust that too?

[glaubinix]

This is a braking change. Any way we can introduce this in a none breaking way e.g. have the method accept RequestFactory|RequestFactoryInterface? Otherwise, we'd have to release this as a new major version, where we should probably remove several of the deprecated methods too but would prefer adding this none breaking first with maybe a deprecation message.

[zanbaldwin]

It also works with guzzlehttp/guzzle - I just used php-http/guzzle7-adapter in the test out of habit. The new PSR17/18 discovery factories auto-detect the correct classes without the adapter.

However, HTTPlug's RequestFactory is not interchangeable with PSR's RequestFactoryInterface and union types RequestFactory|RequestFactoryInterface aren't valid until PHP 8.0 and this library supports 7.2+

I don't think there's any way to do this without a breaking change.

[zanbaldwin]

We could just not type-hint the argument, and have runtime variable checks instead?

[glaubinix]

Using PHPDoc instead of a typehint sounds good. Can then using something like https://packagist.org/packages/doctrine/deprecations or https://packagist.org/packages/symfony/deprecation-contracts to trigger a deprecation if the old service is passed.

[glaubinix]

Can this be two different tests? One that throws the exception and one that doesn't? Makes it easier to see what is actually necessary for the test case with the exception

[glaubinix]

Suggested change

	protected function matchRequestIncludingHeaders(): RequestMatcherInterface
	private function matchRequestIncludingHeaders(): RequestMatcherInterface

[glaubinix]

Just noticed this PR and it seems like we never finish it. @zanbaldwin do you remember what is still to do here? If I update one of my projects to use your branch then running

    $token = $client->tokens()->create([
        'description' => 'Team-Token ' . time(),
        'access' => 'read',
        'accessToAllPackages' => true,
    ]);

errors with

Uncaught RuntimeException: Cannot create request: A stream factory is required to create a request with a non-empty string body. in vendor/php-http/client-common/src/HttpMethodsClient.php:135

So there is definitely still something off with the HttpPluginClientBuilder when we create the HttpMethodsClient