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

diff --git a/includes/libs/objectcache/MediumSpecificBagOStuff.php b/includes/libs/objectcache/MediumSpecificBagOStuff.php
index fe17628..62a8aec 100644 (file)
--- 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 ff9dedf..dc40931 100644 (file)
--- 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 ffbc378..3bb0771 100644 (file)
--- 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 aeb0a70..9ec86bc 100644 (file)
--- 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 ) );