8000 bug #53282 [RateLimiter] Fix RateLimit->getRetryAfter() return value … · symfony/symfony@4e10a49 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4e10a49

Browse files
nicolas-grekasnicolas-grekas
committed
bug #53282 [RateLimiter] Fix RateLimit->getRetryAfter() return value when consuming 0 or last tokens (wouterj, ERuban)
This PR was merged into the 6.4 branch. Discussion ---------- [RateLimiter] Fix RateLimit->getRetryAfter() return value when consuming 0 or last tokens | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Replaces #52835 Original description: > Have got some BC after updating the project to Symfony 6.4 (after that PR #51676). > > Sometimes I need to get `RateLimit` object without consuming just before consuming a try, in example: > ```php > $rateLimit = $limiter->consume(0); > if ($rateLimit->getRemainingTokens() === 0) { > throw new SomeException($rateLimit->getRetryAfter()); > } > ... > $limiter->consume(1) > ... > ``` > and found that in that case `$rateLimit->getRetryAfter()` always returns `now`. > > So this PR fixes it. Commits ------- 677b8b7 [RateLimit] Allow to get RateLimit without consuming again 169e383 Reintroduce peek consume test for sliding window policy
2 parents b83b5f7 + 677b8b7 commit 4e10a49

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,18 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
6565
$now = microtime(true);
6666
$hitCount = $window->getHitCount();
6767
$availableTokens = $this->getAvailableTokens($hitCount);
68+
if (0 === $tokens) {
69+
$resetDuration = $window->calculateTimeForTokens($this->limit, $window->getHitCount());
70+
$resetTime = \DateTimeImmutable::createFromFormat('U', $availableTokens ? floor($now) : floor($now + $resetDuration));
71+
72+
return new Reservation($now, new RateLimit($availableTokens, $resetTime, true, $this->limit));
73+
}
6874
if ($availableTokens >= $tokens) {
6975
$window->add($tokens);
7076

7177
$reservation = new Reservation($now, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit));
7278
} else {
73-
$waitDuration = $window->calculateTimeForTokens($this->limit, max(1, $tokens));
79+
$waitDuration = $window->calculateTimeForTokens($this->limit, $tokens);
7480

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

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public function testConsume()
5252
$rateLimit = $limiter->consume(10);
5353
$this->assertTrue($rateLimit->isAccepted());
5454
$this->assertSame(10, $rateLimit->getLimit());
55+
$this->assertSame(0, $rateLimit->getRemainingTokens());
5556
}
5657

5758
public function testWaitIntervalOnConsumeOverLimit()
@@ -76,6 +77,37 @@ public function testReserve()
7677

7778
// 2 over the limit, causing the WaitDuration to become 2/10th of the 12s interval
7879
$this->assertEqualsWithDelta(12 / 5, $limiter->reserve(4)->getWaitDuration(), 1);
80+
81+
$limiter->reset();
82+
$this->assertEquals(0, $limiter->reserve(10)->getWaitDuration());
83+
}
84+
85+
public function testPeekConsume()
86+
{
87+
$limiter = $this->createLimiter();
88+
89+
$limiter->consume(9);
90+
91+
// peek by consuming 0 tokens twice (making sure peeking doesn't claim a token)
92+
for ($i = 0; $i < 2; ++$i) {
93+
$rateLimit = $limiter->consume(0);
94+
$this->assertTrue($rateLimit->isAccepted());
95+
$this->assertSame(10, $rateLimit->getLimit());
96+
$this->assertEquals(
97+
\DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true))),
98+
$rateLimit->getRetryAfter()
99+
);
100+
}
101+
102+
$limiter->consume();
103+
104+
$rateLimit = $limiter->consume(0);
105+
$this->assertEquals(0, $rateLimit->getRemainingTokens());
106+
$this->assertTrue($rateLimit->isAccepted());
107+
$this->assertEquals(
108+
\DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true) + 12)),
109+
$rateLimit->getRetryAfter()
110+
);
79111
}
80112

81113
private function createLimiter(): SlidingWindowLimiter

0 commit comments

Comments
 (0)
0