Migrate BagOStuff::incr() calls to incrWithInit()
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 9 Aug 2019 22:56:18 +0000 (15:56 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Sun, 25 Aug 2019 01:15:32 +0000 (01:15 +0000)
Change-Id: I722d6fe3d50c594858e3e7960fb99ef2fc1664b3

includes/auth/Throttler.php
includes/cache/FileCacheBase.php
includes/user/User.php

index e6d9bd8..9d0175a 100644 (file)
@@ -127,14 +127,16 @@ class Throttler implements LoggerAwareInterface {
                                continue;
                        }
 
-                       $throttleKey = $this->cache->makeGlobalKey( 'throttler', $this->type, $index, $ipKey, $userKey );
+                       $throttleKey = $this->cache->makeGlobalKey(
+                               'throttler',
+                               $this->type,
+                               $index,
+                               $ipKey,
+                               $userKey
+                       );
                        $throttleCount = $this->cache->get( $throttleKey );
-
-                       if ( !$throttleCount ) { // counter not started yet
-                               $this->cache->add( $throttleKey, 1, $expiry );
-                       } elseif ( $throttleCount < $count ) { // throttle limited not yet reached
-                               $this->cache->incr( $throttleKey );
-                       } else { // throttled
+                       if ( $throttleCount && $throttleCount >= $count ) {
+                               // Throttle limited reached
                                $this->logRejection( [
                                        'throttle' => $this->type,
                                        'index' => $index,
@@ -147,13 +149,12 @@ class Throttler implements LoggerAwareInterface {
                                        // @codeCoverageIgnoreEnd
                                ] );
 
-                               return [
-                                       'throttleIndex' => $index,
-                                       'count' => $count,
-                                       'wait' => $expiry,
-                               ];
+                               return [ 'throttleIndex' => $index, 'count' => $count, 'wait' => $expiry ];
+                       } else {
+                               $this->cache->incrWithInit( $throttleKey, $expiry, 1 );
                        }
                }
+
                return false;
        }
 
index ce5a019..fb42539 100644 (file)
@@ -230,31 +230,26 @@ abstract class FileCacheBase {
         */
        public function incrMissesRecent( WebRequest $request ) {
                if ( mt_rand( 0, self::MISS_FACTOR - 1 ) == 0 ) {
-                       $cache = ObjectCache::getLocalClusterInstance();
                        # Get a large IP range that should include the user  even if that
                        # person's IP address changes
                        $ip = $request->getIP();
                        if ( !IP::isValid( $ip ) ) {
                                return;
                        }
+
                        $ip = IP::isIPv6( $ip )
                                ? IP::sanitizeRange( "$ip/32" )
                                : IP::sanitizeRange( "$ip/16" );
 
                        # Bail out if a request already came from this range...
+                       $cache = ObjectCache::getLocalClusterInstance();
                        $key = $cache->makeKey( static::class, 'attempt', $this->mType, $this->mKey, $ip );
-                       if ( $cache->get( $key ) ) {
+                       if ( !$cache->add( $key, 1, self::MISS_TTL_SEC ) ) {
                                return; // possibly the same user
                        }
-                       $cache->set( $key, 1, self::MISS_TTL_SEC );
 
                        # Increment the number of cache misses...
-                       $key = $this->cacheMissKey( $cache );
-                       if ( $cache->get( $key ) === false ) {
-                               $cache->set( $key, 1, self::MISS_TTL_SEC );
-                       } else {
-                               $cache->incr( $key );
-                       }
+                       $cache->incrWithInit( $this->cacheMissKey( $cache ), self::MISS_TTL_SEC );
                }
        }
 
index 7c2f038..91b9420 100644 (file)
@@ -2042,14 +2042,10 @@ class User implements IDBAccessObject, UserIdentity {
                        $summary = "(limit $max in {$period}s)";
                        $count = $cache->get( $key );
                        // Already pinged?
-                       if ( $count ) {
-                               if ( $count >= $max ) {
-                                       wfDebugLog( 'ratelimit', "User '{$this->getName()}' " .
-                                               "(IP {$this->getRequest()->getIP()}) tripped $key at $count $summary" );
-                                       $triggered = true;
-                               } else {
-                                       wfDebug( __METHOD__ . ": ok. $key at $count $summary\n" );
-                               }
+                       if ( $count && $count >= $max ) {
+                               wfDebugLog( 'ratelimit', "User '{$this->getName()}' " .
+                                       "(IP {$this->getRequest()->getIP()}) tripped $key at $count $summary" );
+                               $triggered = true;
                        } else {
                                wfDebug( __METHOD__ . ": adding record for $key $summary\n" );
                                if ( $incrBy > 0 ) {
@@ -2057,7 +2053,7 @@ class User implements IDBAccessObject, UserIdentity {
                                }
                        }
                        if ( $incrBy > 0 ) {
-                               $cache->incr( $key, $incrBy );
+                               $cache->incrWithInit( $key, (int)$period, $incrBy, $incrBy );
                        }
                }