Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types missmatches for the client configuration #77

Open
ragusa87 opened this issue Dec 15, 2024 · 8 comments
Open

Types missmatches for the client configuration #77

ragusa87 opened this issue Dec 15, 2024 · 8 comments

Comments

@ragusa87
Copy link

Description

Passing an instance of PSR-18 ClientInterface to the configuration fails.

Steps to reproduce

use Http\Discovery\Psr18Client;
use Typesense\Client;

new Client([...] + ['client' => new Psr18Client()]);

Expected Behavior

You should be able to use the client

Actual Behavior

TypeError: Http\Client\Common\HttpMethodsClient::__construct(): Argument #1 ($httpClient) must be of type Http\Client\HttpClient, Http\Discovery\Psr18Client given, called in /var/www/html/vendor/typesense/typesense-php/src/Lib/Configuration.php on line 220

It seems that the configuration expects a ClientInterface

        if (true === \array_key_exists('client', $config) && $config['client'] instanceof ClientInterface) {
            $this->client = $config['client'];
        }

But the getClient method, will pass the ClientInterface to HttpsMethodsClient that expect an HttpClient.
Meaning that currently you need to pass an implementation compatible with both PSR-7 & PSR-18 standards.

    public function getClient(): ClientInterface
    {
        return new HttpMethodsClient(
            $this->client ?? Psr18ClientDiscovery::find(),
                Psr17FactoryDiscovery::findRequestFactory()
        );
    }

Metadata

Typesense Version:
v4.9.0
OS:
Linux

@ragusa87
Copy link
Author

Using the code below fixes the issue:

    public function getClient(): ClientInterface
    {
        return $this->client ?? Psr18ClientDiscovery::find();
    }

@tharropoulos
Copy link

Hey there, I'll investigate and report back

@ragusa87
Copy link
Author

ragusa87 commented Dec 17, 2024

Hello, thanks a lot. FTR I got the issue using composer install --prefer-lowest
Also adding the snippet below fixed the issue:

  "conflict": {
    "php-http/httplug": "<1.5"
  },

Example at https://github.com/ragusa87/TypesenseBundle/blob/main/composer.json

@tharropoulos
Copy link

tharropoulos commented Dec 18, 2024

Hey there, could you update your composer.json to point to my repo and branch on the PR I've posted? That way you can use the new changes and verify if that solves the problem

@ragusa87
Copy link
Author

I will try to have a look today, thanks a lot for the fix.

@ragusa87
Copy link
Author

ragusa87 commented Dec 20, 2024

Hello @tharropoulos,

I still have the same issue with your MR.
See https://github.com/ragusa87/TypesenseBundle/actions/runs/12433977885/job/34716680092?pr=8

To me the issue is that the declaration below is not valid with older versions of HttpClient (<2.4 PSR-18)

interface HttpClient extends ClientInterface

I think you can just fix the requirements, because you still expect the ClientInterface to be implementing HttpClient at the same time.

   } elseif ($config['client'] instanceof HttpClient || $config['client'] instanceof ClientInterface) {
                $this->client = new HttpMethodsClient(
                    $config['client'],
                    Psr17FactoryDiscovery::findRequestFactory(),
                    Psr17FactoryDiscovery::findStreamFactory()
                );
There was 1 error:

1) Biblioteca\TypesenseBundle\Tests\ContainerTest::testClientFactory
TypeError: Http\Client\Common\HttpMethodsClient::__construct(): Argument #1 ($httpClient) must be of type Http\Client\HttpClient, Symfony\Component\HttpClient\Psr18Client given, called in /home/runner/work/TypesenseBundle/TypesenseBundle/vendor/typesense/typesense-php/src/Lib/Configuration.php on line 111

/home/runner/work/TypesenseBundle/TypesenseBundle/vendor/php-http/client-common/src/HttpMethodsClient.php:43
/home/runner/work/TypesenseBundle/TypesenseBundle/vendor/typesense/typesense-php/src/Lib/Configuration.php:111
/home/runner/work/TypesenseBundle/TypesenseBundle/vendor/typesense/typesense-php/src/Client.php:97
/home/runner/work/TypesenseBundle/TypesenseBundle/src/Client/ClientFactory.php:29

@tharropoulos
Copy link

Could you also check again? Made some changes, and they seemed to work on act

@ragusa87
Copy link
Author

Hello @tharropoulos , I ran the tests again on my side and it works indeed. Thanks a lot !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants