jenkins-bot has submitted this change and it was merged.
Change subject: Use PSR logging in GatewayPage classes
......................................................................
Use PSR logging in GatewayPage classes
And remove adapter public log functions
Bug: T86266
Change-Id: I546573526362519aea0fe4b19d61f27a116439e8
---
M amazon_gateway/amazon_gateway.body.php
M gateway_common/GatewayPage.php
M gateway_common/gateway.adapter.php
M gateway_forms/Form.php
M gateway_forms/RapidHtml.php
M globalcollect_gateway/globalcollect_resultswitcher.body.php
M paypal_gateway/paypal_resultswitcher.body.php
M special/GatewayFormChooser.php
M tests/includes/TestingGatewayPage.php
M tests/includes/test_gateway/TestingAdyenAdapter.php
M tests/includes/test_gateway/TestingGlobalCollectAdapter.php
M tests/includes/test_gateway/TestingGlobalCollectOrphanAdapter.php
M tests/includes/test_gateway/TestingWorldPayAdapter.php
13 files changed, 47 insertions(+), 141 deletions(-)
Approvals:
Awight: Looks good to me, approved
jenkins-bot: Verified
diff --git a/amazon_gateway/amazon_gateway.body.php
b/amazon_gateway/amazon_gateway.body.php
index 6f356d9..a20a901 100644
--- a/amazon_gateway/amazon_gateway.body.php
+++ b/amazon_gateway/amazon_gateway.body.php
@@ -54,7 +54,7 @@
}
// TODO: move resultswitching out
- $this->log( 'At gateway return with params: ' .
json_encode( $this->getRequest()->getValues() ), LOG_INFO );
+ $this->logger->info( 'At gateway return with params: '
. json_encode( $this->getRequest()->getValues() ) );
if ( $this->adapter->checkTokens() &&
$this->getRequest()->getText( 'status' ) ) {
$this->adapter->do_transaction(
'ProcessAmazonReturn' );
@@ -71,7 +71,7 @@
if ( !is_null( $specialform ) &&
$this->adapter->isValidSpecialForm( $specialform ) ){
$this->displayForm();
} else {
- $this->log( 'Failed to process gateway
return. Tokens bad or no status.', LOG_ERR );
+ $this->logger->error( 'Failed to
process gateway return. Tokens bad or no status.' );
}
}
}
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 28c806d..09fdd12 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -38,9 +38,16 @@
public $adapter;
/**
+ * Gateway-specific logger
+ * @var \Psr\Log\LoggerInterface
+ */
+ protected $logger;
+
+ /**
* Constructor
*/
public function __construct() {
+ $this->logger = DonationLoggerFactory::getLogger(
$this->adapter );
$this->getOutput()->addModules(
'donationInterface.skinOverride' );
$me = get_called_class();
@@ -119,7 +126,7 @@
$page = $this->adapter->getGlobal( "FailPage" );
$log_message = '"Redirecting to [ ' . $page . ' ] "';
- $this->log( $log_message, LOG_INFO );
+ $this->logger->info( $log_message );
if ( $page ) {
@@ -201,15 +208,6 @@
} else {
$wgOut->addHTML( "No Debug Array" );
}
- }
-
- /**
- * logs messages to the current gateway adapter's configured log
location
- * @param string $msg The message to log
- * @param int|string $log_level The severity level of the message.
- */
- public function log( $msg, $log_level=LOG_INFO ) {
- $this->adapter->log( $msg, $log_level );
}
/**
@@ -332,7 +330,7 @@
'currency_code' => $defaultCurrency
) );
- $this->adapter->log( "Unsupported currency $oldCurrency forced
to $defaultCurrency" );
+ $this->logger->info( "Unsupported currency $oldCurrency forced
to $defaultCurrency" );
return true;
}
@@ -409,11 +407,11 @@
global $wgServer;
if ( ( strpos( $referrer, $wgServer ) === false ) &&
!$liberated ) {
$_SESSION[ 'order_status' ][ $oid ] = 'liberated';
- $this->adapter->log("Resultswitcher: Popping out of
iframe for Order ID " . $oid);
+ $this->logger->info( "Resultswitcher: Popping out of
iframe for Order ID " . $oid );
//TODO: Move the $forbidden check back to the beginning
of this if block, once we know this doesn't happen a lot.
//TODO: If we get a lot of these messages, we need to
redirect to something more friendly than FORBIDDEN, RAR RAR RAR.
if ( $forbidden ) {
- $this->adapter->log("Resultswitcher: $oid
SHOULD BE FORBIDDEN. Reason: $f_message", LOG_ERR);
+ $this->logger->error( "Resultswitcher: $oid
SHOULD BE FORBIDDEN. Reason: $f_message" );
}
$this->getOutput()->allowClickjacking();
$this->getOutput()->addModules( 'iframe.liberator' );
@@ -423,10 +421,10 @@
$this->setHeaders();
if ( $forbidden ){
- $this->adapter->log( "Resultswitcher: Request
forbidden. " . $f_message . " Adapter Order ID: $oid", LOG_CRIT );
+ $this->logger->critical( "Resultswitcher: Request
forbidden. " . $f_message . " Adapter Order ID: $oid" );
return;
} else {
- $this->adapter->log( "Resultswitcher: OK to process
Order ID: " . $oid );
+ $this->logger->info( "Resultswitcher: OK to process
Order ID: " . $oid );
}
if ( $this->adapter->checkTokens() ) {
@@ -444,7 +442,7 @@
$this->getOutput()->redirect(
$this->adapter->getFailPage() );
}
} else {
- $this->adapter->log( "Resultswitcher: Token Check
Failed. Order ID: $oid", LOG_ERR );
+ $this->logger->error( "Resultswitcher: Token Check
Failed. Order ID: $oid" );
}
}
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index f2c6af8..5ab6582 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1555,62 +1555,6 @@
return $result;
}
- /**
- * Log messages out to syslog (if configured), or the wfDebugLog
- * @param string $msg The message to log
- * @param int $log_level Should be one of the following:
- * * LOG_EMERG - Actual meltdown in progress: Get everyone.
- * * LOG_ALERT - Time to start paging people and failing things
over
- * * LOG_CRIT - Corrective action required, but you probably have
some time.
- * * LOG_ERR - Probably denotes a bug in the system.
- * * LOG_WARNING - Not good, but will require eventual action to
preserve stability
- * * LOG_NOTICE - Unusual circumstances, but nothing imediately
alarming
- * * LOG_INFO - Nothing to see here. Business as usual.
- * * LOG_DEBUG - Probably shouldn't use these unless we're in the
process
- * of diagnosing a relatively esoteric problem that only happens in the
prod
- * environment, which will require a settings change to start the data
avalanche.
- * @param string $log_id_suffix Primarily used for shunting syslog
messages off into alternative buckets.
- * @return null
- */
- public function log( $msg, $log_level = LOG_INFO, $log_id_suffix = '' )
{
- if ( !self::getGlobal('LogDebug') && $log_level === LOG_DEBUG ){
- //stfu, then.
- return;
- }
-
- $msg = $this->getLogMessagePrefix() . $msg;
- self::_log( $msg, $log_level, $log_id_suffix );
- }
-
- /**
- * DO NOT USE THIS FUNCTION UNLESS YOU CAN'T INSTANTIATE.
- * /sadface
- * @param type $msg
- * @param type $log_level
- * @param type $log_id_suffix
- * @return type
- */
- public static function _log( $msg, $log_level = LOG_INFO,
$log_id_suffix = '' ) {
- if ( !self::getGlobal( 'LogDebug' ) && $log_level === LOG_DEBUG
) {
- //stfu, then.
- return;
- }
-
- $identifier = self::getIdentifier() . "_gateway" .
$log_id_suffix;
-
- // if we're not using the syslog facility, use wfDebugLog
- if ( !self::getGlobal( 'UseSyslog' ) ) {
- WmfFramework::debugLog( $identifier, $msg );
- return;
- }
-
- // otherwise, use syslogging
- openlog( $identifier, LOG_ODELAY, LOG_SYSLOG );
- $msg = str_replace( "\t", " ", $msg );
- syslog( $log_level, $msg );
- closelog();
- }
-
//To avoid reinventing the wheel: taken from
http://recursive-design.com/blog/2007/04/05/format-xml-with-php/
function formatXmlString( $xml ) {
// add marker linefeeds to aid the pretty-tokeniser (adds a
linefeed between all tag-end boundaries)
diff --git a/gateway_forms/Form.php b/gateway_forms/Form.php
index 100a633..7204bf4 100644
--- a/gateway_forms/Form.php
+++ b/gateway_forms/Form.php
@@ -8,6 +8,12 @@
public $form_errors;
/**
+ * Gateway-specific logger
+ * @var \Psr\Log\LoggerInterface
+ */
+ protected $logger;
+
+ /**
* Required method for returning the full HTML for a form.
*
* Code invoking forms will expect this method to be set. Requiring
only
@@ -21,6 +27,7 @@
public function __construct( &$gateway ) {
$this->gateway = & $gateway;
+ $this->logger = DonationLoggerFactory::getLogger( $gateway );
$gateway_errors = $this->gateway->getAllErrors();
// @codeCoverageIgnoreStart
diff --git a/gateway_forms/RapidHtml.php b/gateway_forms/RapidHtml.php
index 5ed47df..fe8f013 100644
--- a/gateway_forms/RapidHtml.php
+++ b/gateway_forms/RapidHtml.php
@@ -133,7 +133,7 @@
$this->set_html_file_path( $ffname );
} catch ( MWException $mwe ) {
$message = "Could not load form '$ffname'";
- $this->gateway->log( $message, LOG_ERR );
+ $this->logger->error( $message );
$this->set_html_file_path( 'error-noform' );
}
}
@@ -491,7 +491,7 @@
if ( $problems ){
if ( $fatal ){
$message = 'Requested an unavailable or
non-existent form.';
- $this->gateway->log( $message . ' ' .
$debug_message . ' ' . $this->gateway->getData_Unstaged_Escaped('utm_source') ,
LOG_ERR );
+ $this->logger->error( $message . ' ' .
$debug_message . ' ' . $this->gateway->getData_Unstaged_Escaped('utm_source') );
throw new MWException( $message ); # TODO:
translate
} else {
return;
diff --git a/globalcollect_gateway/globalcollect_resultswitcher.body.php
b/globalcollect_gateway/globalcollect_resultswitcher.body.php
index 123094a..0e8e86e 100644
--- a/globalcollect_gateway/globalcollect_resultswitcher.body.php
+++ b/globalcollect_gateway/globalcollect_resultswitcher.body.php
@@ -70,18 +70,18 @@
$f_message = "Requested order id not present in the
session. (session_oid = '$session_oid')";
if ( !$_SESSION ) {
- $this->adapter->log( "Resultswitcher:
{$this->qs_oid} Is popped out, but still has no session data.", LOG_ERR );
+ $this->logger->error( "Resultswitcher:
{$this->qs_oid} Is popped out, but still has no session data." );
}
}
if ( $forbidden ){
- $this->adapter->log( $this->qs_oid . " Resultswitcher:
forbidden for reason: {$f_message}", LOG_ERR );
+ $this->logger->error( $this->qs_oid . " Resultswitcher:
forbidden for reason: {$f_message}" );
wfHttpError( 403, 'Forbidden', wfMessage(
'donate_interface-error-http-403' )->text() );
return;
}
$this->setHeaders();
- $this->adapter->log( "Resultswitcher: OK to process Order ID: "
. $this->qs_oid );
+ $this->logger->info( "Resultswitcher: OK to process Order ID: "
. $this->qs_oid );
// dispatch forms/handling
if ( $this->adapter->checkTokens() ) {
@@ -97,7 +97,7 @@
if ( array_key_exists( 'pending',
$_SESSION ) ){
$started = $_SESSION['pending'];
//not sure what to do with this
yet, but I sure want to know if it's happening.
- $this->adapter->log(
"Resultswitcher: Parallel Universe Unlocked. Start time: $started", LOG_ALERT );
+ $this->logger->alert(
"Resultswitcher: Parallel Universe Unlocked. Start time: $started" );
}
$_SESSION['pending'] = microtime( true
); //We couldn't have gotten this far if the server wasn't sticky.
@@ -106,7 +106,7 @@
$_SESSION['order_status'][$oid]['data']['count'] = 0;
} else {
$_SESSION['order_status'][$oid]['data']['count'] =
$_SESSION['order_status'][$oid]['data']['count'] + 1;
- $this->adapter->log( "Resultswitcher:
Multiple attempts to process. " .
$_SESSION['order_status'][$oid]['data']['count'], LOG_ERR );
+ $this->logger->error( "Resultswitcher:
Multiple attempts to process. " .
$_SESSION['order_status'][$oid]['data']['count'] );
}
$result = $_SESSION['order_status'][$oid];
$this->displayResultsForDebug( $result );
@@ -128,16 +128,16 @@
$this->getOutput()->addHTML(
"<br>Redirecting to page $go" );
$this->getOutput()->redirect(
$go );
} else {
-
$this->adapter->log("Resultswitcher: No redirect defined. Order ID: $oid",
LOG_ERR);
+ $this->logger->error(
"Resultswitcher: No redirect defined. Order ID: $oid" );
}
} else {
- $this->adapter->log("Resultswitcher: No
FinalStatus. Order ID: $oid", LOG_ERR);
+ $this->logger->error( "Resultswitcher:
No FinalStatus. Order ID: $oid" );
}
} else {
- $this->adapter->log("Resultswitcher: Payment
method is not cc. Order ID: $oid", LOG_ERR);
+ $this->logger->error( "Resultswitcher: Payment
method is not cc. Order ID: $oid" );
}
} else {
- $this->adapter->log("Resultswitcher: Token Check
Failed. Order ID: $oid", LOG_ERR);
+ $this->logger->error("Resultswitcher: Token Check
Failed. Order ID: $oid" );
}
}
@@ -168,17 +168,17 @@
$referrer = $this->getRequest()->getHeader( 'referer' );
if ( ( strpos( $referrer, $wgServer ) === false ) ) {
if ( !$_SESSION ) {
- $this->adapter->log( "Resultswitcher:
{$this->qs_oid} warning: iframed script cannot see session cookie.", LOG_ERR );
+ $this->logger->error( "Resultswitcher:
{$this->qs_oid} warning: iframed script cannot see session cookie." );
}
$_SESSION['order_status'][$this->qs_oid] = 'liberated';
- $this->adapter->log("Resultswitcher: Popping out of
iframe for Order ID " . $this->qs_oid);
+ $this->logger->info( "Resultswitcher: Popping out of
iframe for Order ID " . $this->qs_oid );
$this->getOutput()->allowClickjacking();
$this->getOutput()->addModules( 'iframe.liberator' );
return true;
}
- $this->adapter->log( "Resultswitcher: good, it appears we are
not in an iframe. Order ID {$this->qs_oid}" );
+ $this->logger->info( "Resultswitcher: good, it appears we are
not in an iframe. Order ID {$this->qs_oid}" );
}
}
diff --git a/paypal_gateway/paypal_resultswitcher.body.php
b/paypal_gateway/paypal_resultswitcher.body.php
index 3056534..729b44d 100644
--- a/paypal_gateway/paypal_resultswitcher.body.php
+++ b/paypal_gateway/paypal_resultswitcher.body.php
@@ -63,7 +63,7 @@
*/
$this->getOutput()->redirect(
$this->adapter->getThankYouPage() );
} else {
- $this->adapter->log("Resultswitcher: Token Check
Failed. Order ID: $oid");
+ $this->logger->info( "Resultswitcher: Token Check
Failed. Order ID: $oid" );
$this->getOutput()->redirect(
$this->adapter->getFailPage() );
}
}
diff --git a/special/GatewayFormChooser.php b/special/GatewayFormChooser.php
index fea164c..8ac5560 100644
--- a/special/GatewayFormChooser.php
+++ b/special/GatewayFormChooser.php
@@ -11,7 +11,13 @@
*/
class GatewayFormChooser extends UnlistedSpecialPage {
+ /**
+ * @var \Psr\Log\LoggerInterface
+ */
+ protected $logger;
+
function __construct() {
+ $this->logger = DonationLoggerFactory::getLogger();
parent::__construct( 'GatewayFormChooser' );
}
@@ -48,9 +54,8 @@
if ( $form === null ) {
$utmSource = $this->getRequest()->getVal( 'utm_source',
'' );
- GatewayAdapter::_log(
- "Not able to find a valid form for country
'$country', currency '$currency', method '$paymentMethod', submethod
'$paymentSubMethod', recurring: '$recurring', gateway '$gateway' for utm source
'$utmSource'",
- LOG_ERR
+ $this->logger->error(
+ "Not able to find a valid form for country
'$country', currency '$currency', method '$paymentMethod', submethod
'$paymentSubMethod', recurring: '$recurring', gateway '$gateway' for utm source
'$utmSource'"
);
$this->getOutput()->showErrorPage(
'donate_interface-error-msg-general', 'donate_interface-error-no-form' );
return;
diff --git a/tests/includes/TestingGatewayPage.php
b/tests/includes/TestingGatewayPage.php
index 79872d6..94b7ba5 100644
--- a/tests/includes/TestingGatewayPage.php
+++ b/tests/includes/TestingGatewayPage.php
@@ -2,6 +2,7 @@
class TestingGatewayPage extends GatewayPage {
public function __construct() {
+ $this->logger = DonationLoggerFactory::getLogger();
//nothing!
}
protected function handleRequest() {
diff --git a/tests/includes/test_gateway/TestingAdyenAdapter.php
b/tests/includes/test_gateway/TestingAdyenAdapter.php
index 2ef8fd5..e2f008a 100644
--- a/tests/includes/test_gateway/TestingAdyenAdapter.php
+++ b/tests/includes/test_gateway/TestingAdyenAdapter.php
@@ -5,8 +5,6 @@
*/
class TestingAdyenAdapter extends AdyenAdapter {
- public $testlog = array ( );
-
public function _buildRequestParams() {
return $this->buildRequestParams();
}
@@ -24,17 +22,6 @@
public function _getData_Staged() {
return call_user_func_array( array ( $this, 'getData_Staged' ),
func_get_args() );
- }
-
- /**
- * Trap the error log so we can use it in testing
- * @param type $msg
- * @param type $log_level
- * @param type $log_id_suffix
- */
- public function log( $msg, $log_level = LOG_INFO, $log_id_suffix = '' )
{
- //I don't care about the suffix right now, particularly.
- $this->testlog[$log_level][] = $msg;
}
/**
diff --git a/tests/includes/test_gateway/TestingGlobalCollectAdapter.php
b/tests/includes/test_gateway/TestingGlobalCollectAdapter.php
index 1bae8ee..6c58e20 100644
--- a/tests/includes/test_gateway/TestingGlobalCollectAdapter.php
+++ b/tests/includes/test_gateway/TestingGlobalCollectAdapter.php
@@ -6,7 +6,6 @@
* TODO: Add dependency injection to the base class so we don't have to repeat
code here.
*/
class TestingGlobalCollectAdapter extends GlobalCollectAdapter {
- public $testlog = array ( );
public $curled = array ( );
@@ -90,16 +89,6 @@
*/
public function doLimboStompTransaction( $antiMessage = false ) {
$this->limbo_stomps[] = $antiMessage;
- }
- /**
- * Trap the error log so we can use it in testing
- * @param type $msg
- * @param type $log_level
- * @param type $log_id_suffix
- */
- public function log( $msg, $log_level = LOG_INFO, $log_id_suffix = ''){
- //I don't care about the suffix right now, particularly.
- $this->testlog[$log_level][] = $msg;
}
//@TODO: That minfraud jerk needs its own isolated tests.
diff --git a/tests/includes/test_gateway/TestingGlobalCollectOrphanAdapter.php
b/tests/includes/test_gateway/TestingGlobalCollectOrphanAdapter.php
index cd088c3..b8c6867 100644
--- a/tests/includes/test_gateway/TestingGlobalCollectOrphanAdapter.php
+++ b/tests/includes/test_gateway/TestingGlobalCollectOrphanAdapter.php
@@ -10,8 +10,6 @@
class TestingGlobalCollectOrphanAdapter extends GlobalCollectOrphanAdapter {
- public $testlog = array ( );
-
public $curled = array ( );
/**
@@ -77,17 +75,6 @@
return;
}
parent::defineOrderIDMeta();
- }
-
- /**
- * Trap the error log so we can use it in testing
- * @param type $msg
- * @param type $log_level
- * @param type $log_id_suffix
- */
- public function log( $msg, $log_level = LOG_INFO, $log_id_suffix = '' )
{
- //I don't care about the suffix right now, particularly.
- $this->testlog[$log_level][] = $msg;
}
//@TODO: That minfraud jerk needs its own isolated tests.
diff --git a/tests/includes/test_gateway/TestingWorldPayAdapter.php
b/tests/includes/test_gateway/TestingWorldPayAdapter.php
index e571781..e670f57 100644
--- a/tests/includes/test_gateway/TestingWorldPayAdapter.php
+++ b/tests/includes/test_gateway/TestingWorldPayAdapter.php
@@ -5,7 +5,6 @@
*/
class TestingWorldPayAdapter extends WorldPayAdapter {
- public $testlog = array ( );
public $curled = '';
//@TODO: That minfraud jerk needs its own isolated tests.
@@ -17,17 +16,6 @@
parent::runAntifraudHooks();
$this->batch = $is_batch;
- }
-
- /**
- * Trap the error log so we can use it in testing
- * @param type $msg
- * @param type $log_level
- * @param type $log_id_suffix
- */
- public function log( $msg, $log_level = LOG_INFO, $log_id_suffix = '' )
{
- //I don't care about the suffix right now, particularly.
- $this->testlog[$log_level][] = $msg;
}
public function getRiskScore() {
--
To view, visit https://gerrit.wikimedia.org/r/193280
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I546573526362519aea0fe4b19d61f27a116439e8
Gerrit-PatchSet: 5
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
