bug #49070 [RateLimiter] fix incorrect retryAfter of FixedWindow (Rob…

…ertMe)

This PR was merged into the 5.4 branch.

Discussion
----------

[RateLimiter] fix incorrect retryAfter of FixedWindow

| Q             | A
| ------------- | ---
| Branch?       |5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |
<!--
Replace this notice by a short README for your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->
A FixedWindow always ends `intervalInSeconds` after the start time (`timer`). So when calculating the time to consume some tokens the tokens will always be available at `timer + intervalInSeconds`. But currently this is reported incorrectly as `calculateTimeForTokens()` calculates the time based on the desired amount of tokens (and cycles) while it doesn't take into account `maxSize` amount of tokens become available at the windows end.

Furthermore calculating the amount of needed cycles is incorrect. This as all tokens become available at once (at the windows end) and you can't consume more tokens than `maxSize` (which is validated at the start of `FixedWindowLimiter::reserve`, in case of `tokens > limit` it throws).

**Note:** I don't think that changing the signature of `calculateTimeForTokens` is a deprecation. This as the `Window` class is marked as ``@internal``. So it should only be used by the `RateLimiter` component.

Commits
-------

2316932 [RateLimiter] fix incorrect retryAfter of FixedWindow

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

@@ -68,7 +68,7 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation
 
                 $reservation = new Reservation($now, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit));
             } else {
-                $waitDuration = $window->calculateTimeForTokens($tokens);
+                $waitDuration = $window->calculateTimeForTokens($tokens, $now);
 
                 if (null !== $maxTime && $waitDuration > $maxTime) {
                     // process needs to wait longer than set interval

src/Symfony/Component/RateLimiter/Policy/Window.php

@@ -75,15 +75,13 @@ public function getAvailableTokens(float $now)
         return $this->maxSize - $this->hitCount;
     }
 
-    public function calculateTimeForTokens(int $tokens): int
+    public function calculateTimeForTokens(int $tokens, float $now): int
     {
         if (($this->maxSize - $this->hitCount) >= $tokens) {
             return 0;
         }
 
-        $cyclesRequired = ceil($tokens / $this->maxSize);
-
-        return $cyclesRequired * $this->intervalInSeconds;
+        return (int) ceil($this->timer + $this->intervalInSeconds - $now);
     }
 
     public function __serialize(): array

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

@@ -37,6 +37,7 @@ protected function setUp(): void
 
     public function testConsume()
     {
+        $now = time();
         $limiter = $this->createLimiter();
 
         // fill 9 tokens in 45 seconds
@@ -51,6 +52,9 @@ public function testConsume()
         $rateLimit = $limiter->consume();
         $this->assertFalse($rateLimit->isAccepted());
         $this->assertSame(10, $rateLimit->getLimit());
+        // Window ends after 1 minute
+        $retryAfter = \DateTimeImmutable::createFromFormat('U', $now + 60);
+        $this->assertEquals($retryAfter, $rateLimit->getRetryAfter());
     }
 
     /**