Improve readme + cleanup + test coverage 100% + psalm level 1 (#14)

yiisoft · Jun 8, 2021 · 4444c0a · 4444c0a
1 parent ed97dc2
commit 4444c0a
Show file tree

Hide file tree

Showing 11 changed files with 121 additions and 66 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -12,3 +12,6 @@ trim_trailing_whitespace = true
 
 [*.md]
 trim_trailing_whitespace = false
+
+[*.yml]
+indent_size = 2
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,10 +1,9 @@
-# RateLimiter Change Log
-
+# Yii Rate Limiter Middleware Change Log
 
 ## 1.0.1 under development
 
-- no changes in this release.
+- Bug #14: Throw exception on call `getCacheKey()` in counter without the specified ID (vjik)
 
 ## 1.0.0 June 07, 2021
 
-- Initial release.
+- Initial release.
diff --git a/README.md b/README.md
@@ -2,16 +2,10 @@
     <a href="https://github.com/yiisoft" target="_blank">
         <img src="https://yiisoft.github.io/docs/images/yii_logo.svg" height="100px">
     </a>
-    <h1 align="center">Yii RateLimiter Middleware</h1>
+    <h1 align="center">Yii Rate Limiter Middleware</h1>
     <br>
 </p>
 
-RateLimiter helps to prevent abuse by limiting the number of requests that could be me made consequentially.
-
-For example, you may want to limit the API usage of each user to be at most 100 API calls within a period of 10 minutes.
-If too many requests are received from a user within the stated period of the time, a response with status code 429
-(meaning "Too Many Requests") should be returned.
-
 [![Latest Stable Version](https://poser.pugx.org/yiisoft/rate-limiter/v/stable.png)](https://packagist.org/packages/yiisoft/rate-limiter)
 [![Total Downloads](https://poser.pugx.org/yiisoft/rate-limiter/downloads.png)](https://packagist.org/packages/yiisoft/rate-limiter)
 [![Build status](https://github.com/yiisoft/rate-limiter/workflows/build/badge.svg)](https://github.com/yiisoft/rate-limiter/actions?query=workflow%3Abuild)
@@ -21,12 +15,22 @@ If too many requests are received from a user within the stated period of the ti
 [![static analysis](https://github.com/yiisoft/rate-limiter/workflows/static%20analysis/badge.svg)](https://github.com/yiisoft/rate-limiter/actions?query=workflow%3A%22static+analysis%22)
 [![type-coverage](https://shepherd.dev/github/yiisoft/rate-limiter/coverage.svg)](https://shepherd.dev/github/yiisoft/rate-limiter)
 
+Rate limiter middleware helps to prevent abuse by limiting the number of requests that could be me made consequentially.
+
+For example, you may want to limit the API usage of each user to be at most 100 API calls within a period of 10 minutes.
+If too many requests are received from a user within the stated period of the time, a response with status code 429
+(meaning "Too Many Requests") should be returned.
+
+## Requirements
+
+- PHP 7.4 or higher.
+
 ## Installation
 
 The package could be installed with composer:
 
-```
-composer install yiisoft/rate-limiter
+```shell
+composer install yiisoft/rate-limiter --prefer-dist
 ```
 
 ## General usage
@@ -54,6 +58,8 @@ that ensures that after reaching the limit further increments are distributed eq
 > or another webserver capabilities for rate limiting. This package allows rate-limiting in the project with deployment
 > environment you cannot control such as installable CMS. 
 
+## Testing
+
 ### Unit testing
 
 The package is tested with [PHPUnit](https://phpunit.de/). To run tests:
@@ -64,10 +70,11 @@ The package is tested with [PHPUnit](https://phpunit.de/). To run tests:
 
 ### Mutation testing
 
-The package tests are checked with [Infection](https://infection.github.io/) mutation framework. To run it:
+The package tests are checked with [Infection](https://infection.github.io/) mutation framework with
+[Infection Static Analysis Plugin](https://github.com/Roave/infection-static-analysis-plugin). To run it:
 
 ```shell
-./vendor/bin/infection
+./vendor/bin/roave-infection-static-analysis-plugin
 ```
 
 ### Static analysis
@@ -78,21 +85,21 @@ The code is statically analyzed with [Psalm](https://psalm.dev/). To run static
 ./vendor/bin/psalm
 ```
 
-### Support the project
+## License
+
+The Yii Rate Limiter Middleware is free software. It is released under the terms of the BSD License.
+Please see [`LICENSE`](./LICENSE.md) for more information.
+
+Maintained by [Yii Software](https://www.yiiframework.com/).
+
+## Support the project
 
 [![Open Collective](https://img.shields.io/badge/Open%20Collective-sponsor-7eadf1?logo=open%20collective&logoColor=7eadf1&labelColor=555555)](https://opencollective.com/yiisoft)
 
-### Follow updates
+## Follow updates
 
 [![Official website](https://img.shields.io/badge/Powered_by-Yii_Framework-green.svg?style=flat)](https://www.yiiframework.com/)
 [![Twitter](https://img.shields.io/badge/twitter-follow-1DA1F2?logo=twitter&logoColor=1DA1F2&labelColor=555555?style=flat)](https://twitter.com/yiiframework)
 [![Telegram](https://img.shields.io/badge/telegram-join-1DA1F2?style=flat&logo=telegram)](https://t.me/yii3en)
 [![Facebook](https://img.shields.io/badge/facebook-join-1DA1F2?style=flat&logo=facebook&logoColor=ffffff)](https://www.facebook.com/groups/yiitalk)
 [![Slack](https://img.shields.io/badge/slack-join-1DA1F2?style=flat&logo=slack)](https://yiiframework.com/go/slack)
-
-## License
-
-The Yii RateLimiter Middleware is free software. It is released under the terms of the BSD License.
-Please see [`LICENSE`](./LICENSE.md) for more information.
-
-Maintained by [Yii Software](https://www.yiiframework.com/).
diff --git a/composer.json b/composer.json
@@ -1,7 +1,7 @@
 {
     "name": "yiisoft/rate-limiter",
     "type": "library",
-    "description": "Yii Framework RateLimiter middleware",
+    "description": "Yii Rate Limiter Middleware",
     "keywords": [
         "yii",
         "rate limiter",

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -16,7 +16,7 @@
     </php>
 
     <testsuites>
-        <testsuite name="RateLimiter">
+        <testsuite name="Yii Rate Limiter Middleware tests">
             <directory>./tests</directory>
         </testsuite>
     </testsuites>

diff --git a/psalm.xml b/psalm.xml
@@ -1,15 +1,11 @@
 <?xml version="1.0"?>
 <psalm
-    errorLevel="3"
-    resolveFromConfigFile="true"
+    errorLevel="1"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns="https://getpsalm.org/schema/config"
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
 >
     <projectFiles>
         <directory name="src" />
-        <ignoreFiles>
-            <directory name="vendor" />
-        </ignoreFiles>
     </projectFiles>
 </psalm>
diff --git a/src/Counter.php b/src/Counter.php
@@ -41,16 +41,9 @@ final class Counter implements CounterInterface
 
     private int $ttlInSeconds = self::DEFAULT_TTL;
 
-    /**
-     * @var int Last increment time.
-     * In GCRA it's known as arrival time.
-     */
-    private int $lastIncrementTimeInMilliseconds;
-
     /**
      * @param int $limit Maximum number of increments that could be performed before increments are limited.
      * @param int $periodInSeconds Period to apply limit to.
-     * @param CacheInterface $storage
      */
     public function __construct(int $limit, int $periodInSeconds, CacheInterface $storage)
     {
@@ -89,6 +82,10 @@ public function setTtlInSeconds(int $secondsTTL): void
      */
     public function getCacheKey(): string
     {
+        if ($this->id === null) {
+            throw new LogicException('The counter ID should be set.');
+        }
+
         return self::ID_PREFIX . $this->id;
     }
 
@@ -98,11 +95,15 @@ public function incrementAndGetState(): CounterState
             throw new LogicException('The counter ID should be set.');
         }
 
-        $this->lastIncrementTimeInMilliseconds = $this->currentTimeInMilliseconds();
+        // Last increment time.
+        // In GCRA it's known as arrival time.
+        $lastIncrementTimeInMilliseconds = $this->currentTimeInMilliseconds();
+
         $theoreticalNextIncrementTime = $this->calculateTheoreticalNextIncrementTime(
-            $this->getLastStoredTheoreticalNextIncrementTime()
+            $lastIncrementTimeInMilliseconds,
+            $this->getLastStoredTheoreticalNextIncrementTime($lastIncrementTimeInMilliseconds)
         );
-        $remaining = $this->calculateRemaining($theoreticalNextIncrementTime);
+        $remaining = $this->calculateRemaining($lastIncrementTimeInMilliseconds, $theoreticalNextIncrementTime);
         $resetAfter = $this->calculateResetAfter($theoreticalNextIncrementTime);
 
         if ($remaining >= 1) {
@@ -113,31 +114,33 @@ public function incrementAndGetState(): CounterState
     }
 
     /**
-     * @param float $storedTheoreticalNextIncrementTime
-     *
-     * @return float Theoretical increment time that would be expected from equally spaced increments at exactly rate limit.
-     * In GCRA it is known as TAT, theoretical arrival time.
+     * @return float Theoretical increment time that would be expected from equally spaced increments at exactly rate
+     * limit. In GCRA it is known as TAT, theoretical arrival time.
      */
-    private function calculateTheoreticalNextIncrementTime(float $storedTheoreticalNextIncrementTime): float
-    {
-        return max($this->lastIncrementTimeInMilliseconds, $storedTheoreticalNextIncrementTime) + $this->incrementIntervalInMilliseconds;
+    private function calculateTheoreticalNextIncrementTime(
+        int $lastIncrementTimeInMilliseconds,
+        float $storedTheoreticalNextIncrementTime
+    ): float {
+        return max($lastIncrementTimeInMilliseconds, $storedTheoreticalNextIncrementTime) +
+            $this->incrementIntervalInMilliseconds;
     }
 
     /**
-     * @param float $theoreticalNextIncrementTime
-     *
      * @return int The number of remaining requests in the current time period.
      */
-    private function calculateRemaining(float $theoreticalNextIncrementTime): int
+    private function calculateRemaining(int $lastIncrementTimeInMilliseconds, float $theoreticalNextIncrementTime): int
     {
         $incrementAllowedAt = $theoreticalNextIncrementTime - $this->periodInMilliseconds;
 
-        return (int)(round($this->lastIncrementTimeInMilliseconds - $incrementAllowedAt) / $this->incrementIntervalInMilliseconds);
+        return (int)(
+            round($lastIncrementTimeInMilliseconds - $incrementAllowedAt) /
+            $this->incrementIntervalInMilliseconds
+        );
     }
 
-    private function getLastStoredTheoreticalNextIncrementTime(): float
+    private function getLastStoredTheoreticalNextIncrementTime(int $lastIncrementTimeInMilliseconds): float
     {
-        return $this->storage->get($this->getCacheKey(), (float)$this->lastIncrementTimeInMilliseconds);
+        return (float)$this->storage->get($this->getCacheKey(), $lastIncrementTimeInMilliseconds);
     }
 
     private function storeTheoreticalNextIncrementTime(float $theoreticalNextIncrementTime): void
@@ -146,8 +149,6 @@ private function storeTheoreticalNextIncrementTime(float $theoreticalNextIncreme
     }
 
     /**
-     * @param float $theoreticalNextIncrementTime
-     *
      * @return int Timestamp to wait until the rate limit resets.
      */
     private function calculateResetAfter(float $theoreticalNextIncrementTime): int

diff --git a/src/CounterInterface.php b/src/CounterInterface.php
@@ -17,8 +17,6 @@ public function setId(string $id): void;
 
     /**
      * Counts one request as done and returns object containing current counter state.
-     *
-     * @return CounterState
      */
     public function incrementAndGetState(): CounterState;
 }
diff --git a/src/Middleware.php b/src/Middleware.php
@@ -14,20 +14,23 @@
 /**
  * RateLimiter helps to prevent abuse by limiting the number of requests that could be me made consequentially.
  *
- * For example, you may want to limit the API usage of each user to be at most 100 API calls within a period of 10 minutes.
- * If too many requests are received from a user within the stated period of the time, a response with status code 429
- * (meaning "Too Many Requests") should be returned.
+ * For example, you may want to limit the API usage of each user to be at most 100 API calls within a period of 10
+ * minutes. If too many requests are received from a user within the stated period of the time, a response with status
+ * code 429 (meaning "Too Many Requests") should be returned.
+ *
+ * @psalm-type CounterIdCallback = callable(ServerRequestInterface):string
  */
 final class Middleware implements MiddlewareInterface
 {
     private CounterInterface $counter;
 
     private ResponseFactoryInterface $responseFactory;
 
-    private ?string $counterId;
+    private ?string $counterId = null;
 
     /**
      * @var callable|null
+     * @psalm-var CounterIdCallback|null
      */
     private $counterIdCallback;
 
@@ -55,7 +58,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
      * @param callable|null $callback Callback to use for generating counter ID. Counters with non-equal IDs
      * are counted separately.
      *
-     * @return self
+     * @psalm-param CounterIdCallback $callback
      */
     public function withCounterIdCallback(?callable $callback): self
     {
@@ -67,8 +70,6 @@ public function withCounterIdCallback(?callable $callback): self
 
     /**
      * @param string $id Counter ID. Counters with non-equal IDs are counted separately.
-     *
-     * @return self
      */
     public function withCounterId(string $id): self
     {

diff --git a/tests/CounterTest.php b/tests/CounterTest.php
@@ -5,6 +5,7 @@
 namespace Yiisoft\Yii\RateLimiter\Tests;
 
 use InvalidArgumentException;
+use LogicException;
 use PHPUnit\Framework\TestCase;
 use Yiisoft\Cache\ArrayCache;
 use Yiisoft\Yii\RateLimiter\Counter;
@@ -43,10 +44,16 @@ public function testStatisticsShouldBeCorrectWhenLimitIsReached(): void
 
     public function testShouldNotBeAbleToSetInvalidId(): void
     {
-        $this->expectException(\LogicException::class);
+        $this->expectException(LogicException::class);
         (new Counter(10, 60, new ArrayCache()))->incrementAndGetState();
     }
 
+    public function testGetCacheKeyShouldFailWithoutId(): void
+    {
+        $this->expectException(LogicException::class);
+        (new Counter(10, 60, new ArrayCache()))->getCacheKey();
+    }
+
     public function testShouldNotBeAbleToSetInvalidLimit(): void
     {
         $this->expectException(InvalidArgumentException::class);
@@ -74,4 +81,19 @@ public function testIncrementMustBeUniformAfterLimitIsReached(): void
             $this->assertEquals(1, $statistics->getRemaining());
         }
     }
+
+    public function testCustomTtl(): void
+    {
+        $cache = new ArrayCache();
+
+        $counter = new Counter(1, 1, $cache);
+        $counter->setId('test');
+        $counter->setTtlInSeconds(1);
+
+        $counter->incrementAndGetState();
+
+        sleep(2);
+
+        self::assertNull($cache->get($counter->getCacheKey()));
+    }
 }