Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

objectcache: Inject Config object to ObjectCache::newFromParams

Browse source 
* Avoid direct $GLOBALS lookups.
* Avoid MediaWikiServices singleton for Config object.

Also given that newFromParams() constructs its Logger object
unconditionally as of I2aa835c5ec79, stop creating it ahead of
time in ServiceWiring.

This code, including the default loggroup value, was duplicated by
me from ObjectCache.php to ServiceWiring.php in commits 3828558140
and bd16c5eb34 because I needed to obtain the Logger object
in order to send a message to it about how it was created.

Solve this debt by letting ServiceWiring access the actual logger
that was created by newFromParams() through a new getLogger()
object.

Change-Id: Ib2de7b22f6c79db0c700fae06269d04fbe27831d
This commit is contained in:
Timo Tijhof 2020-03-02 14:56:08 +00:00 committed by Krinkle
parent ab79c3861e
commit 8d4aaa43cb
3 changed files with 35 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
includes/ServiceWiring.php
View file

@ -470,7 +470,7 @@ return [
$config = $services->getMainConfig();
$cacheId = ObjectCache::detectLocalServerCache();
return ObjectCache::newFromParams( $config->get( 'ObjectCaches' )[$cacheId] );
return ObjectCache::newFromParams( $config->get( 'ObjectCaches' )[$cacheId], $config );
},
'LockManagerGroupFactory' => function ( MediaWikiServices $services ) : LockManagerGroupFactory {
@ -500,10 +500,9 @@ return [
}
$params = $mainConfig->get( 'ObjectCaches' )[$id];
$logger = $params['logger'] = LoggerFactory::getInstance( $params['loggroup'] ?? 'objectcache' );
$store = ObjectCache::newFromParams( $params );
$logger->debug( 'MainObjectStash using store {class}', [
$store = ObjectCache::newFromParams( $params, $mainConfig );
$store->getLogger()->debug( 'MainObjectStash using store {class}', [
'class' => get_class( $store )
] );
@ -521,15 +520,14 @@ return [
$params = $mainConfig->get( 'WANObjectCaches' )[$id];
$logger = LoggerFactory::getInstance( $params['loggroup'] ?? 'objectcache' );
$objectCacheId = $params['cacheId'];
if ( !isset( $mainConfig->get( 'ObjectCaches' )[$objectCacheId] ) ) {
throw new UnexpectedValueException(
"Cache type \"$objectCacheId\" is not present in \$wgObjectCaches." );
}
$storeParams = $mainConfig->get( 'ObjectCaches' )[$objectCacheId];
$store = ObjectCache::newFromParams( $storeParams );
$store = ObjectCache::newFromParams( $storeParams, $mainConfig );
$logger = $store->getLogger();
$logger->debug( 'MainWANObjectCache using store {class}', [
'class' => get_class( $store )
] );

8
includes/libs/objectcache/BagOStuff.php
View file

@ -113,6 +113,14 @@ abstract class BagOStuff implements
$this->logger = $logger;
}
/**
* @since 1.35
* @return LoggerInterface
*/
public function getLogger() : LoggerInterface {
return $this->logger;
}
/**
* @param bool $enabled
*/

28
includes/objectcache/ObjectCache.php
View file

@ -136,16 +136,17 @@ class ObjectCache {
* - class: BagOStuff subclass constructed with $params.
* - loggroup: Alias to set 'logger' key with LoggerFactory group.
* - .. Other parameters passed to factory or class.
* @param Config|null $conf (Since 1.35)
* @return BagOStuff
* @throws InvalidArgumentException
*/
public static function newFromParams( $params ) {
public static function newFromParams( array $params, Config $conf = null ) {
// Apply default parameters and resolve the logger instance
$params += [
'logger' => LoggerFactory::getInstance( $params['loggroup'] ?? 'objectcache' ),
'keyspace' => self::getDefaultKeyspace(),
'asyncHandler' => [ DeferredUpdates::class, 'addCallableUpdate' ],
'reportDupes' => true
'reportDupes' => true,
];
if ( isset( $params['factory'] ) ) {
@ -159,6 +160,21 @@ class ObjectCache {
}
$class = $params['class'];
// Not passing $conf is deprecated since 1.35
// NOTE: We cannot use MediaWikiServices::getMainConfig here as fallback,
// because ObjectCache::newFromParams is used for service wiring and
// in the ExtensionRegistry, and must not itself cause MediaWikiServices
// to be initialised. In particular, doing so would break the
// GlobalPreferences extension, which is overriding a service and then
// hard-requiring their additional methods to exist on the service object
// (T210449, T238466).
// TODO: Hard-deprecate before 1.35 gets cut, and remove in 1.36.
$conf = $conf ?? new HashConfig( [
'SQLiteDataDir' => $GLOBALS['wgSQLiteDataDir'],
'MemCachedServers' => $GLOBALS['wgMemCachedServers'],
'MemCachedPersistent' => $GLOBALS['wgMemCachedPersistent'],
'MemCachedTimeout' => $GLOBALS['wgMemCachedTimeout'],
] );
// Do b/c logic for SqlBagOStuff
if ( is_a( $class, SqlBagOStuff::class, true ) ) {
@ -170,7 +186,7 @@ class ObjectCache {
if ( isset( $params['servers'] ) ) {
foreach ( $params['servers'] as &$server ) {
if ( $server['type'] === 'sqlite' && !isset( $server['dbDirectory'] ) ) {
$server['dbDirectory'] = $GLOBALS['wgSQLiteDataDir'];
$server['dbDirectory'] = $conf->get( 'SQLiteDataDir' );
}
}
}
@ -179,9 +195,9 @@ class ObjectCache {
// Do b/c logic for MemcachedBagOStuff
if ( is_subclass_of( $class, MemcachedBagOStuff::class ) ) {
$params += [
'servers' => $GLOBALS['wgMemCachedServers'],
'persistent' => $GLOBALS['wgMemCachedPersistent'],
'timeout' => $GLOBALS['wgMemCachedTimeout']
'servers' => $conf->get( 'MemCachedServers' ),
'persistent' => $conf->get( 'MemCachedPersistent' ),
'timeout' => $conf->get( 'MemCachedTimeout' ),
];
}