[MediaWiki-commits] [Gerrit] Generate new order IDs for each NewInvoice call - change (mediawiki...DonationInterface)
Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/227256
Change subject: Generate new order IDs for each NewInvoice call
..
Generate new order IDs for each NewInvoice call
AstroPay needs a different merchant-side identifier for each
NewInvoice call. This commit introduces a 'sequence' key so as not
to abuse the numAttempt, which should only be incremented when
setting final status for a payment attempt.
Bug: T106039
Change-Id: I91b49430aeab43150f63751c51f56bb4c906051e
---
M astropay_gateway/astropay.adapter.php
M gateway_common/gateway.adapter.php
M tests/Adapter/Astropay/AstropayTest.php
M tests/Adapter/GatewayAdapterTest.php
M tests/Adapter/Worldpay/WorldpayTest.php
M worldpay_gateway/worldpay.adapter.php
6 files changed, 70 insertions(+), 13 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/56/227256/1
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index 2db25b9..08b0950 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -127,7 +127,7 @@
/**
* Sets up the $order_id_meta array.
-* For Astropay, we use the ct_id.numAttempt format because we don't get
+* For Astropay, we use the ct_id.sequence format because we don't get
* a gateway transaction ID until the user has actually paid. If the
user
* doesn't return to the result switcher, we will need to use the
order_id
* to find a pending queue message with donor details to flesh out the
@@ -422,6 +422,11 @@
}
function doPayment() {
+ // If this is not our first NewInvoice call, get a fresh order
ID
+ if ( $this-session_getData( 'sequence' ) ) {
+ $this-regenerateOrderID();
+ }
+
$transaction_result = $this-do_transaction( 'NewInvoice' );
$this-runAntifraudHooks();
if ( $this-getValidationAction() !== 'process' ) {
@@ -603,6 +608,8 @@
* @param array $response
*/
protected function processNewInvoiceResponse( $response ) {
+ // Increment sequence number so next NewInvoice call gets a new
order ID
+ $this-incrementSequenceNumber();
if ( !isset( $response['status'] ) ) {
$this-transaction_response-setCommunicationStatus(
false );
$this-logger-error( 'Astropay response does not have
a status code' );
@@ -627,8 +634,6 @@
// have to parse the description.
if ( preg_match( '/invoice already used/i',
$response['desc'] ) ) {
$this-logger-error( 'Order ID
collision! Starting again.' );
- // Increment numAttempt to get a new
order ID
- $this-incrementNumAttempt();
throw new ResponseProcessingException(
'Order ID collision! Starting
again.',
ResponseCodes::DUPLICATE_ORDER_ID,
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index d81ffa9..f834c10 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -126,7 +126,7 @@
* order IDs, false if we are deferring order_id generation to the
* gateway.
* 'ct_id' = boolean value. If True, when generating order ID use
-* the contribution tracking ID with the attempt number appended
+* the contribution tracking ID with the sequence number appended
*
* Will eventually contain the following keys/values:
* 'final'= The value that we have chosen as the valid order ID for
@@ -2445,6 +2445,26 @@
$_SESSION['numAttempt'] = $attempts;
}
+ /**
+* Some payment gateways require a distinct identifier for each API call
+* or for each new payment attempt, even if retrying an attempt that
failed
+* validation. This is slightly different from numAttempt, which is
only
+* incremented when setting a final status for a payment attempt.
+* It is the child class's responsibility to increment this at the
+* appropriate time.
+*/
+ protected function incrementSequenceNumber() {
+ self::session_ensure();
+ $sequence = self::session_getData( 'sequence' );
//intentionally outside the 'Donor' key.
+ if ( is_numeric( $sequence ) ) {
+ $sequence += 1;
+ } else {
+ $sequence = 1;
+ }
+
+ $_SESSION['sequence'] =
[MediaWiki-commits] [Gerrit] Generate new order IDs for each NewInvoice call - change (mediawiki...DonationInterface)
jenkins-bot has submitted this change and it was merged.
Change subject: Generate new order IDs for each NewInvoice call
..
Generate new order IDs for each NewInvoice call
AstroPay needs a different merchant-side identifier for each
NewInvoice call. This commit introduces a 'sequence' key so as not
to abuse the numAttempt, which should only be incremented when
setting final status for a payment attempt.
Bug: T106039
Change-Id: I91b49430aeab43150f63751c51f56bb4c906051e
---
M astropay_gateway/astropay.adapter.php
M gateway_common/gateway.adapter.php
M tests/Adapter/Astropay/AstropayTest.php
M tests/Adapter/GatewayAdapterTest.php
M tests/Adapter/Worldpay/WorldpayTest.php
A tests/includes/Responses/astropay/NewInvoice_collision.testresponse
M worldpay_gateway/worldpay.adapter.php
7 files changed, 98 insertions(+), 13 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index 2db25b9..08b0950 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -127,7 +127,7 @@
/**
* Sets up the $order_id_meta array.
-* For Astropay, we use the ct_id.numAttempt format because we don't get
+* For Astropay, we use the ct_id.sequence format because we don't get
* a gateway transaction ID until the user has actually paid. If the
user
* doesn't return to the result switcher, we will need to use the
order_id
* to find a pending queue message with donor details to flesh out the
@@ -422,6 +422,11 @@
}
function doPayment() {
+ // If this is not our first NewInvoice call, get a fresh order
ID
+ if ( $this-session_getData( 'sequence' ) ) {
+ $this-regenerateOrderID();
+ }
+
$transaction_result = $this-do_transaction( 'NewInvoice' );
$this-runAntifraudHooks();
if ( $this-getValidationAction() !== 'process' ) {
@@ -603,6 +608,8 @@
* @param array $response
*/
protected function processNewInvoiceResponse( $response ) {
+ // Increment sequence number so next NewInvoice call gets a new
order ID
+ $this-incrementSequenceNumber();
if ( !isset( $response['status'] ) ) {
$this-transaction_response-setCommunicationStatus(
false );
$this-logger-error( 'Astropay response does not have
a status code' );
@@ -627,8 +634,6 @@
// have to parse the description.
if ( preg_match( '/invoice already used/i',
$response['desc'] ) ) {
$this-logger-error( 'Order ID
collision! Starting again.' );
- // Increment numAttempt to get a new
order ID
- $this-incrementNumAttempt();
throw new ResponseProcessingException(
'Order ID collision! Starting
again.',
ResponseCodes::DUPLICATE_ORDER_ID,
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index d81ffa9..f834c10 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -126,7 +126,7 @@
* order IDs, false if we are deferring order_id generation to the
* gateway.
* 'ct_id' = boolean value. If True, when generating order ID use
-* the contribution tracking ID with the attempt number appended
+* the contribution tracking ID with the sequence number appended
*
* Will eventually contain the following keys/values:
* 'final'= The value that we have chosen as the valid order ID for
@@ -2445,6 +2445,26 @@
$_SESSION['numAttempt'] = $attempts;
}
+ /**
+* Some payment gateways require a distinct identifier for each API call
+* or for each new payment attempt, even if retrying an attempt that
failed
+* validation. This is slightly different from numAttempt, which is
only
+* incremented when setting a final status for a payment attempt.
+* It is the child class's responsibility to increment this at the
+* appropriate time.
+*/
+ protected function incrementSequenceNumber() {
+ self::session_ensure();
+ $sequence = self::session_getData( 'sequence' );
//intentionally outside the 'Donor' key.
+ if ( is_numeric( $sequence ) ) {
+ $sequence += 1;
+ } else {
+ $sequence = 1;
+ }
+
+
