rdbms: Add a deprecationLogger callback
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 5 Apr 2018 16:13:08 +0000 (12:13 -0400)
committerAaron Schulz <aschulz@wikimedia.org>
Thu, 5 Apr 2018 22:25:08 +0000 (22:25 +0000)
Much like the existing errorLogger, but for logging deprecation
warnings.

The default in the RDBMS layer is to call trigger_error() with
E_USER_DEPRECATED. The default in MediaWiki (via MWLBFactory) is to log
to the 'deprecated' log group, much like wfDeprecated() does, although
unfortunately we can't effectively use that directly since we have no
idea of a proper $callerOffset to pass.

Change-Id: Id13625e249516e84d72b6310953bb338a90976da

includes/db/MWLBFactory.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/lbfactory/ILBFactory.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
tests/phpunit/includes/Storage/RevisionStoreDbTest.php

index f0a17f7..82d9c1d 100644 (file)
@@ -31,6 +31,10 @@ use Wikimedia\Rdbms\DatabaseDomain;
  * @ingroup Database
  */
 abstract class MWLBFactory {
+
+       /** @var array Cache of already-logged deprecation messages */
+       private static $loggedDeprecations = [];
+
        /**
         * @param array $lbConf Config for LBFactory::__construct()
         * @param Config $mainConfig Main config object from MediaWikiServices
@@ -57,6 +61,7 @@ abstract class MWLBFactory {
                        'connLogger' => LoggerFactory::getInstance( 'DBConnection' ),
                        'perfLogger' => LoggerFactory::getInstance( 'DBPerformance' ),
                        'errorLogger' => [ MWExceptionHandler::class, 'logException' ],
+                       'deprecationLogger' => [ static::class, 'logDeprecation' ],
                        'cliMode' => $wgCommandLineMode,
                        'hostname' => wfHostname(),
                        'readOnlyReason' => $readOnlyMode->getReason(),
@@ -228,4 +233,22 @@ abstract class MWLBFactory {
                        ] );
                }
        }
+
+       /**
+        * Log a database deprecation warning
+        * @param string $msg Deprecation message
+        */
+       public static function logDeprecation( $msg ) {
+               global $wgDevelopmentWarnings;
+
+               if ( isset( self::$loggedDeprecations[$msg] ) ) {
+                       return;
+               }
+               self::$loggedDeprecations[$msg] = true;
+
+               if ( $wgDevelopmentWarnings ) {
+                       trigger_error( $msg, E_USER_DEPRECATED );
+               }
+               wfDebugLog( 'deprecated', $msg, 'private' );
+       }
 }
index 2ef8822..5336b25 100644 (file)
@@ -101,6 +101,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected $queryLogger;
        /** @var callback Error logging callback */
        protected $errorLogger;
+       /** @var callback Deprecation logging callback */
+       protected $deprecationLogger;
 
        /** @var resource|null Database connection */
        protected $conn = null;
@@ -312,6 +314,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->connLogger = $params['connLogger'];
                $this->queryLogger = $params['queryLogger'];
                $this->errorLogger = $params['errorLogger'];
+               $this->deprecationLogger = $params['deprecationLogger'];
 
                if ( isset( $params['nonNativeInsertSelectBatchSize'] ) ) {
                        $this->nonNativeInsertSelectBatchSize = $params['nonNativeInsertSelectBatchSize'];
@@ -396,6 +399,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *      includes the agent as a SQL comment.
         *   - trxProfiler: Optional TransactionProfiler instance.
         *   - errorLogger: Optional callback that takes an Exception and logs it.
+        *   - deprecationLogger: Optional callback that takes a string and logs it.
         *   - cliMode: Whether to consider the execution context that of a CLI script.
         *   - agent: Optional name used to identify the end-user in query profiling/logging.
         *   - srvCache: Optional BagOStuff instance to an APC-style cache.
@@ -437,6 +441,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
                                };
                        }
+                       if ( !isset( $p['deprecationLogger'] ) ) {
+                               $p['deprecationLogger'] = function ( $msg ) {
+                                       trigger_error( $msg, E_USER_DEPRECATED );
+                               };
+                       }
 
                        /** @var Database $conn */
                        $conn = new $class( $p );
index 32d9008..1e8838e 100644 (file)
@@ -55,6 +55,7 @@ interface ILBFactory {
         *  - queryLogger: PSR-3 logger instance. [optional]
         *  - perfLogger: PSR-3 logger instance. [optional]
         *  - errorLogger: Callback that takes an Exception and logs it. [optional]
+        *  - deprecationLogger: Callback to log a deprecation warning. [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $conf );
index bc428ec..b1ea810 100644 (file)
@@ -52,6 +52,8 @@ abstract class LBFactory implements ILBFactory {
        protected $perfLogger;
        /** @var callable Error logger */
        protected $errorLogger;
+       /** @var callable Deprecation logger */
+       protected $deprecationLogger;
        /** @var BagOStuff */
        protected $srvCache;
        /** @var BagOStuff */
@@ -109,7 +111,12 @@ abstract class LBFactory implements ILBFactory {
                $this->errorLogger = isset( $conf['errorLogger'] )
                        ? $conf['errorLogger']
                        : function ( Exception $e ) {
-                               trigger_error( E_USER_WARNING, get_class( $e ) . ': ' . $e->getMessage() );
+                               trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
+                       };
+               $this->deprecationLogger = isset( $conf['deprecationLogger'] )
+                       ? $conf['deprecationLogger']
+                       : function ( $msg ) {
+                               trigger_error( $msg, E_USER_DEPRECATED );
                        };
 
                $this->profiler = isset( $conf['profiler'] ) ? $conf['profiler'] : null;
@@ -514,6 +521,7 @@ abstract class LBFactory implements ILBFactory {
                        'connLogger' => $this->connLogger,
                        'replLogger' => $this->replLogger,
                        'errorLogger' => $this->errorLogger,
+                       'deprecationLogger' => $this->deprecationLogger,
                        'hostname' => $this->hostname,
                        'cliMode' => $this->cliMode,
                        'agent' => $this->agent,
index 767cc49..715f4e4 100644 (file)
@@ -109,6 +109,7 @@ interface ILoadBalancer {
         *  - queryLogger: PSR-3 logger instance. [optional]
         *  - perfLogger: PSR-3 logger instance. [optional]
         *  - errorLogger : Callback that takes an Exception and logs it. [optional]
+        *  - deprecationLogger: Callback to log a deprecation warning. [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $params );
index 7c1b9d9..db2ab1f 100644 (file)
@@ -111,6 +111,8 @@ class LoadBalancer implements ILoadBalancer {
 
        /** @var callable Exception logger */
        private $errorLogger;
+       /** @var callable Deprecation logger */
+       private $deprecationLogger;
 
        /** @var bool */
        private $disabled = false;
@@ -223,6 +225,11 @@ class LoadBalancer implements ILoadBalancer {
                        : function ( Exception $e ) {
                                trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
                        };
+               $this->deprecationLogger = isset( $params['deprecationLogger'] )
+                       ? $params['deprecationLogger']
+                       : function ( $msg ) {
+                               trigger_error( $msg, E_USER_DEPRECATED );
+                       };
 
                foreach ( [ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ] as $key ) {
                        $this->$key = isset( $params[$key] ) ? $params[$key] : new NullLogger();
@@ -1067,6 +1074,7 @@ class LoadBalancer implements ILoadBalancer {
                $server['connLogger'] = $this->connLogger;
                $server['queryLogger'] = $this->queryLogger;
                $server['errorLogger'] = $this->errorLogger;
+               $server['deprecationLogger'] = $this->deprecationLogger;
                $server['profiler'] = $this->profiler;
                $server['trxProfiler'] = $this->trxProfiler;
                // Use the same agent and PHP mode for all DB handles
index 719a3bf..7d6906c 100644 (file)
@@ -117,7 +117,10 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                                'trxProfiler' => new TransactionProfiler(),
                                'connLogger' => new \Psr\Log\NullLogger(),
                                'queryLogger' => new \Psr\Log\NullLogger(),
-                               'errorLogger' => new \Psr\Log\NullLogger(),
+                               'errorLogger' => function () {
+                               },
+                               'deprecationLogger' => function () {
+                               },
                                'type' => 'test',
                                'dbname' => $dbName,
                                'tablePrefix' => $dbPrefix,