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

Refactor SettingsBuilder to use BagOStuff.

Browse source 
Removed use of Psr\SimpleCache\CacheInterface in favor of BagOStuff, as
the latter is a tried-and-true abstraction and the former offers no real
upfront benefits since the caching patterns of SettingsBuilder are quite
basic at this time.

The simplicity of cache interface use is largely in part to the minimal
probabilistic stampede protection implementation within CachedSource
which is left untouched by this change.

Bug: T294748
Change-Id: Ie59b37a8d5c7bf96225757fa9eb9d2c762476713
This commit is contained in:
Dan Duvall 2021-12-10 10:20:05 -08:00
parent 8190a51c43
commit 9778adf1c5
12 changed files with 54 additions and 557 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
composer.json
View file

@ -44,7 +44,6 @@
"php": ">=7.2.22",
"psr/container": "1.1.1",
"psr/log": "1.1.4",
"psr/simple-cache": "1.0.1",
"ralouphie/getallheaders": "3.0.3",
"symfony/polyfill-php80": "1.23.1",
"symfony/yaml": "5.3.6",

65
includes/Settings/Cache/ArrayCache.php
View file

@ -1,65 +0,0 @@
<?php
namespace MediaWiki\Settings\Cache;
use Psr\SimpleCache\CacheInterface;
/**
* A PSR-16 compliant array based cache used for testing.
*
* @since 1.38
*/
class ArrayCache implements CacheInterface {
/** @var array */
private $store = [];
public function clear(): bool {
$this->store = [];
return true;
}
public function delete( $key ): bool {
unset( $this->store[$key] );
return true;
}
public function deleteMultiple( $keys ): bool {
foreach ( $keys as $key ) {
$this->delete( $key );
}
return true;
}
public function get( $key, $default = null ) {
if ( !array_key_exists( $key, $this->store ) ) {
return $default;
}
return $this->store[$key];
}
public function getMultiple( $keys, $default = null ) {
$results = [];
foreach ( $keys as $key ) {
$results[$key] = $this->get( $key, $default );
}
return $results;
}
public function has( $key ): bool {
return array_key_exists( $key, $this->store );
}
public function set( $key, $value, $_ttl = null ): bool {
$this->store[$key] = $value;
return true;
}
public function setMultiple( $values, $ttl = null ): bool {
foreach ( $values as $key => $value ) {
$this->set( $key, $value, $ttl );
}
return true;
}
}

12
includes/Settings/Cache/CacheArgumentException.php
View file

@ -1,12 +0,0 @@
<?php
namespace MediaWiki\Settings\Cache;
use MediaWiki\Settings\SettingsBuilderException;
use Psr\SimpleCache\InvalidArgumentException;
class CacheArgumentException
extends SettingsBuilderException
implements InvalidArgumentException
{
}

17
includes/Settings/Cache/CachedSource.php
View file

@ -2,8 +2,8 @@
namespace MediaWiki\Settings\Cache;
use BagOStuff;
use MediaWiki\Settings\Source\SettingsSource;
use Psr\SimpleCache\CacheInterface;
/**
* Provides a caching layer for a {@link CacheableSource}.
@ -12,7 +12,7 @@ use Psr\SimpleCache\CacheInterface;
* @todo mark as stable before the 1.38 release
*/
class CachedSource implements SettingsSource {
/** @var CacheInterface */
/** @var BagOStuff */
private $cache;
/** @var CacheableSource */
@ -22,11 +22,11 @@ class CachedSource implements SettingsSource {
* Constructs a new CachedSource using an instantiated cache and
* {@link CacheableSource}.
*
* @param CacheInterface $cache
* @param BagOStuff $cache
* @param CacheableSource $source
*/
public function __construct(
CacheInterface $cache,
BagOStuff $cache,
CacheableSource $source
) {
$this->cache = $cache;
@ -40,11 +40,14 @@ class CachedSource implements SettingsSource {
* @return array
*/
public function load(): array {
$key = $this->source->getHashKey();
$item = $this->cache->get( $key, null );
$key = $this->cache->makeGlobalKey(
__CLASS__,
$this->source->getHashKey()
);
$item = $this->cache->get( $key );
$miss =
$item === null ||
$item === false ||
$this->expiresEarly( $item, $this->source->getExpiryWeight() );
if ( $miss ) {

274
includes/Settings/Cache/SharedMemoryCache.php
View file

@ -1,274 +0,0 @@
<?php
namespace MediaWiki\Settings\Cache;
use APCuIterator;
use DateInterval;
use DateTime;
use MediaWiki\Settings\SettingsBuilderException;
use Psr\SimpleCache\CacheInterface;
use Traversable;
/**
* A PSR-16 compliant APCu based shared memory settings cache.
*
* @since 1.38
*/
class SharedMemoryCache implements CacheInterface {
private const VERSION = '0';
/** @var string */
private $namespace;
/** @var int */
private $ttl;
/**
* Checks whether APCu is available.
*
* @return bool
*/
public static function isSupported() {
return function_exists( 'apcu_fetch' )
&& filter_var( ini_get( 'apc.enabled' ), FILTER_VALIDATE_BOOLEAN );
}
/**
* @param string $namespace APCu namespace.
* @param int $ttl Cached settings TTL in seconds.
*
* @throws SettingsBuilderException
*/
public function __construct(
string $namespace = 'mediawiki.settings',
int $ttl = 60 * 60 * 24
) {
$this->namespace = $namespace;
$this->ttl = $ttl;
if ( !static::isSupported() ) {
throw new SettingsBuilderException( 'APCu is not enabled' );
}
}
/**
* Clear all items from this cache.
*
* @return bool True on success, false on failure.
*/
public function clear(): bool {
return apcu_delete(
new APCuIterator(
sprintf( '/^%s/', preg_quote( $this->qualifyKey( '' ) ) ),
APC_ITER_KEY
)
);
}
/**
* Delete an item from the cache.
*
* @param string $key
*
* @return bool True if removed, false if not.
*
* @throws CacheArgumentException
*/
public function delete( $key ): bool {
return $this->deleteMultiple( [ $key ] );
}
/**
* Deletes multiple cache items.
*
* @param iterable $keys
*
* @return bool True if all were removed, false if not.
*
* @throws CacheArgumentException
*/
public function deleteMultiple( $keys ): bool {
if ( !( is_array( $keys ) || $keys instanceof Traversable ) ) {
throw new CacheArgumentException(
'given cache $keys is not an iterable'
);
}
$apcuKeys = [];
foreach ( $keys as $key ) {
$this->assertValidKey( $key );
$apcuKeys[] = $this->qualifyKey( $key );
}
$deleted = apcu_delete( $apcuKeys );
if ( is_array( $deleted ) ) {
return count( $deleted ) === count( $keys );
}
return $deleted;
}
/**
* Fetches a value from the cache.
*
* @param string $key
* @param mixed $default
*
* @return mixed
*
* @throws CacheArgumentException
*/
public function get( $key, $default = null ) {
$result = $default;
foreach ( $this->getMultiple( [ $key ], $default ) as $value ) {
$result = $value;
}
return $result;
}
/**
* Fetches values from the cache.
*
* @param iterable $keys
* @param mixed $default
*
* @return iterable
*
* @throws CacheArgumentException
*/
public function getMultiple( $keys, $default = null ) {
if ( !( is_array( $keys ) || $keys instanceof Traversable ) ) {
throw new CacheArgumentException(
'given cache $keys is not an iterable'
);
}
$results = [];
$keyMap = [];
foreach ( $keys as $key ) {
$this->assertValidKey( $key );
$keyMap[ $this->qualifyKey( $key ) ] = $key;
$results[$key] = $default;
}
$apcuResults = apcu_fetch( array_keys( $keyMap ), $ok ) ?: [];
if ( $ok ) {
foreach ( $apcuResults as $qualifiedKey => $value ) {
$results[ $keyMap[ $qualifiedKey ] ] = $value;
}
}
return $results;
}
/**
* Checks for the presence of a given item in the cache.
*
* @param string $key
*
* @return bool
*
* @throws CacheArgumentException
*/
public function has( $key ): bool {
$this->assertValidKey( $key );
return apcu_exists( $this->qualifyKey( $key ) );
}
/**
* Store a value in the cache.
*
* Note that while object values are possible given the underlying APCU
* serializer, caching of objects directly should be avoided due to
* incompatibilities in serialized representation from PHP version to PHP
* version and when simple differences in member scope are made (private
* to protected, etc.). Wherever possible, convert objects to arrays
* before storing.
*
* @param string $key
* @param mixed $value
* @param null|int|DateInterval $ttl Optional. By default, the $ttl
* argument passed to the constructor will be used.
*
* @return bool True on success, false on failure.
*
* @throws CacheArgumentException
*/
public function set( $key, $value, $ttl = null ): bool {
return $this->setMultiple( [ $key => $value ], $ttl );
}
/**
* Store multiple values in the cache.
*
* @param iterable $values An iterable of key => value pairs.
* @param null|int|DateInterval $ttl Optional. By default, the $ttl
* argument passed to the constructor will be used.
*
* @return bool True on success, false on failure.
*
* @throws CacheArgumentException
*/
public function setMultiple( $values, $ttl = null ): bool {
if ( $ttl === null ) {
$ttl = $this->ttl;
}
if ( !( is_array( $values ) || $values instanceof Traversable ) ) {
throw new CacheArgumentException(
'given cache $values is not an iterable'
);
}
// To be compliant with the PSR-16 interface, we must accept a
// DateInterval TTL. Convert it to an int.
if ( $ttl instanceof DateInterval ) {
$ttl = ( new DateTime( '@0' ) )->add( $ttl )->getTimestamp();
}
$apcuValues = [];
foreach ( $values as $key => $value ) {
$this->assertValidKey( $key );
$apcuValues[ $this->qualifyKey( $key ) ] = $value;
}
$failures = apcu_store( $apcuValues, null, $ttl );
return empty( $failures );
}
/**
* Fully qualifies a relative key, prefixing it with the internal
* namespace and cache implementation version.
*
* @param string $key Relative key value
*
* @return string
*/
private function qualifyKey( string $key ): string {
return $this->namespace . '@' . self::VERSION . ':' . $key;
}
/**
* @param string $key
*
* @throws CacheArgumentException
*/
private function assertValidKey( $key ) {
if ( !is_string( $key ) ) {
throw new CacheArgumentException( 'key must be a string' );
}
if ( empty( $key ) ) {
throw new CacheArgumentException( 'key cannot be an empty string' );
}
}
}

14
includes/Settings/SettingsBuilder.php
View file

@ -2,6 +2,7 @@
namespace MediaWiki\Settings;
use BagOStuff;
use ExtensionRegistry;
use JsonSchema\Constraints\Constraint;
use JsonSchema\Validator;
@ -13,7 +14,6 @@ use MediaWiki\Settings\Config\PhpIniSink;
use MediaWiki\Settings\Source\ArraySource;
use MediaWiki\Settings\Source\FileSource;
use MediaWiki\Settings\Source\SettingsSource;
use Psr\SimpleCache\CacheInterface;
use StatusValue;
/**
@ -27,7 +27,7 @@ class SettingsBuilder {
/** @var ExtensionRegistry */
private $extensionRegistry;
/** @var CacheInterface */
/** @var BagOStuff */
private $cache;
/** @var ConfigBuilder */
@ -54,17 +54,17 @@ class SettingsBuilder {
* @param ExtensionRegistry $extensionRegistry
* @param ConfigBuilder $configSink
* @param PhpIniSink $phpIniSink
* @param CacheInterface|null $cache PSR-16 compliant cache interface used
* to cache settings loaded from each source. The caller should beware
* that secrets contained in any source passed to {@link load} or {@link
* loadFile} will be cached as well.
* @param BagOStuff|null $cache BagOStuff used to cache settings loaded
* from each source. The caller should beware that secrets contained in
* any source passed to {@link load} or {@link loadFile} will be cached as
* well.
*/
public function __construct(
string $baseDir,
ExtensionRegistry $extensionRegistry,
ConfigBuilder $configSink,
PhpIniSink $phpIniSink,
CacheInterface $cache = null
BagOStuff $cache = null
) {
$this->baseDir = $baseDir;
$this->extensionRegistry = $extensionRegistry;

3
tests/common/TestsAutoLoader.php
View file

@ -268,9 +268,6 @@ $wgAutoloadClasses += [
'MediaWiki\Tests\Unit\Revision\RevisionSlotsTest' => "$testDir/phpunit/unit/includes/Revision/RevisionSlotsTest.php",
'MediaWiki\Tests\Unit\Revision\RevisionStoreRecordTest' => "$testDir/phpunit/unit/includes/Revision/RevisionStoreRecordTest.php",
# tests/phpunit/unit/includes/Settings/Cache
'MediaWiki\Tests\Unit\Settings\Cache\CacheInterfaceTestTrait' => "$testDir/phpunit/unit/includes/Settings/Cache/CacheInterfaceTestTrait.php",
# tests/phpunit/unit/includes/Settings/Config
'MediaWiki\Tests\Unit\Settings\Config\ConfigSinkTestTrait' => "$testDir/phpunit/unit/includes/Settings/Config/ConfigSinkTestTrait.php",

17
tests/phpunit/unit/includes/Settings/Cache/ArrayCacheTest.php
View file

@ -1,17 +0,0 @@
<?php
namespace MediaWiki\Tests\Unit\Settings\Cache;
use MediaWiki\Settings\Cache\ArrayCache;
use PHPUnit\Framework\TestCase;
/**
* @covers \MediaWiki\Settings\Cache\ArrayCache
*/
class ArrayCacheTest extends TestCase {
use CacheInterfaceTestTrait;
protected function newCache(): ArrayCache {
return new ArrayCache();
}
}

94
tests/phpunit/unit/includes/Settings/Cache/CacheInterfaceTestTrait.php
View file

@ -1,94 +0,0 @@
<?php
namespace MediaWiki\Tests\Unit\Settings\Cache;
use DateInterval;
use Psr\SimpleCache\CacheInterface;
trait CacheInterfaceTestTrait {
abstract protected function newCache(): CacheInterface;
public function testSetAndGet() {
$cache = $this->newCache();
$cache->set( 'one', 'fish1' );
$cache->set( 'two', 'fish2', 60 );
$cache->set( 'red', 'fish3', DateInterval::createFromDateString( '@60' ) );
$this->assertSame( 'fish1', $cache->get( 'one' ) );
$this->assertSame( 'fish2', $cache->get( 'two' ) );
$this->assertSame( 'fish3', $cache->get( 'red' ) );
$this->assertSame( 'fish4', $cache->get( 'blue', 'fish4' ) );
}
public function testSetAndGetMultiple() {
$cache = $this->newCache();
$cache->setMultiple(
[
'one' => 'fish1',
'two' => 'fish2',
'red' => 'fish3',
]
);
$this->assertSame(
[
'one' => 'fish1',
'two' => 'fish2',
'red' => 'fish3',
'blue' => 'fish4',
],
$cache->getMultiple( [ 'one', 'two', 'red', 'blue' ], 'fish4' )
);
}
public function testDeleteAndHas() {
$cache = $this->newCache();
$cache->set( 'red', 'fish' );
$cache->set( 'blue', 'fish' );
$cache->delete( 'red' );
$this->assertFalse( $cache->has( 'red' ) );
$this->assertTrue( $cache->has( 'blue' ) );
}
public function testDeleteMultiple() {
$cache = $this->newCache();
$cache->set( 'one', 'fish' );
$cache->set( 'two', 'fish' );
$cache->set( 'red', 'fish' );
$cache->set( 'blue', 'fish' );
$cache->deleteMultiple( [ 'two', 'red' ] );
$this->assertTrue( $cache->has( 'one' ) );
$this->assertFalse( $cache->has( 'two' ) );
$this->assertFalse( $cache->has( 'red' ) );
$this->assertTrue( $cache->has( 'blue' ) );
}
public function testClear() {
$cache = $this->newCache();
$cache->set( 'one', 'fish' );
$cache->set( 'two', 'fish' );
$cache->set( 'red', 'fish' );
$cache->set( 'blue', 'fish' );
$this->assertTrue( $cache->has( 'one' ) );
$this->assertTrue( $cache->has( 'two' ) );
$this->assertTrue( $cache->has( 'red' ) );
$this->assertTrue( $cache->has( 'blue' ) );
$cache->clear();
$this->assertFalse( $cache->has( 'one' ) );
$this->assertFalse( $cache->has( 'two' ) );
$this->assertFalse( $cache->has( 'red' ) );
$this->assertFalse( $cache->has( 'blue' ) );
}
}

32
tests/phpunit/unit/includes/Settings/Cache/CachedSourceTest.php
View file

@ -2,34 +2,41 @@
namespace MediaWiki\Tests\Unit\Settings\Cache;
use BagOStuff;
use MediaWiki\Settings\Cache\CacheableSource;
use MediaWiki\Settings\Cache\CachedSource;
use PHPUnit\Framework\TestCase;
use Psr\SimpleCache\CacheInterface;
/**
* @covers \MediaWiki\Settings\Cache\CachedSource
*/
class CachedSourceTest extends TestCase {
public function testLoadWithMiss() {
$cache = $this->createMock( CacheInterface::class );
$cache = $this->createMock( BagOStuff::class );
$source = $this->createMock( CacheableSource::class );
$cacheSource = new CachedSource( $cache, $source );
$settings = [ 'config' => [ 'Foo' => 'value' ] ];
$key = 'abc123';
$hashKey = 'abc123';
$key = 'global:MediaWiki\Tests\Unit\Settings\Cache\CachedSourceTest:' . $hashKey;
$ttl = 123;
$source
->expects( $this->once() )
->method( 'getHashKey' )
->willReturn( $hashKey );
$cache
->expects( $this->once() )
->method( 'makeGlobalKey' )
->with( 'MediaWiki\Settings\Cache\CachedSource', $hashKey )
->willReturn( $key );
$cache
->expects( $this->once() )
->method( 'get' )
->with( $key, null )
->willReturn( null );
->with( $key )
->willReturn( false );
$source
->expects( $this->atLeastOnce() )
@ -65,22 +72,29 @@ class CachedSourceTest extends TestCase {
}
public function testLoadWithHit() {
$cache = $this->createMock( CacheInterface::class );
$cache = $this->createMock( BagOStuff::class );
$source = $this->createMock( CacheableSource::class );
$cacheSource = new CachedSource( $cache, $source );
$settings = [ 'config' => [ 'Foo' => 'value' ] ];
$key = 'abc123';
$hashKey = 'abc123';
$key = 'global:MediaWiki\Tests\Unit\Settings\Cache\CachedSourceTest:' . $hashKey;
$source
->expects( $this->once() )
->method( 'getHashKey' )
->willReturn( 'abc123' );
->willReturn( $hashKey );
$cache
->expects( $this->once() )
->method( 'makeGlobalKey' )
->with( 'MediaWiki\Settings\Cache\CachedSource', $hashKey )
->willReturn( $key );
$cache
->expects( $this->once() )
->method( 'get' )
->with( $key, null )
->with( $key )
->willReturn(
[
'value' => $settings,

63
tests/phpunit/unit/includes/Settings/Cache/SharedMemoryCacheTest.php
View file

@ -1,63 +0,0 @@
<?php
namespace MediaWiki\Tests\Unit\Settings\Cache;
use MediaWiki\Settings\Cache\CacheArgumentException;
use MediaWiki\Settings\Cache\SharedMemoryCache;
use PHPUnit\Framework\TestCase;
/**
* @covers \MediaWiki\Settings\Cache\SharedMemoryCache
*/
class SharedMemoryCacheTest extends TestCase {
use CacheInterfaceTestTrait;
protected function newCache(): SharedMemoryCache {
return new SharedMemoryCache( 'phpunit.mediawiki.settings' );
}
public function setUp(): void {
if ( !SharedMemoryCache::isSupported() || !ini_get( 'apc.enable_cli' ) ) {
$this->markTestSkipped(
'skipped shared memory cache tests. set apc.enable_cli=on to run them'
);
}
}
public function tearDown(): void {
if ( SharedMemoryCache::isSupported() && ini_get( 'apc.enable_cli' ) ) {
$this->newCache()->clear();
}
}
public function testSetWithZeroLengthKey() {
$cache = $this->newCache();
$this->expectException( CacheArgumentException::class );
$cache->set( '', 'foo' );
}
public function testNamespacing() {
$cache1 = new SharedMemoryCache( 'phpunit.mediawiki.settings.one' );
$cache2 = new SharedMemoryCache( 'phpunit.mediawiki.settings.two' );
$cache1->set( 'one', 'fish' );
$cache1->set( 'two', 'fish' );
$cache2->set( 'red', 'fish' );
$cache2->set( 'blue', 'fish' );
try {
$cache1->clear();
$this->assertFalse( $cache1->has( 'one' ) );
$this->assertFalse( $cache1->has( 'two' ) );
$this->assertTrue( $cache2->has( 'red' ) );
$this->assertTrue( $cache2->has( 'blue' ) );
} finally {
$cache2->clear();
}
}
}

19
tests/phpunit/unit/includes/Settings/SettingsBuilderTest.php
View file

@ -2,6 +2,7 @@
namespace phpunit\unit\includes\Settings;
use BagOStuff;
use ExtensionRegistry;
use InvalidArgumentException;
use MediaWiki\Settings\Cache\CacheableSource;
@ -11,7 +12,6 @@ use MediaWiki\Settings\Config\PhpIniSink;
use MediaWiki\Settings\SettingsBuilder;
use MediaWiki\Settings\SettingsBuilderException;
use PHPUnit\Framework\TestCase;
use Psr\SimpleCache\CacheInterface;
/**
* @covers \MediaWiki\Settings\SettingsBuilder
@ -308,7 +308,7 @@ class SettingsBuilderTest extends TestCase {
public function testLoadsCacheableSource() {
$mockSource = $this->createMock( CacheableSource::class );
$mockCache = $this->createMock( CacheInterface::class );
$mockCache = $this->createMock( BagOStuff::class );
$configBuilder = new ArrayConfigBuilder();
$builder = $this
->newSettingsBuilder( [
@ -317,17 +317,26 @@ class SettingsBuilderTest extends TestCase {
] )
->load( $mockSource );
$hashKey = 'abc123';
$key = 'global:MediaWiki\Tests\Unit\Settings\Cache\CachedSourceTest:' . $hashKey;
// Mock a cache miss
$mockSource
->expects( $this->once() )
->method( 'getHashKey' )
->willReturn( 'abc123' );
->willReturn( $hashKey );
$mockCache
->expects( $this->once() )
->method( 'makeGlobalKey' )
->with( 'MediaWiki\Settings\Cache\CachedSource', $hashKey )
->willReturn( $key );
$mockCache
->expects( $this->once() )
->method( 'get' )
->with( 'abc123' )
->willReturn( null );
->with( $key )
->willReturn( false );
$mockSource
->expects( $this->once() )