yii\filters\RateLimiter refactored (#13994)

Refactored yii\filters\RateLimiter and added tests

framework/CHANGELOG.md

@@ -8,6 +8,7 @@ Yii Framework 2 Change Log
 - Bug #14012: `yii\db\pgsql\Schema::findViewNames()` was skipping materialized views (insolita)
 - Bug #13362: Fixed return value of `yii\caching\MemCache::setValues()`  (masterklavi)
 - Enh #13963: Added tests for yii\behaviors\TimestampBehavior (vladis84)
+- Enh #13994: Refactored `yii\filters\RateLimiter`. Added tests (vladis84)
 - Enh #13820: Add new HTTP status code 451 (yyxx9988)
 - Bug #13671: Fixed error handler trace to work correctly with XDebug (samdark)
 - Bug #13657: Fixed `yii\helpers\StringHelper::truncateHtml()` skip extra tags at the end (sam002)

framework/filters/RateLimiter.php

@@ -65,22 +65,34 @@ class RateLimiter extends ActionFilter
     /**
      * @inheritdoc
      */
+    public function init()
+    {
+        if ($this->request === null) {
+            $this->request = Yii::$app->getRequest();
+        }
+        if ($this->response === null) {
+            $this->response = Yii::$app->getResponse();
+        }
+    }
+
+    /**
+     * @inheritdoc
+     */
     public function beforeAction($action)
     {
-        $user = $this->user ? : (Yii::$app->getUser() ? Yii::$app->getUser()->getIdentity(false) : null);
+        if ($this->user === null && Yii::$app->getUser()) {
-        if ($user instanceof RateLimitInterface) {
+            $this->user = Yii::$app->getUser()->getIdentity(false);
+        }
+
+        if ($this->user instanceof RateLimitInterface) {
             Yii::trace('Check rate limit', __METHOD__);
-            $this->checkRateLimit(
+            $this->checkRateLimit($this->user, $this->request, $this->response, $action);
-                $user,
+        } elseif ($this->user) {
-                $this->request ? : Yii::$app->getRequest(),
-                $this->response ? : Yii::$app->getResponse(),
-                $action
-            );
-        } elseif ($user) {
             Yii::info('Rate limit skipped: "user" does not implement RateLimitInterface.', __METHOD__);
         } else {
             Yii::info('Rate limit skipped: user not logged in.', __METHOD__);
         }
+
         return true;
     }
 

tests/TestCase.php

@@ -152,7 +152,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase
         }
         $property = $class->getProperty($propertyName);
         $property->setAccessible(true);
-        $property->setValue($value);
+        $property->setValue($object, $value);
         if ($revoke) {
             $property->setAccessible(false);
         }

tests/framework/filters/RateLimiterTest.php

@@ -0,0 +1,155 @@
+<?php
+
+namespace yiiunit\framework\filters;
+
+use Yii;
+use yiiunit\TestCase;
+use Prophecy\Argument;
+use yiiunit\framework\filters\stubs\RateLimit;
+use yii\web\User;
+use yii\web\Request;
+use yii\web\Response;
+use yii\log\Logger;
+use yii\filters\RateLimiter;
+
+/**
+ *  @group filters
+ */
+class RateLimiterTest extends TestCase
+{
+    protected function setUp()
+    {
+        parent::setUp();
+
+        /* @var $logger Logger|\Prophecy\ObjectProphecy */
+        $logger = $this->prophesize(Logger::className());
+        $logger
+            ->log(Argument::any(), Argument::any(), Argument::any())
+            ->will(function ($parameters, $logger) {
+                $logger->messages = $parameters;
+            });
+
+        Yii::setLogger($logger->reveal());
+
+        $this->mockWebApplication();
+    }
+    protected function tearDown()
+    {
+        parent::tearDown();
+        Yii::setLogger(null);
+    }
+    
+    public function testInitFilledRequest()
+    {
+        $rateLimiter = new RateLimiter(['request' => 'Request']);
+
+        $this->assertEquals('Request', $rateLimiter->request);
+    }
+
+    public function testInitNotFilledRequest()
+    {
+        $rateLimiter = new RateLimiter();
+
+        $this->assertInstanceOf(Request::className(), $rateLimiter->request);
+    }
+
+    public function testInitFilledResponse()
+    {
+        $rateLimiter = new RateLimiter(['response' => 'Response']);
+
+        $this->assertEquals('Response', $rateLimiter->response);
+    }
+
+    public function testInitNotFilledResponse()
+    {
+        $rateLimiter = new RateLimiter();
+
+        $this->assertInstanceOf(Response::className(), $rateLimiter->response);
+    }
+
+    public function testBeforeActionUserInstanceOfRateLimitInterface()
+    {
+        $rateLimiter = new RateLimiter();
+        $rateLimit = new RateLimit();
+        $rateLimit->setAllowance([1, time()])
+            ->setRateLimit([1, 1]);
+        $rateLimiter->user = $rateLimit;
+
+        $result = $rateLimiter->beforeAction('test');
+
+        $this->assertContains('Check rate limit', Yii::getLogger()->messages);
+        $this->assertTrue($result);
+    }
+
+    public function testBeforeActionUserNotInstanceOfRateLimitInterface()
+    {
+        $rateLimiter = new RateLimiter(['user' => 'User']);
+
+        $result = $rateLimiter->beforeAction('test');
+
+        $this->assertContains('Rate limit skipped: "user" does not implement RateLimitInterface.', Yii::getLogger()->messages);
+        $this->assertTrue($result);
+    }
+
+    public function testBeforeActionEmptyUser()
+    {
+        $user = new User(['identityClass' => RateLimit::className()]);
+        Yii::$app->set('user', $user);
+        $rateLimiter = new RateLimiter();
+
+        $result = $rateLimiter->beforeAction('test');
+
+        $this->assertContains('Rate limit skipped: user not logged in.', Yii::getLogger()->messages);
+        $this->assertTrue($result);
+    }
+
+    public function testCheckRateLimitTooManyRequests()
+    {
+        /* @var $rateLimit UserIdentity|\Prophecy\ObjectProphecy */
+        $rateLimit = new RateLimit;
+        $rateLimit
+            ->setRateLimit([1, 1])
+            ->setAllowance([1, time() + 2]);
+        $rateLimiter = new RateLimiter();
+
+        $this->setExpectedException('yii\web\TooManyRequestsHttpException');
+        $rateLimiter->checkRateLimit($rateLimit, Yii::$app->request, Yii::$app->response, 'testAction');
+    }
+
+    public function testCheckRateaddRateLimitHeaders()
+    {
+        /* @var $user UserIdentity|\Prophecy\ObjectProphecy */
+        $rateLimit = new RateLimit;
+        $rateLimit
+            ->setRateLimit([1, 1])
+            ->setAllowance([1, time()]);
+        $rateLimiter = $this->getMockBuilder(RateLimiter::className())
+            ->setMethods(['addRateLimitHeaders'])
+            ->getMock();
+        $rateLimiter->expects(self::at(0))
+            ->method('addRateLimitHeaders')
+            ->willReturn(null);
+
+        $rateLimiter->checkRateLimit($rateLimit, Yii::$app->request, Yii::$app->response, 'testAction');
+    }
+
+    public function testAddRateLimitHeadersDisabledRateLimitHeaders()
+    {
+        $rateLimiter = new RateLimiter();
+        $rateLimiter->enableRateLimitHeaders = false;
+        $response = Yii::$app->response;
+
+        $rateLimiter->addRateLimitHeaders($response, 1, 0, 0);
+        $this->assertCount(0, $response->getHeaders());
+    }
+
+    public function testAddRateLimitHeadersEnabledRateLimitHeaders()
+    {
+        $rateLimiter = new RateLimiter();
+        $rateLimiter->enableRateLimitHeaders = true;
+        $response = Yii::$app->response;
+
+        $rateLimiter->addRateLimitHeaders($response, 1, 0, 0);
+        $this->assertCount(3, $response->getHeaders());
+    }
+}

tests/framework/filters/stubs/RateLimit.php

@@ -0,0 +1,44 @@
+<?php
+
+namespace yiiunit\framework\filters\stubs;
+
+use yii\base\Object;
+use yii\filters\RateLimitInterface;
+
+class RateLimit extends Object implements RateLimitInterface
+{
+    private $_rateLimit;
+
+    private $_allowance;
+
+    public function getRateLimit($request, $action)
+    {
+        return $this->_rateLimit;
+    }
+
+    public function setRateLimit($rateLimit)
+    {
+        $this->_rateLimit = $rateLimit;
+        
+        return $this;
+    }
+
+    public function loadAllowance($request, $action)
+    {
+        return $this->_allowance;
+    }
+
+    public function setAllowance($allowance)
+    {
+        $this->_allowance = $allowance;
+
+        return $this;
+    }
+
+
+    public function saveAllowance($request, $action, $allowance, $timestamp)
+    {
+        return [$action, $allowance, $timestamp];
+    }
+
+}