objectcache: Use Psr\Log\LoggerInterface instead of wfDebug

ObjectCache::newFromParams() will handle a 'loggroup' parameter
specially, getting an instance of \Psr\Log\LoggerInterface to
pass to the BagsOStuff.

BagOStuff implementations can use $this->logger which will be
an implementation of \Psr\Log\LoggerInterface. As this is set in
BagOStuff::__construct(), all subclasses must now call the
parent constructor.

The goal of this is to make the logging in BagOStuff non-MediaWiki
specific, in the hopes of separating it out into a separate library
in the future.

Change-Id: I8a8e278e6f028814499d8457d6d5341d03eabc7a

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 83870c2..0b37b97 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2111,17 +2111,17 @@ $wgLanguageConverterCacheType = CACHE_ANYTHING;
  */
 $wgObjectCaches = array(
        CACHE_NONE => array( 'class' => 'EmptyBagOStuff' ),
-       CACHE_DB => array( 'class' => 'SqlBagOStuff' ),
+       CACHE_DB => array( 'class' => 'SqlBagOStuff', 'loggroup' => 'SQLBagOStuff' ),
 
        CACHE_ANYTHING => array( 'factory' => 'ObjectCache::newAnything' ),
        CACHE_ACCEL => array( 'factory' => 'ObjectCache::newAccelerator' ),
-       CACHE_MEMCACHED => array( 'factory' => 'ObjectCache::newMemcached' ),
+       CACHE_MEMCACHED => array( 'factory' => 'ObjectCache::newMemcached', 'loggroup' => 'memcached' ),
 
        'apc' => array( 'class' => 'APCBagOStuff' ),
        'xcache' => array( 'class' => 'XCacheBagOStuff' ),
        'wincache' => array( 'class' => 'WinCacheBagOStuff' ),
-       'memcached-php' => array( 'class' => 'MemcachedPhpBagOStuff' ),
-       'memcached-pecl' => array( 'class' => 'MemcachedPeclBagOStuff' ),
+       'memcached-php' => array( 'class' => 'MemcachedPhpBagOStuff', 'loggroup' => 'memcached' ),
+       'memcached-pecl' => array( 'class' => 'MemcachedPeclBagOStuff', 'loggroup' => 'memcached' ),
        'hash' => array( 'class' => 'HashBagOStuff' ),
 );
 

diff --git a/includes/objectcache/BagOStuff.php b/includes/objectcache/BagOStuff.php
index 85b0b62..ae49564 100644 (file)
--- a/includes/objectcache/BagOStuff.php
+++ b/includes/objectcache/BagOStuff.php
@@ -28,6 +28,10 @@
  * @defgroup Cache Cache
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
  * interface is intended to be more or less compatible with
  * the PHP memcached client.
@@ -40,17 +44,38 @@
  *
  * @ingroup Cache
  */
-abstract class BagOStuff {
+abstract class BagOStuff implements LoggerAwareInterface {
        private $debugMode = false;
 
        protected $lastError = self::ERR_NONE;
 
+       /**
+        * @var LoggerInterface
+        */
+       protected $logger;
+
        /** Possible values for getLastError() */
        const ERR_NONE = 0; // no error
        const ERR_NO_RESPONSE = 1; // no response
        const ERR_UNREACHABLE = 2; // can't connect
        const ERR_UNEXPECTED = 3; // response gave some error
 
+       public function __construct( array $params = array() ) {
+               if ( isset( $params['logger'] ) ) {
+                       $this->setLogger( $params['logger'] );
+               } else {
+                       $this->setLogger( new NullLogger() );
+               }
+       }
+
+       /**
+        * @param LoggerInterface $logger
+        * @return null
+        */
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
+
        /**
         * @param bool $bool
         */
@@ -358,8 +383,9 @@ abstract class BagOStuff {
         */
        public function debug( $text ) {
                if ( $this->debugMode ) {
-                       $class = get_class( $this );
-                       wfDebug( "$class debug: $text\n" );
+                       $this->logger->debug( "{class} debug: $text", array(
+                               'class' => get_class( $this ),
+                       ) );
                }
        }
 

diff --git a/includes/objectcache/HashBagOStuff.php b/includes/objectcache/HashBagOStuff.php
index 06a0865..6da1882 100644 (file)
--- a/includes/objectcache/HashBagOStuff.php
+++ b/includes/objectcache/HashBagOStuff.php
@@ -31,7 +31,8 @@ class HashBagOStuff extends BagOStuff {
        /** @var array */
        protected $bag;
 
-       function __construct() {
+       function __construct( $params = array() ) {
+               parent::__construct( $params );
                $this->bag = array();
        }
 

diff --git a/includes/objectcache/MemcachedBagOStuff.php b/includes/objectcache/MemcachedBagOStuff.php
index 0e133a8..6a98a8a 100644 (file)
--- a/includes/objectcache/MemcachedBagOStuff.php
+++ b/includes/objectcache/MemcachedBagOStuff.php
@@ -174,6 +174,6 @@ class MemcachedBagOStuff extends BagOStuff {
         * @param string $text
         */
        protected function debugLog( $text ) {
-               wfDebugLog( 'memcached', $text );
+               $this->logger->debug( $text );
        }
 }

diff --git a/includes/objectcache/MemcachedClient.php b/includes/objectcache/MemcachedClient.php
index 1e04d45..bc4a00b 100644 (file)
--- a/includes/objectcache/MemcachedClient.php
+++ b/includes/objectcache/MemcachedClient.php
@@ -64,6 +64,9 @@
  * @version 0.1.2
  */
 
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 // {{{ requirements
 // }}}
 
@@ -233,6 +236,11 @@ class MWMemcached {
         */
        public $_connect_attempts;
 
+       /**
+        * @var LoggerInterface
+        */
+       private $_logger;
+
        // }}}
        // }}}
        // {{{ methods
@@ -263,6 +271,8 @@ class MWMemcached {
 
                $this->_connect_timeout = isset( $args['connect_timeout'] ) ? $args['connect_timeout'] : 0.1;
                $this->_connect_attempts = 2;
+
+               $this->_logger = isset( $args['logger'] ) ? $args['logger'] : new NullLogger();
        }
 
        // }}}
@@ -1104,14 +1114,14 @@ class MWMemcached {
         * @param string $text
         */
        function _debugprint( $text ) {
-               wfDebugLog( 'memcached', $text );
+               $this->_logger->debug( $text );
        }
 
        /**
         * @param string $text
         */
        function _error_log( $text ) {
-               wfDebugLog( 'memcached-serious', "Memcached error: $text" );
+               $this->_logger->error( "Memcached error: $text" );
        }
 
        /**

diff --git a/includes/objectcache/MemcachedPeclBagOStuff.php b/includes/objectcache/MemcachedPeclBagOStuff.php
index 7c0a645..4ead417 100644 (file)
--- a/includes/objectcache/MemcachedPeclBagOStuff.php
+++ b/includes/objectcache/MemcachedPeclBagOStuff.php
@@ -46,6 +46,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
         * @throws MWException
         */
        function __construct( $params ) {
+               parent::__construct( $params );
                $params = $this->applyDefaultParams( $params );
 
                if ( $params['persistent'] ) {
@@ -54,7 +55,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
                        // We can only reuse a pool ID if we keep the config consistent.
                        $this->client = new Memcached( md5( serialize( $params ) ) );
                        if ( count( $this->client->getServerList() ) ) {
-                               wfDebug( __METHOD__ . ": persistent Memcached object already loaded.\n" );
+                               $this->logger->debug( __METHOD__ . ": persistent Memcached object already loaded." );
                                return; // already initialized; don't add duplicate servers
                        }
                } else {
@@ -223,14 +224,16 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
                                break;
                        default:
                                $msg = $this->client->getResultMessage();
+                               $logCtx = array();
                                if ( $key !== false ) {
                                        $server = $this->client->getServerByKey( $key );
-                                       $serverName = "{$server['host']}:{$server['port']}";
-                                       $msg = "Memcached error for key \"$key\" on server \"$serverName\": $msg";
+                                       $logCtx['memcached-server'] = "{$server['host']}:{$server['port']}";
+                                       $logCtx['memcached-key'] = $key;
+                                       $msg = "Memcached error for key \"{memcached-key}\" on server \"{memcached-server}\": $msg";
                                } else {
                                        $msg = "Memcached error: $msg";
                                }
-                               wfDebugLog( 'memcached-serious', $msg );
+                               $this->logger->error( $msg, $logCtx );
                                $this->setLastError( BagOStuff::ERR_UNEXPECTED );
                }
                return $result;

diff --git a/includes/objectcache/MemcachedPhpBagOStuff.php b/includes/objectcache/MemcachedPhpBagOStuff.php
index 939a715..6fba61b 100644 (file)
--- a/includes/objectcache/MemcachedPhpBagOStuff.php
+++ b/includes/objectcache/MemcachedPhpBagOStuff.php
@@ -42,6 +42,7 @@ class MemcachedPhpBagOStuff extends MemcachedBagOStuff {
         * @param array $params
         */
        function __construct( $params ) {
+               parent::__construct( $params );
                $params = $this->applyDefaultParams( $params );
 
                $this->client = new MemCachedClientforWiki( $params );

diff --git a/includes/objectcache/MultiWriteBagOStuff.php b/includes/objectcache/MultiWriteBagOStuff.php
index 04ed894..dbaccd6 100644 (file)
--- a/includes/objectcache/MultiWriteBagOStuff.php
+++ b/includes/objectcache/MultiWriteBagOStuff.php
@@ -43,6 +43,7 @@ class MultiWriteBagOStuff extends BagOStuff {
         * @throws MWException
         */
        public function __construct( $params ) {
+               parent::__construct( $params );
                if ( !isset( $params['caches'] ) ) {
                        throw new MWException( __METHOD__ . ': the caches parameter is required' );
                }

diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php
index 62856f9..1f2c9c0 100644 (file)
--- a/includes/objectcache/ObjectCache.php
+++ b/includes/objectcache/ObjectCache.php
@@ -81,6 +81,13 @@ class ObjectCache {
         * @return BagOStuff
         */
        static function newFromParams( $params ) {
+               if ( isset( $params['loggroup'] ) ) {
+                       $params['logger'] = MWLoggerFactory::getInstance( $params['loggroup'] );
+               } else {
+                       // For backwards-compatability with custom parameters, lets not
+                       // have all logging suddenly disappear
+                       $params['logger'] = MWLoggerFactory::getInstance( 'objectcache' );
+               }
                if ( isset( $params['factory'] ) ) {
                        return call_user_func( $params['factory'], $params );
                } elseif ( isset( $params['class'] ) ) {

diff --git a/includes/objectcache/RedisBagOStuff.php b/includes/objectcache/RedisBagOStuff.php
index 6836f74..fef7155 100644 (file)
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -56,6 +56,7 @@ class RedisBagOStuff extends BagOStuff {
         * @param array $params
         */
        function __construct( $params ) {
+               parent::__construct( $params );
                $redisConf = array( 'serializer' => 'none' ); // manage that in this class
                foreach ( array( 'connectTimeout', 'persistent', 'password' ) as $opt ) {
                        if ( isset( $params[$opt] ) ) {
@@ -363,7 +364,7 @@ class RedisBagOStuff extends BagOStuff {
         * @param string $msg
         */
        protected function logError( $msg ) {
-               wfDebugLog( 'redis', "Redis error: $msg" );
+               $this->logger->error( "Redis error: $msg" );
        }
 
        /**

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index 7524240..2a92bee 100644 (file)
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -88,6 +88,7 @@ class SqlBagOStuff extends BagOStuff {
         * @param array $params
         */
        public function __construct( $params ) {
+               parent::__construct( $params );
                if ( isset( $params['servers'] ) ) {
                        $this->serverInfos = $params['servers'];
                        $this->numServers = count( $this->serverInfos );
@@ -138,12 +139,12 @@ class SqlBagOStuff extends BagOStuff {
                        # If server connection info was given, use that
                        if ( $this->serverInfos ) {
                                if ( $wgDebugDBTransactions ) {
-                                       wfDebug( "Using provided serverInfo for SqlBagOStuff\n" );
+                                       $this->logger->debug( "Using provided serverInfo for SqlBagOStuff" );
                                }
                                $info = $this->serverInfos[$serverIndex];
                                $type = isset( $info['type'] ) ? $info['type'] : 'mysql';
                                $host = isset( $info['host'] ) ? $info['host'] : '[unknown]';
-                               wfDebug( __CLASS__ . ": connecting to $host\n" );
+                               $this->logger->debug( __CLASS__ . ": connecting to $host" );
                                $db = DatabaseBase::factory( $type, $info );
                                $db->clearFlag( DBO_TRX );
                        } else {
@@ -161,7 +162,7 @@ class SqlBagOStuff extends BagOStuff {
                                }
                        }
                        if ( $wgDebugDBTransactions ) {
-                               wfDebug( sprintf( "Connection %s will be used for SqlBagOStuff\n", $db ) );
+                               $this->logger->debug( sprintf( "Connection %s will be used for SqlBagOStuff", $db ) );
                        }
                        $this->conns[$serverIndex] = $db;
                }
@@ -702,13 +703,13 @@ class SqlBagOStuff extends BagOStuff {
                if ( $exception instanceof DBConnectionError ) {
                        $this->markServerDown( $exception, $serverIndex );
                }
-               wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" );
+               $this->logger->error( "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
                        $this->setLastError( BagOStuff::ERR_UNREACHABLE );
-                       wfDebug( __METHOD__ . ": ignoring connection error\n" );
+                       $this->logger->debug( __METHOD__ . ": ignoring connection error" );
                } else {
                        $this->setLastError( BagOStuff::ERR_UNEXPECTED );
-                       wfDebug( __METHOD__ . ": ignoring query error\n" );
+                       $this->logger->debug( __METHOD__ . ": ignoring query error" );
                }
        }
 
@@ -728,13 +729,14 @@ class SqlBagOStuff extends BagOStuff {
                        } catch ( DBError $e ) {
                        }
                }
-               wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" );
+
+               $this->logger->error( "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
                        $this->setLastError( BagOStuff::ERR_UNREACHABLE );
-                       wfDebug( __METHOD__ . ": ignoring connection error\n" );
+                       $this->logger->debug( __METHOD__ . ": ignoring connection error" );
                } else {
                        $this->setLastError( BagOStuff::ERR_UNEXPECTED );
-                       wfDebug( __METHOD__ . ": ignoring query error\n" );
+                       $this->logger->debug( __METHOD__ . ": ignoring query error" );
                }
        }
 
@@ -750,12 +752,12 @@ class SqlBagOStuff extends BagOStuff {
                                unset( $this->connFailureTimes[$serverIndex] );
                                unset( $this->connFailureErrors[$serverIndex] );
                        } else {
-                               wfDebug( __METHOD__ . ": Server #$serverIndex already down\n" );
+                               $this->logger->debug( __METHOD__ . ": Server #$serverIndex already down" );
                                return;
                        }
                }
                $now = time();
-               wfDebug( __METHOD__ . ": Server #$serverIndex down until " . ( $now + 60 ) . "\n" );
+               $this->logger->info( __METHOD__ . ": Server #$serverIndex down until " . ( $now + 60 ) );
                $this->connFailureTimes[$serverIndex] = $now;
                $this->connFailureErrors[$serverIndex] = $exception;
        }