From 9689e5ed3818dbfcc2fc1e667283aee8dafe2dba Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Mon, 31 Mar 2025 11:48:18 -0400 Subject: [PATCH] [FrameworkBundle][RateLimiter] default `lock_factory` to `auto` --- .../Bundle/FrameworkBundle/CHANGELOG.md | 1 + .../DependencyInjection/Configuration.php | 2 +- .../FrameworkExtension.php | 4 +++ .../PhpFrameworkExtensionTest.php | 31 +++++++++++++++++-- 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 6c4daeb6df85d..b7efe5a18bbf7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -22,6 +22,7 @@ CHANGELOG * Enable service argument resolution on classes that use the `#[Route]` attribute, the `#[AsController]` attribute is no longer required * Deprecate setting the `framework.profiler.collect_serializer_data` config option to `false` + * Set `framework.rate_limiter.limiters.*.lock_factory` to `auto` by default 7.2 --- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index aa61cb12c56f4..7f37b52166cfe 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -2504,7 +2504,7 @@ private function addRateLimiterSection(ArrayNodeDefinition $rootNode, callable $ ->children() ->scalarNode('lock_factory') ->info('The service ID of the lock factory used by this limiter (or null to disable locking).') - ->defaultValue('lock.factory') + ->defaultValue('auto') ->end() ->scalarNode('cache_pool') ->info('The cache pool to use for storing the current limiter state.') diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 1a1bcdd162d5d..98e2e8904c3f2 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -3239,6 +3239,10 @@ private function registerRateLimiterConfiguration(array $config, ContainerBuilde $limiter = $container->setDefinition($limiterId = 'limiter.'.$name, new ChildDefinition('limiter')) ->addTag('rate_limiter', ['name' => $name]); + if ('auto' === $limiterConfig['lock_factory']) { + $limiterConfig['lock_factory'] = $this->isInitializedConfigEnabled('lock') ? 'lock.factory' : null; + } + if (null !== $limiterConfig['lock_factory']) { if (!interface_exists(LockInterface::class)) { throw new LogicException(\sprintf('Rate limiter "%s" requires the Lock component to be installed. Try running "composer require symfony/lock".', $name)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php index deac159b6f9b0..ea8d481e0f0f0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/PhpFrameworkExtensionTest.php @@ -188,7 +188,7 @@ public function testWorkflowDefaultMarkingStoreDefinition() $this->assertNull($argumentsB['index_1'], 'workflow_b marking_store argument is null'); } - public function testRateLimiterWithLockFactory() + public function testRateLimiterLockFactoryWithLockDisabled() { try { $this->createContainerFromClosure(function (ContainerBuilder $container) { @@ -199,7 +199,7 @@ public function testRateLimiterWithLockFactory() 'php_errors' => ['log' => true], 'lock' => false, 'rate_limiter' => [ - 'with_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour'], + 'with_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour', 'lock_factory' => 'lock.factory'], ], ]); }); @@ -208,7 +208,10 @@ public function testRateLimiterWithLockFactory() } catch (LogicException $e) { $this->assertEquals('Rate limiter "with_lock" requires the Lock component to be configured.', $e->getMessage()); } + } + public function testRateLimiterAutoLockFactoryWithLockEnabled() + { $container = $this->createContainerFromClosure(function (ContainerBuilder $container) { $container->loadFromExtension('framework', [ 'annotations' => false, @@ -226,13 +229,35 @@ public function testRateLimiterWithLockFactory() $this->assertEquals('lock.factory', (string) $withLock->getArgument(2)); } - public function testRateLimiterLockFactory() + public function testRateLimiterAutoLockFactoryWithLockDisabled() { $container = $this->createContainerFromClosure(function (ContainerBuilder $container) { $container->loadFromExtension('framework', [ 'annotations' => false, 'http_method_override' => false, 'handle_all_throwables' => true, + 'lock' => false, + 'php_errors' => ['log' => true], + 'rate_limiter' => [ + 'without_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour'], + ], + ]); + }); + + $this->expectException(OutOfBoundsException::class); + $this->expectExceptionMessageMatches('/^The argument "2" doesn\'t exist.*\.$/'); + + $container->getDefinition('limiter.without_lock')->getArgument(2); + } + + public function testRateLimiterDisableLockFactory() + { + $container = $this->createContainerFromClosure(function (ContainerBuilder $container) { + $container->loadFromExtension('framework', [ + 'annotations' => false, + 'http_method_override' => false, + 'handle_all_throwables' => true, + 'lock' => true, 'php_errors' => ['log' => true], 'rate_limiter' => [ 'without_lock' => ['policy' => 'fixed_window', 'limit' => 10, 'interval' => '1 hour', 'lock_factory' => null],