jenkins-bot has submitted this change and it was merged.
Change subject: Use PSR logging in DonationData
......................................................................
Use PSR logging in DonationData
Bug: T86266
Change-Id: I2dd11b0ee78872320fb57facb43cdaf5355d15e3
---
M DonationInterface.php
M gateway_common/DonationData.php
M gateway_common/gateway.adapter.php
M tests/DonationDataTest.php
M tests/DonationInterfaceTestCase.php
A tests/includes/TestingDonationLogger.php
6 files changed, 88 insertions(+), 30 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/DonationInterface.php b/DonationInterface.php
index 5486f53..d0d2e64 100644
--- a/DonationInterface.php
+++ b/DonationInterface.php
@@ -1047,6 +1047,7 @@
$wgAutoloadClasses['TestingAdyenAdapter'] = $testDir .
'includes/test_gateway/TestingAdyenAdapter.php';
$wgAutoloadClasses['TestingAmazonAdapter'] = $testDir .
'includes/test_gateway/TestingAmazonAdapter.php';
$wgAutoloadClasses['TestingAmazonGateway'] = $testDir .
'includes/test_page/TestingAmazonGateway.php';
+ $wgAutoloadClasses['TestingDonationLogger'] = $testDir .
'includes/TestingDonationLogger.php';
$wgAutoloadClasses['TestingGatewayPage'] = $testDir .
'includes/TestingGatewayPage.php';
$wgAutoloadClasses['TestingGenericAdapter'] = $testDir .
'includes/test_gateway/TestingGenericAdapter.php';
$wgAutoloadClasses['TestingGlobalCollectAdapter'] = $testDir .
'includes/test_gateway/TestingGlobalCollectAdapter.php';
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 49b0fd6..0a2c673 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -13,10 +13,14 @@
*
* @author khorn
*/
-class DonationData {
+class DonationData implements LogPrefixProvider {
protected $normalized = array( );
protected $gateway;
protected $validationErrors = null;
+ /**
+ * @var \Psr\Log\LoggerInterface
+ */
+ protected $logger;
/**
* DonationData constructor
@@ -24,11 +28,12 @@
* @param mixed $data An optional array of donation data that will, if
* present, circumvent the usual process of gathering the data from
various
* places in $wgRequest, or 'false' to gather the data the usual way.
- * Default is false.
+ * Default is false.
*/
function __construct( $gateway, $data = false ) {
$this->gateway = $gateway;
$this->gatewayID = $this->gateway->getIdentifier();
+ $this->logger = DonationLoggerFactory::getLogger( $gateway, '',
$this );
$this->populateData( $data );
}
@@ -400,11 +405,11 @@
//we're still going to try to regen.
$near_countries = array ( 'XX', 'EU', 'AP',
'A1', 'A2', 'O1' );
if ( !in_array( $country, $near_countries ) ) {
- $this->log( __FUNCTION__ . ": $country
is not a country, or a recognized placeholder.", LOG_WARNING );
+ $this->logger->warning( __FUNCTION__ .
": $country is not a country, or a recognized placeholder." );
}
}
} else {
- $this->log( __FUNCTION__ . ': Country not set.',
LOG_WARNING );
+ $this->logger->warning( __FUNCTION__ . ': Country not
set.' );
}
//try to regenerate the country if we still don't have a valid
one yet
@@ -420,11 +425,11 @@
// TODO: to change error_reporting is
less worse?
$country = @geoip_country_code_by_name(
$ip );
if ( !$country ) {
- $this->log( __FUNCTION__ . ":
GeoIP lookup function found nothing for $ip! No country available.",
LOG_WARNING );
+ $this->logger->warning(
__FUNCTION__ . ": GeoIP lookup function found nothing for $ip! No country
available." );
}
}
} else {
- $this->log( 'GeoIP lookup function is missing!
No country available.', LOG_WARNING );
+ $this->logger->warning( 'GeoIP lookup function
is missing! No country available.' );
}
//still nothing good? Give up.
@@ -452,16 +457,16 @@
if ( $this->isSomething( 'currency' ) ) {
$currency = $this->getVal( 'currency' );
$this->expunge( 'currency' );
- $this->log( "Got currency from 'currency', now:
$currency", LOG_DEBUG );
+ $this->logger->debug( "Got currency from 'currency',
now: $currency" );
} elseif ( $this->isSomething( 'currency_code' ) ) {
$currency = $this->getVal( 'currency_code' );
- $this->log( "Got currency from 'currency_code', now:
$currency", LOG_DEBUG );
+ $this->logger->debug( "Got currency from
'currency_code', now: $currency" );
}
//TODO: This is going to fail miserably if there's no country
yet.
if ( !$currency ){
$currency = NationalCurrencies::getNationalCurrency(
$this->getVal( 'country' ) );
- $this->log( "Got currency from 'country', now:
$currency", LOG_DEBUG );
+ $this->logger->debug( "Got currency from 'country',
now: $currency" );
}
$this->setVal( 'currency_code', $currency );
@@ -531,7 +536,7 @@
foreach ( $keys as $key ){
$mess .= ' ' . $key . '=' . $this->getVal( $key
);
}
- $this->log( $mess, LOG_DEBUG );
+ $this->logger->debug( $mess );
$this->setVal('amount', 'invalid');
return;
}
@@ -603,7 +608,7 @@
$message = "Submethod normalization
conflict!: ";
$message .= 'payment_submethod = ' .
$this->getVal( 'payment_submethod' );
$message .= ", and exploded
payment_method = '$submethod'. Going with the first option.";
- $this->log( $message, LOG_DEBUG );
+ $this->logger->debug( $message );
}
}
$submethod = $this->getVal( 'payment_submethod' );
@@ -636,21 +641,6 @@
*/
protected function sanitizeInput( &$value, $key ) {
$value = htmlspecialchars( $value, ENT_COMPAT, 'UTF-8', false );
- }
-
- /**
- * log: This grabs the adapter class that instantiated DonationData, and
- * uses its log function.
- * @TODO: Once the DonationData constructor does less, we can stop using
- * the static log function in the gateway. As it is, we're trying to log
- * things as we're constructing, when as far as the gateway cares we
- * don't exist yet. Very circular.
- * @param string $message The message to log.
- * @param int|string $log_level
- */
- protected function log( $message, $log_level = LOG_INFO ) {
- $message = $this->getLogMessagePrefix() . $message;
- $this->gateway->_log( $message, $log_level );
}
/**
@@ -760,7 +750,7 @@
}
$recurring_str = var_export( $this->getVal( 'recurring' ), true
);
- $this->log( __FUNCTION__ . ": Payment method is {$this->getVal(
'payment_method' )}, recurring = {$recurring_str}, utm_source =
{$utm_payment_method_family}", LOG_DEBUG );
+ $this->logger->debug( __FUNCTION__ . ": Payment method is
{$this->getVal( 'payment_method' )}, recurring = {$recurring_str}, utm_source =
{$utm_payment_method_family}" );
// split the utm_source into its parts for easier manipulation
$source_parts = explode( ".", $utm_source );
@@ -855,7 +845,7 @@
if ( !$db ) {
// TODO: This might be a critical failure; do we want
to throw an exception instead?
- $this->log( 'Failed to create a connect to
contribution_tracking database', LOG_ERR );
+ $this->logger->error( 'Failed to create a connect to
contribution_tracking database' );
return false;
}
@@ -879,7 +869,7 @@
if ( $db->insert( 'contribution_tracking',
$tracking_data ) ) {
$ctid = $db->insertId();
} else {
- $this->log( 'Failed to create a new
contribution_tracking record', LOG_ERR );
+ $this->logger->error( 'Failed to create a new
contribution_tracking record' );
return false;
}
}
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index cc7a2ee..5be7917 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -173,7 +173,7 @@
* GatewayAdapter
*
*/
-abstract class GatewayAdapter implements GatewayType {
+abstract class GatewayAdapter implements GatewayType, LogPrefixProvider {
/**
* $dataConstraints provides information on how to handle variables.
diff --git a/tests/DonationDataTest.php b/tests/DonationDataTest.php
index bc8dc9a..b6e8d94 100644
--- a/tests/DonationDataTest.php
+++ b/tests/DonationDataTest.php
@@ -15,11 +15,13 @@
* GNU General Public License for more details.
*
*/
+use Psr\Log\LogLevel;
/**
* @group Fundraising
* @group DonationInterface
* @group Splunge
+ * @group DonationData
*/
class DonationInterface_DonationDataTest extends DonationInterfaceTestCase {
@@ -210,6 +212,28 @@
}
/**
+ * Check that constructor outputs certain information to logs
+ */
+ public function testDebugLog() {
+ $expected = array (
+ 'payment_method' => 'cc',
+ 'utm_source' => 'test_src..cc',
+ 'utm_medium' => 'test_medium',
+ 'utm_campaign' => 'test_campaign',
+ 'payment_submethod' => 'amex',
+ 'currency_code' => 'USD',
+ );
+
+ $this->setMwGlobals( 'wgRequest', new FauxRequest( $expected,
false ) );
+
+ $ddObj = new DonationData( $this->getFreshGatewayObject( ) );
+ $matches = $this->getLogMatches( LogLevel::DEBUG,
'/setUtmSource: Payment method is cc, recurring = false, utm_source = cc$/' );
+ $this->assertNotEmpty( $matches );
+ $matches = $this->getLogMatches( LogLevel::DEBUG, "/Got
currency from 'currency_code', now: USD$/" );
+ $this->assertNotEmpty( $matches );
+ }
+
+ /**
*
*/
public function testRepopulate(){
diff --git a/tests/DonationInterfaceTestCase.php
b/tests/DonationInterfaceTestCase.php
index d7bad81..85d77b9 100644
--- a/tests/DonationInterfaceTestCase.php
+++ b/tests/DonationInterfaceTestCase.php
@@ -48,6 +48,11 @@
protected $gatewayAdapter;
/**
+ * @var TestingDonationLogger
+ */
+ protected $testLogger;
+
+ /**
* @param $name string The name of the test case
* @param $data array Any parameters read from a dataProvider
* @param $dataName string|int The name or index of the data set
@@ -65,11 +70,14 @@
}
protected function setUp() {
+ $this->testLogger = new TestingDonationLogger();
+ DonationLoggerFactory::$overrideLogger = $this->testLogger;
parent::setUp();
}
protected function tearDown() {
$this->resetAllEnv();
+ DonationLoggerFactory::$overrideLogger = null;
parent::tearDown();
}
@@ -598,6 +606,26 @@
}
/**
+ * Version of @see getGatewayLogMatches, but checking the mock PSR-3
logger.
+ * @param string $log_level One of the constants in \Psr\Log\LogLevel
+ * @param string $match A regex to match against the log lines.
+ * @return array All log lines that match $match.
+ */
+ public function getLogMatches( $log_level, $match ) {
+ $log = $this->testLogger->messages;
+ if ( !array_key_exists( $log_level, $log ) ) {
+ return false;
+ }
+ $return = array ( );
+ foreach ( $log[$log_level] as $line ) {
+ if ( preg_match( $match, $line ) ) {
+ $return[] = $line;
+ }
+ }
+ return $return;
+ }
+
+ /**
* Set global language for the duration of the test
*
* @param string $language language code to force
diff --git a/tests/includes/TestingDonationLogger.php
b/tests/includes/TestingDonationLogger.php
new file mode 100644
index 0000000..0a519a5
--- /dev/null
+++ b/tests/includes/TestingDonationLogger.php
@@ -0,0 +1,15 @@
+<?php
+use Psr\Log\AbstractLogger;
+
+/**
+ * Dummy logger
+ *
+ * @author Elliott Eggleston <[email protected]>
+ */
+class TestingDonationLogger extends AbstractLogger {
+ public $messages = array();
+
+ public function log( $level, $message, array $context = array() ) {
+ $this->messages[$level][] = $message;
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/192393
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2dd11b0ee78872320fb57facb43cdaf5355d15e3
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Katie Horn <[email protected]>
Gerrit-Reviewer: Ssmith <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
