Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/312563
Change subject: WIP use ct_id to find completed, avoid race
......................................................................
WIP use ct_id to find completed, avoid race
We don't want to drop pending messages just because the payments-init
table says 'completed'. We can consult payments-init for failed
donations, but we should check a different table to determine
whether a completed donation has actually been imported.
TODO: test for the completed condition.
We'll want a matching patch for the fredge consumer in the crm repo.
So, now the contribution tracking schema lives in 3 places... Sorry!
Bug: T141477
Change-Id: I39d958b17347728873e6e3f416b32b0f97f781da
---
A Core/DataStores/ContributionTrackingDatabase.php
M Core/DataStores/PaymentsInitialDatabase.php
M Core/QueueConsumers/PendingQueueConsumer.php
A Schema/mysql/004_CreateContributionTrackingTable.sql
A Schema/sqlite/004_CreateContributionTrackingTable.sql
M SmashPig.yaml
M Tests/PendingQueueConsumerTest.php
M Tests/data/config_smashpig_db.yaml
8 files changed, 123 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/SmashPig
refs/changes/63/312563/1
diff --git a/Core/DataStores/ContributionTrackingDatabase.php
b/Core/DataStores/ContributionTrackingDatabase.php
new file mode 100644
index 0000000..c908a01
--- /dev/null
+++ b/Core/DataStores/ContributionTrackingDatabase.php
@@ -0,0 +1,39 @@
+<?php
+namespace SmashPig\Core\DataStores;
+
+use PDO;
+
+class ContributionTrackingDatabase extends SmashPigDatabase {
+
+ public function contributionExistsForTrackingId( $id ) {
+ $record = $this->fetchRecordByContributionTrackingId( $id );
+ return $record && !empty( $record['contribution_id'] );
+ }
+
+ public function fetchRecordByContributionTrackingId( $id ) {
+ $prepared = self::$db->prepare( '
+ SELECT * FROM contribution_tracking
+ WHERE id = :id' );
+ $prepared->bindValue( ':id', $id, PDO::PARAM_INT );
+ $prepared->execute();
+ $row = $prepared->fetch( PDO::FETCH_ASSOC );
+ if ( !$row ) {
+ return null;
+ }
+ return $row;
+ }
+
+ /**
+ * @return string Key in configuration pointing to backing PDO object
+ */
+ protected function getConfigKey() {
+ return 'data-store/contribution-tracking-db';
+ }
+
+ /**
+ * @return string Name of file (no directory) containing table creation
SQL
+ */
+ protected function getTableScriptFile() {
+ return '004_CreateContributionTrackingTable.sql';
+ }
+}
diff --git a/Core/DataStores/PaymentsInitialDatabase.php
b/Core/DataStores/PaymentsInitialDatabase.php
index d18701f..834b04e 100644
--- a/Core/DataStores/PaymentsInitialDatabase.php
+++ b/Core/DataStores/PaymentsInitialDatabase.php
@@ -18,13 +18,13 @@
* isTransactionFinalizedByGatewayOrderId??
* @return boolean
*/
- public function isTransactionFinalized( $message ) {
+ public function isTransactionFailed( $message ) {
$message = $this->fetchMessageByGatewayOrderId(
$message['gateway'], $message['order_id'] );
if ( $message === null ) {
return false;
}
- if ( in_array( $message['payments_final_status'], array(
'failed', 'complete' ) ) ) {
+ if ( $message['payments_final_status'] === 'failed' ) {
return true;
}
return false;
diff --git a/Core/QueueConsumers/PendingQueueConsumer.php
b/Core/QueueConsumers/PendingQueueConsumer.php
index 733d9b7..ac2f8b8 100644
--- a/Core/QueueConsumers/PendingQueueConsumer.php
+++ b/Core/QueueConsumers/PendingQueueConsumer.php
@@ -1,5 +1,6 @@
<?php namespace SmashPig\Core\QueueConsumers;
+use SmashPig\Core\DataStores\ContributionTrackingDatabase;
use SmashPig\Core\DataStores\PaymentsInitialDatabase;
use SmashPig\Core\DataStores\PendingDatabase;
use SmashPig\Core\Logging\Logger;
@@ -16,18 +17,28 @@
*/
protected $paymentsInitialDatabase;
+ /**
+ * @var ContributionTrackingDatabase
+ */
+ protected $contributionTrackingDatabase;
+
public function __construct( $queueName, $timeLimit, $messageLimit ) {
parent::__construct( $queueName, $timeLimit, $messageLimit );
$this->pendingDatabase = PendingDatabase::get();
$this->paymentsInitialDatabase = PaymentsInitialDatabase::get();
+ $this->contributionTrackingDatabase =
ContributionTrackingDatabase::get();
}
public function processMessage( $message ) {
$logIdentifier = "message with gateway {$message['gateway']}" .
" and order ID {$message['order_id']}";
- if ( $this->paymentsInitialDatabase->isTransactionFinalized(
$message ) ) {
- // Throw the message out if it's already completed or
failed, and
- // exists in the fredge database.
+ if (
+ $this->paymentsInitialDatabase->isTransactionFailed(
$message ) ||
+
$this->contributionTrackingDatabase->contributionExistsForTrackingId(
+ $message['contribution_tracking_id']
+ )
+ ) {
+ // Throw the message out if it's already completed or
failed
Logger::info( "Skipping finalized $logIdentifier" );
} else {
Logger::info( "Storing $logIdentifier in database" );
diff --git a/Schema/mysql/004_CreateContributionTrackingTable.sql
b/Schema/mysql/004_CreateContributionTrackingTable.sql
new file mode 100644
index 0000000..02fd0b0
--- /dev/null
+++ b/Schema/mysql/004_CreateContributionTrackingTable.sql
@@ -0,0 +1,27 @@
+CREATE TABLE IF NOT EXISTS `contribution_tracking` (
+ `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
+ `contribution_id` int(10) unsigned DEFAULT NULL,
+ `form_amount` varchar(20) DEFAULT NULL,
+ `usd_amount` decimal(20,2) DEFAULT NULL,
+ `note` text,
+ `referrer` varchar(4096) DEFAULT NULL,
+ `anonymous` tinyint(1) unsigned DEFAULT NULL,
+ `utm_source` varchar(128) DEFAULT NULL,
+ `utm_medium` varchar(128) DEFAULT NULL,
+ `utm_campaign` varchar(128) DEFAULT NULL,
+ `utm_key` varchar(128) DEFAULT NULL,
+ `payments_form` varchar(128) DEFAULT NULL,
+ `optout` tinyint(1) unsigned DEFAULT NULL,
+ `language` varchar(8) DEFAULT NULL,
+ `country` varchar(2) DEFAULT NULL,
+ `ts` char(14) DEFAULT NULL,
+ `owa_session` varbinary(255) DEFAULT NULL,
+ `owa_ref` int(11) DEFAULT NULL,
+ PRIMARY KEY (`id`),
+ UNIQUE KEY `contribution_id` (`contribution_id`),
+ KEY `ts` (`ts`),
+ KEY `utm_source_id` (`utm_source`),
+ KEY `utm_medium_id` (`utm_medium`),
+ KEY `utm_campaign_id` (`utm_campaign`),
+ KEY `utm_campsign_id` (`utm_campaign`)
+) ENGINE=InnoDB AUTO_INCREMENT=23320382 DEFAULT CHARSET=utf8
diff --git a/Schema/sqlite/004_CreateContributionTrackingTable.sql
b/Schema/sqlite/004_CreateContributionTrackingTable.sql
new file mode 100644
index 0000000..83b829a
--- /dev/null
+++ b/Schema/sqlite/004_CreateContributionTrackingTable.sql
@@ -0,0 +1,20 @@
+CREATE TABLE IF NOT EXISTS `contribution_tracking` (
+ `id` int(10) NOT NULL PRIMARY KEY,
+ `contribution_id` int(10) DEFAULT NULL,
+ `form_amount` varchar(20) DEFAULT NULL,
+ `usd_amount` decimal(20,2) DEFAULT NULL,
+ `note` text,
+ `referrer` varchar(4096) DEFAULT NULL,
+ `anonymous` tinyint(1) DEFAULT NULL,
+ `utm_source` varchar(128) DEFAULT NULL,
+ `utm_medium` varchar(128) DEFAULT NULL,
+ `utm_campaign` varchar(128) DEFAULT NULL,
+ `utm_key` varchar(128) DEFAULT NULL,
+ `payments_form` varchar(128) DEFAULT NULL,
+ `optout` tinyint(1) DEFAULT NULL,
+ `language` varchar(8) DEFAULT NULL,
+ `country` varchar(2) DEFAULT NULL,
+ `ts` char(14) DEFAULT NULL,
+ `owa_session` varbinary(255) DEFAULT NULL,
+ `owa_ref` int(11) DEFAULT NULL
+)
diff --git a/SmashPig.yaml b/SmashPig.yaml
index f078a36..dece735 100644
--- a/SmashPig.yaml
+++ b/SmashPig.yaml
@@ -46,6 +46,11 @@
constructor-parameters:
- 'mysql:host=127.0.0.1;dbname=fredge'
+ contribution-tracking-db:
+ class: PDO
+ constructor-parameters:
+ - 'mysql:host=127.0.0.1;dbname=drupal'
+
recurring:
class: SmashPig\Core\DataStores\MultiQueueWriter
constructor-parameters:
diff --git a/Tests/PendingQueueConsumerTest.php
b/Tests/PendingQueueConsumerTest.php
index 5c7aecc..fae4657 100644
--- a/Tests/PendingQueueConsumerTest.php
+++ b/Tests/PendingQueueConsumerTest.php
@@ -3,6 +3,7 @@
namespace SmashPig\Tests;
use SmashPig\Core\Context;
+use SmashPig\Core\DataStores\ContributionTrackingDatabase;
use SmashPig\Core\DataStores\PaymentsInitialDatabase;
use SmashPig\Core\DataStores\PendingDatabase;
use SmashPig\Core\QueueConsumers\PendingQueueConsumer;
@@ -19,6 +20,11 @@
*/
protected $paymentsInitialDb;
+ /**
+ * @var ContributionTrackingDatabase
+ */
+ protected $contributionTrackingDb;
+
public function setUp() {
parent::setUp();
// Merge db and queue test configs.
@@ -32,6 +38,8 @@
$this->pendingDb->createTable();
$this->paymentsInitialDb = PaymentsInitialDatabase::get();
$this->paymentsInitialDb->createTable();
+ $this->contributionTrackingDb =
ContributionTrackingDatabase::get();
+ $this->contributionTrackingDb->createTable();
}
public function tearDown() {
@@ -93,28 +101,6 @@
/**
* We refuse to consume a message and drop it if the corresponding
- * payments_initial row is complete.
- */
- public function testPendingMessageInitialComplete() {
- $initRow = PaymentsInitialDatabaseTest::generateTestMessage();
- $initRow['payments_final_status'] = 'complete';
-
- $this->paymentsInitialDb->storeMessage( $initRow );
-
- $message = self::generatePendingMessageFromInitial( $initRow );
- $consumer = new PendingQueueConsumer( 'pending', 1000, 1000 );
-
- $consumer->processMessage( $message );
-
- $fetched = $this->pendingDb->fetchMessageByGatewayOrderId(
- $message['gateway'], $message['order_id'] );
-
- $this->assertNull( $fetched,
- 'Message consumed and not stored in the pending
database.' );
- }
-
- /**
- * We refuse to consume a message and drop it if the corresponding
* payments_initial row is failed.
*/
public function testPendingMessageInitialFailed() {
@@ -139,6 +125,7 @@
$message = array(
'gateway' => 'test',
'date' => time(),
+ 'contribution_tracking_id' => mt_rand(),
'order_id' => mt_rand(),
'cousin' => 'itt',
'kookiness' => mt_rand(),
@@ -158,6 +145,7 @@
'gateway' => $initialRow['gateway'],
'date' => $initialRow['date'],
'order_id' => $initialRow['order_id'],
+ 'contribution_tracking_id' =>
$initialRow['contribution_tracking_id'],
'cousin' => 'itt',
'kookiness' => mt_rand(),
);
diff --git a/Tests/data/config_smashpig_db.yaml
b/Tests/data/config_smashpig_db.yaml
index e3bd4f3..dad34a9 100644
--- a/Tests/data/config_smashpig_db.yaml
+++ b/Tests/data/config_smashpig_db.yaml
@@ -14,3 +14,9 @@
class: PDO
constructor-parameters:
- 'sqlite::memory:'
+
+ contribution-tracking-db:
+ class: PDO
+ constructor-parameters:
+ - 'sqlite::memory:'
+
--
To view, visit https://gerrit.wikimedia.org/r/312563
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I39d958b17347728873e6e3f416b32b0f97f781da
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
