jenkins-bot has submitted this change and it was merged.
Change subject: Quit deleting from pending queue, stop saying limbo
......................................................................
Quit deleting from pending queue, stop saying limbo
Get rid of 'limbo' references and old orphan rectifier.
Bug: T133433
Change-Id: Ied22c6057496c550f1283489bc13ca4f81ab639a
---
M README.txt
M adyen_gateway/adyen.adapter.php
M amazon_gateway/amazon.adapter.php
M astropay_gateway/astropay.adapter.php
M extension.json
M gateway_common/gateway.adapter.php
D globalcollect_gateway/GlobalCollectOrphanRectifier_pooled.php
M globalcollect_gateway/globalcollect.adapter.php
D globalcollect_gateway/scripts/orphans.php
M paypal_gateway/express_checkout/paypal_express.adapter.php
M tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
M tests/phpunit/includes/test_gateway/TestingGlobalCollectAdapter.php
12 files changed, 19 insertions(+), 318 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/README.txt b/README.txt
index e7b1d48..317bca0 100644
--- a/README.txt
+++ b/README.txt
@@ -332,16 +332,12 @@
// Incoming donations that we think have been paid for.
'completed' => array(),
- // So-called limbo queue for GlobalCollect, where we store donor
personal
- // information while waiting for the donor to return from iframe or a
- // redirect. It's very important that this data is not stored anywhere
- // permanent such as logs or the database, until we know this person
- // finished making a donation.
- // FIXME: Note that this must be an instance of KeyValueStore.
- //
+ // Transactions still needing action before they are settled.
+ 'pending' => array(),
+
// Example of a PCI-compliant queue configuration:
//
- // 'globalcollect-cc-limbo' => array(
+ // 'pending' => array(
// 'type' => 'PHPQueue\Backend\Predis',
// # Note that servers cannot be an array, due to some incompatibility
// # with aggregate connections.
@@ -353,14 +349,10 @@
//
// Example of aliasing a queue
//
- // 'globalcollect-cc-limbo' => array(
- // # Point at the main CC limbo queue.
- // 'queue' => 'cc-limbo',
+ // 'pending' => array(
+ // # Point at a queue named pending-new
+ // 'queue' => 'pending-new',
// ),
-
- // Transactions still needing action before they are settled.
- // FIXME: who reads from this queue?
- 'pending' => array(),
// Non-critical queues
diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index 7c12752..ae047e3 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -190,7 +190,7 @@
$this->logger->info( "launching
external iframe request: " . print_r( $requestParams, true )
);
$this->logPaymentDetails();
- $this->setLimboMessage();
+ $this->sendPendingMessage();
break;
}
}
@@ -269,7 +269,6 @@
}
}
else {
- $this->deleteLimboMessage( 'pending' );
$this->finalizeInternalStatus( FinalStatus::FAILED );
$this->logger->info( "Negative response from gateway.
Full response: " . print_r( $response, TRUE ) );
}
diff --git a/amazon_gateway/amazon.adapter.php
b/amazon_gateway/amazon.adapter.php
index 3615702..13ccdc1 100644
--- a/amazon_gateway/amazon.adapter.php
+++ b/amazon_gateway/amazon.adapter.php
@@ -206,7 +206,7 @@
// audit and IPN messages
$details = $this->getStompTransaction();
$this->logger->info( 'Got info for Amazon donation: ' .
json_encode( $details ) );
- $this->setLimboMessage();
+ $this->sendPendingMessage();
}
/**
@@ -251,7 +251,6 @@
$this->finalizeInternalStatus(
$this->capture_status_map[$captureState] );
$this->postProcessDonation();
- $this->deleteLimboMessage( 'pending' );
}
/**
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index 7b971bd..da6b8c8 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -289,7 +289,6 @@
$this->logger->info( "Payment status $status coming
back to ResultSwitcher" );
$this->finalizeInternalStatus( $status );
$this->postProcessDonation();
- $this->deleteLimboMessage( 'pending' );
break;
case 'NewInvoice':
$this->processNewInvoiceResponse( $response );
diff --git a/extension.json b/extension.json
index 9ee40cd..494c229 100644
--- a/extension.json
+++ b/extension.json
@@ -114,7 +114,6 @@
"GlobalCollectAdapter":
"globalcollect_gateway/globalcollect.adapter.php",
"GlobalCollectOrphanAdapter":
"globalcollect_gateway/orphan.adapter.php",
"GlobalCollectOrphanRectifier":
"globalcollect_gateway/GlobalCollectOrphanRectifier.php",
- "GlobalCollectOrphanRectifier_pooled":
"globalcollect_gateway/GlobalCollectOrphanRectifier_pooled.php",
"IngenicoFinancialNumber":
"globalcollect_gateway/IngenicoFinancialNumber.php",
"IngenicoLanguage":
"globalcollect_gateway/IngenicoLanguage.php",
"IngenicoMethodCodec":
"globalcollect_gateway/IngenicoMethodCodec.php",
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 307b0a7..67f016a 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1436,7 +1436,7 @@
$this->logPaymentDetails();
// Feed the message into the pending queue, so the CRM queue
consumer
// can read it to fill in donor details when it gets a partial
message
- $this->setLimboMessage();
+ $this->sendPendingMessage();
// Avoid 'bad ffname' logspam on return and try again links.
$this->session_pushFormName( $this->getData_Unstaged_Escaped(
'ffname' ) );
}
@@ -2254,19 +2254,10 @@
DonationQueue::instance()->push( $this->getStompTransaction(),
$queue );
}
- protected function setLimboMessage( $queue = 'pending' ) {
- // FIXME: log the key and raw queue name.
- $this->logger->info( "Setting transaction in limbo store
[$queue]" );
- DonationQueue::instance()->push( $this->getStompTransaction(),
$queue );
- }
-
- protected function deleteLimboMessage( $queue = 'pending' ) {
- $this->logger->info( "Clearing transaction from limbo store
[$queue]" );
- try {
- DonationQueue::instance()->delete(
$this->getCorrelationID(), $queue );
- } catch( BadMethodCallException $ex ) {
- $this->logger->warning( "Backend for queue [$queue]
does not support deletion. Hope your message had an expiration date!" );
- }
+ protected function sendPendingMessage() {
+ $order_id = $this->getData_Unstaged_Escaped( 'order_id' );
+ $this->logger->info( "Sending donor details for $order_id to
pending queue" );
+ DonationQueue::instance()->push( $this->getStompTransaction(),
'pending' );
}
/**
diff --git a/globalcollect_gateway/GlobalCollectOrphanRectifier_pooled.php
b/globalcollect_gateway/GlobalCollectOrphanRectifier_pooled.php
deleted file mode 100644
index 87f7430..0000000
--- a/globalcollect_gateway/GlobalCollectOrphanRectifier_pooled.php
+++ /dev/null
@@ -1,230 +0,0 @@
-<?php
-
-use Predis\Connection\ConnectionException;
-
-/**
- * Legacy STOMP orphan rectifier
- * @deprecated
- *
- * TODO: Generalize to all gateways, using hooks to implement polymorphism.
- */
-class GlobalCollectOrphanRectifier_pooled {
-
- protected $killfiles = array();
- protected $order_ids = array();
- protected $target_execute_time;
- protected $max_per_execute; //only really used if you're going by-file.
- protected $adapter;
-
- public function __construct() {
- // Have to turn this off here, until we know it's using the
user's ip, and
- // not 127.0.0.1 during the batch process. Otherwise, we'll
immediately
- // lock ourselves out when processing multiple charges.
- global $wgDonationInterfaceEnableIPVelocityFilter;
- $wgDonationInterfaceEnableIPVelocityFilter = false;
-
- if ( !$this->getOrphanGlobal( 'enable' ) ){
- $this->info( 'Orphan cron disabled. Have a nice day!' );
- return;
- }
-
- $this->target_execute_time = $this->getOrphanGlobal(
'target_execute_time' );
- $this->max_per_execute = $this->getOrphanGlobal(
'max_per_execute' );
-
- // FIXME: Is this just to trigger batch mode?
- $data = array(
- 'wheeee' => 'yes'
- );
- $this->adapter = new
GlobalCollectOrphanAdapter(array('external_data' => $data));
- $this->logger = DonationLoggerFactory::getLogger(
$this->adapter );
- }
-
- protected function keepGoing(){
- $elapsed = $this->getProcessElapsed();
- if ( $elapsed < $this->target_execute_time ) {
- return true;
- } else {
- return false;
- }
- }
-
- /**
- * This will both return the elapsed process time, and echo something
for
- * the cronspammer.
- * @return int elapsed time since start in seconds
- */
- protected function getProcessElapsed(){
- $elapsed = time() - $this->start_time;
- $this->info( "Elapsed Time: {$elapsed}" );
- return $elapsed;
- }
-
- protected function deleteMessage( $correlation_id, $queue ) {
- DonationQueue::instance()->delete( $correlation_id, $queue );
- }
-
- public function processOrphans() {
- // TODO: Make this configurable.
- // 20 minutes: this is exactly equal to something on
Globalcollect's side.
- $time_buffer = 60*20;
-
- $queue_pool = new CyclicalArray( $this->getOrphanGlobal(
'gc_cc_limbo_queue_pool' ) );
- if ( $queue_pool->isEmpty() ) {
- // FIXME: cheesy inline default
- $queue_pool = new CyclicalArray(
GlobalCollectAdapter::GC_CC_LIMBO_QUEUE );
- }
-
- $this->info( "Slaying orphans..." );
- $this->start_time = time();
-
- //I want to be clear on the problem I hope to prevent with
this. Say,
- //for instance, we pull a legit orphan, and for whatever
reason, can't
- //completely rectify it. Then, we go back and pull more... and
that
- //same one is in the list again. We should stop after one try
per
- //message per execute. We should also be smart enough to not
process
- //things we believe we just deleted.
- $this->handled_ids = array();
-
- while ( $this->keepGoing() && !$queue_pool->isEmpty() ) {
- $current_queue = $queue_pool->current();
- try {
- $message = DonationQueue::instance()->peek(
$current_queue );
-
- if ( !$message ) {
- $this->info( "Emptied queue
[{$current_queue}], removing from pool." );
- $queue_pool->dropCurrent();
- continue;
- }
-
- $correlation_id = 'globalcollect-' .
$message['gateway_txn_id'];
- if ( array_key_exists( $correlation_id,
$this->handled_ids ) ) {
- // We already did this one, keep going.
It's fine to draw
- // again from the same queue.
- DonationQueue::instance()->delete(
$correlation_id, $current_queue );
- continue;
- }
-
- // Check the timestamp to see if it's old
enough, and stop when
- // we're below the threshold. Messages are
guaranteed to pop in
- // chronological order.
- $elapsed = $this->start_time - $message['date'];
- if ( $elapsed < $time_buffer ) {
- $this->info( "Exhausted new messages in
[{$current_queue}], removing from pool..." );
- $queue_pool->dropCurrent();
-
- continue;
- }
-
- // We got ourselves an orphan!
- if ( $this->rectifyOrphan( $message ) ) {
- $this->handled_ids[$correlation_id] =
'rectified';
- } else {
- $this->handled_ids[$correlation_id] =
'error';
- }
-
- // Throw out the message either way.
- $this->deleteMessage( $correlation_id,
$current_queue );
-
- // Round-robin the pool before we complete the
loop.
- $queue_pool->rotate();
- } catch ( ConnectionException $ex ) {
- // Drop this server, for the duration of this
batch.
- $this->error( "Queue server for
[$current_queue] is down! Ignoring for this run..." );
- $queue_pool->dropCurrent();
- }
- }
-
- //TODO: Make stats squirt out all over the place.
- $rec = 0;
- $err = 0;
- foreach( $this->handled_ids as $id=>$whathappened ){
- switch ( $whathappened ){
- case 'rectified':
- $rec += 1;
- break;
- case 'error':
- $err += 1;
- break;
- }
- }
- $final = "\nDone! Final results: \n";
- $final .= " $rec rectified orphans \n";
- $final .= " $err errored out \n";
- if ( isset( $this->adapter->orphanstats ) ){
- foreach ( $this->adapter->orphanstats as $status =>
$count ) {
- $final .= "\n Status $status = $count";
- }
- }
- $final .= "\n Approximately " . $this->getProcessElapsed() . "
seconds to execute.\n";
- $this->info( $final );
- }
-
- /**
- * Uses the Orphan Adapter to rectify (complete the charge for) a
single orphan. Returns a boolean letting the caller know if
- * the orphan has been fully rectified or not.
- * @param array $data Some set of orphan data.
- * @param boolean $query_contribution_tracking A flag specifying if we
should query the contribution_tracking table or not.
- * @return boolean True if the orphan has been rectified, false if not.
- */
- protected function rectifyOrphan( $data, $query_contribution_tracking =
true ){
- $data['order_id'] = $data['gateway_txn_id'];
-
- $this->info( "Rectifying orphan: {$data['order_id']}" );
- $rectified = false;
-
- $normalized = DonationQueue::queueMessageToNormalized( $data );
- $this->adapter->loadDataAndReInit( $normalized,
$query_contribution_tracking );
- $results = $this->adapter->do_transaction( 'Confirm_CreditCard'
);
- $message = $results->getMessage();
- if ( $results->getCommunicationStatus() ){
- $this->info( $data['contribution_tracking_id'] . ":
FINAL: " . $this->adapter->getValidationAction() );
- $rectified = true;
- } else {
- $this->info( $data['contribution_tracking_id'] . ":
ERROR: " . $message );
- if ( strpos( $message, "GET_ORDERSTATUS reports that
the payment is already complete." ) === 0 ){
- $rectified = true;
- }
-
- //handles the transactions we've cancelled ourselves...
though if they got this far, that's a problem too.
- $errors = $results->getErrors();
- if ( !empty( $errors ) && array_key_exists( '1000001',
$errors ) ){
- $rectified = true;
- }
-
- //apparently this is well-formed GlobalCollect for
"iono". Get rid of it.
- if ( strpos( $message, "No processors are available." )
=== 0 ){
- $rectified = true;
- }
- }
-
- $this->info( $message );
-
- return $rectified;
- }
-
- /**
- * Gets the global setting for the key passed in.
- * @param string $key
- *
- * FIXME: Reuse GatewayAdapter::getGlobal.
- * @return mixed
- */
- protected static function getOrphanGlobal( $key ){
- global $wgDonationInterfaceOrphanCron;
- if ( array_key_exists( $key, $wgDonationInterfaceOrphanCron ) ){
- return $wgDonationInterfaceOrphanCron[$key];
- } else {
- return NULL;
- }
- }
-
- protected function info( $msg ) {
- $this->logger->info( $msg );
- print( "{$msg}\n" );
- }
-
- protected function error( $msg ) {
- $this->logger->error( $msg );
- error_log( $msg );
- }
-}
diff --git a/globalcollect_gateway/globalcollect.adapter.php
b/globalcollect_gateway/globalcollect.adapter.php
index abc5d9d..1550afa 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -25,8 +25,6 @@
const GATEWAY_NAME = 'Global Collect';
const IDENTIFIER = 'globalcollect';
const GLOBAL_PREFIX = 'wgGlobalCollectGateway';
- // @deprecated
- const GC_CC_LIMBO_QUEUE = 'globalcollect-cc-limbo';
public function getCommunicationType() {
return 'xml';
@@ -625,13 +623,7 @@
*/
private function transactionConfirm_CreditCard(){
$is_orphan = $this->isBatchProcessor();
- if ( !$is_orphan ) {
- // This was a normal front-end donation.
-
- // @deprecated We should be able to skip any deletion.
- $this->logger->info( 'Donor returned, deleting limbo
message' );
- $this->deleteLimboMessage( self::GC_CC_LIMBO_QUEUE );
- } else {
+ if ( $is_orphan ) {
// We're in orphan processing mode, so a "pending
waiting for donor
// input" status means that we'll never complete. Set
this range
// to map to "failed".
@@ -881,10 +873,6 @@
//get the old status from the
first txn, and add in the part where we set the payment.
$this->transaction_response->setTxnMessage( "Original Response Status
(pre-SET_PAYMENT): " . $original_status_code );
}
-
- // We won't need the limbo message
again, either way, so cancel it.
- // @deprecated
- $this->deleteLimboMessage();
}
}
}
@@ -1659,7 +1647,7 @@
if ( $action != FinalStatus::FAILED ){
// TODO: if method_loses_control rather than
hardcode cc.
if ( $this->getData_Unstaged_Escaped(
'payment_method' ) === 'cc' ) {
- $this->setLimboMessage(
self::GC_CC_LIMBO_QUEUE );
+ $this->sendPendingMessage();
}
}
}
diff --git a/globalcollect_gateway/scripts/orphans.php
b/globalcollect_gateway/scripts/orphans.php
deleted file mode 100644
index 1d0ce87..0000000
--- a/globalcollect_gateway/scripts/orphans.php
+++ /dev/null
@@ -1,21 +0,0 @@
-<?php
-//actually, as a maintenance script, this totally is a valid entry point.
-// FIXME: Prevent web access even if the security is misconfigured so this is
runnable.
-
-$IP = getenv( 'MW_INSTALL_PATH' );
-if ( $IP === false ) {
- $IP = __DIR__ . '/../../../..';
-}
-
-//If you get errors on this next line, set (and export) your MW_INSTALL_PATH
var.
-require_once( "$IP/maintenance/Maintenance.php" );
-
-class OrphanMaintenance extends Maintenance {
- public function execute() {
- $rectifier = new GlobalCollectOrphanRectifier_pooled();
- $rectifier->processOrphans();
- }
-}
-
-$maintClass = 'OrphanMaintenance';
-require_once RUN_MAINTENANCE_IF_MAIN;
diff --git a/paypal_gateway/express_checkout/paypal_express.adapter.php
b/paypal_gateway/express_checkout/paypal_express.adapter.php
index 306ebb7..6cc85d7 100644
--- a/paypal_gateway/express_checkout/paypal_express.adapter.php
+++ b/paypal_gateway/express_checkout/paypal_express.adapter.php
@@ -406,8 +406,6 @@
// response and sort it into complete or
pending.
$this->finalizeInternalStatus(
FinalStatus::COMPLETE );
$this->postProcessDonation();
- // FIXME: deprecated
- $this->deleteLimboMessage( 'pending' );
break;
case 'SetExpressCheckout':
case 'SetExpressCheckout_recurring':
@@ -476,8 +474,6 @@
// TODO: Can we do this from do_transaction
instead, or at least protect with !recurring...
$this->finalizeInternalStatus( $status );
$this->postProcessDonation();
- // FIXME: deprecated
- $this->deleteLimboMessage( 'pending' );
break;
}
diff --git a/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
b/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
index 70c1ce9..482bb9d 100644
--- a/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
+++ b/tests/phpunit/Adapter/GlobalCollect/GlobalCollectTest.php
@@ -456,10 +456,6 @@
$gateway->setDummyGatewayResponseCode( $code );
$gateway->do_transaction( 'Confirm_CreditCard' );
$this->assertEquals( 1, count( $gateway->curled ), "Gateway
kept trying even with response code $code! MasterCard could fine us a thousand
bucks for that!" );
-
- // Test limbo queue contents.
- $this->assertEquals( array( true ), $gateway->limbo_messages,
- "Gateway did not delete limbo message for code $code!"
);
}
/**
diff --git
a/tests/phpunit/includes/test_gateway/TestingGlobalCollectAdapter.php
b/tests/phpunit/includes/test_gateway/TestingGlobalCollectAdapter.php
index e940ae9..a1c196e 100644
--- a/tests/phpunit/includes/test_gateway/TestingGlobalCollectAdapter.php
+++ b/tests/phpunit/includes/test_gateway/TestingGlobalCollectAdapter.php
@@ -9,7 +9,7 @@
public $curled = array ( );
- public $limbo_messages = array();
+ public $pending_messages = array();
public $dummyGatewayResponseCode;
@@ -48,15 +48,8 @@
}
// TODO: Store and test the actual messages.
- public function setLimboMessage( $queue = 'pending' ) {
- $this->limbo_messages[] = false;
- }
-
- /**
- * Stub out the limboStomp fn and record the calls
- */
- public function deleteLimboMessage( $queue = 'pending' ) {
- $this->limbo_messages[] = true;
+ public function sendPendingMessage() {
+ $this->pending_messages[] = false;
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/301630
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ied22c6057496c550f1283489bc13ca4f81ab639a
Gerrit-PatchSet: 15
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org>
Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
