[!!!][TASK] Do not magically register validators 98/60298/12
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Tue, 19 Mar 2019 14:21:54 +0000 (14:21 +0000)
committerBenni Mack <benni@typo3.org>
Mon, 17 Jun 2019 12:30:54 +0000 (14:30 +0200)
This patch removes the automatical registration of two types
of validators.

- model validators
- type validators

Model validators are those validators that follow a specific
namespace and class naming derived from domain models.

Given a model \Vendor\Extension\Domain\Model\Foo, extbase searched
for a valiator \Vendor\Extension\Domain\Validator\FooValidator.

If it existed, it had been registered automatically and could not
be disabled at all.

Type validators are similiar to model validators. Given a non
model action parameter or model property like int, string, float,
DateTime and such, extbase searched for validators in the
namespace TYPO3\CMS\Extbase\Validation\Validator.

There is a TYPO3\CMS\Extbase\Validation\Validator\StringValidator
for example which had been registered for string type params and
properties.

Said validators could not be disabled at all.

There are several reasons why the automatic registration has been
removed:

- First of all, this behaviour led to an unknown amount of actual
  registered validators. Developers that are new or simply not
  familiar with the concept of validation magic could easily
  become frustrated.

- Then there is the problem that validation takes place, no matter
  if it was needed or wanted. A domain validator, which looked
  quite handy in the first place, had to be reduced to the
  validation logic that would fit all cases where according
  objects had been passed into methods. No matter the context.
  The context however matters a lot. One might want to have
  different validation rules depending on if objects are
  created, updated or deleted. This distinction was impossible
  and therefore model validators could be a burden.

- Last but not least, the automatic registration is a problem
  when introducing validation groups. Validation groups cover
  the context aspect mentioned earlier. By grouping validations
  one can create and register different validators and apply
  them given by contexts like create, update, delete and such.

Releases: master
Resolves: #87957
Change-Id: If8f590a1bedb428c8884cd61828d8cc671ee92e1
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60298
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Stefan Froemken <froemken@gmail.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Stefan Froemken <froemken@gmail.com>
Reviewed-by: Benni Mack <benni@typo3.org>
typo3/sysext/core/Classes/Utility/ClassNamingUtility.php
typo3/sysext/core/Documentation/Changelog/master/Breaking-87957-DoNotMagicallyRegisterValidators.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Utility/ClassNamingUtilityTest.php
typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
typo3/sysext/extbase/Classes/Validation/ValidatorResolver.php
typo3/sysext/extbase/Tests/Functional/Mvc/Controller/ActionControllerTest.php
typo3/sysext/extbase/Tests/Functional/Mvc/Validation/ActionControllerValidationTest.php
typo3/sysext/extbase/Tests/Functional/Validation/ValidatorResolverTest.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php

index 19bf53e..dd1de7d 100644 (file)
@@ -38,23 +38,6 @@ class ClassNamingUtility
         ) . 'Repository';
     }
 
-    /**
-     * Translates a model name to an appropriate validator name
-     * e.g. Tx_Extbase_Domain_Model_Foo to Tx_Extbase_Domain_Validator_FooValidator
-     * or \TYPO3\CMS\Extbase\Domain\Model\Foo to \TYPO3\CMS\Extbase\Domain\Validator\FooValidator
-     *
-     * @param string $modelName Name of the model to translate
-     * @return string Name of the repository
-     */
-    public static function translateModelNameToValidatorName($modelName)
-    {
-        return str_replace(
-            '\\Domain\\Model\\',
-            '\\Domain\\Validator\\',
-            $modelName
-        ) . 'Validator';
-    }
-
     /**
      * Translates a repository name to an appropriate model name
      * e.g. Tx_Extbase_Domain_Repository_FooRepository to Tx_Extbase_Domain_Model_Foo
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-87957-DoNotMagicallyRegisterValidators.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-87957-DoNotMagicallyRegisterValidators.rst
new file mode 100644 (file)
index 0000000..b343cc5
--- /dev/null
@@ -0,0 +1,162 @@
+.. include:: ../../Includes.txt
+
+=================================================================================
+Breaking: #87957 - Validators are not registered automatically in Extbase anymore
+=================================================================================
+
+See :issue:`87957`
+
+Description
+===========
+
+There are several validators that Extbase automatically applies. One of these is domain validators that are registered
+if created in a specific directory. Another one is the type validator which is created if a validator with a specific
+name exists.
+
+For this reason, the method :php:`\TYPO3\CMS\Extbase\Utility\ClassNamingUtility::translateModelNameToValidatorName` has
+been removed without substitution.
+
+Domain Validators
+=================
+
+Given that there is a model :php:`\TYPO3\CMS\Extbase\Domain\Model\BackendUser`, extbase searched for a validator named
+:php:`\TYPO3\CMS\Extbase\Domain\Validator\BackendUserValidator`. The `Model` part of the namespace had been replaced
+with `Validator` and another `Validator` string had been added to the actual class name. In this example, `BackendUser`
+has been replaced with `BackendUserValidator`.
+
+If such a validator class existed it had been magically applied and used during the validation of the model.
+
+Example::
+
+   <?php
+   namespace ExtbaseTeam\BlogExample\Domain\Validator;
+
+   use TYPO3\CMS\Extbase\Validation\Validator;
+
+   class BlogValidator implements ValidatorInterface
+   {
+      public function validate($value);
+      {
+         // ...
+      }
+   }
+
+::
+
+   <?php
+   namespace ExtbaseTeam\BlogExample\Controller;
+
+   use ExtbaseTeam\BlogExample\Domain\Model\Blog;
+   use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
+
+   class BlogController extends ActionController
+   {
+      public function showAction(Blog $blog)
+      {
+         // ...
+      }
+   }
+
+In this example there is a model validator :php:`ExtbaseTeam\BlogExample\Domain\Validator\BlogValidator` defined for
+model :php:`ExtbaseTeam\BlogExample\Domain\Model\Blog`, which had been automatically registered before calling action
+:php:`ExtbaseTeam\BlogExample\Controller\BlogController::showAction`.
+
+From now on the validator needs to be registered manually.
+
+::
+
+   <?php
+   namespace ExtbaseTeam\BlogExample\Controller;
+
+   use ExtbaseTeam\BlogExample\Domain\Model\Blog;
+   use TYPO3\CMS\Extbase\Annotation as Extbase;
+   use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
+
+   class BlogController extends ActionController
+   {
+      /**
+       * @Extbase\Validate(param="blog", validator="ExtbaseTeam\BlogExample\Domain\Validator\BlogValidator")
+       */
+      public function showAction(Blog $blog)
+      {
+         // ...
+      }
+   }
+
+
+Type Validators
+===============
+
+Given that there is any kind of simple type param or property that is to be validated, e.g. a property of a model or an
+action method param, extbase tried to apply a validator for that param/property derived from its type. If there was an
+action param of type string, extbase searched for a `StringValidator` in the namespace
+`TYPO3\CMS\Extbase\Validation\Validator`. The :php:`TYPO3\CMS\Extbase\Validation\Validator\StringValidator` does
+actually exist, as well as :php:`TYPO3\CMS\Extbase\Validation\Validator\IntegerValidator` and others.
+
+If a validator for a specific type existed it had been magically applied and used during the validation of models and
+action arguments.
+
+Example:
+
+::
+
+   <?php
+   namespace ExtbaseTeam\BlogExample\Controller;
+
+   use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
+
+   class BlogController extends ActionController
+   {
+      public function showAction(int $blogUid)
+      {
+         // ...
+      }
+   }
+
+In this example there is a simple type param, extbase automatically registered a type validator for. First, `int` had
+been normalized to `integer`, then :php:`ucfirst($type)` had been called, resulting in `Integer` and then extbase looked
+for a :php:`TYPO3\CMS\Extbase\Validation\Validator\IntegerValidator`. As this Validator exists, it had been
+automatically registered.
+
+If this behaviour is desired, the validator needs to be registered manually from now on.
+
+::
+
+   <?php
+   namespace ExtbaseTeam\BlogExample\Controller;
+
+   use TYPO3\CMS\Extbase\Annotation as Extbase;
+   use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
+
+   class BlogController extends ActionController
+   {
+      /**
+       * @Extbase\Validate(param="blogUid", validator="TYPO3\CMS\Extbase\Validation\Validator\IntegerValidator")
+       */
+      public function showAction(int $blogUid)
+      {
+         // ...
+      }
+   }
+
+
+Impact
+======
+
+With these mentioned validators no longer being applied automatically, developers actively need to apply those
+validators if needed. Most developers might want to register existing domain validators manually while leaving the type
+validators unregistered. This however will vary from project to project.
+
+
+Affected Installations
+======================
+
+All installations that use the extbase validation framework.
+
+
+Migration
+=========
+
+There is no automatic migration. Validators need to be re-applied manually if needed.
+
+.. index:: PHP-API, PartiallyScanned, ext:extbase
index 0baff28..ae5cd37 100644 (file)
@@ -27,7 +27,6 @@ class ClassNamingUtilityTest extends UnitTestCase
     /**
      * DataProvider for translateModelNameToRepositoryName
      * and translateRepositoryNameToModelName
-     * and translateModelNameToValidatorName
      *
      * @return array
      */
@@ -37,37 +36,30 @@ class ClassNamingUtilityTest extends UnitTestCase
             [
                 'VENDOR\\EXT\\Domain\\Repository\\BlogRepository',
                 'VENDOR\\EXT\\Domain\\Model\\Blog',
-                'VENDOR\\EXT\\Domain\\Validator\\BlogValidator'
             ],
             [
                 'VENDOR\\EXT\\Domain\\Repository\\_PageRepository',
                 'VENDOR\\EXT\\Domain\\Model\\_Page',
-                'VENDOR\\EXT\\Domain\\Validator\\_PageValidator'
             ],
             [
                 'VENDOR\\Repository\\Domain\\Repository\\SomeModelRepository',
                 'VENDOR\\Repository\\Domain\\Model\\SomeModel',
-                'VENDOR\\Repository\\Domain\\Validator\\SomeModelValidator'
             ],
             [
                 'VENDOR\\EXT\\Domain\\Repository\\RepositoryRepository',
                 'VENDOR\\EXT\\Domain\\Model\\Repository',
-                'VENDOR\\EXT\\Domain\\Validator\\RepositoryValidator'
             ],
             [
                 'VENDOR\\Repository\\Domain\\Repository\\RepositoryRepository',
                 'VENDOR\\Repository\\Domain\\Model\\Repository',
-                'VENDOR\\Repository\\Domain\\Validator\\RepositoryValidator'
             ],
             [
                 'VENDOR\\ModelCollection\\Domain\\Repository\\ModelRepository',
                 'VENDOR\\ModelCollection\\Domain\\Model\\Model',
-                'VENDOR\\ModelCollection\\Domain\\Validator\\ModelValidator'
             ],
             [
                 'VENDOR\\Model\\Domain\\Repository\\ModelRepository',
                 'VENDOR\\Model\\Domain\\Model\\Model',
-                'VENDOR\\Model\\Domain\\Validator\\ModelValidator'
             ],
         ];
     }
@@ -76,10 +68,9 @@ class ClassNamingUtilityTest extends UnitTestCase
      * @dataProvider repositoryAndModelClassNames
      * @param string $expectedRepositoryName
      * @param string $modelName
-     * @param string $dummyValidatorName not needed here - just a dummy to be able to cleanly use the same dataprovider
      * @test
      */
-    public function translateModelNameToRepositoryName($expectedRepositoryName, $modelName, $dummyValidatorName)
+    public function translateModelNameToRepositoryName($expectedRepositoryName, $modelName)
     {
         $translatedRepositoryName = ClassNamingUtility::translateModelNameToRepositoryName($modelName);
         $this->assertSame($expectedRepositoryName, $translatedRepositoryName);
@@ -89,28 +80,14 @@ class ClassNamingUtilityTest extends UnitTestCase
      * @dataProvider repositoryAndModelClassNames
      * @param string $repositoryName
      * @param string $expectedModelName
-     * @param string $dummyValidatorName not needed here - just a dummy to be able to use the same dataprovider
      * @test
      */
-    public function translateRepositoryNameToModelName($repositoryName, $expectedModelName, $dummyValidatorName)
+    public function translateRepositoryNameToModelName($repositoryName, $expectedModelName)
     {
         $translatedModelName = ClassNamingUtility::translateRepositoryNameToModelName($repositoryName);
         $this->assertSame($expectedModelName, $translatedModelName);
     }
 
-    /**
-     * @dataProvider repositoryAndModelClassNames
-     * @param string $repositoryName
-     * @param string $modelName
-     * @param string $expectedValidatorName
-     * @test
-     */
-    public function translateModelNameToValidatorName($repositoryName, $modelName, $expectedValidatorName)
-    {
-        $translatedModelName = ClassNamingUtility::translateModelNameToValidatorName($modelName);
-        $this->assertSame($expectedValidatorName, $translatedModelName);
-    }
-
     /**
      * DataProvider for explodeObjectControllerName
      *
index 9eedfd0..9881373 100644 (file)
@@ -265,12 +265,6 @@ class ActionController extends AbstractController
                 );
             }
 
-            /*
-             * At this point, a validator based on the argument's type is added. If the argument is of type float,
-             * an instance of \TYPO3\CMS\Extbase\Validation\Validator\FloatValidator is applied.
-             *
-             * todo: this should be removed (breaking) in TYPO3 10.0. as the use of validators should be verbose
-             */
             $baseValidatorConjunction = $this->validatorResolver->getBaseValidatorConjunction($argument->getDataType());
             if ($baseValidatorConjunction->count() > 0) {
                 $validator->addValidator($baseValidatorConjunction);
index 3a9cfbb..4529ba6 100644 (file)
@@ -15,7 +15,6 @@ namespace TYPO3\CMS\Extbase\Validation;
  */
 
 use TYPO3\CMS\Core\Log\LogManager;
-use TYPO3\CMS\Core\Utility\ClassNamingUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Utility\TypeHandlingUtility;
 use TYPO3\CMS\Extbase\Validation\Exception\NoSuchValidatorException;
@@ -219,38 +218,5 @@ class ValidatorResolver implements \TYPO3\CMS\Core\SingletonInterface
                 $conjunctionValidator->addValidator($objectValidator);
             }
         }
-
-        $this->addCustomValidators($targetClassName, $conjunctionValidator);
-    }
-
-    /**
-     * This adds custom validators to the passed $conjunctionValidator.
-     *
-     * A custom validator is found if it follows the naming convention "Replace '\Model\' by '\Validator\' and
-     * append 'Validator'". If found, it will be added to the $conjunctionValidator.
-     *
-     * In addition canValidate() will be called on all implementations of the ObjectValidatorInterface to find
-     * all validators that could validate the target. The one with the highest priority will be added as well.
-     * If multiple validators have the same priority, which one will be added is not deterministic.
-     *
-     * @param string $targetClassName
-     * @param ConjunctionValidator $conjunctionValidator
-     */
-    protected function addCustomValidators($targetClassName, ConjunctionValidator &$conjunctionValidator)
-    {
-        // todo: this method should be renamed to addDomainValidator as this method neither adds custom validators,
-        // todo: nor does it add multiple validators. If there is a Validator that fits the class naming scheme
-        // todo: defined in ClassNamingUtility::translateModelNameToValidatorName, a single validator is added.
-
-        // todo: magically adding validators based on a directory structure is questionable. It should be discussed
-        // todo: if this functionality should be removed (breaking) in TYPO3 10.0.
-        $possibleValidatorClassName = ClassNamingUtility::translateModelNameToValidatorName($targetClassName);
-
-        $customValidator = $this->createValidator($possibleValidatorClassName);
-        if ($customValidator !== null) {
-            $conjunctionValidator->addValidator($customValidator);
-        }
-
-        // @todo: find polytype validator for class
     }
 }
index 7b63c4d..53a8545 100644 (file)
@@ -22,7 +22,6 @@ use TYPO3\CMS\Extbase\Mvc\Web\Response;
 use TYPO3\CMS\Extbase\Object\ObjectManager;
 use TYPO3\CMS\Extbase\Validation\Validator\ConjunctionValidator;
 use TYPO3\CMS\Extbase\Validation\Validator\NotEmptyValidator;
-use TYPO3\CMS\Extbase\Validation\Validator\StringValidator;
 
 /**
  * Test case
@@ -62,51 +61,6 @@ class ActionControllerTest extends \TYPO3\TestingFramework\Core\Functional\Funct
         $this->controller = $objectManager->get(Fixture\Controller\TestController::class);
     }
 
-    /**
-     * @test
-     */
-    public function modelValidatorsAreProperlyResolved()
-    {
-        // Setup
-        $this->request->setControllerActionName('foo');
-        $this->request->setArgument('fooParam', []);
-
-        // Test run
-        $this->controller->processRequest($this->request, $this->response);
-
-        // Open arguments property
-        $reflectionClass = new \ReflectionClass($this->controller);
-        $argumentsProperty = $reflectionClass->getProperty('arguments');
-        $argumentsProperty->setAccessible(true);
-
-        // Assertions
-
-        /** @var Arguments $arguments */
-        $arguments = $argumentsProperty->getValue($this->controller);
-        $argument = $arguments->getArgument('fooParam');
-
-        /** @var ConjunctionValidator $validator */
-        $conjunctionValidator = $argument->getValidator();
-        static::assertInstanceOf(ConjunctionValidator::class, $conjunctionValidator);
-
-        /** @var \SplObjectStorage $validators */
-        $validators = $conjunctionValidator->getValidators();
-        static::assertInstanceOf(\SplObjectStorage::class, $validators);
-
-        $validators->rewind();
-
-        /** @var ConjunctionValidator $subConjunctionValidator */
-        $subConjunctionValidator = $validators->current();
-        static::assertInstanceOf(ConjunctionValidator::class, $subConjunctionValidator);
-
-        /** @var \SplObjectStorage $subValidators */
-        $subValidators = $subConjunctionValidator->getValidators();
-        static::assertInstanceOf(\SplObjectStorage::class, $subValidators);
-
-        $subValidators->rewind();
-        static::assertInstanceOf(Fixture\Domain\Validator\ModelValidator::class, $subValidators->current());
-    }
-
     /**
      * @test
      */
@@ -140,19 +94,6 @@ class ActionControllerTest extends \TYPO3\TestingFramework\Core\Functional\Funct
 
         $validators->rewind();
         static::assertInstanceOf(Fixture\Validation\Validator\CustomValidator::class, $validators->current());
-
-        $validators->next();
-
-        /** @var ConjunctionValidator $subConjunctionValidator */
-        $subConjunctionValidator = $validators->current();
-        static::assertInstanceOf(ConjunctionValidator::class, $subConjunctionValidator);
-
-        /** @var \SplObjectStorage $subValidators */
-        $subValidators = $subConjunctionValidator->getValidators();
-        static::assertInstanceOf(\SplObjectStorage::class, $subValidators);
-
-        $subValidators->rewind();
-        static::assertInstanceOf(StringValidator::class, $subValidators->current());
     }
 
     /**
index 3e6226f..87ab33f 100644 (file)
@@ -50,11 +50,6 @@ class ActionControllerValidationTest extends FunctionalTestCase
                 ['blogPost[__identity]', 'blogPost[title]'],
                 [1428504122]
             ],
-            'existing blog post custom validator' => [
-                ['__identity' => 1, 'title' => '77'],
-                ['blogPost[__identity]', 'blogPost[title]'],
-                [1428504122, 1480872650]
-            ],
         ];
     }
 
@@ -163,7 +158,7 @@ class ActionControllerValidationTest extends FunctionalTestCase
         $errors = $request->getOriginalRequestMappingResults()->getFlattenedErrors();
         $this->assertCount(1, $errors['blog.title']);
         $this->assertCount(1, $errors['blog.description']);
-        $this->assertCount(2, $errors['blogPost.title']);
+        $this->assertCount(1, $errors['blogPost.title']);
 
         $this->assertEquals('testFormAction', $response->getContent());
     }
index 309f73f..3fd5d94 100644 (file)
@@ -135,9 +135,5 @@ class ValidatorResolverTest extends \TYPO3\TestingFramework\Core\Functional\Func
             $baseValidatorConjunctions[\TYPO3\CMS\Extbase\Tests\Functional\Validation\Fixture\Domain\Model\AnotherModel::class],
             $propertyValidator
         );
-
-        $baseValidators->next();
-        $validator = $baseValidators->current();
-        static::assertInstanceOf(Fixture\Domain\Validator\ModelValidator::class, $validator);
     }
 }
index 0da9ecc..3e7895f 100644 (file)
@@ -903,4 +903,11 @@ return [
             'Deprecation-88554-DeprecatedMethodsInVersionNumberUtility.rst',
         ],
     ],
+    'TYPO3\CMS\Extbase\Utility\ClassNamingUtility::translateModelNameToValidatorName' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Breaking-87957-DoNotMagicallyRegisterValidators.rst',
+        ],
+    ],
 ];