From 49025f52b2865c771dc3270e5e29122e8d52a46b Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 8 Aug 2019 09:38:55 -0700 Subject: [PATCH] objectcache: clean up MemcachedBagOStuff expiry handling Partly a follow-up to 88640fd902900. Use real time in changeTTL() tests to fix all remaining failures for BagOStuff sub-classes. Change-Id: I537d665d6c8770a68a5a79233a913f1714881dfb --- .../objectcache/MediumSpecificBagOStuff.php | 6 +-- .../libs/objectcache/MemcachedBagOStuff.php | 25 ++++++++----- includes/objectcache/ObjectCache.php | 3 +- .../libs/objectcache/BagOStuffTest.php | 37 ++++++++++++++++--- 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/includes/libs/objectcache/MediumSpecificBagOStuff.php b/includes/libs/objectcache/MediumSpecificBagOStuff.php index fe17628e81..62a8aec967 100644 --- a/includes/libs/objectcache/MediumSpecificBagOStuff.php +++ b/includes/libs/objectcache/MediumSpecificBagOStuff.php @@ -783,12 +783,12 @@ abstract class MediumSpecificBagOStuff extends BagOStuff { } /** - * @param int $exptime - * @return bool + * @param int|float $exptime + * @return bool Whether the expiry is non-infinite, and, negative or not a UNIX timestamp * @since 1.34 */ final protected function isRelativeExpiration( $exptime ) { - return ( $exptime != self::TTL_INDEFINITE && $exptime < ( 10 * self::TTL_YEAR ) ); + return ( $exptime !== self::TTL_INDEFINITE && $exptime < ( 10 * self::TTL_YEAR ) ); } /** diff --git a/includes/libs/objectcache/MemcachedBagOStuff.php b/includes/libs/objectcache/MemcachedBagOStuff.php index ff9dedf3a1..dc409315f8 100644 --- a/includes/libs/objectcache/MemcachedBagOStuff.php +++ b/includes/libs/objectcache/MemcachedBagOStuff.php @@ -96,17 +96,24 @@ abstract class MemcachedBagOStuff extends MediumSpecificBagOStuff { } /** - * TTLs higher than 30 days will be detected as absolute TTLs - * (UNIX timestamps), and will result in the cache entry being - * discarded immediately because the expiry is in the past. - * Clamp expires >30d at 30d, unless they're >=1e9 in which - * case they are likely to really be absolute (1e9 = 2011-09-09) - * @param int $exptime + * @param int|float $exptime * @return int */ protected function fixExpiry( $exptime ) { - return ( $exptime > self::TTL_MONTH && !$this->isRelativeExpiration( $exptime ) ) - ? self::TTL_MONTH - : (int)$exptime; + if ( $exptime < 0 ) { + // The PECL driver does not seem to like negative relative values + $expiresAt = $this->getCurrentTime() + $exptime; + } elseif ( $this->isRelativeExpiration( $exptime ) ) { + // TTLs higher than 30 days will be detected as absolute TTLs + // (UNIX timestamps), and will result in the cache entry being + // discarded immediately because the expiry is in the past. + // Clamp expires >30d at 30d, unless they're >=1e9 in which + // case they are likely to really be absolute (1e9 = 2011-09-09) + $expiresAt = min( $exptime, self::TTL_MONTH ); + } else { + $expiresAt = $exptime; + } + + return (int)$expiresAt; } } diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php index ffbc3783c4..3bb077173f 100644 --- a/includes/objectcache/ObjectCache.php +++ b/includes/objectcache/ObjectCache.php @@ -178,7 +178,8 @@ class ObjectCache { } elseif ( isset( $params['class'] ) ) { $class = $params['class']; // Automatically set the 'async' update handler - $params['asyncHandler'] = $params['asyncHandler'] ?? 'DeferredUpdates::addCallableUpdate'; + $params['asyncHandler'] = $params['asyncHandler'] + ?? [ DeferredUpdates::class, 'addCallableUpdate' ]; // Enable reportDupes by default $params['reportDupes'] = $params['reportDupes'] ?? true; // Do b/c logic for SqlBagOStuff diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php index aeb0a70d7d..9ec86bce79 100644 --- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php +++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php @@ -112,21 +112,48 @@ class BagOStuffTest extends MediaWikiTestCase { /** * @covers MediumSpecificBagOStuff::changeTTL */ - public function testChangeTTL() { - $now = 1563892142; + public function testChangeTTLRenew() { + $now = microtime( true ); // need real time $this->cache->setMockTime( $now ); $key = $this->cache->makeKey( self::TEST_KEY ); $value = 'meow'; - $this->cache->add( $key, $value, 5 ); + $this->cache->add( $key, $value, 60 ); $this->assertEquals( $value, $this->cache->get( $key ) ); - $this->assertTrue( $this->cache->changeTTL( $key, 10 ) ); - $this->assertTrue( $this->cache->changeTTL( $key, 10 ) ); + $this->assertTrue( $this->cache->changeTTL( $key, 120 ) ); + $this->assertTrue( $this->cache->changeTTL( $key, 120 ) ); $this->assertTrue( $this->cache->changeTTL( $key, 0 ) ); $this->assertEquals( $this->cache->get( $key ), $value ); + $this->cache->delete( $key ); $this->assertFalse( $this->cache->changeTTL( $key, 15 ) ); + } + + /** + * @covers MediumSpecificBagOStuff::changeTTL + */ + public function testChangeTTLExpireRel() { + $now = microtime( true ); // need real time + $this->cache->setMockTime( $now ); + + $key = $this->cache->makeKey( self::TEST_KEY ); + $value = 'meow'; + + $this->cache->add( $key, $value, 5 ); + $this->assertTrue( $this->cache->changeTTL( $key, -3600 ) ); + $this->assertFalse( $this->cache->get( $key ) ); + } + + /** + * @covers MediumSpecificBagOStuff::changeTTL + */ + public function testChangeTTLExpireAbs() { + $now = microtime( true ); // need real time + $this->cache->setMockTime( $now ); + + $key = $this->cache->makeKey( self::TEST_KEY ); + $value = 'meow'; $this->cache->add( $key, $value, 5 ); $this->assertTrue( $this->cache->changeTTL( $key, $now - 3600 ) ); -- 2.20.1