getTransactionId needed for NotificationInterface?

[judgej]

https://github.com/thephpleague/omnipay-common/blob/master/src/Common/Message/NotificationInterface.php

The NotificationInterface has a getTransactionReference() method to fetch the remote transaction reference. For some gateways the reference is already supplied to the merchant site when the transaction was initiated. For other gateways this is the first point at which the reference is provided, so it can be logged, but cannot be used to look up the transaction in storage.

The transactionId is the only identifier that the merchant site can use to guarantee being able to look up the transaction, and it is supplied in every notification callback that I am aware of (except Authorize.Net DPM, but our driver defines a custom field to put that in, so we know it will be there).

My proposal is to add getTransactionId() to the NotificationInterface so that all drivers know this needs to be implemented.

[barryvdh]

Hmm, I feel like the reference should always be used, of at all possible.

[judgej]

The assumption is that the gateways all support a merchant site transaction ID because they all know the merchant site will have its own way to index and reference the transactions internally.

Plus the fact that some gateways do not allocate a reference it their own until the notification comes in, and so ALL you have is the merchant transaction ID.

I'll gather some evidence to see what is generally available.

[barryvdh]

A gateway can and should still add it when available, but not sure it should be in the interface.

But I'm also not sure about this interface, I've never used it. Do you have a good example? Can't is just be the complete purchase response, but returning a 'reply' response?

[aimeos]

It depends on how the process for accepting remote notifications should work. If it's

if( $provider->supportsAcceptNotification() )
{
	$request = $provider->acceptNotification();
	$response = $request->send();

	if( $response->isSuccessful() ) {
		// get $response->getTransationId();
		// save $request->getTransactionStatus();
	}
}

then getTransactionId() should go into the ResponseInterface. The ResonseInterface already contains a default implementation of getTransactionId() but though it's not in the interface, we can't be sure that it will be really there.
Some gateways like PayPal will only send a token which must be used to retrieve the details including the transaction ID. Therefore, I would vote for adding it to the ResonseInterface.

[aimeos]

After discussing that a bit more in detail, we think that the usage of acceptNotification() should be like this:

if( $provider->supportsAcceptNotification() )
{
	$notification = $provider->acceptNotification();
	// get $notification->getTransationId();
	// save $notification->getTransactionStatus();
	// save $notification->getTransactionRefernce();
}

The reason is the PayPal driver which needs to contact the payment server first before the transaction ID and status will be available and the NotificationInterface does not extend from ResponseInterface.

As no isSuccessful() method is available then, acceptNotification() must throw an exception (InvalidRequestException?) in case the request is tampered.

In Omnipay 3.x we also need an optional PSR-7 request as parameter for acceptNotification() and a PSR-7 response that contains the data that must be returned to the payment gateway so it knowns the payment status update was OK:

if( $provider->supportsAcceptNotification() )
{
	$notification = $provider->acceptNotification( $psr7request );
	// get $notification->getTransationId();
	// save $notification->getTransactionStatus();
	// save $notification->getTransactionRefernce();
	return $notification->getResponse(); // PSR-7 repsonse
}

The reason is that in environments with separated front-ends and back-ends which communication e.g. via JSON, acceptNotification() can't create a request from the environment itself. Also, being able to inject a PSR-7 request makes testing easier.

[barryvdh]

But still, what is the difference with completePurchase?

You can already always inject the Symfony request object.

And we are currently still using Symfony for server request and response, to make upgrades easier. We could perhaps add methods to convert those.

[judgej]

A problem with throwing an exception at that stage is that you do not get a notification object to either analyse or to respond with. Logging the details is very useful for analysis.

The response is always needed - there MUST be a http response, whether the data has been tampered with or not, and whether the transaction is successful or not. So you don't want an exception, since the processing is not exceptional so far as omnipay is concerned; a http response can be returned as normal. The application still needs to know if the data is valid and untampered, and needs to know whether the transaction is successful so it can decide what to do with that data before sending the response it is given.

[aimeos]

@barryvdh The main difference is an object with another interface which doesn't need the transactionId in advance I think.

@judgej Our suggestion to throw an exception is based on the current NotificationInterface. If you would like to always return a ResponseInterface, the NotificationInterface must then inherit from the ResponseInterface. Throwing an exception makes analysis different (you have to log the request yourself instead of using getData() or something like that) but would be OK in my opinion. You don't need to do anything besides logging and return a HTTP code 400 if the request is invalid at all.

At the end it all boils down to how you would like the process of acceptNotification() to work.

[judgej]

Here are the differences I can see:

area	completeAuhthorize/Purchase	acceptNotification
objects	returns an Omnipay response object on a send(); this allows for various redirects, or no response needed at all	only needs to return a HTTP response object
session	has a user http session available	no user session available
browser	may interact with a user browser (e.g. POST redirects)	no browser interaction
repeats	user will only be sent to this page once	notifications may be sent multiple times over short or extended periods
payment transaction	completes a transaction in progress	notifications can be for other purposes too, not just for completing transactions; e.g. status changes, automated regular payment notifications

I am beginning to see that the notification object MUST return a HTTP response. That response may be decorated with data from the application (this will be gateway-specific).

The general flow of the notification will be:

1. Receive notification.
2. Process the data and states it gives.
3. Get the HTTP response object.
4. Send the HTTP response
5. Application may close down nicely, but do it quietly.

Step 2 will involve logging details if the server request has been tampered with, looking up the transaction if not, updating the transaction as necessary (remembering that a notification can be sent multiple times, so be prepared for repeats).

Step 3 will include providing any final result data to the notification object (e.g. message, error state if the application could not process the data), then ask for the HTTP response object.

So I think the isSuccessful() and related methods need to be in the notification object, not the second object we get back after issuing a $notify->send().

Q: Will the driver ever need to do a callback to the gateway (in an ever-depending hole) in this process, or is ALL the data needed in the notification request the gateway has sent and the transaction details in local storage? Is Paypal an example of this callback to a callback?

[barryvdh]

The http session is not required for complete, and it usually doesn't redirect I think.

Imho this difference is application logic. Most gateways I encounter can use complete* in the same way.

redirectUrl: /payment/return/<paymentId>
notifyUrl: /payment/notify/<paymentId>

Both can get the payment details from the database (no session required), check the payment status (usually by sending the original parameters + the current HttpRequest). Only redirect shows 'THank you, your status is: ...' page and notify will return something back to the gateway (most cases just an HTTP status 200 is ok, sometimes the reference or whatever).

So in my view, you could just extend the complete* message, with a default response of '200 ok', and override it when needed. So you can call something like this:

$reponse = $gateway->comletePurchase($params)->send();

if ($respose->isSuccessfull()) {
	//..
}

$response->sendReply(); // or: return $response->getReplyResponse();

[barryvdh]

Q: Will the driver ever need to do a callback to the gateway (in an ever-depending hole) in this process, or is ALL the data needed in the notification request the gateway has sent and the transaction details in local storage? Is Paypal an example of this callback to a callback?

Some gateways require a callback indeed. Eg they just notify that the status change, but a HTTP request is required to verify the actual result.

[aimeos]

@barryvdh Both redirectUrl (back to the application) and notifyUrl have to contain the order ID (transaction ID) besides the transaction reference in some way. The main difference is that in the redirectUrl you can add the order ID like you want, in the notifyUrl the order ID parameter name is often named by the payment gateway (and therefore different between gateways) or you only get a token and have to request the details including the order ID yourself (PayPal).

[aimeos]

According to our thoughts it should be like this now:

if( $provider->supportsAcceptNotification() )
{
	// process the data in PSR-7 request
	// send request to gateway if necessary (PayPal)
	// return final response of evaluation
	$response = $provider->acceptNotification( $psr7request );

	// verify that the notify request hasn't been tampered
	if( $response->isSuccessful() ) {
		// get $response->getTransationId();
		// save $response->getTransactionStatus();
	}

	// return PSR-7 response to finish the update for the gateway (like a transaction)
	$psr7response = $response->getReply();
}

This also means that the NotificationInterface must inherit from the ResponseInterface, not the MessageInterface because what must be returned is always a response to have isSuccessful() available for retrieving the raw data.

[barryvdh]

So these are different issues I think:

1. completePurchase is not the same for redirectUrl and notifyUrl. So you want acceptNotification to be used when available and different, for the notifyUrl return request?
2. The transactionReference is not always available upon the initial request, so transactionId should be made available?
3. There must be a default way to respond to a notifyUrl request.

For 3, I think we all agree.
For 2, as far as Omnipay is (currently) concerned, only the transactionReference is required. So this should be supplied always, and can be used to lookup transactions in your database. But gateways (who divert from this) can still choose to make the transactionId available. I don't think this needs to be in the interface.

As for 1, I can sort of see this might be the case sometimes, but for the implementations I have seen, this usually isn't a problem. Sometimes you can call completePurchase only on the notifyUrl, sometimes also on the redirectUrl. It is perfectly fine to make another request to the gateway, using the provided token. But this shouldn't matter to the user. Or do you mean that you need the transactionId from the HttpRequest, to load the correct parameters BEFORE calling completePurchase?

[aimeos]

1.) The suggestion would be to use completePurchase() for the returnUrl and acceptNotification() for the notifyUrl only to have a clear distinction between both and an explicit handling. The "sometimes this, sometimes that way" is one of Omnipay's biggest problems and I think we should make this more explicit in 3.x and give driver developers a clear guideline.

2.) A transactionReference is NOT always available, in fact most of the time its not. Think about gateways where you need to redirect to the gateway and have no transactionReference in advance for storing it in the database for later lookup of the order. Jason opened this issue because a the moment, there are concrete problems with gateways requiring a redirect.

Edit: returnUrl instead of redirectUrl in 1.)

[judgej]

@aimeos It's interesting that $response->isSuccessful() is used in your example to check for tampering and not to check for a successful authorisation, which seems to be the convention in most of OmniPay. I think isSuccessful() should certainly be false if tampering has been detected, but a true does not necessarily mean the transaction auth/payment was successful, at least in the gateways I've seen.

Just a small note on the redirect and notify URLs: not all gateways allow you to customise this URL at runtime. For some gateways the URLs are fixed in the gateway configuration, and only data POSTed can be accessed. For some gateways the redirect URL returns the user to the site using a GET request with a fixed URL, so all you have to work with is what you put in the user session before you sent them off to the remote site. There seems to be an exception to every rule.

[aimeos]

@judgej Seemed a possible way as there's no other API method that could do the job. Additionally, you've said acceptNotification() must return an object implementing ResponsInterface. And yes, isSuccessful() doesn't say anything about the transaction status.

[judgej]

At some point you need a response object, since a response must be sent, and since the notification responses for gateways are very specific and particular about what is returned, the gateway driver- with the knowledge of what the gateway is expecting - is the thing that should generate that response.

Now where that is done, I'm not entirely sure, since there are a number of things that can happen in the notification handler flow:

• The notification POST may contain all the information that is needed for the merchant site to update the status of the transaction in storage. The NotificationInterface object returned by acceptNotification() can provide all that information (from the POST data) and can also generate the ResponsInterface object for responding to the gateway.
• The merchant site may need to provide some input before the ResponsInterface object is generated, so some means to inject that is needed. Input may be signalling "yes, I'm happy with that notification" or "what is that?! here is a message detailing why I am rejecting the notification". Some gateways don't need that - you just return, "got it" with no clues as to whether you accept it or not.
• Some gateways need an extra call back to the gateway to get further details. I'm not sure where that is best done. Perhaps in the instantiation of the notification object? Doesn't feel right though. The merchant site should not need to know about that callback though. In the front end this is where a completeAuthorization would have its send() invoked to send a final message to the gateway and get a final response. Doing this for all the gateways means many use the sent() but no message is actually sent to the gateway. Instead it just returns another object with the final results in. That is kind of what the acceptNotification has inherited.

So $gateway->acceptNotification() returns a NotificationInterface, and it is this that then should lead on to a ResponsInterface. Whether NotificationInterface returns the ResponsInterface directly (using getResponse() perhaps), or whether it needs send() to be invoked to get another object that generates the ResponsInterface and contains the final result, I don't know. That additional object is the point at which the NotificationInterface can go back to the gateway to get its additional information. But is there another place that can happen?

Edit: too many words. I think I need to sketch this out.

[barryvdh]

But for example Mollie, which has an offline gateway. You can either pass the transactionReference directly (eg. from database) or it will try to get it from the current http request (eg. notify); https://github.com/thephpleague/omnipay-mollie/blob/master/src/Message/CompletePurchaseRequest.php

I think you should, even in the case of the notification, always call the ->send() method to verify the request. In some gateways this will not be an actual request but just checking the hash, but still.

The notification interface could be used to get some data without actually checking, but then that data will still need to be passed to completePurchase to verify the request, so you could just as well do it there.

Only case I can think of is that you need to load the parameters from the database, based on the notification HttpRequest, so it would be something like:

$notification = $gateway->acceptNofication();
$params = TransactionRepository::findByReference($notification->transactionReference())->params;
$response = $gateway->completePurchase($params)->send();
if ($response->isSuccessful()) { ... }

But do you have an example of where you can't request the final status based on the notification alone?

[aimeos]

To be able to add order data for generating the final response, the code should look like this now:

if( $provider->supportsAcceptNotification() )
{
	// process the data in PSR-7 request
	$notification = $provider->acceptNotification( $psr7request );

	$params['transactionId'] = $notification->getTransationId();
	$params['transactionReference'] = 'xxx'; // Tx reference from database
	$params['amount'] = 'xxx'; // Total amount from order
	$params['currency'] = 'xxx'; // Currency from order

	// send request to gateway if necessary (PayPal)
	// return final response of evaluation
	$notification->getResponse( $param )->send();

	// verify that the notify request hasn't been tampered
	if( $response->isSuccessful() ) {
		// save $response->getTransactionStatus();
	}

	// return PSR-7 response to finish the update for the gateway (like a transaction)
	$psr7response = $response->getReply();
}

[barryvdh]

Ok, but remember that we are still using Symfony HttpFoundation for both incoming requests and responses. And the HttpResponse is already available inside the gateway.
But what I'm saying is, the only responsibility of the acceptNotification would have to be to get the transactionReference and/or transactionId based on the incoming HttpRequest, in order to load the database details.
The rest doesn't have to change, it can process the completePurchase etc just like before (like with redirect with session or route parameters).

if( $gateway->supportsAcceptNotification() )
{
        // Get the parameters from the incoming HttpRequest
	$notification = $gateway->acceptNotification();

	$params['transactionId'] = $notification->getTransationId();
	$params['transactionReference'] = 'xxx'; // Tx reference from database
	$params['amount'] = 'xxx'; // Total amount from order
	$params['currency'] = 'xxx'; // Currency from order

	// send request to gateway if necessary (PayPal)
	// return final response of evaluation
	$response = $gateway->completePurchase( $params )->send();

	// verify that the notify request hasn't been tampered
	if( $response->isSuccessful() ) {
		// save $response->getTransactionStatus();
	}

	// return HttpResponse to finish the update for the gateway (like a transaction)
	return $response->getReplyResponse();
}

So imho, it would be okay to have something like this to extract those parameters. But the handling of the status should be left to the complete* requests/responses.

[judgej]

Remember, there are no session parameters, because there is no session. There is also no browser to send a POST redirect to. I don't know of any gateways that expect a HTTP redirect as a response to the notify. They all just need text, JSON, XML, or just a HTTP code. One gateway requests you to send a HTML page back, that it then passes on to the user via the gateway, but that's a weird one.

[barryvdh]

I know, this is as for the notifyUrl when no session parameters are available, so INSTEAD OF the session parameters and not always possible to set the url.

And the response would indeed be just a 200 'ok' in 99% of the cases, but some gateways require something like 'success' or the transaction id back.

[aimeos]

What about drivers supporting completeAuthorize()? Must this be used instead of completePurchase() when in authorization mode?

The parameters sent by the gateway may differ for returnUrl and notifyUrl so this might end up in methods doing two completely different things, so I would suggest to avoid using completePurchase() for notifications.

[barryvdh]

Can you provide a real example?

[judgej]

@aimeos I agree that just extending the completeAuthorize to completePurchase classes would fill in that gap. It's one thing documenting what you need to use, but you are trying to automate this. Bear in mind you will probably need different routes (or GET parameters) for the two if using as a notification handler, since you would need to know whether you are responding to an authorize or a purchase before you instantiate the relevant complete* class.

[barryvdh]

But this is only an issue when BOTH of the following is true:

• It is not possible to define your own notifyUrl/returnUrl
• The httpRequest from notify/return does not provide enough information to complete the request.

So let's use an actual real example where this is a problem.

[aimeos]

Yes, the routes for handling notifications and for handling returnUrls with completeAuthorize/completePurchase are very different and therefore I suggest to not mix up both code paths. Instead, I suggest to use something like completeNotification:

if( $provider->supportsAcceptNotification() )
{
	// get data from incoming HttpRequest ($psr7request is optional)
	$notification = $gateway->acceptNotification( $psr7request );

	$params['transactionId'] = $notification->getTransationId();
	$params['transactionReference'] = 'xxx'; // Tx reference from database
	$params['amount'] = 'xxx'; // Total amount from order
	$params['currency'] = 'xxx'; // Currency from order

	// send request to gateway if necessary (PayPal)
	// return final response of evaluation
	$response = $gateway->completeNotification( $params )->send();

	// verify that the notify request hasn't been tampered
	if( $response->isSuccessful() ) {
		// save $response->getTransactionStatus();
	}

	// return HttpResponse to finish the update for the gateway (like a transaction)
	$psr7response = $response->getReply();
}

The method getTransactionStatus() is currently part of the NotificationInterface but the status is not always available at this stage. Move that to ResponseInterface instead?

@barryvdh I remember a payment gateway from Deutsche Bank where this was the case.

[aimeos]

More on the Deutsche Bank example: They only allowed URLs without GET parameters and sent an encrypted token that must be used to fetch the details.

[barryvdh]

You don't need GET parameters; /transaction/purchase/complete/234234

[aimeos]

Not all applications are based an Laravel and Symfony where you can define your own routes. There are a lot of applications where you will have problems and Aimeos for example can't make sure there are no URLs without GET parameters as we are not responsible for the host application.

[barryvdh]

And do you need all the parameters? Or just the token? Can't find the omnipay deutsche bank repo.

[aimeos]

As far as I remember, they only send the token as parameter.
We did a custom implementation not based on Omnipay years ago.