From d83a7bcd09d1e291ac91e5b9c1edee6a75a23ef1 Mon Sep 17 00:00:00 2001 From: Dan Duvall Date: Fri, 12 Nov 2021 20:21:34 -0800 Subject: [PATCH] Cache loading of SettingsBuilder sources The `SettingsBuilder` now accepts a PSR-16 cache interface with which to store and query settings before attempting to load from each source. By default, no cache is used, but any object that implements the `Psr\SimpleCache\CacheInterface` may be provided to the constructor. An explicit dependency on "psr/simple-cache" has been added to `composer.json`. Note that this dependency already existed in vendor albeit it as a transitive one. An APCu based `SharedMemoryCache` adapter is provided as a canonical PSR-16 compliant interface for production use. Sources are now queued by the `SettingsBuilder` when calling `load()`. If a cache interface has been provided, and the source is considered cacheable (implements `CacheableSource`), then it is wrapped as a `CachedSource` which will query the cache first before loading from the wrapped source. Cache stampedes are mitigated using probabilistic early expiry. The implementation for this was partially based on symfony/cache-contract source code but also from the Wikipedia article and paper referenced therein. See https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration Bug: T294748 Change-Id: I52ab3899731546876ee58265bd4a1927886746dc --- composer.json | 1 + includes/Settings/Cache/ArrayCache.php | 65 +++++ .../Settings/Cache/CacheArgumentException.php | 12 + includes/Settings/Cache/CacheableSource.php | 52 ++++ includes/Settings/Cache/CachedSource.php | 123 ++++++++ includes/Settings/Cache/SharedMemoryCache.php | 274 ++++++++++++++++++ includes/Settings/SettingsBuilder.php | 24 +- includes/Settings/Source/ArraySource.php | 1 - includes/Settings/Source/FileSource.php | 45 ++- tests/common/TestsAutoLoader.php | 3 + .../Settings/Cache/ArrayCacheTest.php | 17 ++ .../Cache/CacheInterfaceTestTrait.php | 94 ++++++ .../Settings/Cache/CachedSourceTest.php | 91 ++++++ .../Settings/Cache/SharedMemoryCacheTest.php | 63 ++++ .../includes/Settings/SettingsBuilderTest.php | 43 ++- .../Settings/Source/FileSourceTest.php | 8 + 16 files changed, 906 insertions(+), 10 deletions(-) create mode 100644 includes/Settings/Cache/ArrayCache.php create mode 100644 includes/Settings/Cache/CacheArgumentException.php create mode 100644 includes/Settings/Cache/CacheableSource.php create mode 100644 includes/Settings/Cache/CachedSource.php create mode 100644 includes/Settings/Cache/SharedMemoryCache.php create mode 100644 tests/phpunit/unit/includes/Settings/Cache/ArrayCacheTest.php create mode 100644 tests/phpunit/unit/includes/Settings/Cache/CacheInterfaceTestTrait.php create mode 100644 tests/phpunit/unit/includes/Settings/Cache/CachedSourceTest.php create mode 100644 tests/phpunit/unit/includes/Settings/Cache/SharedMemoryCacheTest.php diff --git a/composer.json b/composer.json index 07dcb10591c..dad87335feb 100644 --- a/composer.json +++ b/composer.json @@ -43,6 +43,7 @@ "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", diff --git a/includes/Settings/Cache/ArrayCache.php b/includes/Settings/Cache/ArrayCache.php new file mode 100644 index 00000000000..bb285feb8f0 --- /dev/null +++ b/includes/Settings/Cache/ArrayCache.php @@ -0,0 +1,65 @@ +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; + } +} diff --git a/includes/Settings/Cache/CacheArgumentException.php b/includes/Settings/Cache/CacheArgumentException.php new file mode 100644 index 00000000000..412d8044cf2 --- /dev/null +++ b/includes/Settings/Cache/CacheArgumentException.php @@ -0,0 +1,12 @@ +0 will + * effectively disable early expiration, and by extension disable stampede + * mitigation altogether. + * + * A greater value may be suitable if a source has a highly variable + * generation duration, but most implementations should simply return + * 1.0. + * + * @link https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration + * @link https://cseweb.ucsd.edu/~avattani/papers/cache_stampede.pdf + * + * Optimal Probabilistic Cache Stampede Prevention + * Vattani, A.; Chierichetti, F.; Lowenstein, K. (2015), "Optimal + * Probabilistic Cache Stampede Prevention" (PDF), Proceedings of the VLDB + * Endowment, VLDB, 8 (8): 886–897, doi:10.14778/2757807.2757813, ISSN + * 2150-8097 https://cseweb.ucsd.edu/~avattani/papers/cache_stampede.pdf + * + * @return float + */ + public function getExpiryWeight(): float; + + /** + * Returns a deterministically computed key for use in caching settings + * from this source. + * + * @return string + */ + public function getHashKey(): string; +} diff --git a/includes/Settings/Cache/CachedSource.php b/includes/Settings/Cache/CachedSource.php new file mode 100644 index 00000000000..9169720f296 --- /dev/null +++ b/includes/Settings/Cache/CachedSource.php @@ -0,0 +1,123 @@ +cache = $cache; + $this->source = $source; + $this->ttl = $ttl; + } + + /** + * Queries cache for source contents and performs loading/caching of the + * source contents on miss. + * + * @return array + */ + public function load(): array { + $key = $this->source->getHashKey(); + $item = $this->cache->get( $key, null ); + + $miss = + $item === null || + $this->expiresEarly( $item, $this->source->getExpiryWeight() ); + + if ( $miss ) { + $item = $this->loadWithMetadata(); + $this->cache->set( $key, $item ); + } + + // This shouldn't be possible but let's make phan happy + if ( !is_array( $item ) || !array_key_exists( 'value', $item ) ) { + return []; + } + + return $item['value']; + } + + /** + * Returns the string representation of the encapsulated source. + * + * @return string + */ + public function __toString(): string { + return $this->source->__toString(); + } + + /** + * Decide whether the cached source should be expired early according to a + * probabilistic calculation that becomes more likely as the normal expiry + * approaches. + * + * @param array $item Cached source with expiry metadata. + * @param float $weight Coefficient used to increase/decrease the + * likelihood of early expiration. + * + * @link https://en.wikipedia.org/wiki/Cache_stampede#Probabilistic_early_expiration + * + * @return bool + */ + private function expiresEarly( array $item, float $weight ): bool { + if ( !isset( $item['expiry'] ) || !isset( $item['generation'] ) ) { + return false; + } + + $expiryOffset = + $item['generation'] * + $weight * + log( random_int( 1, PHP_INT_MAX ) / PHP_INT_MAX ); + + return ( $item['expiry'] + $expiryOffset ) <= microtime( true ); + } + + /** + * Wraps cached source with the metadata needed to perform probabilistic + * early expiration to help mitigate cache stampedes. + * + * @return array + */ + private function loadWithMetadata(): array { + $start = microtime( true ); + $value = $this->source->load(); + $finish = microtime( true ); + + return [ + 'value' => $value, + 'expiry' => $start + $this->ttl, + 'generation' => $start - $finish, + ]; + } +} diff --git a/includes/Settings/Cache/SharedMemoryCache.php b/includes/Settings/Cache/SharedMemoryCache.php new file mode 100644 index 00000000000..896e68da444 --- /dev/null +++ b/includes/Settings/Cache/SharedMemoryCache.php @@ -0,0 +1,274 @@ +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' ); + } + } +} diff --git a/includes/Settings/SettingsBuilder.php b/includes/Settings/SettingsBuilder.php index 8c17c74ac22..935049c5e99 100644 --- a/includes/Settings/SettingsBuilder.php +++ b/includes/Settings/SettingsBuilder.php @@ -2,26 +2,31 @@ namespace MediaWiki\Settings; +use MediaWiki\Settings\Cache\CacheableSource; +use MediaWiki\Settings\Cache\CachedSource; use MediaWiki\Settings\Config\ConfigSchemaAggregator; use MediaWiki\Settings\Config\ConfigSink; use MediaWiki\Settings\Config\PhpIniSink; use MediaWiki\Settings\Source\ArraySource; use MediaWiki\Settings\Source\FileSource; use MediaWiki\Settings\Source\SettingsSource; +use Psr\SimpleCache\CacheInterface; /** * Utility for loading settings files. * @since 1.38 */ class SettingsBuilder { - /** @var string */ private $baseDir; + /** @var CacheInterface */ + private $cache; + /** @var ConfigSink */ private $configSink; - /** @var SettingsSource[] */ + /** @var array */ private $currentBatch; /** @var ConfigSchemaAggregator */ @@ -34,13 +39,19 @@ class SettingsBuilder { * @param string $baseDir * @param ConfigSink $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. */ public function __construct( string $baseDir, ConfigSink $configSink, - PhpIniSink $phpIniSink + PhpIniSink $phpIniSink, + CacheInterface $cache = null ) { $this->baseDir = $baseDir; + $this->cache = $cache; $this->configSink = $configSink; $this->configSchema = new ConfigSchemaAggregator(); $this->phpIniSink = $phpIniSink; @@ -56,7 +67,12 @@ class SettingsBuilder { * @return $this */ public function load( SettingsSource $source ): self { + if ( $this->cache !== null && $source instanceof CacheableSource ) { + $source = new CachedSource( $this->cache, $source ); + } + $this->currentBatch[] = $source; + return $this; } @@ -108,7 +124,7 @@ class SettingsBuilder { } /** - * Apply the settings file. + * Apply the settings array. * * @param array $settings */ diff --git a/includes/Settings/Source/ArraySource.php b/includes/Settings/Source/ArraySource.php index a7a1cdfbf66..eed59495dfa 100644 --- a/includes/Settings/Source/ArraySource.php +++ b/includes/Settings/Source/ArraySource.php @@ -8,7 +8,6 @@ namespace MediaWiki\Settings\Source; * @since 1.38 */ class ArraySource implements SettingsSource { - private $settings; public function __construct( array $settings ) { diff --git a/includes/Settings/Source/FileSource.php b/includes/Settings/Source/FileSource.php index 16f806dd677..d4a5abb9f5a 100644 --- a/includes/Settings/Source/FileSource.php +++ b/includes/Settings/Source/FileSource.php @@ -2,6 +2,7 @@ namespace MediaWiki\Settings\Source; +use MediaWiki\Settings\Cache\CacheableSource; use MediaWiki\Settings\SettingsBuilderException; use MediaWiki\Settings\Source\Format\JsonFormat; use MediaWiki\Settings\Source\Format\SettingsFormat; @@ -14,13 +15,26 @@ use Wikimedia\AtEase\AtEase; * * @since 1.38 */ -class FileSource implements SettingsSource { +class FileSource implements CacheableSource { private const BUILT_IN_FORMATS = [ JsonFormat::class, YamlFormat::class, ]; + /** + * Early expiry weight. This value influences the margin by which + * processes are selected to expire cached local-file settings early to + * avoid cache stampedes. Changes to this value are not likely to be + * necessary as time spent loading from local files should not have much + * variation and should already be well served by the default early expiry + * calculation. + * + * @see getExpiryWeight() + * @see CacheableSource::getExpiryWeight() + */ + private const EXPIRY_WEIGHT = 1.0; + /** * Format to use for reading the file, if given. * @@ -98,6 +112,35 @@ class FileSource implements SettingsSource { ); } + /** + * Coefficient used in determining early expiration of cached settings to + * avoid stampedes. + * + * @return float + */ + public function getExpiryWeight(): float { + return self::EXPIRY_WEIGHT; + } + + /** + * Returns a hash key computed from the file's inode, size, and last + * modified timestamp. + * + * @return string + */ + public function getHashKey(): string { + $stat = stat( $this->path ); + + if ( $stat === false ) { + throw new SettingsBuilderException( + "Failed to stat file '{path}'", + [ 'path' => $this->path ] + ); + } + + return sprintf( '%x-%x-%x', $stat['ino'], $stat['size'], $stat['mtime'] ); + } + /** * Returns this file source as a string. * diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 8470984b094..da625891faf 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -266,6 +266,9 @@ $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", diff --git a/tests/phpunit/unit/includes/Settings/Cache/ArrayCacheTest.php b/tests/phpunit/unit/includes/Settings/Cache/ArrayCacheTest.php new file mode 100644 index 00000000000..28335a638e8 --- /dev/null +++ b/tests/phpunit/unit/includes/Settings/Cache/ArrayCacheTest.php @@ -0,0 +1,17 @@ +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' ) ); + } +} diff --git a/tests/phpunit/unit/includes/Settings/Cache/CachedSourceTest.php b/tests/phpunit/unit/includes/Settings/Cache/CachedSourceTest.php new file mode 100644 index 00000000000..142a68fda9c --- /dev/null +++ b/tests/phpunit/unit/includes/Settings/Cache/CachedSourceTest.php @@ -0,0 +1,91 @@ +createMock( CacheableSource::class ); + $cacheSource = new CachedSource( $cache, $source ); + + $settings = [ 'config' => [ 'Foo' => 'value' ] ]; + + $source + ->expects( $this->once() ) + ->method( 'getHashKey' ) + ->willReturn( 'abc123' ); + + $source + ->expects( $this->once() ) + ->method( 'load' ) + ->willReturn( $settings ); + + $this->assertSame( $settings, $cacheSource->load() ); + } + + public function testLoadWithHit() { + $cache = new ArrayCache(); + $source = $this->createMock( CacheableSource::class ); + $cacheSource = new CachedSource( $cache, $source ); + + $settings = [ 'config' => [ 'Foo' => 'value' ] ]; + + $cache->set( 'abc123', [ 'value' => $settings ] ); + + $source + ->expects( $this->once() ) + ->method( 'getHashKey' ) + ->willReturn( 'abc123' ); + + $source + ->expects( $this->never() ) + ->method( 'load' ); + + $this->assertSame( $settings, $cacheSource->load() ); + } + + public function testLoadWithNoEarlyExpiry() { + $cache = new ArrayCache(); + $source = $this->createMock( CacheableSource::class ); + $cacheSource = new CachedSource( $cache, $source ); + + $settings = [ 'config' => [ 'Foo' => 'value' ] ]; + + $cache->set( + 'abc123', + [ + 'value' => $settings, + 'expiry' => microtime( true ) + 60, + 'generation' => 1.0 + ] + ); + + $source + ->expects( $this->once() ) + ->method( 'getHashKey' ) + ->willReturn( 'abc123' ); + + // Effectively disable early expiry for the test since it's + // probabilistic and thus cannot be tested reliably without more + // dependency injection, but at least ensure that the code path is + // exercised. + $source + ->expects( $this->once() ) + ->method( 'getExpiryWeight' ) + ->willReturn( 0.0 ); + + $source + ->expects( $this->never() ) + ->method( 'load' ); + + $this->assertSame( $settings, $cacheSource->load() ); + } +} diff --git a/tests/phpunit/unit/includes/Settings/Cache/SharedMemoryCacheTest.php b/tests/phpunit/unit/includes/Settings/Cache/SharedMemoryCacheTest.php new file mode 100644 index 00000000000..3f4798198fe --- /dev/null +++ b/tests/phpunit/unit/includes/Settings/Cache/SharedMemoryCacheTest.php @@ -0,0 +1,63 @@ +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(); + } + } +} diff --git a/tests/phpunit/unit/includes/Settings/SettingsBuilderTest.php b/tests/phpunit/unit/includes/Settings/SettingsBuilderTest.php index ba0432d709d..dbf3044b67e 100644 --- a/tests/phpunit/unit/includes/Settings/SettingsBuilderTest.php +++ b/tests/phpunit/unit/includes/Settings/SettingsBuilderTest.php @@ -2,6 +2,7 @@ namespace phpunit\unit\includes\Settings; +use MediaWiki\Settings\Cache\CacheableSource; use MediaWiki\Settings\Config\ArrayConfigBuilder; use MediaWiki\Settings\Config\ConfigSink; use MediaWiki\Settings\Config\MergeStrategy; @@ -9,6 +10,7 @@ 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 @@ -18,16 +20,19 @@ class SettingsBuilderTest extends TestCase { /** * @param ConfigSink|null $configBuilder * @param PhpIniSink|null $phpIniSink + * @param CacheInterface|null $cache * @return SettingsBuilder */ private function newSettingsBuilder( ConfigSink $configBuilder = null, - PhpIniSink $phpIniSink = null + PhpIniSink $phpIniSink = null, + CacheInterface $cache = null ): SettingsBuilder { return new SettingsBuilder( __DIR__, $configBuilder ?? new ArrayConfigBuilder(), - $phpIniSink ?? new PhpIniSink() + $phpIniSink ?? new PhpIniSink(), + $cache ); } @@ -186,8 +191,8 @@ class SettingsBuilderTest extends TestCase { } public function testApplyDefaultDoesNotOverwriteExisting() { - $configBuilder = ( new ArrayConfigBuilder() ) - ->set( 'MySetting', 'existing' ); + $configBuilder = new ArrayConfigBuilder(); + $configBuilder->set( 'MySetting', 'existing' ); $this->newSettingsBuilder( $configBuilder ) ->loadArray( [ 'config-schema' => [ 'MySetting' => [ 'default' => 'default' ], ], ] ) ->apply(); @@ -201,4 +206,34 @@ class SettingsBuilderTest extends TestCase { ->loadArray( [ 'config-schema' => [ 'MySetting' => [ 'default' => 'override' ], ], ] ) ->apply(); } + + public function testLoadsCacheableSource() { + $mockSource = $this->createMock( CacheableSource::class ); + $mockCache = $this->createMock( CacheInterface::class ); + $configBuilder = new ArrayConfigBuilder(); + $builder = $this + ->newSettingsBuilder( $configBuilder, null, $mockCache ) + ->load( $mockSource ); + + // Mock a cache miss + $mockSource + ->expects( $this->once() ) + ->method( 'getHashKey' ) + ->willReturn( 'abc123' ); + + $mockCache + ->expects( $this->once() ) + ->method( 'get' ) + ->with( 'abc123' ) + ->willReturn( null ); + + $mockSource + ->expects( $this->once() ) + ->method( 'load' ) + ->willReturn( [ 'config' => [ 'MySetting' => 'BlaBla' ] ] ); + + $builder->apply(); + + $this->assertSame( 'BlaBla', $configBuilder->build()->get( 'MySetting' ) ); + } } diff --git a/tests/phpunit/unit/includes/Settings/Source/FileSourceTest.php b/tests/phpunit/unit/includes/Settings/Source/FileSourceTest.php index e39547dfa88..e1782af427c 100644 --- a/tests/phpunit/unit/includes/Settings/Source/FileSourceTest.php +++ b/tests/phpunit/unit/includes/Settings/Source/FileSourceTest.php @@ -54,4 +54,12 @@ class FileSourceTest extends TestCase { $settings = $source->load(); } + + public function testGetHashKey() { + $source = new FileSource( __DIR__ . '/fixtures/settings.json' ); + + // We can't reliably mock the filesystem stat so simply ensure the + // method returns and is non-zero in length + $this->assertNotEmpty( $source->getHashKey() ); + } }