8000 bug #42168 [RateLimiter] Fix wait duration for fixed window policy (j… · symfony/symfony@a972a6a · GitHub
[go: up one dir, main page]
Skip to content

Commit a972a6a

Browse files
fabpotfabpot
committed
bug #42168 [RateLimiter] Fix wait duration for fixed window policy (jlekowski)
This PR was merged into the 5.3 branch. Discussion ---------- [RateLimiter] Fix wait duration for fixed window policy | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> When using `fixed_window` policy and calling `->wait()` after overconsuming, there might be no `sleep()` called. Reproducer below: ```php <?php use Symfony\Component\RateLimiter\RateLimiterFactory; use Symfony\Component\RateLimiter\Storage\InMemoryStorage; require_once './vendor/autoload.php'; $factory = new RateLimiterFactory([ 'id' => 'some_id', 'policy' => 'fixed_window', 'limit' => 10, 'interval' => '2 seconds', ], new InMemoryStorage()); $limiter = $factory->create(); $limit = $limiter->consume(8); $limit = $limiter->consume(4); $start = microtime(true); $limit->wait(); echo 'WAITED: ' . 1000 * (microtime(true) - $start); // WAITED: 0.0030994415283203 - instantaneous instead of WAITED: 2000.3020763397 after the fix ``` Commits ------- f4b5d8a Fix wait duration for fixed window policy
2 parents f7319e6 + f4b5d8a commit a972a6a

File tree

4 files changed

+49
-2
lines changed

4 files changed

+49
-2
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
7070

7171
$reservation = new Reservation($now, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit));
7272
} else {
73-
$remainingTokens = $tokens - $availableTokens;
74-
$waitDuration = $window->calculateTimeForTokens($remainingTokens);
73+
$waitDuration = $window->calculateTimeForTokens($tokens);
7574

7675
if (null !== $maxTime && $waitDuration > $maxTime) {
7776
// process needs to wait longer than set interval

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\PhpUnit\ClockMock;
1616
use Symfony\Component\RateLimiter\Policy\FixedWindowLimiter;
17+
use Symfony\Component\RateLimiter\RateLimit;
1718
use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
1819
use Symfony\Component\RateLimiter\Tests\Resources\DummyWindow;
1920

@@ -29,6 +30,7 @@ protected function setUp(): void
2930
$this->storage = new InMemoryStorage();
3031

3132
ClockMock::register(InMemoryStorage::class);
33+
ClockMock::register(RateLimit::class);
3234
}
3335

3436
public function testConsume()
@@ -65,6 +67,20 @@ public function testConsumeOutsideInterval()
6567
$this->assertTrue($rateLimit->isAccepted());
6668
}
6769

70+
public function testWaitIntervalOnConsumeOverLimit()
71+
{
72+
$limiter = $this->createLimiter();
73+
74+
// initial consume
75+
$limiter->consume(8);
76+
// consumer over the limit
77+
$rateLimit = $limiter->consume(4);
78+
79+
$start = microtime(true);
80+
$rateLimit->wait(); // wait 1 minute
81+
$this->assertEqualsWithDelta($start + 60, microtime(true), 0.5);
82+
}
83+
6884
public function testWrongWindowFromCache()
6985
{
7086
$this->storage->save(new DummyWindow());

‎src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php

Lines changed: 16 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\Exception\ReserveNotSupportedException;
1717
use Symfony\Component\RateLimiter\Policy\SlidingWindowLimiter;
18+
use Symfony\Component\RateLimiter\RateLimit;
1819
use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
1920

2021
/**
@@ -29,6 +30,7 @@ protected function setUp(): void
2930
$this->storage = new InMemoryStorage();
3031

3132
ClockMock::register(InMemoryStorage::class);
33+
ClockMock::register(RateLimit::class);
3234
}
3335

3436
public function testConsume()
@@ -53,6 +55,20 @@ public function testConsume()
5355
$this->assertSame(10, $rateLimit->getLimit());
5456
}
5557

58+
public function testWaitIntervalOnConsumeOverLimit()
59+
{
60+
$limiter = $this->createLimiter();
61+
62+
// initial consume
63+
$limiter->consume(8);
64+
// consumer over the limit
65+
$rateLimit = $limiter->consume(4);
66+
67+
$start = microtime(true);
68+
$rateLimit->wait(); // wait 12 seconds
69+
$this->assertEqualsWithDelta($start + 12, microtime(true), 0.5);
70+
}
71+
5672
public function testReserve()
5773
{
5874
$this->expectException(ReserveNotSupportedException::class);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\RateLimiter\Policy\Rate;
1818
use Symfony\Component\RateLimiter\Policy\TokenBucket;
1919
use Symfony\Component\RateLimiter\Policy\TokenBucketLimiter;
20+
use Symfony\Component\RateLimiter\RateLimit;
2021
use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
2122
use Symfony\Component\RateLimiter\Tests\Resources\DummyWindow;
2223

@@ -34,6 +35,7 @@ protected function setUp(): void
3435
ClockMock::register(TokenBucketLimiter::class);
3536
ClockMock::register(InMemoryStorage::class);
3637
ClockMock::register(TokenBucket::class);
38+
ClockMock::register(RateLimit::class);
3739
}
3840

3941
public function testReserve()
@@ -89,6 +91,20 @@ public function testConsume()
8991
$this->assertSame(10, $rateLimit->getLimit());
9092
}
9193

94+
public function testWaitIntervalOnConsumeOverLimit()
95+
{
96+
$limiter = $this->createLimiter();
97+
98+
// initial consume
99+
$limiter->consume(8);
100+
// consumer over the limit
101+
$rateLimit = $limiter->consume(4);
102+
103+
$start = microtime(true);
104+
$rateLimit->wait(); // wait 1 second
105+
$this->assertEqualsWithDelta($start + 1, microtime(true), 0.5);
106+
}
107+
92108
public function testWrongWindowFromCache()
93109
{
94110
$this->storage->save(new DummyWindow());

0 commit comments

Comments
 (0)
0