diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a8e9da..873e568 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,20 +10,12 @@ jobs: strategy: fail-fast: false matrix: - php: [ "7.2", "7.3", "7.4", "8.0", "8.1", "8.2" ] + php: [ "8.0", "8.1", "8.2" ] composer_flags: [ "", "--prefer-lowest" ] - symfony_version: [ "^3.4", "^4.4", "^5.4", "^6.3"] + symfony_version: [ "^5.4", "^6.1"] exclude: - - php: "7.2" - symfony_version: "^6.3" - - php: "7.3" - symfony_version: "^6.3" - - php: "7.4" - symfony_version: "^6.3" - php: "8.0" - symfony_version: "^6.3" - - php: "8.2" - symfony_version: "^4.4" + symfony_version: "^6.1" name: PHP ${{ matrix.php }} SF ${{ matrix.symfony_version }} ${{ matrix.composer_flags}} env: PHP: ${{ matrix.os }} @@ -40,6 +32,8 @@ jobs: ini-values: memory_limit=256M,post_max_size=256M - name: Checkout ratelimit bundle uses: actions/checkout@v2 + with: + fetch-depth: 2 - name: Install dependencies run: | composer self-update @@ -48,4 +42,9 @@ jobs: COMPOSER_MEMORY_LIMIT=-1 composer update --prefer-dist --no-interaction $COMPOSER_FLAGS - name: Run tests run: | - SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/simple-phpunit + SYMFONY_DEPRECATIONS_HELPER=weak vendor/bin/simple-phpunit --coverage-text --coverage-clover=coverage.clover + - name: Upload coverage + if: ${{ matrix.php == '8.1' && github.repository == 'jaytaph/RateLimitBundle' }} + uses: sudo-bot/action-scrutinizer@latest + with: + cli-args: "--format=php-clover coverage.clover" diff --git a/Annotation/RateLimit.php b/Annotation/RateLimit.php deleted file mode 100644 index 022c04d..0000000 --- a/Annotation/RateLimit.php +++ /dev/null @@ -1,118 +0,0 @@ -limit; - } - - /** - * @param int $limit - */ - public function setLimit($limit) - { - $this->limit = $limit; - } - - /** - * @return array - */ - public function getMethods() - { - return $this->methods; - } - - /** - * @param array $methods - */ - public function setMethods($methods) - { - $this->methods = (array) $methods; - } - - /** - * @return int - */ - public function getPeriod() - { - return $this->period; - } - - /** - * @param int $period - */ - public function setPeriod($period) - { - $this->period = $period; - } - - /** - * @return mixed - */ - public function getPayload() - { - return $this->payload; - } - - /** - * @param mixed $payload - */ - public function setPayload($payload) - { - $this->payload = $payload; - } - -} diff --git a/Attribute/RateLimit.php b/Attribute/RateLimit.php new file mode 100644 index 0000000..a4cf08a --- /dev/null +++ b/Attribute/RateLimit.php @@ -0,0 +1,78 @@ +methods = [$methods]; + } else { + $this->methods = $methods; + } + } + + public function getLimit(): int + { + return $this->limit; + } + + public function setLimit(int $limit): void + { + $this->limit = $limit; + } + + public function getMethods(): array + { + return $this->methods; + } + + public function setMethods($methods): void + { + $this->methods = (array) $methods; + } + + public function getPeriod(): int + { + return $this->period; + } + + public function setPeriod(int $period): void + { + $this->period = $period; + } + + public function getPayload(): mixed + { + return $this->payload; + } + + public function setPayload(mixed $payload): void + { + $this->payload = $payload; + } +} diff --git a/DependencyInjection/NoxlogicRateLimitExtension.php b/DependencyInjection/NoxlogicRateLimitExtension.php index f23404e..8042930 100755 --- a/DependencyInjection/NoxlogicRateLimitExtension.php +++ b/DependencyInjection/NoxlogicRateLimitExtension.php @@ -18,7 +18,7 @@ class NoxlogicRateLimitExtension extends Extension /** * {@inheritDoc} */ - public function load(array $configs, ContainerBuilder $container) + public function load(array $configs, ContainerBuilder $container): void { $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); @@ -26,7 +26,7 @@ public function load(array $configs, ContainerBuilder $container) } - private function loadServices(ContainerBuilder $container, array $config) + private function loadServices(ContainerBuilder $container, array $config): void { $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.xml'); diff --git a/EventListener/RateLimitAnnotationListener.php b/EventListener/RateLimitAnnotationListener.php index 1461614..662f295 100644 --- a/EventListener/RateLimitAnnotationListener.php +++ b/EventListener/RateLimitAnnotationListener.php @@ -2,44 +2,29 @@ namespace Noxlogic\RateLimitBundle\EventListener; -use Noxlogic\RateLimitBundle\Annotation\RateLimit; +use Noxlogic\RateLimitBundle\Attribute\RateLimit; use Noxlogic\RateLimitBundle\Events\CheckedRateLimitEvent; use Noxlogic\RateLimitBundle\Events\GenerateKeyEvent; use Noxlogic\RateLimitBundle\Events\RateLimitEvents; use Noxlogic\RateLimitBundle\Exception\RateLimitExceptionInterface; use Noxlogic\RateLimitBundle\Service\RateLimitService; use Noxlogic\RateLimitBundle\Util\PathLimitProcessor; -use Symfony\Component\EventDispatcher\EventDispatcherInterface as LegacyEventDispatcherInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\ControllerEvent; -use Symfony\Component\HttpKernel\Event\FilterControllerEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; class RateLimitAnnotationListener extends BaseListener { + protected EventDispatcherInterface $eventDispatcher; - /** - * @var EventDispatcherInterface | LegacyEventDispatcherInterface - */ - protected $eventDispatcher; - - /** - * @var \Noxlogic\RateLimitBundle\Service\RateLimitService - */ - protected $rateLimitService; + protected RateLimitService $rateLimitService; - /** - * @var \Noxlogic\RateLimitBundle\Util\PathLimitProcessor - */ - protected $pathLimitProcessor; + protected PathLimitProcessor $pathLimitProcessor; - /** - * @param RateLimitService $rateLimitService - */ public function __construct( - $eventDispatcher, + EventDispatcherInterface $eventDispatcher, RateLimitService $rateLimitService, PathLimitProcessor $pathLimitProcessor ) { @@ -48,10 +33,7 @@ public function __construct( $this->pathLimitProcessor = $pathLimitProcessor; } - /** - * @param ControllerEvent|FilterControllerEvent $event - */ - public function onKernelController($event) + public function onKernelController(ControllerEvent $event): void { // Skip if the bundle isn't enabled (for instance in test environment) if( ! $this->getParameter('enabled', true)) { @@ -59,25 +41,32 @@ public function onKernelController($event) } // Skip if we aren't the main request - if ($event->getRequestType() != HttpKernelInterface::MASTER_REQUEST) { + if ($event->getRequestType() !== HttpKernelInterface::MASTER_REQUEST) { return; } - // Find the best match - $annotations = $event->getRequest()->attributes->get('_x-rate-limit', array()); - $rateLimit = $this->findBestMethodMatch($event->getRequest(), $annotations); + // RateLimits used to be set by sensio/framework-extra-bundle by reading annotations + // Tests also use that mechanism, we should probably keep it for retrocompatibility + $request = $event->getRequest(); + if ($request->attributes->has('_x-rate-limit')) { + /** @var RateLimit[] $rateLimits */ + $rateLimits = $request->attributes->get('_x-rate-limit', []); + } else { + $rateLimits = $this->getRateLimitsFromAttributes($event->getController()); + } + $rateLimit = $this->findBestMethodMatch($request, $rateLimits); // Another treatment before applying RateLimit ? - $checkedRateLimitEvent = new CheckedRateLimitEvent($event->getRequest(), $rateLimit); - $this->dispatch(RateLimitEvents::CHECKED_RATE_LIMIT, $checkedRateLimitEvent); + $checkedRateLimitEvent = new CheckedRateLimitEvent($request, $rateLimit); + $this->eventDispatcher->dispatch($checkedRateLimitEvent, RateLimitEvents::CHECKED_RATE_LIMIT); $rateLimit = $checkedRateLimitEvent->getRateLimit(); - // No matching annotation found + // No matching RateLimit found if (! $rateLimit) { return; } - $key = $this->getKey($event, $rateLimit, $annotations); + $key = $this->getKey($event, $rateLimit, $rateLimits); // Ratelimit the call $rateLimitInfo = $this->rateLimitService->limitRate($key); @@ -93,7 +82,6 @@ public function onKernelController($event) // Store the current rating info in the request attributes - $request = $event->getRequest(); $request->attributes->set('rate_limit_info', $rateLimitInfo); // Reset the rate limits @@ -137,71 +125,59 @@ public function onKernelController($event) /** - * @param RateLimit[] $annotations + * @param RateLimit[] $rateLimits */ - protected function findBestMethodMatch(Request $request, array $annotations) + protected function findBestMethodMatch(Request $request, array $rateLimits): ?RateLimit { // Empty array, check the path limits - if (count($annotations) == 0) { + if (count($rateLimits) === 0) { return $this->pathLimitProcessor->getRateLimit($request); } $best_match = null; - foreach ($annotations as $annotation) { - // cast methods to array, even method holds a string - $methods = is_array($annotation->getMethods()) ? $annotation->getMethods() : array($annotation->getMethods()); - - if (in_array($request->getMethod(), $methods)) { - $best_match = $annotation; + foreach ($rateLimits as $rateLimit) { + if (in_array($request->getMethod(), $rateLimit->getMethods(), true)) { + $best_match = $rateLimit; } // Only match "default" annotation when we don't have a best match - if (count($annotation->getMethods()) == 0 && $best_match == null) { - $best_match = $annotation; + if ($best_match === null && count($rateLimit->methods) === 0) { + $best_match = $rateLimit; } } return $best_match; } - /** - * @param ControllerEvent|FilterControllerEvent $event - * @param RateLimit $rateLimit - * @param array $annotations - * @return string - */ - private function getKey($event, RateLimit $rateLimit, array $annotations) + /** @param RateLimit[] $rateLimits */ + private function getKey(ControllerEvent $event, RateLimit $rateLimit, array $rateLimits): string { // Let listeners manipulate the key - $keyEvent = new GenerateKeyEvent($event->getRequest(), '', $rateLimit->getPayload()); + $request = $event->getRequest(); + $keyEvent = new GenerateKeyEvent($request, '', $rateLimit->getPayload()); $rateLimitMethods = implode('.', $rateLimit->getMethods()); $keyEvent->addToKey($rateLimitMethods); - $rateLimitAlias = count($annotations) === 0 - ? str_replace('/', '.', $this->pathLimitProcessor->getMatchedPath($event->getRequest())) + $rateLimitAlias = count($rateLimits) === 0 + ? str_replace('/', '.', $this->pathLimitProcessor->getMatchedPath($request)) : $this->getAliasForRequest($event); $keyEvent->addToKey($rateLimitAlias); - - $this->dispatch(RateLimitEvents::GENERATE_KEY, $keyEvent); + $this->eventDispatcher->dispatch($keyEvent, RateLimitEvents::GENERATE_KEY); return $keyEvent->getKey(); } - /** - * @param string $route - * @param ControllerEvent|FilterControllerEvent $controller - * @return mixed|string - */ - private function getAliasForRequest($event) + private function getAliasForRequest(ControllerEvent $event): string { - if (($route = $event->getRequest()->attributes->get('_route'))) { + $route = $event->getRequest()->attributes->get('_route'); + if ($route) { return $route; } $controller = $event->getController(); - if (is_string($controller) && false !== strpos($controller, '::')) { + if (is_string($controller) && str_contains($controller, '::')) { $controller = explode('::', $controller); } @@ -220,15 +196,30 @@ private function getAliasForRequest($event) return 'other'; } - private function dispatch($eventName, $event) + /** + * @return RateLimit[] + */ + private function getRateLimitsFromAttributes(string|array|object $controller): array { - if ($this->eventDispatcher instanceof EventDispatcherInterface) { - // Symfony >= 4.3 - $this->eventDispatcher->dispatch($event, $eventName); + $rClass = $rMethod = null; + if (\is_array($controller) && method_exists(...$controller)) { + $rClass = new \ReflectionClass($controller[0]); + $rMethod = new \ReflectionMethod($controller[0], $controller[1]); + } elseif (\is_string($controller) && false !== $i = strpos($controller, '::')) { + $rClass = new \ReflectionClass(substr($controller, 0, $i)); + } elseif (\is_object($controller) && \is_callable([$controller, '__invoke'])) { + $rMethod = new \ReflectionMethod($controller, '__invoke'); } else { - // Symfony 3.4 - $this->eventDispatcher->dispatch($eventName, $event); + $rMethod = new \ReflectionFunction($controller); } - } + $attributes = []; + foreach (array_merge($rClass?->getAttributes() ?? [], $rMethod?->getAttributes() ?? []) as $attribute) { + if (RateLimit::class === $attribute->getName()) { + $attributes[] = $attribute->newInstance(); + } + } + + return $attributes; + } } diff --git a/Events/AbstractEvent.php b/Events/AbstractEvent.php deleted file mode 100644 index b6c2d1b..0000000 --- a/Events/AbstractEvent.php +++ /dev/null @@ -1,22 +0,0 @@ -= 4.3 - */ - abstract class AbstractEvent extends Event - { - } -} diff --git a/Events/CheckedRateLimitEvent.php b/Events/CheckedRateLimitEvent.php index 0d12acb..012b6fa 100644 --- a/Events/CheckedRateLimitEvent.php +++ b/Events/CheckedRateLimitEvent.php @@ -2,21 +2,15 @@ namespace Noxlogic\RateLimitBundle\Events; -use Noxlogic\RateLimitBundle\Annotation\RateLimit; +use Noxlogic\RateLimitBundle\Attribute\RateLimit; use Symfony\Component\HttpFoundation\Request; +use Symfony\Contracts\EventDispatcher\Event; -class CheckedRateLimitEvent extends AbstractEvent +class CheckedRateLimitEvent extends Event { + protected Request $request; - /** - * @var Request - */ - protected $request; - - /** - * @var RateLimit|null - */ - protected $rateLimit; + protected ?RateLimit $rateLimit; public function __construct(Request $request, RateLimit $rateLimit = null) { @@ -24,26 +18,17 @@ public function __construct(Request $request, RateLimit $rateLimit = null) $this->rateLimit = $rateLimit; } - /** - * @return RateLimit|null - */ - public function getRateLimit() + public function getRateLimit(): ?RateLimit { return $this->rateLimit; } - /** - * @param RateLimit|null $rateLimit - */ - public function setRateLimit(RateLimit $rateLimit = null) + public function setRateLimit(?RateLimit $rateLimit = null): void { $this->rateLimit = $rateLimit; } - /** - * @return Request - */ - public function getRequest() + public function getRequest(): Request { return $this->request; } diff --git a/Events/GenerateKeyEvent.php b/Events/GenerateKeyEvent.php index 6b65370..7be4e41 100644 --- a/Events/GenerateKeyEvent.php +++ b/Events/GenerateKeyEvent.php @@ -3,8 +3,9 @@ namespace Noxlogic\RateLimitBundle\Events; use Symfony\Component\HttpFoundation\Request; +use Symfony\Contracts\EventDispatcher\Event; -class GenerateKeyEvent extends AbstractEvent +class GenerateKeyEvent extends Event { /** @var Request */ diff --git a/Events/RateLimitEvents.php b/Events/RateLimitEvents.php index 15054e4..c176214 100644 --- a/Events/RateLimitEvents.php +++ b/Events/RateLimitEvents.php @@ -4,6 +4,6 @@ final class RateLimitEvents { - const GENERATE_KEY = 'ratelimit.generate.key'; - const CHECKED_RATE_LIMIT = 'ratelimit.checked.ratelimit'; + public const GENERATE_KEY = 'ratelimit.generate.key'; + public const CHECKED_RATE_LIMIT = 'ratelimit.checked.ratelimit'; } diff --git a/README.md b/README.md index 4c6dbcc..bd3af2c 100755 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ NoxlogicRateLimitBundle [![Latest Stable Version](https://poser.pugx.org/noxlogic/ratelimit-bundle/v/stable.svg)](https://packagist.org/packages/noxlogic/ratelimit-bundle) [![Total Downloads](https://poser.pugx.org/noxlogic/ratelimit-bundle/downloads.svg)](https://packagist.org/packages/noxlogic/ratelimit-bundle) [![Latest Unstable Version](https://poser.pugx.org/noxlogic/ratelimit-bundle/v/unstable.svg)](https://packagist.org/packages/noxlogic/ratelimit-bundle) [![License](https://poser.pugx.org/noxlogic/ratelimit-bundle/license.svg)](https://packagist.org/packages/noxlogic/ratelimit-bundle) -This bundle provides enables the `@RateLimit` annotation which allows you to limit the number of connections to actions. +This bundle provides enables the `#[RateLimit()]` attribute which allows you to limit the number of connections to actions. This is mostly useful in APIs. The bundle is prepared to work by default in cooperation with the `FOSOAuthServerBundle`. It contains a listener that adds the OAuth token to the cache-key. However, you can create your own key generator to allow custom rate limiting based on the request. See *Create a custom key generator* below. @@ -16,7 +16,7 @@ This bundle is partially inspired by a GitHub gist from Ruud Kamphuis: https://g ## Features - * Simple usage through annotations + * Simple usage through attributes * Customize rates per controller, action and even per HTTP method * Multiple storage backends: Redis, Memcached and Doctrine cache @@ -29,10 +29,10 @@ Installation takes just few easy steps: If you're not yet familiar with Composer see http://getcomposer.org. Add the NoxlogicRateLimitBundle in your composer.json: -```js +```json { "require": { - "noxlogic/ratelimit-bundle": "1.x" + "noxlogic/ratelimit-bundle": "2.x" } } ``` @@ -170,19 +170,16 @@ noxlogic_rate_limit: ### Simple rate limiting -To enable rate limiting, you only need to add the annotation to the docblock of the specified action +To enable rate limiting, you only need to add the attribute to the specified action ```php assertEquals('x-rate-limit', $annot->getAliasName()); - $this->assertTrue($annot->allowArray()); - - $this->assertEquals(-1, $annot->getLimit()); - $this->assertEmpty($annot->getMethods()); - $this->assertEquals(3600, $annot->getPeriod()); - } - - public function testConstructionWithValues() - { - $annot = new RateLimit(array('limit' => 1234, 'period' => 1000)); - $this->assertEquals(1234, $annot->getLimit()); - $this->assertEquals(1000, $annot->getPeriod()); - - - $annot = new RateLimit(array('methods' => 'POST', 'limit' => 1234, 'period' => 1000)); - $this->assertEquals(1234, $annot->getLimit()); - $this->assertEquals(1000, $annot->getPeriod()); - $this->assertEquals(['POST'], $annot->getMethods()); - } - - public function testConstructionWithMethods() - { - $annot = new RateLimit(array('limit' => 1234, 'period' => 1000, 'methods' => array('POST', 'GET'))); - $this->assertCount(2, $annot->getMethods()); - - $annot->setMethods(array()); - $this->assertCount(0, $annot->getMethods()); - } -} diff --git a/Tests/Attribute/RateLimitTest.php b/Tests/Attribute/RateLimitTest.php new file mode 100644 index 0000000..ddda5fd --- /dev/null +++ b/Tests/Attribute/RateLimitTest.php @@ -0,0 +1,58 @@ +assertEquals(-1, $attribute->limit); + $this->assertEmpty($attribute->methods); + $this->assertEquals(3600, $attribute->period); + } + + public function testConstructionWithValues(): void + { + $attribute = new RateLimit( + [], + 1234, + 1000 + ); + $this->assertEquals(1234, $attribute->limit); + $this->assertEquals(1000, $attribute->period); + + $attribute = new RateLimit( + ['POST'], + 1234, + 1000 + ); + $this->assertEquals(1234, $attribute->limit); + $this->assertEquals(1000, $attribute->period); + $this->assertEquals(['POST'], $attribute->methods); + } + + public function testConstructionWithMethods(): void + { + $attribute = new RateLimit( + ['POST', 'GET'], + 1234, + 1000 + ); + $this->assertCount(2, $attribute->methods); + } + + public function testConstructWithStringAsMethods(): void + { + $attribute = new RateLimit( + 'POST', + 1234, + 1000 + ); + $this->assertEquals(['POST'], $attribute->methods); + } +} diff --git a/Tests/EventListener/MockControllerWithAttributes.php b/Tests/EventListener/MockControllerWithAttributes.php new file mode 100644 index 0000000..5f455db --- /dev/null +++ b/Tests/EventListener/MockControllerWithAttributes.php @@ -0,0 +1,12 @@ +createListener($this->never()); $listener->setParameter('enabled', false); @@ -62,7 +48,7 @@ public function testReturnedWhenNotEnabled() } - public function testReturnedWhenNotAMasterRequest() + public function testReturnedWhenNotAMasterRequest(): void { $listener = $this->createListener($this->never()); @@ -71,24 +57,20 @@ public function testReturnedWhenNotAMasterRequest() } - public function testReturnedWhenNoControllerFound() + public function testReturnedWhenNoControllerFound(): void { $listener = $this->createListener($this->once()); $kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\HttpKernelInterface')->getMock(); $request = new Request(); - if (class_exists('Symfony\\Component\\HttpKernel\\Event\\ControllerEvent')) { - $event = new ControllerEvent($kernel, function() {}, $request, HttpKernelInterface::MASTER_REQUEST); - } else { - $event = new FilterControllerEvent($kernel, function () {}, $request, HttpKernelInterface::MASTER_REQUEST); - } + $event = new ControllerEvent($kernel, static function() {}, $request, HttpKernelInterface::MASTER_REQUEST); $listener->onKernelController($event); } - public function testReturnedWhenNoAnnotationsFound() + public function testReturnedWhenNoAttributesFound(): void { $listener = $this->createListener($this->once()); @@ -96,7 +78,7 @@ public function testReturnedWhenNoAnnotationsFound() $listener->onKernelController($event); } - public function testDelegatesToPathLimitProcessorWhenNoAnnotationsFound() + public function testDelegatesToPathLimitProcessorWhenNoAttributesFound(): void { $request = new Request(); $event = $this->createEvent(HttpKernelInterface::MASTER_REQUEST, $request); @@ -110,43 +92,57 @@ public function testDelegatesToPathLimitProcessorWhenNoAnnotationsFound() $listener->onKernelController($event); } - public function testDispatchIsCalled() + public function testDispatchIsCalled(): void { $listener = $this->createListener($this->exactly(2)); $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 100, 'period' => 3600)), + new RateLimit([], 100, 3600), )); $listener->onKernelController($event); } - public function testDispatchIsCalledIfThePathLimitProcessorReturnsARateLimit() + public function testDispatchIsCalledWithAttributes(): void + { + $listener = $this->createListener($this->exactly(2)); + + $event = $this->createEvent( + HttpKernelInterface::MASTER_REQUEST, + null, + new MockControllerWithAttributes() + ); + + $listener->onKernelController($event); + } + + public function testDispatchIsCalledIfThePathLimitProcessorReturnsARateLimit(): void { $event = $this->createEvent(HttpKernelInterface::MASTER_REQUEST); $listener = $this->createListener($this->exactly(2)); + $rateLimit = new RateLimit( + [], + 100, + 200 + ); - $rateLimit = new RateLimit(array( - 'limit' => 100, - 'period' => 200 - )); - - $this->mockPathLimitProcessor->expects($this->any()) - ->method('getRateLimit') - ->will($this->returnValue($rateLimit)); + $this->mockPathLimitProcessor + ->expects($this->any()) + ->method('getRateLimit') + ->willReturn($rateLimit); $listener->onKernelController($event); } - public function testIsRateLimitSetInRequest() + public function testIsRateLimitSetInRequest(): void { $listener = $this->createListener($this->any()); $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 10)), + new RateLimit([], 5, 10), )); @@ -161,66 +157,66 @@ public function testIsRateLimitSetInRequest() $this->assertArrayHasKey('rate_limit_info', $event->getRequest()->attributes->all()); } - public function testRateLimit() + public function testRateLimit(): void { $listener = $this->createListener($this->any()); $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 5)), + new RateLimit([], 5, 5), )); $listener->onKernelController($event); - $this->assertIsArray($event->getController()); + self::assertIsArray($event->getController()); $listener->onKernelController($event); - $this->assertIsArray( $event->getController()); + self::assertIsArray( $event->getController()); $listener->onKernelController($event); - $this->assertIsArray($event->getController()); + self::assertIsArray($event->getController()); $listener->onKernelController($event); - $this->assertIsArray($event->getController()); + self::assertIsArray($event->getController()); $listener->onKernelController($event); - $this->assertIsArray($event->getController()); + self::assertIsArray($event->getController()); $listener->onKernelController($event); - $this->assertIsNotArray($event->getController()); + self::assertIsNotArray($event->getController()); $listener->onKernelController($event); - $this->assertIsNotArray($event->getController()); + self::assertIsNotArray($event->getController()); $listener->onKernelController($event); - $this->assertIsNotArray($event->getController()); + self::assertIsNotArray($event->getController()); } - public function testRateLimitThrottling() + public function testRateLimitThrottling(): void { $listener = $this->createListener($this->any()); $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 3)), + new RateLimit([], 5, 3), )); // Throttled $storage = $this->getMockStorage(); $storage->createMockRate('Noxlogic.RateLimitBundle.Tests.EventListener.MockController.mockAction', 5, 10, 6); $listener->onKernelController($event); - $this->assertIsNotArray($event->getController()); + self::assertIsNotArray($event->getController()); } - public function testRateLimitExpiring() + public function testRateLimitExpiring(): void { $listener = $this->createListener($this->any()); $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 3)), + new RateLimit([], 5, 3), )); // Expired $storage = $this->getMockStorage(); $storage->createMockRate('Noxlogic.RateLimitBundle.Tests.EventListener.MockController.mockAction', 5, -10, 12); $listener->onKernelController($event); - $this->assertIsArray($event->getController()); + self::assertIsArray($event->getController()); } - public function testBestMethodMatch() + public function testBestMethodMatch(): void { $listener = $this->createListener($this->any()); $method = new ReflectionMethod(get_class($listener), 'findBestMethodMatch'); @@ -229,9 +225,9 @@ public function testBestMethodMatch() $request = new Request(); $annotations = array( - new RateLimit(array('limit' => 100, 'period' => 3600)), - new RateLimit(array('methods' => 'GET', 'limit' => 100, 'period' => 3600)), - new RateLimit(array('methods' => array('POST', 'PUT'), 'limit' => 100, 'period' => 3600)), + new RateLimit([], 100, 3600), + new RateLimit('GET', 100, 3600), + new RateLimit(['POST', 'PUT'], 100, 3600), ); // Find the method that matches the string @@ -257,7 +253,7 @@ public function testBestMethodMatch() } - public function testFindNoAnnotations() + public function testFindNoAttributes(): void { $listener = $this->createListener($this->any()); $method = new ReflectionMethod(get_class($listener), 'findBestMethodMatch'); @@ -275,7 +271,7 @@ public function testFindNoAnnotations() } - public function testFindBestMethodMatchNotMatchingAnnotations() + public function testFindBestMethodMatchNotMatchingAnnotations(): void { $listener = $this->createListener($this->any()); $method = new ReflectionMethod(get_class($listener), 'findBestMethodMatch'); @@ -284,7 +280,7 @@ public function testFindBestMethodMatchNotMatchingAnnotations() $request = new Request(); $annotations = array( - new RateLimit(array('methods' => 'GET', 'limit' => 100, 'period' => 3600)), + new RateLimit('GET', 100, 3600), ); $request->setMethod('PUT'); @@ -298,7 +294,7 @@ public function testFindBestMethodMatchNotMatchingAnnotations() } - public function testFindBestMethodMatchMatchingMultipleAnnotations() + public function testFindBestMethodMatchMatchingMultipleAnnotations(): void { $listener = $this->createListener($this->any()); $method = new ReflectionMethod(get_class($listener), 'findBestMethodMatch'); @@ -307,8 +303,8 @@ public function testFindBestMethodMatchMatchingMultipleAnnotations() $request = new Request(); $annotations = array( - new RateLimit(array('methods' => 'GET', 'limit' => 100, 'period' => 3600)), - new RateLimit(array('methods' => array('GET','PUT'), 'limit' => 200, 'period' => 7200)), + new RateLimit('GET', 100, 3600), + new RateLimit(['GET','PUT'], 200, 7200), ); $request->setMethod('PUT'); @@ -318,31 +314,26 @@ public function testFindBestMethodMatchMatchingMultipleAnnotations() $this->assertEquals($annotations[1], $method->invoke($listener, $request, $annotations)); } - /** - * @param int $requestType - * @param Request|null $request - * @return ControllerEvent|FilterControllerEvent - */ - protected function createEvent($requestType = HttpKernelInterface::MASTER_REQUEST, Request $request = null) + protected function createEvent( + int $requestType = HttpKernelInterface::MASTER_REQUEST, + ?Request $request = null, + ?MockController $controller = null, + ): ControllerEvent { $kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\HttpKernelInterface')->getMock(); - $controller = new MockController(); + $controller = $controller ?? new MockController(); $action = 'mockAction'; - $request = $request === null ? new Request() : $request; - - if (class_exists('Symfony\\Component\\HttpKernel\\Event\\ControllerEvent')) { - return new ControllerEvent($kernel, array($controller, $action), $request, $requestType); - } + $request = $request ?? new Request(); - return new FilterControllerEvent($kernel, array($controller, $action), $request, $requestType); + return new ControllerEvent($kernel, array($controller, $action), $request, $requestType); } - protected function createListener($expects) + protected function createListener($expects): RateLimitAnnotationListener { - $mockDispatcher = $this->getMockBuilder(self::$usedDispatcher)->getMock(); + $mockDispatcher = $this->getMockBuilder('Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface')->getMock(); $mockDispatcher ->expects($expects) ->method('dispatch'); @@ -357,16 +348,16 @@ protected function createListener($expects) ); } - public function testRateLimitKeyGenerationEventHasPayload() + public function testRateLimitKeyGenerationEventHasPayload(): void { $event = $this->createEvent(); $request = $event->getRequest(); $request->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 3, 'payload' => ['foo'])), + new RateLimit([], 5, 3, ['foo']), )); $generated = false; - $mockDispatcher = $this->getMockBuilder(self::$usedDispatcher)->getMock(); + $mockDispatcher = $this->getMockBuilder('Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface')->getMock(); $generatedCallback = function ($name, $event) use ($request, &$generated) { if ($name !== RateLimitEvents::GENERATE_KEY) { return; @@ -381,7 +372,7 @@ public function testRateLimitKeyGenerationEventHasPayload() ->expects($this->any()) ->method('dispatch') ->willReturnCallback(function ($arg1, $arg2) use ($generatedCallback) { - if ($arg1 instanceof AbstractEvent) { + if ($arg1 instanceof Event) { $generatedCallback($arg2, $arg1); return $arg1; } else { @@ -402,7 +393,7 @@ public function testRateLimitKeyGenerationEventHasPayload() $this->assertTrue($generated, 'Generate key event not dispatched'); } - public function testRateLimitThrottlingWithExceptionAndPayload() + public function testRateLimitThrottlingWithExceptionAndPayload(): void { $listener = $this->createListener($this->any()); $listener->setParameter('rate_response_exception', 'Noxlogic\RateLimitBundle\Tests\Exception\TestException'); @@ -411,7 +402,7 @@ public function testRateLimitThrottlingWithExceptionAndPayload() $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 3, 'payload' => ['foo'])), + new RateLimit([], 5, 3, ['foo']), )); // Throttled @@ -430,7 +421,7 @@ public function testRateLimitThrottlingWithExceptionAndPayload() } } - public function testRateLimitThrottlingWithException() + public function testRateLimitThrottlingWithException(): void { $this->expectException(\BadFunctionCallException::class); $this->expectExceptionCode(123); @@ -442,7 +433,7 @@ public function testRateLimitThrottlingWithException() $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 3)), + new RateLimit([], 5, 3), )); // Throttled @@ -451,7 +442,7 @@ public function testRateLimitThrottlingWithException() $listener->onKernelController($event); } - public function testRateLimitThrottlingWithMessages() + public function testRateLimitThrottlingWithMessages(): void { $listener = $this->createListener($this->any()); $listener->setParameter('rate_response_code', 123); @@ -459,7 +450,7 @@ public function testRateLimitThrottlingWithMessages() $event = $this->createEvent(); $event->getRequest()->attributes->set('_x-rate-limit', array( - new RateLimit(array('limit' => 5, 'period' => 3)), + new RateLimit([], 5, 3), )); // Throttled diff --git a/Tests/Events/CheckedRateLimitEventsTest.php b/Tests/Events/CheckedRateLimitEventsTest.php index 9eedc1f..d15dfaf 100644 --- a/Tests/Events/CheckedRateLimitEventsTest.php +++ b/Tests/Events/CheckedRateLimitEventsTest.php @@ -2,7 +2,7 @@ namespace Noxlogic\RateLimitBundle\Tests\Events; -use Noxlogic\RateLimitBundle\Annotation\RateLimit; +use Noxlogic\RateLimitBundle\Attribute\RateLimit; use Noxlogic\RateLimitBundle\Events\CheckedRateLimitEvent; use Noxlogic\RateLimitBundle\Tests\TestCase; use Symfony\Component\HttpFoundation\Request; @@ -10,7 +10,7 @@ class CheckedRateLimitEventsTest extends TestCase { - public function testConstruction() + public function testConstruction(): void { $request = new Request(); $event = new CheckedRateLimitEvent($request, null); @@ -18,7 +18,7 @@ public function testConstruction() $this->assertEquals(null, $event->getRateLimit()); } - public function testRequest() + public function testRequest(): void { $request = new Request(); $event = new CheckedRateLimitEvent($request, null); @@ -26,10 +26,10 @@ public function testRequest() $this->assertEquals($request, $event->getRequest()); } - public function testSetRateLimit() + public function testSetRateLimit(): void { $request = new Request(); - $rateLimit = new RateLimit([]); + $rateLimit = new RateLimit(); $event = new CheckedRateLimitEvent($request, $rateLimit); diff --git a/Tests/Service/RateLimitInfoTest.php b/Tests/Service/RateLimitInfoTest.php index 82ce36e..bb4cd06 100644 --- a/Tests/Service/RateLimitInfoTest.php +++ b/Tests/Service/RateLimitInfoTest.php @@ -2,16 +2,13 @@ namespace Noxlogic\RateLimitBundle\Tests\Service; -use Noxlogic\RateLimitBundle\EventListener\OauthKeyGenerateListener; -use Noxlogic\RateLimitBundle\Events\GenerateKeyEvent; use Noxlogic\RateLimitBundle\Service\RateLimitInfo; use Noxlogic\RateLimitBundle\Tests\TestCase; -use Symfony\Component\HttpFoundation\Request; class RateLimitInfoTest extends TestCase { - public function testRateInfoSetters() + public function testRateInfoSetters(): void { $rateInfo = new RateLimitInfo(); diff --git a/Tests/Service/Storage/PhpRedisClusterTest.php b/Tests/Service/Storage/PhpRedisClusterTest.php index 151148f..372f331 100755 --- a/Tests/Service/Storage/PhpRedisClusterTest.php +++ b/Tests/Service/Storage/PhpRedisClusterTest.php @@ -1,24 +1,24 @@ -markTestSkipped('Php Redis client not installed'); - } - } - - protected function getRedisMock() { - return $this->getMockBuilder('\RedisCluster')->disableOriginalConstructor(); - } - - protected function getStorage($client) { - return new PhpRedisCluster($client); - } +markTestSkipped('Php Redis client not installed'); + } + } + + protected function getRedisMock() { + return $this->getMockBuilder('\RedisCluster')->disableOriginalConstructor(); + } + + protected function getStorage($client) { + return new PhpRedisCluster($client); + } } \ No newline at end of file diff --git a/Tests/Util/PathLimitProcessorTest.php b/Tests/Util/PathLimitProcessorTest.php index 6ac2ce1..8b0e07f 100644 --- a/Tests/Util/PathLimitProcessorTest.php +++ b/Tests/Util/PathLimitProcessorTest.php @@ -13,7 +13,7 @@ class PathLimitProcessorTest extends TestCase { /** @test */ - function itReturnsNullIfThereAreNoPathLimits() + public function itReturnsNullIfThereAreNoPathLimits(): void { $plp = new PathLimitProcessor(array()); @@ -23,7 +23,7 @@ function itReturnsNullIfThereAreNoPathLimits() } /** @test */ - function itReturnARateLimitIfItMatchesPathAndMethod() + public function itReturnARateLimitIfItMatchesPathAndMethod(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -39,7 +39,7 @@ function itReturnARateLimitIfItMatchesPathAndMethod() ); $this->assertInstanceOf( - 'Noxlogic\RateLimitBundle\Annotation\RateLimit', + 'Noxlogic\RateLimitBundle\Attribute\RateLimit', $result ); @@ -49,7 +49,7 @@ function itReturnARateLimitIfItMatchesPathAndMethod() } /** @test */ - function itReturnARateLimitIfItMatchesSubPathWithUrlEncodedString() + public function itReturnARateLimitIfItMatchesSubPathWithUrlEncodedString() { $plp = new PathLimitProcessor(array( 'api' => array( @@ -65,7 +65,7 @@ function itReturnARateLimitIfItMatchesSubPathWithUrlEncodedString() ); $this->assertInstanceOf( - 'Noxlogic\RateLimitBundle\Annotation\RateLimit', + 'Noxlogic\RateLimitBundle\Attribute\RateLimit', $result ); @@ -75,7 +75,7 @@ function itReturnARateLimitIfItMatchesSubPathWithUrlEncodedString() } /** @test */ - function itWorksWhenMultipleMethodsAreSpecified() + public function itWorksWhenMultipleMethodsAreSpecified(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -96,7 +96,7 @@ function itWorksWhenMultipleMethodsAreSpecified() } /** @test */ - function itReturnsTheCorrectRateLimitWithMultiplePathLimits() + public function itReturnsTheCorrectRateLimitWithMultiplePathLimits(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -123,7 +123,7 @@ function itReturnsTheCorrectRateLimitWithMultiplePathLimits() } /** @test */ - function itWorksWithLimitsOnSamePathButDifferentMethods() + public function itWorksWithLimitsOnSamePathButDifferentMethods(): void { $plp = new PathLimitProcessor(array( 'api_get' => array( @@ -150,7 +150,7 @@ function itWorksWithLimitsOnSamePathButDifferentMethods() } /** @test */ - function itMatchesAstrixAsAnyMethod() + public function itMatchesAstrixAsAnyMethod(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -218,7 +218,7 @@ function itMatchesAstrixAsAnyPath() } /** @test */ - function itMatchesWhenAccessSubPaths() + public function itMatchesWhenAccessSubPaths(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -239,7 +239,7 @@ function itMatchesWhenAccessSubPaths() } /** @test */ - function itReturnsNullIfThereIsNoMatchingPath() + public function itReturnsNullIfThereIsNoMatchingPath(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -258,7 +258,7 @@ function itReturnsNullIfThereIsNoMatchingPath() } /** @test */ - function itMatchesTheMostSpecificPathFirst() + public function itMatchesTheMostSpecificPathFirst(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -285,7 +285,7 @@ function itMatchesTheMostSpecificPathFirst() } /** @test */ - function itReturnsTheMatchedPath() + public function itReturnsTheMatchedPath(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -304,7 +304,7 @@ function itReturnsTheMatchedPath() } /** @test */ - function itReturnsTheCorrectPathForADifferentSetup() + public function itReturnsTheCorrectPathForADifferentSetup(): void { $plp = new PathLimitProcessor(array( 'api' => array( @@ -329,7 +329,7 @@ function itReturnsTheCorrectPathForADifferentSetup() } /** @test */ - function itReturnsTheCorrectMatchedPathForSubPaths() + public function itReturnsTheCorrectMatchedPathForSubPaths(): void { $plp = new PathLimitProcessor(array( 'api' => array( diff --git a/Tests/bootstrap.php b/Tests/bootstrap.php index eee59a5..2fb0639 100644 --- a/Tests/bootstrap.php +++ b/Tests/bootstrap.php @@ -23,4 +23,4 @@ function includeIfExists($file) } // force loading the XRateLimit annotation since the composer target-dir autoloader does not run through $loader::loadClass -class_exists('Noxlogic\RateLimitBundle\Annotation\RateLimit'); +class_exists('Noxlogic\RateLimitBundle\Attribute\RateLimit'); diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md new file mode 100644 index 0000000..8346488 --- /dev/null +++ b/UPGRADE-2.0.md @@ -0,0 +1,10 @@ +UPGRADE FROM 1.x to 2.0 +======================= +Before: + ```php +use Noxlogic\RateLimitBundle\Annotation\RateLimit; +``` +After: + ```php +use Noxlogic\RateLimitBundle\Attribute\RateLimit; +``` \ No newline at end of file diff --git a/Util/PathLimitProcessor.php b/Util/PathLimitProcessor.php index a2c0a4c..3846f8b 100644 --- a/Util/PathLimitProcessor.php +++ b/Util/PathLimitProcessor.php @@ -3,14 +3,14 @@ namespace Noxlogic\RateLimitBundle\Util; -use Noxlogic\RateLimitBundle\Annotation\RateLimit; +use Noxlogic\RateLimitBundle\Attribute\RateLimit ; use Symfony\Component\HttpFoundation\Request; class PathLimitProcessor { - private $pathLimits; + private array $pathLimits; - function __construct(array $pathLimits) + public function __construct(array $pathLimits) { $this->pathLimits = $pathLimits; @@ -21,23 +21,23 @@ function __construct(array $pathLimits) // Order the configs so that the most specific paths // are matched first - usort($this->pathLimits, function($a, $b) { + usort($this->pathLimits, static function($a, $b) { return substr_count($b['path'], '/') - substr_count($a['path'], '/'); }); } - public function getRateLimit(Request $request) + public function getRateLimit(Request $request): ?RateLimit { $path = trim(urldecode($request->getPathInfo()), '/'); $method = $request->getMethod(); foreach ($this->pathLimits as $pathLimit) { if ($this->requestMatched($pathLimit, $path, $method)) { - return new RateLimit(array( - 'limit' => $pathLimit['limit'], - 'period' => $pathLimit['period'], - 'methods' => $pathLimit['methods'] - )); + return new RateLimit( + $pathLimit['methods'], + $pathLimit['limit'], + $pathLimit['period'] + ); } } @@ -58,13 +58,13 @@ public function getMatchedPath(Request $request) return ''; } - private function requestMatched($pathLimit, $path, $method) + private function requestMatched($pathLimit, $path, $method): bool { return $this->methodMatched($pathLimit['methods'], $method) && $this->pathMatched($pathLimit['path'], $path); } - private function methodMatched(array $expectedMethods, $method) + private function methodMatched(array $expectedMethods, $method): bool { foreach ($expectedMethods as $expectedMethod) { if ($expectedMethod === '*' || $expectedMethod === $method) { @@ -75,7 +75,7 @@ private function methodMatched(array $expectedMethods, $method) return false; } - private function pathMatched($expectedPath, $path) + private function pathMatched($expectedPath, $path): bool { if ($expectedPath === '*') { return true; diff --git a/composer.json b/composer.json index b57bc72..008b27f 100644 --- a/composer.json +++ b/composer.json @@ -11,9 +11,8 @@ } ], "require": { - "php": "^7.2|^8.0", - "symfony/framework-bundle": "^3.4.23|^4.4|^5.4.2|^6.1", - "sensio/framework-extra-bundle": "^3.0|^4.0|^5.0|^6.0" + "php": "^8.0", + "symfony/framework-bundle": "^5.4.2|^6.1" }, "require-dev": { "symfony/phpunit-bridge": ">=5.4",