[FrameworkBundle][RateLimiter] default lock_factory to auto

Author: kbond

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
 ---

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.')

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));

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],