Ejegg has submitted this change and it was merged.
Change subject: Clean up antifraud queue usage
......................................................................
Clean up antifraud queue usage
No more KeyedOpaqueStorableObject or mirroring to ActiveMQ, just push
an array straight to Redis
Bug: T131273
Change-Id: Ib06f946899c2217f52b6277233b679dca95f69c6
---
D CrmLink/Messages/DonationInterfaceAntifraud.php
A CrmLink/Messages/DonationInterfaceAntifraudFactory.php
M PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
M PaymentProviders/Adyen/Tests/config_test.yaml
M PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
M SmashPig.yaml
M Tests/MessageTest.php
7 files changed, 75 insertions(+), 87 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/CrmLink/Messages/DonationInterfaceAntifraud.php
b/CrmLink/Messages/DonationInterfaceAntifraud.php
deleted file mode 100644
index f462300..0000000
--- a/CrmLink/Messages/DonationInterfaceAntifraud.php
+++ /dev/null
@@ -1,44 +0,0 @@
-<?php namespace SmashPig\CrmLink\Messages;
-
-use SmashPig\Core\DataStores\KeyedOpaqueStorableObject;
-
-/**
- * Message encapsulating fraud scores and outcome
- */
-class DonationInterfaceAntifraud extends KeyedOpaqueStorableObject {
- public $contribution_tracking_id = '';
- public $date = '';
- public $gateway = '';
- public $gateway_txn_id = '';
- public $order_id = '';
- public $payment_method = '';
- public $risk_score = '';
- public $score_breakdown = array();
- public $server = '';
- public $user_ip = '';
- public $validation_action = '';
-
- public static function factory(
- $message,
- $riskScore,
- $scoreBreakdown = array(),
- $validationAction = 'process'
- ) {
-
- $obj = new DonationInterfaceAntifraud();
- $obj->risk_score = $riskScore;
- $obj->score_breakdown = $scoreBreakdown;
- $obj->validation_action = $validationAction;
-
- $obj->contribution_tracking_id =
$message['contribution_tracking_id'];
- $obj->date = $message['date'];
- $obj->gateway = $message['gateway'];
- $obj->gateway_txn_id = $message['gateway_txn_id'];
- $obj->order_id = $message['order_id'];
- $obj->payment_method = $message['payment_method'];
- // no 'server' available
- $obj->user_ip = $message['user_ip'];
-
- return $obj;
- }
-}
diff --git a/CrmLink/Messages/DonationInterfaceAntifraudFactory.php
b/CrmLink/Messages/DonationInterfaceAntifraudFactory.php
new file mode 100644
index 0000000..dc8c7f5
--- /dev/null
+++ b/CrmLink/Messages/DonationInterfaceAntifraudFactory.php
@@ -0,0 +1,38 @@
+<?php namespace SmashPig\CrmLink\Messages;
+
+/**
+ * Message encapsulating fraud scores and outcome
+ */
+class DonationInterfaceAntifraudFactory {
+
+ public static function create(
+ $donationMessage,
+ $riskScore,
+ $scoreBreakdown = array(),
+ $validationAction = 'process'
+ ) {
+
+ $antifraud = array(
+ 'risk_score' => $riskScore,
+ 'score_breakdown' => $scoreBreakdown,
+ 'validation_action' => $validationAction,
+ );
+
+ $keysToCopy = array(
+ 'contribution_tracking_id',
+ 'date',
+ 'gateway',
+ 'gateway_txn_id',
+ 'order_id',
+ 'payment_method',
+ 'user_ip'
+ // no 'server' available
+ );
+
+ foreach( $keysToCopy as $key ) {
+ $antifraud[$key] = $donationMessage[$key];
+ }
+
+ return $antifraud;
+ }
+}
diff --git a/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
b/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
index 8a06d51..fb39f08 100644
--- a/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
+++ b/PaymentProviders/Adyen/Jobs/ProcessCaptureRequestJob.php
@@ -5,7 +5,7 @@
use SmashPig\Core\Jobs\RunnableJob;
use SmashPig\Core\Logging\Logger;
use SmashPig\Core\Logging\TaggedLogger;
-use SmashPig\CrmLink\Messages\DonationInterfaceAntifraud;
+use SmashPig\CrmLink\Messages\DonationInterfaceAntifraudFactory;
use SmashPig\PaymentProviders\Adyen\AdyenPaymentsInterface;
use SmashPig\PaymentProviders\Adyen\ExpatriatedMessages\Authorisation;
@@ -189,11 +189,13 @@
}
protected function sendAntifraudMessage( $dbMessage, $riskScore,
$scoreBreakdown, $action ) {
- $antifraudMessage = DonationInterfaceAntifraud::factory(
+ $antifraudMessage = DonationInterfaceAntifraudFactory::create(
$dbMessage, $riskScore, $scoreBreakdown, $action
);
$this->logger->debug( "Sending antifraud message with risk
score $riskScore and action $action." );
- Configuration::getDefaultConfig()->object(
'data-store/antifraud' )->push( $antifraudMessage );
+ Configuration::getDefaultConfig()
+ ->object( 'data-store/payments-antifraud' )
+ ->push( $antifraudMessage );
}
/**
diff --git a/PaymentProviders/Adyen/Tests/config_test.yaml
b/PaymentProviders/Adyen/Tests/config_test.yaml
index 40185fa..2be830d 100644
--- a/PaymentProviders/Adyen/Tests/config_test.yaml
+++ b/PaymentProviders/Adyen/Tests/config_test.yaml
@@ -1,8 +1,11 @@
adyen:
data-store:
- antifraud-stomp:
- class: SmashPig\Tests\MockDataStore
+ payments-antifraud:
+ class: PHPQueue\Backend\PDO
+ constructor-parameters:
+ -
+ connection_string: 'sqlite::memory:'
verified:
class: PHPQueue\Backend\PDO
diff --git a/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
b/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
index 8c1e270..efcabbc 100644
--- a/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
+++ b/PaymentProviders/Adyen/Tests/phpunit/CaptureJobTest.php
@@ -1,9 +1,11 @@
<?php namespace SmashPig\PaymentProviders\Adyen\Test;
+use PHPQueue\Backend\PDO;
use SmashPig\Core\Configuration;
use SmashPig\Core\Context;
use SmashPig\Core\DataStores\KeyedOpaqueStorableObject;
use SmashPig\Core\DataStores\PendingDatabase;
+use SmashPig\Core\QueueConsumers\BaseQueueConsumer;
use SmashPig\PaymentProviders\Adyen\Jobs\ProcessCaptureRequestJob;
use SmashPig\PaymentProviders\Adyen\Tests\AdyenTestConfiguration;
use SmashPig\Tests\BaseSmashPigUnitTestCase;
@@ -22,6 +24,10 @@
*/
protected $pendingDatabase;
protected $pendingMessage;
+ /**
+ * @var PDO
+ */
+ protected $antifraudQueue;
public function setUp() {
parent::setUp();
@@ -32,6 +38,7 @@
file_get_contents( __DIR__ . '/../Data/pending.json' )
, true
);
$this->pendingDatabase->storeMessage( $this->pendingMessage );
+ $this->antifraudQueue = BaseQueueConsumer::getQueue(
'payments-antifraud' );
}
public function tearDown() {
@@ -41,10 +48,9 @@
/**
* For a legit donation, ProcessCaptureJob should leave donor data
- * on the pending queue, add an antifraud message, and return true.
+ * in the pending database, add an antifraud message, and return true.
*/
public function testSuccessfulCapture() {
- $antifraudQueue = $this->config->object(
'data-store/antifraud-stomp', true );
$api = $this->config->object( 'payment-provider/adyen/api',
true );
$auth = KeyedOpaqueStorableObject::fromJsonProxy(
@@ -78,15 +84,14 @@
'RequestCaptureJob did not make the right capture call'
);
- // Blank correlation ID on antifraud messages
- $antifraudMessage = $antifraudQueue->queueGetObject( null, '' );
+ $antifraudMessage = $this->antifraudQueue->pop();
$this->assertNotNull(
$antifraudMessage,
'RequestCaptureJob did not send antifraud message'
);
$this->assertEquals(
'process',
- $antifraudMessage->validation_action,
+ $antifraudMessage['validation_action'],
'Successful capture should get "process" validation
action'
);
}
@@ -96,7 +101,6 @@
* we should not capture the payment, but leave the donor details.
*/
public function testReviewThreshold() {
- $antifraudQueue = $this->config->object(
'data-store/antifraud-stomp', true );
$api = $this->config->object( 'payment-provider/adyen/api',
true );
$auth = KeyedOpaqueStorableObject::fromJsonProxy(
@@ -126,14 +130,14 @@
'RequestCaptureJob tried to capture above review
threshold'
);
- $antifraudMessage = $antifraudQueue->queueGetObject( null, '' );
+ $antifraudMessage = $this->antifraudQueue->pop();
$this->assertNotNull(
$antifraudMessage,
'RequestCaptureJob did not send antifraud message'
);
$this->assertEquals(
'review',
- $antifraudMessage->validation_action,
+ $antifraudMessage['validation_action'],
'Suspicious auth should get "review" validation action'
);
}
@@ -143,7 +147,6 @@
* we should cancel the authorization and delete the donor details.
*/
public function testRejectThreshold() {
- $antifraudQueue = $this->config->object(
'data-store/antifraud-stomp', true );
$api = $this->config->object( 'payment-provider/adyen/api',
true );
$auth = KeyedOpaqueStorableObject::fromJsonProxy(
@@ -175,14 +178,14 @@
'Did not cancel the fraudulent authorization'
);
- $antifraudMessage = $antifraudQueue->queueGetObject( null, '' );
+ $antifraudMessage = $this->antifraudQueue->pop();
$this->assertNotNull(
$antifraudMessage,
'RequestCaptureJob did not send antifraud message'
);
$this->assertEquals(
'reject',
- $antifraudMessage->validation_action,
+ $antifraudMessage['validation_action'],
'Obvious fraud should get "reject" validation action'
);
}
@@ -225,7 +228,7 @@
$this->pendingDatabase->fetchMessageByGatewayOrderId(
'adyen', $auth1->merchantReference
),
- 'Capture job should leave donor details on queue'
+ 'Capture job should leave donor details in database'
);
}
diff --git a/SmashPig.yaml b/SmashPig.yaml
index 8403293..8f0f69d 100644
--- a/SmashPig.yaml
+++ b/SmashPig.yaml
@@ -18,23 +18,11 @@
constructor-parameters:
- /tmp/
- antifraud:
- class: SmashPig\Core\DataStores\MultiQueueWriter
- constructor-parameters:
- -
- - antifraud-stomp
-
- antifraud-stomp:
- class: SmashPig\Core\DataStores\StompDataStore
- constructor-parameters:
- - antifraud
-
payments-antifraud:
class: PHPQueue\Backend\Predis
constructor-parameters:
-
<<: *REDIS
- queue: payments-antifraud
pending:
class: PHPQueue\Backend\Predis
@@ -129,7 +117,6 @@
convert-string-expressions: false
queues:
- antifraud: /queue/payments-antifraud
verified: /queue/donations
failed: /queue/failed
recurring: /queue/donations_recurring
diff --git a/Tests/MessageTest.php b/Tests/MessageTest.php
index f8675a3..07c1488 100644
--- a/Tests/MessageTest.php
+++ b/Tests/MessageTest.php
@@ -1,8 +1,7 @@
<?php
namespace SmashPig\Tests;
-use SmashPig\CrmLink\Messages\DonationInterfaceMessage;
-use SmashPig\CrmLink\Messages\DonationInterfaceAntifraud;
+use SmashPig\CrmLink\Messages\DonationInterfaceAntifraudFactory;
/**
* Test CrmLink message functions
@@ -25,19 +24,19 @@
'getScoreCountry' => 25,
'getScoreEmailDomain' => 10,
);
- $afMessage = DonationInterfaceAntifraud::factory(
+ $afMessage = DonationInterfaceAntifraudFactory::create(
$diMessage, 12.5, $scoreBreakdown, 'process'
);
- $this->assertEquals( $diMessage['contribution_tracking_id'],
$afMessage->contribution_tracking_id );
- $this->assertEquals( 1455128736, $afMessage->date );
- $this->assertEquals( 'adyen', $afMessage->gateway );
- $this->assertEquals( $diMessage['gateway_txn_id'],
$afMessage->gateway_txn_id );
- $this->assertEquals( $diMessage['order_id'],
$afMessage->order_id );
- $this->assertEquals( 'cc', $afMessage->payment_method );
- $this->assertEquals( 12.5, $afMessage->risk_score );
- $this->assertEquals( $scoreBreakdown,
$afMessage->score_breakdown );
- $this->assertEquals( '8.8.4.4', $afMessage->user_ip );
- $this->assertEquals( 'process', $afMessage->validation_action );
+ $this->assertEquals( $diMessage['contribution_tracking_id'],
$afMessage['contribution_tracking_id'] );
+ $this->assertEquals( 1455128736, $afMessage['date'] );
+ $this->assertEquals( 'adyen', $afMessage['gateway'] );
+ $this->assertEquals( $diMessage['gateway_txn_id'],
$afMessage['gateway_txn_id'] );
+ $this->assertEquals( $diMessage['order_id'],
$afMessage['order_id'] );
+ $this->assertEquals( 'cc', $afMessage['payment_method'] );
+ $this->assertEquals( 12.5, $afMessage['risk_score'] );
+ $this->assertEquals( $scoreBreakdown,
$afMessage['score_breakdown'] );
+ $this->assertEquals( '8.8.4.4', $afMessage['user_ip'] );
+ $this->assertEquals( 'process', $afMessage['validation_action']
);
}
}
--
To view, visit https://gerrit.wikimedia.org/r/310670
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib06f946899c2217f52b6277233b679dca95f69c6
Gerrit-PatchSet: 3
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
