Changed some return types

DivineOmega / password_exposed

🔒 Password Exposed Helper Function - Check if a password has been exposed in a data breach.

GNU Lesser General Public License v3.0

213 stars 34 forks source link

Changed some return types #24

Closed tswestendorp closed 5 years ago

tswestendorp commented 5 years ago

\Http\Client\HttpClient doesn't implement \Psr\Http\Client\ClientInterface in php-http/guzzle6-adapter ^1.1 which makes the package incompatible with recent versions of laravel/framework

DivineOmega commented 5 years ago

Hi there @tswestendorp,

Thanks for your contribution, though I'd prefer to keep this following the PSR-18 Client Interface if possible. It is a new feature of the recent major release (v3.0.0). I think we may be able to find an alternative solution.

Could I ask you which version of Laravel you are using?

Could I also ask you to run composer why php-http/guzzle6-adapter and copy-paste the output here? We'll then be able to see why you have the older version of the Guzzle adapter installed.

tswestendorp commented 5 years ago

Hi @DivineOmega,

Structure is as following

laravel/framework:5.7.26
    laravel/nexmo-notification-channel:^1.0
        nexmo/client:1.0
            php-http/guzzle6-adapter:^1.0

Output (ignore dev-master, I'm using my fork temporarily)

composer why php-http/guzzle6-adapter
divineomega/password_exposed  dev-master  requires  php-http/guzzle6-adapter (^1.1|^2.0)  
nexmo/client                  1.6.2       requires  php-http/guzzle6-adapter (^1.0)

DivineOmega commented 5 years ago

Thanks for the info @tswestendorp.

It seems this package should not indicate support for v1 of the guzzle6-adapter package, because the type of object returned differs between v1 and v2 of the guzzle6-adapter.

So, the Laravel incompatibility affects Laravel 5.7. It is caused by Laravel 5.7 requiring the laravel/nexmo-notification-channel as standard, which depends on the nexmo/client which enforces the use v1 of the guzzle6-adapter.

My plan of action is going to be:

Create a simple PSR18 Guzzle adapter package, that returns an object which implements the correct PSR18 client interface.
Modify this package to make use of the new PSR18 Guzzle adapter package.

DivineOmega commented 5 years ago

I'm going to close this PR now, but thanks very much for bringing this to my attention.

Further discussion will be occurring #25, so follow that issue for updates. If you need to use this in Laravel 5.7 immediately, feel free to install v2.8.0, which is not affected by this issue.