feature #21730 [DependencyInjection] Use a service locator in AddCons…

…traintValidatorsPass (GuilhemN)

This PR was squashed before being merged into the 3.3-dev branch (closes #21730).

Discussion
----------

[DependencyInjection] Use a service locator in AddConstraintValidatorsPass

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Use a service locator to load constraint validators: it allows them to be private.

Commits
-------

597b6bc [DependencyInjection] Use a service locator in AddConstraintValidatorsPass

UPGRADE-3.3.md

@@ -143,6 +143,11 @@ FrameworkBundle
    deprecated and will be removed in 4.0. Use the `Symfony\Component\PropertyInfo\DependencyInjection\PropertyInfoPass`
    class instead.
 
+ * The `ConstraintValidatorFactory::$validators` and `$container` properties
+   have been deprecated and will be removed in 4.0.
+
+ * Extending `ConstraintValidatorFactory` is deprecated and won't be supported in 4.0.
+
 HttpKernel
 -----------
 

UPGRADE-4.0.md

@@ -202,6 +202,11 @@ FrameworkBundle
    removed. Use the `Symfony\Component\PropertyInfo\DependencyInjection\PropertyInfoPass`
    class instead.
 
+ * The `ConstraintValidatorFactory::$validators` and `$container` properties
+   have been removed.
+
+ * Extending `ConstraintValidatorFactory` is not supported anymore.
+
 HttpFoundation
 ---------------
 
@@ -243,7 +248,7 @@ HttpKernel
 
  * The `Psr6CacheClearer::addPool()` method has been removed. Pass an array of pools indexed
    by name to the constructor instead.
-   
+
  * The `LazyLoadingFragmentHandler::addRendererService()` method has been removed.
 
  * The `X-Status-Code` header method of setting a custom status code in the
@@ -310,7 +315,7 @@ Translation
 TwigBundle
 ----------
 
-* The `ContainerAwareRuntimeLoader` class has been removed. Use the 
+* The `ContainerAwareRuntimeLoader` class has been removed. Use the
   Twig `Twig_ContainerRuntimeLoader` class instead.
 
 TwigBridge

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

@@ -17,9 +17,10 @@ CHANGELOG
  * Deprecated `FormPass`, use `Symfony\Component\Form\DependencyInjection\FormPass` instead
  * Deprecated `SessionListener`
  * Deprecated `TestSessionListener`
- * Deprecated `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ConfigCachePass`. 
+ * Deprecated `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ConfigCachePass`.
    Use `Symfony\Component\Console\DependencyInjection\ConfigCachePass` instead.
  * Deprecated `PropertyInfoPass`, use `Symfony\Component\PropertyInfo\DependencyInjection\PropertyInfoPass` instead
+ * Deprecated extending `ConstraintValidatorFactory`
 
 3.2.0
 -----
@@ -31,7 +32,7 @@ CHANGELOG
  * Removed `symfony/asset` from the list of required dependencies in `composer.json`
  * The `Resources/public/images/*` files have been removed.
  * The `Resources/public/css/*.css` files have been removed (they are now inlined in TwigBundle).
- * Added possibility to prioritize form type extensions with `'priority'` attribute on tags `form.type_extension` 
+ * Added possibility to prioritize form type extensions with `'priority'` attribute on tags `form.type_extension`
 
 3.1.0
 -----

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddConstraintValidatorsPass.php

@@ -11,9 +11,10 @@
 
 namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler;
 
+use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
-use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Reference;
 
 class AddConstraintValidatorsPass implements CompilerPassInterface
 {
@@ -25,23 +26,19 @@ public function process(ContainerBuilder $container)
 
         $validators = array();
         foreach ($container->findTaggedServiceIds('validator.constraint_validator') as $id => $attributes) {
-            if (isset($attributes[0]['alias'])) {
-                $validators[$attributes[0]['alias']] = $id;
-            }
-
             $definition = $container->getDefinition($id);
 
-            if (!$definition->isPublic()) {
-                throw new InvalidArgumentException(sprintf('The service "%s" must be public as it can be lazy-loaded.', $id));
+            if ($definition->isAbstract()) {
+                continue;
             }
 
-            if ($definition->isAbstract()) {
-                throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as it can be lazy-loaded.', $id));
+            if (isset($attributes[0]['alias'])) {
+                $validators[$attributes[0]['alias']] = new Reference($id);
             }
 
-            $validators[$definition->getClass()] = $id;
+            $validators[$definition->getClass()] = new Reference($id);
         }
 
-        $container->getDefinition('validator.validator_factory')->replaceArgument(1, $validators);
+        $container->getDefinition('validator.validator_factory')->replaceArgument(0, new ServiceLocatorArgument($validators));
     }
 }

src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml

@@ -57,8 +57,7 @@
         </service>
 
         <service id="validator.validator_factory" class="Symfony\Bundle\FrameworkBundle\Validator\ConstraintValidatorFactory" public="false">
-            <argument type="service" id="service_container" />
-            <argument type="collection" />
+            <argument type="service-locator" /> <!-- Constraint validators locator -->
         </service>
 
         <service id="validator.expression" class="Symfony\Component\Validator\Constraints\ExpressionValidator">

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/AddConstraintValidatorsPassTest.php

@@ -13,56 +13,34 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddConstraintValidatorsPass;
+use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Reference;
 
 class AddConstraintValidatorsPassTest extends TestCase
 {
     public function testThatConstraintValidatorServicesAreProcessed()
     {
-        $services = array(
-            'my_constraint_validator_service1' => array(0 => array('alias' => 'my_constraint_validator_alias1')),
-            'my_constraint_validator_service2' => array(),
-        );
-
-        $validatorFactoryDefinition = $this->getMockBuilder('Symfony\Component\DependencyInjection\Definition')->getMock();
-        $container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerBuilder')->setMethods(array('findTaggedServiceIds', 'getDefinition', 'hasDefinition'))->getMock();
-
-        $validatorDefinition1 = $this->getMockBuilder('Symfony\Component\DependencyInjection\Definition')->setMethods(array('getClass'))->getMock();
-        $validatorDefinition2 = $this->getMockBuilder('Symfony\Component\DependencyInjection\Definition')->setMethods(array('getClass'))->getMock();
-
-        $validatorDefinition1->expects($this->atLeastOnce())
-            ->method('getClass')
-            ->willReturn('My\Fully\Qualified\Class\Named\Validator1');
-        $validatorDefinition2->expects($this->atLeastOnce())
-            ->method('getClass')
-            ->willReturn('My\Fully\Qualified\Class\Named\Validator2');
-
-        $container->expects($this->any())
-            ->method('getDefinition')
-            ->with($this->anything())
-            ->will($this->returnValueMap(array(
-                array('my_constraint_validator_service1', $validatorDefinition1),
-                array('my_constraint_validator_service2', $validatorDefinition2),
-                array('validator.validator_factory', $validatorFactoryDefinition),
-            )));
-
-        $container->expects($this->atLeastOnce())
-            ->method('findTaggedServiceIds')
-            ->will($this->returnValue($services));
-        $container->expects($this->atLeastOnce())
-            ->method('hasDefinition')
-            ->with('validator.validator_factory')
-            ->will($this->returnValue(true));
-
-        $validatorFactoryDefinition->expects($this->once())
-            ->method('replaceArgument')
-            ->with(1, array(
-                'My\Fully\Qualified\Class\Named\Validator1' => 'my_constraint_validator_service1',
-                'my_constraint_validator_alias1' => 'my_constraint_validator_service1',
-                'My\Fully\Qualified\Class\Named\Validator2' => 'my_constraint_validator_service2',
-            ));
+        $container = new ContainerBuilder();
+        $validatorFactory = $container->register('validator.validator_factory')
+            ->setArguments(array(new ServiceLocatorArgument()));
+
+        $container->register('my_constraint_validator_service1', Validator1::class)
+            ->addTag('validator.constraint_validator', array('alias' => 'my_constraint_validator_alias1'));
+        $container->register('my_constraint_validator_service2', Validator2::class)
+            ->addTag('validator.constraint_validator');
+        $container->register('my_abstract_constraint_validator')
+            ->setAbstract(true)
+            ->addTag('validator.constraint_validator');
 
         $addConstraintValidatorsPass = new AddConstraintValidatorsPass();
         $addConstraintValidatorsPass->process($container);
+
+        $this->assertEquals(new ServiceLocatorArgument(array(
+            Validator1::class => new Reference('my_constraint_validator_service1'),
+            'my_constraint_validator_alias1' => new Reference('my_constraint_validator_service1'),
+            Validator2::class => new Reference('my_constraint_validator_service2'),
+        )), $validatorFactory->getArgument(0));
     }
 
     public function testThatCompilerPassIsIgnoredIfThereIsNoConstraintValidatorFactoryDefinition()

src/Symfony/Bundle/FrameworkBundle/Validator/ConstraintValidatorFactory.php

@@ -11,7 +11,7 @@
 
 namespace Symfony\Bundle\FrameworkBundle\Validator;
 
-use Symfony\Component\DependencyInjection\ContainerInterface;
+use Psr\Container\ContainerInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidatorFactoryInterface;
 use Symfony\Component\Validator\ConstraintValidatorInterface;
@@ -37,6 +37,8 @@
  *     }
  *
  * @author Kris Wallsmith <kris@symfony.com>
+ *
+ * @final since version 3.3
  */
 class ConstraintValidatorFactory implements ConstraintValidatorFactoryInterface
 {
@@ -70,11 +72,15 @@ public function getInstance(Constraint $constraint)
         $name = $constraint->validatedBy();
 
         if (!isset($this->validators[$name])) {
-            if (!class_exists($name)) {
-                throw new ValidatorException(sprintf('Constraint validator "%s" does not exist or it is not enabled. Check the "validatedBy" method in your constraint class "%s".', $name, get_class($constraint)));
-            }
+            if ($this->container->has($name)) {
+                $this->validators[$name] = $this->container->get($name);
+            } else {
+                if (!class_exists($name)) {
+                    throw new ValidatorException(sprintf('Constraint validator "%s" does not exist or it is not enabled. Check the "validatedBy" method in your constraint class "%s".', $name, get_class($constraint)));
+                }
 
-            $this->validators[$name] = new $name();
+                $this->validators[$name] = new $name();
+            }
         } elseif (is_string($this->validators[$name])) {
             $this->validators[$name] = $this->container->get($this->validators[$name]);
         }

src/Symfony/Bundle/FrameworkBundle/composer.json

@@ -49,7 +49,7 @@
         "symfony/serializer": "~3.3",
         "symfony/translation": "~2.8|~3.0",
         "symfony/templating": "~2.8|~3.0",
-        "symfony/validator": "~3.2",
+        "symfony/validator": "~3.3",
         "symfony/yaml": "~3.2",
         "symfony/property-info": "~3.3",
         "doctrine/annotations": "~1.0",
@@ -65,7 +65,8 @@
         "symfony/console": "<3.3",
         "symfony/serializer": "<3.3",
         "symfony/form": "<3.3",
-        "symfony/property-info": "<3.3"
+        "symfony/property-info": "<3.3",
+        "symfony/validator": "<3.3"
     },
     "suggest": {
         "ext-apcu": "For best performance of the system caches",