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

Use Guzzle7 as default implementation #615

Merged
merged 11 commits into from
Sep 22, 2020
Merged

Use Guzzle7 as default implementation #615

merged 11 commits into from
Sep 22, 2020

Conversation

barryvdh
Copy link
Member

@barryvdh barryvdh commented Sep 9, 2020

Guzzle 7 implements the HttpClient interface directly. So no longer required to use the adapter.

This is just the wrapper package, so 3.0 will still be fine for those on older versions/guzzle (I think). Composer should detect that 3.1 only works with Guzzle 7/newer PHP versions hopefully.

We might need to run some actual tests, but http-client/discovery 1.9+ should find Guzzle7 already: https://github.com/php-http/discovery/releases

@barryvdh barryvdh mentioned this pull request Sep 9, 2020
@judgej
Copy link
Member

judgej commented Sep 10, 2020

Just for a bit of background, are the drivers likely to need any updates for this, or should it be working seamlessly?

@barryvdh
Copy link
Member Author

They should work without issue, if Guzzle 7 implements the interface correctly.

@judgej
Copy link
Member

judgej commented Sep 13, 2020

I'm trying to pull this into a dev env using composer, and getting this error:

Problem 1
    - The requested package omnipay/common dev-feat-guzzle7 exists as omnipay/common[2.3.2, 2.4.0, 2.5.2, 2.5.x-dev, dev-feat-amountinteger, dev-feat-customer, dev-feat-httpdecorator, dev-feat-psr18, dev-feat/symfony5, dev-fix-gateway-params, dev-future, dev-master, 3.0.x-dev, dev-revert-184-feat-parameters, dev-revert-67-sf3-compat, v2.0.0, v2.1.0, v2.2.0, v2.3.0, v2.3.1, v2.3.3, v2.3.4, v2.4.1, v2.5.0, v2.5.1, v3.0-RC1, v3.0-RC2, v3.0-alpha.1, v3.0-alpha.2, v3.0-alpha.3, v3.0-alpha.4, v3.0-beta.1, v3.0-beta.2, v3.0-beta.3, v3.0.0, v3.0.1, v3.0.2, v3.0.3, v3.0.4] but these are rejected by your constraint.

The feat-guzzle7 branch isn't in the list, but clearly it is in the repository. I'm probably being dumb here, but what am I missing? Only requirement is:

    "require": {
        "omnipay/common": "dev-feat-guzzle7"
    },

Same with or without:

    "repositories": [
      {
        "type": "vcs",
        "url": "https://github.com/thephpleague/omnipay.git"
      }
    ]

Using PHP 7.3.

@barryvdh
Copy link
Member Author

This is not omnipay/common, but the league/omnipay package

@judgej
Copy link
Member

judgej commented Sep 13, 2020

I did say I was probably being dumb...thanks.

@judgej
Copy link
Member

judgej commented Sep 13, 2020

I'm getting no client discovery with Guzzle 7.0.1

Fatal error: Uncaught Http\Discovery\Exception\DiscoveryFailedException: Could not find resource using any discovery strategy. Find more information at http://docs.php-http.org/en/latest/discovery.html#common-errors

  • Puli Factory is not available
  • No valid candidate found using strategy "Http\Discovery\Strategy\CommonClassesStrategy". We tested the following candidates: Symfony\Component\HttpClient\HttplugClient, Http\Adapter\Guzzle6\Client, Http\Adapter\Guzzle5\Client, Http\Client\Curl\Client, Http\Client\Socket\Client, Http\Adapter\Buzz\Client, Http\Adapter\React\Client, Http\Adapter\Cake\Client, Http\Adapter\Zend\Client, Http\Adapter\Artax\Client, Http\Discovery\Strategy\CommonClassesStrategy::buzzInstantiate.
  • No valid candidate found using strategy "Http\Discovery\Strategy\CommonPsr17ClassesStrategy". We tested the following candidates: .

in .../vendor/php-http/discovery/src/Exception/DiscoveryFailedException.php:41
Stack trace:
#0 /var in .../vendor/php-http/discovery/src/HttpClientDiscovery.php on line 27

clue/stream-filter            v1.4.1                   A simple and modern approach to stream filtering in PHP
guzzlehttp/guzzle             7.0.1                    Guzzle is a PHP HTTP client library
guzzlehttp/promises           v1.3.1                   Guzzle promises library
guzzlehttp/psr7               1.6.1                    PSR-7 message implementation that also provides common utility methods
league/omnipay                dev-feat-guzzle7 8983ec9 Omnipay payment processing library
moneyphp/money                v3.3.1                   PHP implementation of Fowler's Money pattern
omnipay/common                v3.0.4                   Common components for Omnipay payment processing library
php-http/client-common        2.3.0                    Common HTTP Client implementations and tools for HTTPlug
php-http/discovery            1.10.0                   Finds installed HTTPlug implementations and PSR-7 message factories
php-http/httplug              2.2.0                    HTTPlug, the HTTP client abstraction for PHP
php-http/message              1.9.0                    HTTP Message related tools
php-http/message-factory      v1.0.2                   Factory interfaces for PSR-7 HTTP Message
php-http/mock-client          1.4.1                    Mock HTTP client
php-http/promise              1.1.0                    Promise used for asynchronous HTTP requests
psr/http-client               1.0.1                    Common interface for HTTP clients
psr/http-factory              1.0.1                    Common interfaces for PSR-7 HTTP message factories
psr/http-message              1.0.1                    Common interface for HTTP messages
ralouphie/getallheaders       3.0.3                    A polyfill for getallheaders.
symfony/deprecation-contracts v2.2.0                   A generic function and convention to trigger deprecation notices
symfony/http-foundation       v5.1.5                   Symfony HttpFoundation Component
symfony/options-resolver      v5.1.5                   Symfony OptionsResolver Component
symfony/polyfill-mbstring     v1.18.1                  Symfony polyfill for the Mbstring extension
symfony/polyfill-php80        v1.18.1                  Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions

php-http/discovery is 1.10.0 so should be able to discover Guzzle PSR-17 factories of Guzzle 7. I'm not quite sure what is missing here - the PSR-18 client PSR-17 message factory, the factory for the PSR-18 client *PSR-17, the discovery of the factory for the PSR-17 factory, or something else...?

Hope this is helpful. It should just work with omnipay common 3.0.4 and Guzzle 7.0.1, and an Omnipay 3.x driver, without any further concrete implementations of factories or clients, shouldn't it?

@barryvdh
Copy link
Member Author

Meh, PSR17 support is not yet released: guzzle/psr7#327
So we could add https://github.com/http-interop/http-factory-guzzle for the factory but that was kinda what we wanted to avoid :/

@nexxai
Copy link

nexxai commented Sep 19, 2020

I know http-interop/http-factory-guzzle isn't a great long-term fix but for those of us who are stuck, is it possible to temporarily switch to it until the Guzzle team decides to implement PSR-17 directly?

@barryvdh
Copy link
Member Author

You can use it directly, if you use omnipay/common

@nexxai
Copy link

nexxai commented Sep 19, 2020

That's the problem - I'm actually using one of your packages (barryvdh/laravel-omnipay) which requires "league/omnipay": "~3.0" :)

@barryvdh
Copy link
Member Author

Ah, we should probably change that to omnipay common with the factory then

@nexxai
Copy link

nexxai commented Sep 21, 2020

Would that just be a matter of updating the composer.json to change the reference from league/omnipay to omnipay/common and add a reference to http-interop/http-factory-guzzle? If so, I will update my open PR to include those changes.

I apologize for the stupid questions, I'm not super clear on how PSR-17 works and if it's really that easy to solve.

@barryvdh
Copy link
Member Author

I've added some tests. I hope this one is failing, then I will add the factory and have it passing.

@barryvdh
Copy link
Member Author

Ah we're using the old HttpClient, should probably change the discovery also..

@barryvdh
Copy link
Member Author

Pff, so guzzle7-adapter is just a few days old, but not supported in the Discovery yet. Guzzle/psr7 2.x is 'close to release' but not ready yet. So if we want to unblock Guzzle 7:

Or wait until either guzzle7 adapter is discovered, of guzzle/psr7 2.x is released.

Preferably we would upgrade the PSR support anyways though

@barryvdh
Copy link
Member Author

Discovery PR is here: php-http/discovery#189

@barryvdh
Copy link
Member Author

Aaaaaand we're green. So what do you think, good like this? Bump to Guzzle 7 for the people using this now, should be able to still use guzzle6 with the older tags, right?

@barryvdh barryvdh merged commit 1ba7c8a into master Sep 22, 2020
@barryvdh
Copy link
Member Author

Merged this so we can test this with 3.1@dev

@barryvdh barryvdh deleted the feat-guzzle7 branch September 22, 2020 14:02
@matthewnessworthy
Copy link

Interface 'Http\Client\HttpClient' not found
#0 /shared/app/vendor/composer/ClassLoader.php(444): include()
#1 /shared/app/vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile('/shared/app/ven...')
#2 [internal function]: Composer\Autoload\ClassLoader->loadClass('Http\\Adapter\\Gu...')
#3 [internal function]: spl_autoload_call('Http\\Adapter\\Gu...')
#4 /shared/app/vendor/php-http/discovery/src/ClassDiscovery.php(241): class_exists('Http\\Adapter\\Gu...')
#5 /shared/app/vendor/php-http/discovery/src/ClassDiscovery.php(178): Http\Discovery\ClassDiscovery::safeClassExists('Http\\Adapter\\Gu...')
#6 /shared/app/vendor/php-http/discovery/src/ClassDiscovery.php(65): Http\Discovery\ClassDiscovery::evaluateCondition('Http\\Adapter\\Gu...')
#7 /shared/app/vendor/php-http/discovery/src/HttpClientDiscovery.php(25): Http\Discovery\ClassDiscovery::findOneByType('Http\\Client\\Htt...')
#8 /shared/app/vendor/omnipay/common/src/Common/Http/Client.php(34): Http\Discovery\HttpClientDiscovery::find()
#9 /shared/app/vendor/omnipay/common/src/Common/AbstractGateway.php(332): Omnipay\Common\Http\Client->__construct()
#10 /shared/app/vendor/omnipay/common/src/Common/AbstractGateway.php(69): Omnipay\Common\AbstractGateway->getDefaultHttpClient()
#11 /shared/app/vendor/omnipay/common/src/Common/GatewayFactory.php(88): Omnipay\Common\AbstractGateway->__construct(NULL, NULL)

@matthewnessworthy
Copy link

@barryvdh still receiving the issue posted above Interface 'Http\Client\HttpClient' not found

@pixobit
Copy link

pixobit commented Oct 2, 2020

Trying to install omnipay with composer require league/omnipay:^3 omnipay/paypal but getting the following error:

Your requirements could not be resolved to an installable set of packages.

Problem 1
- Installation request for league/omnipay 3 -> satisfiable by league/omnipay[v3.0.0].
- Conclusion: remove guzzlehttp/guzzle 7.1.1
- Conclusion: don't install guzzlehttp/guzzle 7.1.1
- league/omnipay v3.0.0 requires php-http/guzzle6-adapter ^1.1 -> satisfiable by php-http/guzzle6-adapter[v1.1.0, v1.1.1].
- php-http/guzzle6-adapter v1.1.0 requires guzzlehttp/guzzle ^6.0 -> satisfiable by guzzlehttp/guzzle[6.0.0, 6.0.1, 6.0.2, 6.1.0, 6.1.1, 6.2.0, 6.2.1, 6.2.2, 6.2.3, 6.3.0, 6.3.1, 6.3.2, 6.3.3, 6.4.0, 6.4.1, 6.5.0, 6.5.1, 6.5.2, 6.5.3, 6.5.4, 6.5.5].
- php-http/guzzle6-adapter v1.1.1 requires guzzlehttp/guzzle ^6.0 -> satisfiable by guzzlehttp/guzzle[6.0.0, 6.0.1, 6.0.2, 6.1.0, 6.1.1, 6.2.0, 6.2.1, 6.2.2, 6.2.3, 6.3.0, 6.3.1, 6.3.2, 6.3.3, 6.4.0, 6.4.1, 6.5.0, 6.5.1, 6.5.2, 6.5.3, 6.5.4, 6.5.5].
- Can only install one of: guzzlehttp/guzzle[6.3.0, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.3.1, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.3.2, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.3.3, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.4.0, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.4.1, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.5.0, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.5.1, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.5.2, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.5.3, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.5.4, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.5.5, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.0.0, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.0.1, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.0.2, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.1.0, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.1.1, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.2.0, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.2.1, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.2.2, 7.1.1].
- Can only install one of: guzzlehttp/guzzle[6.2.3, 7.1.1].
- Installation request for guzzlehttp/guzzle (locked at 7.1.1) -> satisfiable by guzzlehttp/guzzle[7.1.1].

Installation failed, reverting ./composer.json to its original content.

What am I missing?

@matthewnessworthy
Copy link

@barryvdh have you had a chance to have a look at the issues posted after the PR was merges?

@barryvdh
Copy link
Member Author

What do you do exactly? If I create a new project:

composer require league/omnipay:^3.1@dev omnipay/mollie
<?php

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

$gateway = \Omnipay\Omnipay::create('Mollie');
$gateway->setApiKey('test_foobar');

$response = $gateway->purchase(
    [
        "amount" => "10.00",
        "currency" => "EUR",
        "description" => "My first Payment",
        "returnUrl" => "https://webshop.example.org/mollie-return.php"
    ]
)->send();

print_r($response->getData());

It works fine for me. It installs Guzzle 7.2.0:

clue/stream-filter            v1.5.0             A simple and modern approa...
guzzlehttp/guzzle             7.2.0              Guzzle is a PHP HTTP clien...
guzzlehttp/promises           1.4.0              Guzzle promises library
guzzlehttp/psr7               1.7.0              PSR-7 message implementati...
league/omnipay                dev-master 1ba7c8a Omnipay payment processing...
moneyphp/money                v3.3.1             PHP implementation of Fowl...
omnipay/common                v3.0.4             Common components for Omni...
omnipay/mollie                v5.2.1             Mollie driver for the Omni...
php-http/discovery            1.12.0             Finds installed HTTPlug im...
php-http/guzzle7-adapter      0.1.1              Guzzle 7 HTTP Adapter
php-http/httplug              2.2.0              HTTPlug, the HTTP client a...
php-http/message              1.9.1              HTTP Message related tools
php-http/message-factory      v1.0.2             Factory interfaces for PSR...
php-http/promise              1.1.0              Promise used for asynchron...
psr/http-client               1.0.1              Common interface for HTTP ...
psr/http-message              1.0.1              Common interface for HTTP ...
ralouphie/getallheaders       3.0.3              A polyfill for getallheaders.
symfony/deprecation-contracts v2.2.0             A generic function and con...
symfony/http-foundation       v5.1.8             Symfony HttpFoundation Com...
symfony/polyfill-mbstring     v1.20.0            Symfony polyfill for the M...
symfony/polyfill-php80        v1.20.0            Symfony polyfill backporti...

And calls the API

php index.php
Array
(
    [status] => 401
    [title] => Unauthorized Request
    [detail] => Missing authentication, or failed to authenticate
    [_links] => Array
        (
            [documentation] => Array
                (
                    [href] => https://docs.mollie.com/guides/authentication
                    [type] => text/html
                )

        )

)

@matthewnessworthy
Copy link

I can't get it to break in the same way anymore. Perhaps I had another dependency causing a conflict. Sorry for the trouble.

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

Successfully merging this pull request may close these issues.

5 participants