jenkins-bot has submitted this change and it was merged.
Change subject: More places where batch jobs shouldn't be pulling from the
request
......................................................................
More places where batch jobs shouldn't be pulling from the request
TODO:
* Deal with contribution tracking correctly.
Bug: T131798
Bug: T131275
Change-Id: I37db9c8096d24bfc8af5c383397a9a003f6ea8d2
---
M gateway_common/ContributionTrackingPlusUnique.php
M gateway_common/DonationData.php
M gateway_common/gateway.adapter.php
M globalcollect_gateway/globalcollect.adapter.php
M globalcollect_gateway/orphan.adapter.php
M tests/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
M tests/DonationDataTest.php
7 files changed, 64 insertions(+), 10 deletions(-)
Approvals:
Ejegg: Looks good to me, approved
jenkins-bot: Verified
diff --git a/gateway_common/ContributionTrackingPlusUnique.php
b/gateway_common/ContributionTrackingPlusUnique.php
index a445e86..c95f45d 100644
--- a/gateway_common/ContributionTrackingPlusUnique.php
+++ b/gateway_common/ContributionTrackingPlusUnique.php
@@ -6,6 +6,14 @@
*/
class ContributionTrackingPlusUnique implements StagingHelper, UnstagingHelper
{
public function stage( GatewayType $adapter, $normalized, &$stagedData
) {
+ if ( !isset( $normalized['contribution_tracking_id'] ) ) {
+ // Note that we should only reach this condition in
batch mode edge
+ // cases where the ctid is unavailable, or during
testing.
+ //
+ // A ctid will have been assigned, so we have to
pointedly not care today.
+
+ return;
+ }
$ctid = $normalized['contribution_tracking_id'];
//append timestamp to ctid
$ctid .= '.' . (( microtime( true ) * 1000 ) % 100000); //least
significant five
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 711fcd2..80ef2bf 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -160,6 +160,9 @@
* @return mixed The final value of the var, or null if we don't
actually have it.
*/
protected function sourceHarvest( $var ) {
+ if ( $this->gateway->isBatchProcessor() ) {
+ return null;
+ }
$ret = $this->gateway->getRequest()->getText( $var, null );
//all strings is just fine.
//getText never returns null: It just casts do an empty string.
Soooo...
if ( $ret === '' && !array_key_exists( $var, $_POST ) &&
!array_key_exists( $var, $_GET ) ) {
@@ -175,6 +178,9 @@
* are populated, and merge that with the data set we already have.
*/
protected function integrateDataFromSession() {
+ if ( $this->gateway->isBatchProcessor() ) {
+ return;
+ }
/**
* if the thing coming in from the session isn't already
something,
* replace it.
@@ -686,7 +692,9 @@
* Normalize referrer either by passing on the original, or grabbing it
in the first place.
*/
protected function setReferrer() {
- if ( !$this->isSomething( 'referrer' ) ) {
+ if ( !$this->isSomething( 'referrer' )
+ && !$this->gateway->isBatchProcessor()
+ ) {
// Remove protocol and query strings to avoid tripping
modsecurity
// TODO it would be a lot more privacy respecting to
omit path too.
$referrer = '';
@@ -834,6 +842,10 @@
* @return mixed Contribution tracking ID or false on failure
*/
public function saveContributionTrackingData() {
+ if ( $this->gateway->isBatchProcessor() ) {
+ // We aren't learning anything new about the donation,
so just return.
+ return false;
+ }
$ctid = $this->getVal( 'contribution_tracking_id' );
$tracking_data = $this->getCleanTrackingData( true );
$db =
ContributionTrackingProcessor::contributionTrackingConnection();
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 23d4340..f37c7ee 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1176,7 +1176,9 @@
// log that the transaction is essentially complete
$this->logger->info( 'Transaction complete.' );
- $this->debugarray[] = 'numAttempt = ' . $this->session_getData(
'numAttempt' );
+ if ( !$this->isBatchProcessor() ) {
+ $this->debugarray[] = 'numAttempt = ' .
$this->session_getData( 'numAttempt' );
+ }
return $this->transaction_response;
}
@@ -2184,6 +2186,9 @@
* be set to '0'.
*/
protected function incrementNumAttempt() {
+ if ( $this->isBatchProcessor() ) {
+ return;
+ }
$this->session_ensure();
$attempts = $this->session_getData( 'numAttempt' );
//intentionally outside the 'Donor' key.
if ( is_numeric( $attempts ) ) {
@@ -2846,6 +2851,9 @@
* This will be used internally every time we call do_transaction.
*/
public function session_addDonorData() {
+ if ( $this->isBatchProcessor() ) {
+ return;
+ }
$this->logger->info( __FUNCTION__ . ': Refreshing all donor
data' );
$this->session_ensure();
$sessionFields = DonationData::getSessionFields();
@@ -2863,6 +2871,9 @@
* reference will be gone.
*/
public function session_killAllEverything() {
+ if ( $this->isBatchProcessor() ) {
+ return;
+ }
SessionManager::getGlobalSession()->clear();
}
@@ -2891,6 +2902,9 @@
* mistake)
*/
public function session_resetForNewAttempt( $force = false ) {
+ if ( $this->isBatchProcessor() ) {
+ return;
+ }
$reset = $force;
if ( $this->session_getData( 'numAttempt' ) > 3 ) {
$reset = true;
@@ -3243,6 +3257,14 @@
}
}
+ if ( $this->isBatchProcessor() ) {
+ // Can't use request or session from here.
+ $locations = array_diff_key( $locations, array_flip(
array(
+ 'request',
+ 'session',
+ ) ) );
+ }
+
//Now pull all the locations and populate the candidate array.
$oid_candidates = array ( );
diff --git a/globalcollect_gateway/globalcollect.adapter.php
b/globalcollect_gateway/globalcollect.adapter.php
index 4f5dc74..5f04df1 100644
--- a/globalcollect_gateway/globalcollect.adapter.php
+++ b/globalcollect_gateway/globalcollect.adapter.php
@@ -613,12 +613,14 @@
'CVVRESULT' => 'cvv_result',
'AVSRESULT' => 'avs_result',
);
- // FIXME: Refactor as normal unstaging.
$qsResults = array();
- foreach ( $pull_vars as $theirkey => $ourkey) {
- $tmp = $this->request->getVal( $theirkey, null );
- if ( !is_null( $tmp ) ) {
- $qsResults[$ourkey] = $tmp;
+ if ( !$this->isBatchProcessor() ) {
+ // FIXME: Refactor as normal unstaging.
+ foreach ( $pull_vars as $theirkey => $ourkey) {
+ $tmp = $this->request->getVal( $theirkey, null
);
+ if ( !is_null( $tmp ) ) {
+ $qsResults[$ourkey] = $tmp;
+ }
}
}
diff --git a/globalcollect_gateway/orphan.adapter.php
b/globalcollect_gateway/orphan.adapter.php
index 91b99d4..a974308 100644
--- a/globalcollect_gateway/orphan.adapter.php
+++ b/globalcollect_gateway/orphan.adapter.php
@@ -107,10 +107,19 @@
}
public function getUTMInfoFromDB() {
+ if ( $this->getData_Unstaged_Escaped( 'utm_source' ) ) {
+ // We already have the info.
+ return array();
+ }
+ if ( !class_exists( 'ContributionTrackingProcessor' ) ) {
+ $this->logger->error( 'We needed to get
contribution_tracking data but cannot on this platform!' );
+ return array();
+ }
$db =
ContributionTrackingProcessor::contributionTrackingConnection();
if ( !$db ) {
- die( "There is something terribly wrong with your
Contribution Tracking database. fixit." );
+ $this->logger->error( 'There is something terribly
wrong with your Contribution Tracking database. fixit.' );
+ throw new RuntimeException( 'Might as well fall over.'
);
}
$ctid = $this->getData_Unstaged_Escaped(
'contribution_tracking_id' );
@@ -139,7 +148,6 @@
$msg .= "$key = $val ";
}
$this->logger->info( "$ctid: Found UTM Data.
$msg" );
- echo "$msg\n";
return $data;
}
}
diff --git a/tests/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
b/tests/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
index 3faa9d9..dc40bef 100644
--- a/tests/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
+++ b/tests/Adapter/GlobalCollect/GlobalCollectOrphanAdapterTest.php
@@ -155,6 +155,7 @@
$init['ffname'] = 'cc-vmad';
$init['order_id'] = '55555';
$init['email'] = '[email protected]';
+ $init['contribution_tracking_id'] = mt_rand();
$gateway->loadDataAndReInit( $init, $useDB = false );
$gateway->setDummyGatewayResponseCode( $code );
@@ -180,6 +181,7 @@
$init['ffname'] = 'cc-vmad';
$init['order_id'] = '55555';
$init['email'] = '[email protected]';
+ $init['contribution_tracking_id'] = mt_rand();
$gateway->loadDataAndReInit( $init, $useDB = false );
$gateway->setDummyGatewayResponseCode( '25' );
diff --git a/tests/DonationDataTest.php b/tests/DonationDataTest.php
index c894a07..6649ba1 100644
--- a/tests/DonationDataTest.php
+++ b/tests/DonationDataTest.php
@@ -116,6 +116,7 @@
$expected = array (
'amount' => '35.00',
'appeal' => 'JimmyQuote',
+ 'contribution_tracking_id' => mt_rand(),
'email' => '[email protected]',
'fname' => 'Tester',
'lname' => 'Testington',
@@ -159,7 +160,6 @@
$this->assertNotEquals( $returned['contribution_tracking_id'],
'', 'There is not a valid contribution tracking ID' );
unset($returned['order_id']);
- unset($returned['contribution_tracking_id']);
$this->assertEquals($expected, $returned, "Staged default test
data does not match expected.");
}
--
To view, visit https://gerrit.wikimedia.org/r/300933
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I37db9c8096d24bfc8af5c383397a9a003f6ea8d2
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Awight <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
