Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rate limiter refactor test #13994

Merged
merged 16 commits into from
Apr 24, 2017
2 changes: 1 addition & 1 deletion framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Yii Framework 2 Change Log

2.0.12 under development
--------------------------

- Enh: Test write for yii\filters\RateLimiter and refactoring (vladis84)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Expand Down
30 changes: 21 additions & 9 deletions framework/filters/RateLimiter.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,38 @@ class RateLimiter extends ActionFilter
*/
public $response;

/**
* {inheritdoc}
*/
public function init()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpdoc missing

Copy link
Contributor Author

@vladis84 vladis84 Apr 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I corrected

{
if (!$this->request) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these initializations for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/yiisoft/yii2/blob/master/framework/filters/RateLimiter.php#L75
It is difficult to say why in the filter these properties

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test for === null instead of !.

$this->request = Yii::$app->getRequest();
}

if (!$this->response) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test for === null instead of !.

$this->response = Yii::$app->getResponse();
}
}

/**
* @inheritdoc
*/
public function beforeAction($action)
{
$user = $this->user ? : (Yii::$app->getUser() ? Yii::$app->getUser()->getIdentity(false) : null);
if ($user instanceof RateLimitInterface) {
if ($this->user === null && Yii::$app->getUser()) {
$this->user = Yii::$app->getUser()->getIdentity(false);
}

if ($this->user instanceof RateLimitInterface) {
Yii::trace('Check rate limit', __METHOD__);
$this->checkRateLimit(
$user,
$this->request ? : Yii::$app->getRequest(),
$this->response ? : Yii::$app->getResponse(),
$action
);
} elseif ($user) {
$this->checkRateLimit($this->user, $this->request, $this->response, $action);
} elseif ($this->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;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ protected function setInaccessibleProperty($object, $propertyName, $value, $revo
}
$property = $class->getProperty($propertyName);
$property->setAccessible(true);
$property->setValue($value);
$property->setValue($object, $value);
if ($revoke) {
$property->setAccessible(false);
}
Expand Down
155 changes: 155 additions & 0 deletions tests/framework/filters/RateLimiterTest.php
Original file line number Diff line number Diff line change
@@ -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());
}
}
44 changes: 44 additions & 0 deletions tests/framework/filters/stubs/RateLimit.php
Original file line number Diff line number Diff line change
@@ -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];
}

}