[3.0] Notification Response / answer

[barryvdh]

Some gateways require a specific answer on the NotifyURL, eg. OK or sometimes something depending on the status/reference.

This would usually be the case for off-site payment, in the CompletePurchaseResponse.

I can't recall any that need a different status, other than 200, but we could have a getNotificationResponse() method which returns a PSR-7 response, or getNotificationMessage/getNotificationReply/getNotificationAnswer etc. Or just use the response getNotificationResponse()->getBody(). Along with an interface + hasReply so it can be checked for all gateways.

[greydnls]

I'm a fan of the latter option.

getNotificationResponse()->getBody() + hasReply

[judgej]

I've been getting my head around PSR-7 to see if I can create a gateway that is 100% PSR-7, and totally client agnostic. So far it makes requests and gets responses as PSR-7 messages and that works fine, with (very simple) adapters for Guzzle and Zend Diactoros (and some auto-discovery to pick up which is installed if you don't specify which you want to use).

Now, handling the Notifications I realised that Guzzle does not do this yet. There is no PSR-7 ServerRequestInterface support from Guzzle. Diactoros does it no problem. For this reason, if we want to support PSR-7 fully (depending on what we mean by that), we cannot rely on Guzzle being present. OmniPay at the moment does not care for PSR-7 server requests (requests coming into the server). With little or no exception the gateway drivers just need GET and POST data already parsed by PHP to do their thing.

So the question is, are we extending to PSR-7 ServerRequestInterface support in OmniPay, or just using PSR-7 for ServerRequest and ServerResponse?

I'm still not convinced that Guzzle should be a requirement of OmniPay, especially since it is Guzzle version dependencies that are making upgrades to Laravel 5.2 difficult for many people, but I suspect the project is not ready for too big a leap into a more portable, but slightly more fiddly to use, package.

I'll carry on with my PSR-7 gateway to see if reality pans out as I hope, or is just an impractical pipe-dream :-) We have to remember what the purpose of PSR-7 support is. How will we use PSR-7 to achieve that end? How important is PSR-7 compared to just having something that works with the least effort, but limits its portability between projects and frameworks?

[aimeos]

+1 for full PSR-7 support :-)

[barryvdh]

I'm not really sure if I understand the issue. We replaced the Request from Symfony with the ServerRequestInterface:

omnipay/src/Common/Message/AbstractRequest.php

Line 111 in 59139cd

public function __construct(ClientInterface $httpClient, ServerRequestInterface $httpRequest)

Which is indeed done by Diactoros because Guzzle does not support it:

omnipay/src/Common/AbstractGateway.php

Line 312 in 59139cd

return ServerRequestFactory::fromGlobals();

It's not really related to Guzzle imho (or Diactoros). Same as Guzzle, we rely on the interface, but use 1 implementation by default.

[judgej]

So long as we have this in composer,json, then both Guzzle and Diactoros are required. Whether using them is a "default" or not, there is a dependency created:

"require": {
        "guzzlehttp/guzzle": "^6.0",
        "zendframework/zend-diactoros": "^1.1.0"
        ...
    },

This is fine within a restricted number of frameworks and versions of those frameworks, but the portability that PSR-7 offers is partially lost because of these dependencies. I just think that libraries like OmniPay have a responsibility to keep their package dependencies down to a minimum while moving more to PSR interface dependencies instead. Dependency requirements have become an order of magnitude more complex over the years since OmniPay first came into being, and that should be recognised.

Anyway, that is a side issue to this main issue. I thought we were locked to Guzzle 6, and that didn't support a ServerRequestInterface implementation. Now I realise to solve that we are locked into two HTTP libraries, and that is a step in further in the wrong direction IMO.

I'll not raise this again. I hope what I've described is clear enough. The PSR-7 stuff that has gone into OmniPay (that you have put into OmniPay) is great - don't get me wrong, love it - I just think we could go that one step further.

[barryvdh]

I understand the issue, but I just don't see a viable way to not require it, but still provide a user friendly setup. I know you and I can just be told to require a valid http driver and psr-7 implementation and inject it somewhere, but not sure if everyone thinks that's easy enough..

[judgej]

Yes, it is an extra composer require, assuming the package is not already installed. I'm trying something like this to discover what's installed and create a factory adapter as a result (this is still a bit experimental, and not OmniPay, but the hope is that I can wrap a thin OmniPay layer around it to create a gateway driver at a later date):

https://github.com/academe/SagePay-Integration/blob/PSR-7/src/Request/AbstractRequest.php#L107

So the idea of a default is there if you don't inject any factories.

[barryvdh]

Just to be clear, the gateways itself should never have knowledge about guzzle or diactoros. So even if we end up with conflicts soonish, it will only affect the common package, no need to update every single gateway.

[judgej]

Yes, totally.

[barryvdh]

Yeah but I mean, that the case already now with Omnipay v3.

[mkrauser]

I understand the issue, but I just don't see a viable way to not require it, but still provide a user friendly setup. I know you and I can just be told to require a valid http driver and psr-7 implementation and inject it somewhere, but not sure if everyone thinks that's easy enough..

@barryvdh @judgej You are both right, including guzzle and diactoros as (hard) dependencies is not pretty. But if omnipay leaves it to the developer to install a client of his or her choosing, that would be more initial work and probably sometimes frustration if the dev does not read the documentation.

Personally I would not like it, if omnipay "requires" diactoros and guzzle. But I don't have a really good solution either.

[aimeos]

There might be one possibility: Composer offers a "provide" section and meta packages. There's already a meta package for PSR-7: https://packagist.org/providers/psr/http-message. The only requirement would be that we convince the Guzzle and Zend teams to add this to their packages.

[mkrauser]

The right virtual package would be psr/http-message-implementation. Guzzle and Diactoros already provide that.
But if that is required and no package which provides this, composer just complains that no package with this name or version is found, which is not very user friendly imho.