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

MapCacheLRU: Remove redundant type checks in favour of native warnings

Browse source 
In commit 70c2223843, the explicit type checks were added as
exceptions because WMF's infra at the time didn't capture stacktraces
for the native warnings that PHP already emits.

That infra issue was resolved years ago (T45086), so we can remove
these again.

I've kept one manual type check, which is in setField, because for
$field in that method we don't first check existence (which emits
the warning normally), and because it does an assignment rather than
a key check, and assignments in PHP still have some invalid types
that don't emit warnings (specifically, booleans).

Bug: T275673
Change-Id: I72388f069afc345da9e9aac330733da49714d1b4
This commit is contained in:
Timo Tijhof 2021-08-17 23:00:45 +01:00
parent 880083a8b7
commit 74b4d669a7
2 changed files with 30 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
includes/libs/MapCacheLRU.php
View file

@ -20,7 +20,6 @@
* @file
* @ingroup Cache
*/
use Wikimedia\Assert\Assert;
use Wikimedia\LightweightObjectStore\ExpirationAwareness;
/**
@ -58,10 +57,11 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
/**
* @param int $maxKeys Maximum number of entries allowed (min 1)
* @throws Exception When $maxKeys is not above zero
*/
public function __construct( int $maxKeys ) {
Assert::parameter( $maxKeys > 0, '$maxKeys', 'must be above zero' );
if ( $maxKeys <= 0 ) {
throw new InvalidArgumentException( '$maxKeys must be above zero' );
}
$this->maxCacheKeys = $maxKeys;
// Use the current time as the default "as of" timestamp of entries
@ -134,17 +134,13 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
/**
* Check if a key exists
*
* @param string $key
* @param string|int $key
* @param float $maxAge Ignore items older than this many seconds [default: INF]
* @return bool
* @since 1.32 Added $maxAge
*/
public function has( $key, $maxAge = INF ) {
if ( !is_int( $key ) && !is_string( $key ) ) {
throw new UnexpectedValueException(
__METHOD__ . ': invalid key; must be string or integer.' );
}
// Optimization: Forego type check because array_key_exists does it already (T275673)
if ( !array_key_exists( $key, $this->cache ) ) {
return false;
}
@ -187,19 +183,18 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
public function setField( $key, $field, $value, $initRank = self::RANK_TOP ) {
if ( $this->has( $key ) ) {
$this->ping( $key );
if ( !is_array( $this->cache[$key] ) ) {
$type = gettype( $this->cache[$key] );
throw new UnexpectedValueException( "Cannot add field to non-array value ('$key' is $type)" );
}
} else {
$this->set( $key, [], $initRank );
}
if ( !is_int( $field ) && !is_string( $field ) ) {
throw new UnexpectedValueException(
__METHOD__ . ": invalid field for '$key'; must be string or integer." );
}
if ( !is_array( $this->cache[$key] ) ) {
$type = gettype( $this->cache[$key] );
throw new UnexpectedValueException( "The value of '$key' ($type) is not an array." );
if ( !is_string( $field ) && !is_int( $field ) ) {
trigger_error( "Field keys must be string or integer (key '$key')", E_USER_WARNING );
return;
}
$this->cache[$key][$field] = $value;
@ -216,11 +211,7 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
public function hasField( $key, $field, $maxAge = INF ) {
$value = $this->get( $key );
if ( !is_int( $field ) && !is_string( $field ) ) {
throw new UnexpectedValueException(
__METHOD__ . ": invalid field for '$key'; must be string or integer." );
}
// Optimization: Forego type check because array_key_exists does it already (T275673)
if ( !is_array( $value ) || !array_key_exists( $field, $value ) ) {
return false;
}
@ -270,7 +261,7 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
if ( $this->has( $key, $maxAge ) ) {
$value = $this->get( $key );
} else {
$value = call_user_func( $callback );
$value = $callback();
if ( $value !== false ) {
$this->set( $key, $value, $rank );
}
@ -312,11 +303,12 @@ class MapCacheLRU implements ExpirationAwareness, Serializable {
*
* @param int $maxKeys Maximum number of entries allowed (min 1)
* @return void
* @throws Exception When $maxKeys is not above zero
* @since 1.32
*/
public function setMaxSize( int $maxKeys ) {
Assert::parameter( $maxKeys > 0, '$maxKeys', 'must be above zero' );
if ( $maxKeys <= 0 ) {
throw new InvalidArgumentException( '$maxKeys must be above zero' );
}
$this->maxCacheKeys = $maxKeys;
while ( count( $this->cache ) > $this->maxCacheKeys ) {

24
tests/phpunit/unit/includes/libs/MapCacheLRUTest.php
View file

@ -250,8 +250,8 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
*/
public function testHasInvalidKey( $key ) {
$cache = new MapCacheLRU( 3 );
$this->expectException( UnexpectedValueException::class );
$this->expectExceptionMessage( 'must be string or integer' );
$this->expectWarning();
$this->expectWarningMessageMatches( '/should be either a string or an integer/' );
$cache->has( $key );
}
@ -261,8 +261,8 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
*/
public function testGetInvalidKey( $key ) {
$cache = new MapCacheLRU( 3 );
$this->expectException( UnexpectedValueException::class );
$this->expectExceptionMessage( 'must be string or integer' );
$this->expectWarning();
$this->expectWarningMessageMatches( '/should be either a string or an integer/' );
$cache->get( $key );
}
@ -272,8 +272,8 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
*/
public function testSetInvalidKey( $key ) {
$cache = new MapCacheLRU( 3 );
$this->expectException( UnexpectedValueException::class );
$this->expectExceptionMessage( 'must be string or integer' );
$this->expectWarning();
$this->expectWarningMessageMatches( '/should be either a string or an integer/' );
$cache->set( $key, 'x' );
}
@ -283,8 +283,8 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
*/
public function testHasFieldInvalidKey( $field ) {
$cache = MapCacheLRU::newFromArray( [ 'key' => [] ], 3 );
$this->expectException( UnexpectedValueException::class );
$this->expectExceptionMessage( 'must be string or integer' );
$this->expectWarning();
$this->expectWarningMessageMatches( '/should be either a string or an integer/' );
$cache->hasField( 'key', $field );
}
@ -294,8 +294,8 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
*/
public function testGetFieldInvalidKey( $field ) {
$cache = MapCacheLRU::newFromArray( [ 'key' => [] ], 3 );
$this->expectException( UnexpectedValueException::class );
$this->expectExceptionMessage( 'must be string or integer' );
$this->expectWarning();
$this->expectWarningMessageMatches( '/should be either a string or an integer/' );
$cache->getField( 'key', $field );
}
@ -305,8 +305,8 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
*/
public function testSetFieldInvalidKey( $field ) {
$cache = new MapCacheLRU( 3 );
$this->expectException( UnexpectedValueException::class );
$this->expectExceptionMessage( 'must be string or integer' );
$this->expectWarning();
$this->expectWarningMessageMatches( '/Field keys must be string or integer/' );
$cache->setField( 'key', $field, 'x' );
}
}