8000 bug #38566 Fix minor issue when sharing windows between Limiters (Nyh… · symfony/symfony@8553361 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8553361

Browse files
nicolas-grekasnicolas-grekas
committed
bug #38566 Fix minor issue when sharing windows between Limiters (Nyholm)
This PR was squashed before being merged into the 5.x branch. Discussion ---------- Fix minor issue when sharing windows between Limiters | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | If I start using my custom Limiter, then change back to `FixedWindowLimiter`, then my cache might contain a value that `FixedWindowLimiter` does not support. This PR makes sure that we handle such switch. Commits ------- e9ac971 Fix minor issue when sharing windows between Limiters
2 parents 2d5c2cf + e9ac971 commit 8553361

File tree

5 files changed

+59
-3
lines changed

5 files changed

+59
-3
lines changed

‎src/Symfony/Component/RateLimiter/FixedWindowLimiter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function consume(int $tokens = 1): Limit
4949

5050
try {
5151
$window = $this->storage->fetch($this->id);
52-
if (null === $window) {
52+
if (!$window instanceof Window) {
5353
$window = new Window($this->id, $this->interval);
5454
}
5555

‎src/Symfony/Component/RateLimiter/Tests/FixedWindowLimiterTest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Bridge\PhpUnit\ClockMock;
1616
use Symfony\Component\RateLimiter\FixedWindowLimiter;
1717
use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
18+
use Symfony\Component\RateLimiter\Tests\Resources\DummyWindow;
1819

1920
/**
2021
* @group time-sensitive
@@ -62,6 +63,15 @@ public function testConsumeOutsideInterval()
6263
$this->assertEquals(time() + 60, $limit->getRetryAfter()->getTimestamp());
6364
}
6465

66+
public function testWrongWindowFromCache()
67+
{
68+
$this->storage->save(new DummyWindow());
69+
$limiter = $this->createLimiter();
70+
$limit = $limiter->consume();
71+
$this->assertTrue($limit->isAccepted());
72+
$this->assertEquals(9, $limit->getRemainingTokens());
73+
}
74+
6575
private function createLimiter(): FixedWindowLimiter
6676
{
6777
return new FixedWindowLimiter('test', 10, new \DateInterval('PT1M'), $this->storage);

‎src/Symfony/Component/RateLimiter/Tests/Resources/DummyWindow.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\RateLimiter\Tests\Resources;
13+
14+
use Symfony\Component\RateLimiter\LimiterStateInterface;
15+
16+
class DummyWindow implements LimiterStateInterface
17+
{
18+
private $id;
19+
private $expirationTime;
20+
21+
public function __construct(string $id = 'test', ?int $expirationTime = 10)
22+
{
23+
$this->id = $id;
24+
$this->expirationTime = $expirationTime;
25+
}
26+
27+
public function getId(): string
28+
{
29+
return $this->id;
30+
}
31+
32+
public function getExpirationTime(): ?int
33+
{
34+
return $this->expirationTime;
35+
}
36+
}

‎src/Symfony/Component/RateLimiter/Tests/TokenBucketLimiterTest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\RateLimiter\Exception\MaxWaitDurationExceededException;
1717
use Symfony\Component\RateLimiter\Rate;
1818
use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
19+
use Symfony\Component\RateLimiter\Tests\Resources\DummyWindow;
1920
use Symfony\Component\RateLimiter\TokenBucket;
2021
use Symfony\Component\RateLimiter\TokenBucketLimiter;
2122

@@ -86,6 +87,15 @@ public function testConsume()
8687
$this->assertEqualsWithDelta(time(), $limit->getRetryAfter()->getTimestamp(), 1);
8788
}
8889

90+
public function testWrongWindowFromCache()
91+
{
92+
$this->storage->save(new DummyWindow());
93+
$limiter = $this->createLimiter();
94+
$limit = $limiter->consume();
95+
$this->assertTrue($limit->isAccepted());
96+
$this->assertEquals(9, $limit->getRemainingTokens());
97+
}
98+
8999
private function createLimiter($initialTokens = 10, Rate $rate = null)
90100
{
91101
return new TokenBucketLimiter('test', $initialTokens, $rate ?? Rate::perSecond(10), $this->storage);

‎src/Symfony/Component/RateLimiter/TokenBucketLimiter.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public function reserve(int $tokens = 1, ?float $maxTime = null): Reservation
6363

6464
try {
6565
$bucket = $this->storage->fetch($this->id);
66-
if (null === $bucket) {
66+
if (!$bucket instanceof TokenBucket) {
6767
$bucket = new TokenBucket($this->id, $this->maxBurst, $this->rate);
6868
}
6969

@@ -106,7 +106,7 @@ public function reserve(int $tokens = 1, ?float $maxTime = null): Reservation
106106
public function consume(int $tokens = 1): Limit
107107
{
108108
$bucket = $this->storage->fetch($this->id);
109-
if (null === $bucket) {
109+
if (!$bucket instanceof TokenBucket) {
110110
$bucket = new TokenBucket($this->id, $this->maxBurst, $this->rate);
111111
}
112112
$now = microtime(true);

0 commit comments

Comments
 (0)
0