Open pkly opened 2 years ago
I agree with all the points!
I would add a couple of ideas:
transactionId
and referenceId
, used ambiguously by different drivers, making them incompatible.isPending()
method was implemented in https://github.com/thephpleague/omnipay-common/blob/master/src/Common/Message/AbstractResponse.php#L69-L77 but not declared in https://github.com/thephpleague/omnipay-common/blob/master/src/Common/Message/ResponseInterface.php. I find it interesting: a CompletePurchaseResponse
could still not be a redirection, nor successful, nor cancelled, so it would be pending. Also there should be an isFailed()
, as a transaction could fail without being a user cancellation.
NotificationInterface
was the candidate to check if a purchase return/notification is still pending payment?At this point I've decided to create a custom solution which can benefit from proper php code instead of holding on to this legacy library. As omnipay is not exactly easy to integrate with something like symfony (and we ended up writing a ton of custom code and our custom gateways anyway) we're moving away from it. I'm not sure that creating an open source solution to match-all gateway needs makes sense, but if at some point later I decide it could be split into a library we might pull it out into an open source project as we've done with our previous 2 libraries.
I don't know how far I'll reach, but I'm going to start a little effort to propose a new major version: https://github.com/devnix-labs/omnipay-common/tree/add-static-analysis
I will consider your original ideas, but I would love to know more about your ideas and experiences two years later and after building a custom solution 😄
Thanks, I'll look into this
@devnix some suggestions then everything as interface, generic gateway interface, but things like we hook/notification support should be optional interfaces instead of whatever it's right now with method annotations use PSR for everything library wise, no more custom omnipay http client, assume a logger and PSR cache may be supplied (only setter method) no more silly property requirements for requests to work probably also only ever return requests for them to be sent explicitly by the implementation instead of omnipay in a hidden http client in request require parameters explicitly for the gateway to work preferably in the individual implementation no more stupid random factory and namespace requirement (it does literally nothing good) and I know it wasn't really a retirement but honestly it shouldn't even be there at all there should be normalized exceptions for all generic gateway issues (not including http issues), for developers to simply implement perhaps rework the line items as they were slightly cumbersome to use, make it easier to specify types of amounts (tax, product, total), get rid of the money library and replace it with something more modern, use integers for all numeric operations (only strings allowed should be in the api format itself) add simple standardized phpunit abstracts you can use to bootstrap your tests (I'm not sure that was that easy last time I wrote a gateway)
all gateway management should be handled by the framework implementing it so the rest depends on how you handle that but you should have a setup simple enough to where it should be possible to simply hook in a setup gateway in a symfony application without maybe any but 1 additional class (something to contain the gateways) a callback interface for handling certain actions like orders being rejected from notifications in gateways could be nice too
if you need more ideas hit me up
Hello.
We've implemented omnipay as v3 about 2 years ago now, and it works fine for the most part. I was wondering if you were considering a new major version which would require a rework?
Things to consider:
This is a breaking change, as the client requirements would change along with the exceptions thrown. I would propose also no default client to be added, instead by going along with what composer wants us to do simply require an implementation. This would allow omnipay to completely decouple from http requests.
This should likely be just removed, instead the gateways should implement a main interface and sit in separate namespaces. A "shared" main namespace could still be considered for official gateways.
The methods for gateways could be split into interfaces instead of an annotation, thus having a proper requirement of implementation on the gateway and allow someone to simply do a
$gateway instanceof PurchaseGatewayInterface
By requiring php 8.1 or simply moving some of the logic elsewhere it would be possible to stop using the set/get pattern along with likely dropping the requirement for ParameterBag and by thus I believe the symfony-framework? If still needed it would be wise to maybe consider an alternative, or simply store parameters in an array.
I'm currently on implementing another gateway for our project and I noticed that barely anything typechecks, as omnipay was designed to work with... php5?
At a time like this it seems a good idea to jump straight to php8.1 allowing a greater freedom of choice and improving development experience. The project could also use an upgrade of all libraries in use (omnipay/tests still uses phpunit 6...)
As code evolves it would be a good idea to use phpstan, psalm and so on as quality assurance tools.