Remove locked dependency with guzzle 6 to allow any psr18 compatible http client

[ppshobi]

No description provided.

[ppshobi]

Todo:

• Update Readme to install with guzzle 6
• Test with Guzzle 7 - Need help [seems like this doesn't work, because from my understanding guzzle 7 is not having psr 18 compatibility]
• Test with symfony/http-client
• Test laravel-scout to make it extra sure that it works. done by @shokme
• Test with php-http/curl-client

[shokme]

Hello @ppshobi,

Using a fresh laravel install with laravel-scout using your PR, I have require only http-interop/http-factory-guzzle with php-http/curl-client OR php-http/guzzle6-adapter and everything is working.

I have done some attempt with symfony/http-client and nyholm/psr7 but can't make it works, I don't know why.

No HTTPlug clients found. Make sure to install a package providing "php-http/client-implementation"

[ppshobi]

Wow, Thank you @shokme for your support with testing, I will try to figure out why the other packages are having no luck with our sdk.

[shokme]

So, I was missing some part when using symfony/http-client and nyholm/psr7. While I was reading the sdk doc, I found that I had to change how I init the client in laravel-scout.

$httpClient = new HttplugClient();
$client = new Client('http://127.0.0.1:7700', 'masterKey', $httpClient);

The first error I have encounter was a missing dependency php-http/httplug. Then, while sending a request to create a new index I have this error: Header values must be RFC 7230 compatible strings.
So, I have check the request and headers to try understand what is the problem, in my case the header X-Meili-API-Key is null, by commenting it I was able to create the index ! 😄

The header need to be fill only if $this->apiKey is not null, I can make a PR if you approve my statement.

About laravel-scout, I need to think how I will allow user to use the factory/client of their choice.

[ppshobi]

@shokme Please do the PR on the meili key

Now, in an ideal situation, the user should install any psr18 compatible http client of choice, and they shouldn't init the client by themselves. But rather we should auto discover it and initialize it, that's the whole point of having interfaces.

Since the PR for the initial implementation of http client (which depends on psr18 interfaces) included a default client of guzzle 6, we had to add this rewiring mechanism in case if we are using another client.

I hope it makes sense.

In production the installation should be,

composer require meilisearch/meilisearch-php
composer require some/other-http-client

So basically meilisearch-php is independent of any concrete implementation of http client,

[ppshobi]

Also in laravel scout, I belive the end user should install laravel-scout-meilisearch and some/other-http-client.

[shokme]

Indeed, I have noticed the different Discovery inside the client.

After adding php-http/httplug, The discovery is able to do its works. Without having to specify:

$httpClient = new HttplugClient();
$client = new Client('http://127.0.0.1:7700', 'masterKey', $httpClient);

But if you don't specify $httpClient, you will not be inform that php-http/httplug dependency is missing when you use symfony/http-client.

Also in laravel scout, I belive the end user should install laravel-scout-meilisearch and some/other-http-client.

Yes.

[ppshobi]

So you are saying that if we add php-http/httplug inside composer.json all the clients keep working as expected?

[shokme]

So you are saying that if we add php-http/httplug inside composer.json all the clients keep working as expected?

I haven't test all clients, but it allows symfony/http-client to works.

I don't think we should add php-http/httplug, because of what they said in the intro

HTTPlug is the predecessor of PSR-18 HTTP Client standard built on PSR-7 HTTP messages. Since there is an entire ecosystem built around HTTPlug which is already widely adopted, we will keep maintaining this package for the time being, but new implementations and consumers should use the PSR-18 interfaces. HTTPlug 2.x extends the PSR-18 interfaces to allow a convenient migration path. In the long term, we expect PSR-18 to completely replace the need for HTTPlug.

source: https://github.com/php-http/httplug/tree/v2.0.0

Right now, I'm lost ^^ I will take a bit of time to uderstand this PSR-18

[ppshobi]

Hmm... I have added httplug: ^2.0 anyway. Because PSR 18 is not yet live and in future we should be able to simply replace it with the psr 18 implementation

[ppshobi]

@Zeichen32 can you please shed some light here?

[ppshobi]

@emulienfou @curquiza Please review the PR. Welcome any suggestions

[Zeichen32]

@Zeichen32 can you please shed some light here?

Sorry for my late replay. Yes you can replace the Guzzle Client with any other implementation. I had choose the Guzzle Client in my PR to prevent breaking changes. In my own project i use the symfony http client without any problems.

[Zeichen32]

@ppshobi To use the Symfony HTTP Client, the documentations says it needs the nyholm/psr7 lib to provide a response and stream factory.

So i thinks the docs for the symfony client should be:
composer require meilisearch/meilisearch-php symfony/http-client nyholm/psr7

https://symfony.com/doc/current/http_client.html#psr-18-and-psr-17

[ppshobi]

@Zeichen32 the tests were passing with just symfony/http-client inside the require-dev section instead of guzzle6-adapter. But with only nyholm/psr7 the tests were failing saying that it was not able to discover a psr 17 compatible client.

Any idea why?

[Zeichen32]

Maybe you can give me a hint. In the test logs the composer is installing the guzzle http client, so the tests are green because it find the guzzle client.

"But with only nyholm/psr7" You need to install the symfony http client AND the psr7 factories, because the symfony client is not PSR 18 compatible out of the box.

[ppshobi]

@Zeichen32 In test logs it shows guzzle client because for the test suite we install guzzle-6 adapter via the require-dev section. But while testing with symfony-client I removed guzzle adapter and installed symfony-client.

[ppshobi]

And yes, with only nyholm/psr7 it was not working, I had to install symfony-http-client

[curquiza]

Thanks for your work @ppshobi!
I put the breaking tag since these changes are breaking for the users!

Merging!