Skip to content

Commit

Permalink
bug #53282 [RateLimiter] Fix RateLimit->getRetryAfter() return value …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
nicolas-grekas committed Dec 30, 2023
2 parents b83b5f7 + 677b8b7 commit 4e10a49
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
Expand Up @@ -65,12 +65,18 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
$now = microtime(true);
$hitCount = $window->getHitCount();
$availableTokens = $this->getAvailableTokens($hitCount);
if (0 === $tokens) {
$resetDuration = $window->calculateTimeForTokens($this->limit, $window->getHitCount());
$resetTime = \DateTimeImmutable::createFromFormat('U', $availableTokens ? floor($now) : floor($now + $resetDuration));

return new Reservation($now, new RateLimit($availableTokens, $resetTime, true, $this->limit));
}
if ($availableTokens >= $tokens) {
$window->add($tokens);

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

if (null !== $maxTime && $waitDuration > $maxTime) {
// process needs to wait longer than set interval
Expand Down
Expand Up @@ -52,6 +52,7 @@ public function testConsume()
$rateLimit = $limiter->consume(10);
$this->assertTrue($rateLimit->isAccepted());
$this->assertSame(10, $rateLimit->getLimit());
$this->assertSame(0, $rateLimit->getRemainingTokens());
}

public function testWaitIntervalOnConsumeOverLimit()
Expand All @@ -76,6 +77,37 @@ public function testReserve()

// 2 over the limit, causing the WaitDuration to become 2/10th of the 12s interval
$this->assertEqualsWithDelta(12 / 5, $limiter->reserve(4)->getWaitDuration(), 1);

$limiter->reset();
$this->assertEquals(0, $limiter->reserve(10)->getWaitDuration());
}

public function testPeekConsume()
{
$limiter = $this->createLimiter();

$limiter->consume(9);

// peek by consuming 0 tokens twice (making sure peeking doesn't claim a token)
for ($i = 0; $i < 2; ++$i) {
$rateLimit = $limiter->consume(0);
$this->assertTrue($rateLimit->isAccepted());
$this->assertSame(10, $rateLimit->getLimit());
$this->assertEquals(
\DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true))),
$rateLimit->getRetryAfter()
);
}

$limiter->consume();

$rateLimit = $limiter->consume(0);
$this->assertEquals(0, $rateLimit->getRemainingTokens());
$this->assertTrue($rateLimit->isAccepted());
$this->assertEquals(
\DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true) + 12)),
$rateLimit->getRetryAfter()
);
}

private function createLimiter(): SlidingWindowLimiter
Expand Down

0 comments on commit 4e10a49

Please sign in to comment.