Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/301630
Change subject: Quit mirroring / deleting pending queue
......................................................................
Quit mirroring / deleting pending queue
Settings change: rename 'pending-new' Redis queue to 'pending'
Do not merge till things are happy reading from the new pending
database and the new orphan rectifier has proven itself!
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/DonationQueue.php
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/Adapter/GlobalCollect/GlobalCollectTest.php
M tests/includes/test_gateway/TestingGlobalCollectAdapter.php
13 files changed, 19 insertions(+), 353 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/30/301630/1
diff --git a/README.txt b/README.txt
index 2999231..d747389 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 c70bd4d..f54c03a 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -189,7 +189,7 @@
$this->logger->info( "launching
external iframe request: " . print_r( $requestParams, true )
);
$this->logPaymentDetails();
- $this->setLimboMessage();
+ $this->sendPendingMessage();
break;
}
}
@@ -268,7 +268,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 25eb149..5b43f57 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->runPostProcessHooks();
- $this->deleteLimboMessage( 'pending' );
}
/**
diff --git a/astropay_gateway/astropay.adapter.php
b/astropay_gateway/astropay.adapter.php
index 7340951..43e8cd4 100644
--- a/astropay_gateway/astropay.adapter.php
+++ b/astropay_gateway/astropay.adapter.php
@@ -218,7 +218,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' ) );
}
@@ -298,7 +298,6 @@
$this->logger->info( "Payment status $status coming
back to ResultSwitcher" );
$this->finalizeInternalStatus( $status );
$this->runPostProcessHooks();
- $this->deleteLimboMessage( 'pending' );
break;
case 'NewInvoice':
$this->processNewInvoiceResponse( $response );
diff --git a/extension.json b/extension.json
index 525c33a..f941dc1 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/DonationQueue.php b/gateway_common/DonationQueue.php
index c1f6ec9..3244c39 100644
--- a/gateway_common/DonationQueue.php
+++ b/gateway_common/DonationQueue.php
@@ -2,18 +2,6 @@
class DonationQueue {
- // Existing queues that should be copied to the new pending queue.
- // TODO: should this be somewhere the consumer can see, so that we can
- // point it at new queues one at a time?
- static $oldPending = array(
- 'pending',
- 'globalcollect-cc-limbo'
- );
-
- // The new pending queue to copy to.
- // TODO: this should all maybe be in config.
- static $newPending = 'pending-new';
-
protected static $instance;
protected function __construct() {
@@ -39,9 +27,6 @@
$properties = $this->buildHeaders( $transaction );
$message = $this->buildBody( $transaction );
$this->newBackend( $queue )->push( $message, $properties );
-
- // FIXME: hack for new queue, remove
- $this->mirror( $message, $queue );
}
public function pop( $queue ) {
@@ -69,9 +54,6 @@
$properties = $this->buildHeaders( $transaction );
$message = $this->buildBody( $transaction );
$this->newBackend( $queue )->set( $correlationId, $message,
$properties );
-
- // FIXME: hack for new queue, remove
- $this->mirror( $message, $queue );
}
public function get( $correlationId, $queue ) {
@@ -86,24 +68,6 @@
return;
}
$this->newBackend( $queue )->clear( $correlationId );
- }
-
- /**
- * Temporary measure to transition 'pending' from key/value ActiveMQ to
a
- * pure FIFO queue. Sends a copy of anything in $oldPending to
$newPending,
- * if $newPending is explicitly configured.
- *
- * @param array $message
- * @param string $queue
- */
- protected function mirror( $message, $queue ) {
- global $wgDonationInterfaceQueues;
- if (
- in_array( $queue, self::$oldPending ) &&
- array_key_exists( self::$newPending,
$wgDonationInterfaceQueues )
- ) {
- $this->newBackend( self::$newPending )->push( $message
);
- }
}
/**
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 8a974ee..a6fd87f 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -2267,19 +2267,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()->set( $this->getCorrelationID(),
$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 5a34426..ccebea7 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';
@@ -635,11 +633,6 @@
$logmsg .= ', AVS Result from querystring: ' .
$this->getData_Unstaged_Escaped( 'avs_result' );
$this->logger->info( $logmsg );
- // If we have a querystring, this means we're
processing a live donor
- // coming back from GlobalCollect, and the transaction
is not orphaned
- // @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 {
// We're in orphan processing mode, so instead of
waiting around for
// the user to add more data and complete pending
transactions,
@@ -940,10 +933,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();
}
}
}
@@ -1709,7 +1698,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 a5f278c..22cc6e1 100644
--- a/paypal_gateway/express_checkout/paypal_express.adapter.php
+++ b/paypal_gateway/express_checkout/paypal_express.adapter.php
@@ -377,7 +377,7 @@
if ( $resultAction->getRedirect() ) {
// FIXME: This stuff should be base behavior for
handling redirect responses.
$this->logPaymentDetails();
- // Don't worry about saving a limbo message, we know
next to nothing yet.
+ // Don't worry about sending a pending message, we know
next to nothing yet.
// TODO: need ffname hack?
}
@@ -415,8 +415,6 @@
// response and sort it into complete or
pending.
$this->finalizeInternalStatus(
FinalStatus::COMPLETE );
$this->runPostProcessHooks();
- // FIXME: deprecated
- $this->deleteLimboMessage( 'pending' );
break;
case 'SetExpressCheckout':
case 'SetExpressCheckout_recurring':
@@ -485,8 +483,6 @@
// TODO: Can we do this from do_transaction
instead, or at least protect with !recurring...
$this->finalizeInternalStatus( $status );
$this->runPostProcessHooks();
- // FIXME: deprecated
- $this->deleteLimboMessage( 'pending' );
break;
}
diff --git a/tests/Adapter/GlobalCollect/GlobalCollectTest.php
b/tests/Adapter/GlobalCollect/GlobalCollectTest.php
index 17ed3a6..86781ac 100644
--- a/tests/Adapter/GlobalCollect/GlobalCollectTest.php
+++ b/tests/Adapter/GlobalCollect/GlobalCollectTest.php
@@ -483,10 +483,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/includes/test_gateway/TestingGlobalCollectAdapter.php
b/tests/includes/test_gateway/TestingGlobalCollectAdapter.php
index e940ae9..a1c196e 100644
--- a/tests/includes/test_gateway/TestingGlobalCollectAdapter.php
+++ b/tests/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: newchange
Gerrit-Change-Id: Ied22c6057496c550f1283489bc13ca4f81ab639a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
