From 6ddfb9c584a0b08511642569c887e5c67b3b5b18 Mon Sep 17 00:00:00 2001
From: Alexander Schnitzler <git@alexanderschnitzler.de>
Date: Tue, 15 May 2018 15:46:29 +0200
Subject: [PATCH] [BUGFIX] Only validate method params if needed
Controller action arguments have been validated on
creation, which caused superfluous CPU cycles if the
action controller later detected, that an argument
should not have been validated at all due to an
@Extbase\IgnoreValidation annotation.
To fix this, arguments get an empty result on creation.
When setting the argument value, only the validation
results of the property mapping are merged with the
argument result.
ActionController::initializeActionMethodValidators does
only create validator instances for method arguments
that need to be validated, thus a whole bunch of checks
disappears in callActionMethod().
Releases: master
Resolves: #85012
Change-Id: Iaecf36718477a9216f8d36a993a137eb7b677227
Reviewed-on: https://review.typo3.org/56970
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
---
...85012-OnlyValidateMethodParamsIfNeeded.rst | 38 ++++++++++++++
.../Mvc/Controller/ActionController.php | 37 +++++---------
.../Classes/Mvc/Controller/Argument.php | 49 ++++++++++++++-----
.../Classes/Mvc/Controller/Arguments.php | 16 +++++-
.../Unit/Mvc/Controller/ArgumentTest.php | 2 +-
.../Php/MethodCallMatcher.php | 14 ++++++
6 files changed, 118 insertions(+), 38 deletions(-)
create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Deprecation-85012-OnlyValidateMethodParamsIfNeeded.rst
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85012-OnlyValidateMethodParamsIfNeeded.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85012-OnlyValidateMethodParamsIfNeeded.rst
new file mode 100644
index 000000000000..6d09a511b3f8
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85012-OnlyValidateMethodParamsIfNeeded.rst
@@ -0,0 +1,38 @@
+.. include:: ../../Includes.txt
+
+===========================================================
+Deprecation: #85012 - Only validate method params if needed
+===========================================================
+
+See :issue:`85012`
+
+Description
+===========
+
+The following public methods have been marked as deprecated:
+
+* :php:`TYPO3\CMS\Extbase\Mvc\Controller\Argument::getValidationResults()`
+* :php:`TYPO3\CMS\Extbase\Mvc\Controller\Arguments::getValidationResults()`
+
+Impact
+======
+
+Calling the method triggers a deprecation log entry.
+
+
+Affected Installations
+======================
+
+Extensions that call that method. The extension scanner will find usages.
+
+
+Migration
+=========
+
+Use the following methods instead:
+
+* :php:`TYPO3\CMS\Extbase\Mvc\Controller\Argument::validate()`
+* :php:`TYPO3\CMS\Extbase\Mvc\Controller\Arguments::validate()`
+
+
+.. index:: PHP-API, FullyScanned, ext:extbase
diff --git a/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php b/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
index 4ace0e0199ca..0b7aa14c323d 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php
@@ -257,8 +257,16 @@ class ActionController extends AbstractController
$classSchema = $this->reflectionService->getClassSchema(static::class);
+ $ignoreValidationAnnotations = array_unique(array_flip(
+ $classSchema->getMethod($this->actionMethodName)['tags']['ignorevalidation'] ?? []
+ ));
+
/** @var \TYPO3\CMS\Extbase\Mvc\Controller\Argument $argument */
foreach ($this->arguments as $argument) {
+ if (isset($ignoreValidationAnnotations[$argument->getName()])) {
+ continue;
+ }
+
$validator = $this->objectManager->get(ConjunctionValidator::class);
$validatorDefinitions = $classSchema->getMethod($this->actionMethodName)['params'][$argument->getName()]['validators'] ?? [];
@@ -313,33 +321,12 @@ class ActionController extends AbstractController
foreach ($this->arguments as $argument) {
$preparedArguments[] = $argument->getValue();
}
- $validationResult = $this->arguments->getValidationResults();
+ $validationResult = $this->arguments->validate();
if (!$validationResult->hasErrors()) {
$this->emitBeforeCallActionMethodSignal($preparedArguments);
- $actionResult = call_user_func_array([$this, $this->actionMethodName], $preparedArguments);
+ $actionResult = $this->{$this->actionMethodName}(...$preparedArguments);
} else {
- $methodTagsValues = $this->reflectionService->getMethodTagsValues(static::class, $this->actionMethodName);
- $ignoreValidationAnnotations = $methodTagsValues['ignorevalidation'] ?? [];
-
- // if there exist errors which are not ignored with @TYPO3\CMS\Extbase\Annotation\IgnoreValidation => call error method
- // else => call action method
- $shouldCallActionMethod = true;
- foreach ($validationResult->getSubResults() as $argumentName => $subValidationResult) {
- if (!$subValidationResult->hasErrors()) {
- continue;
- }
- if (in_array($argumentName, $ignoreValidationAnnotations, true)) {
- continue;
- }
- $shouldCallActionMethod = false;
- break;
- }
- if ($shouldCallActionMethod) {
- $this->emitBeforeCallActionMethodSignal($preparedArguments);
- $actionResult = call_user_func_array([$this, $this->actionMethodName], $preparedArguments);
- } else {
- $actionResult = call_user_func([$this, $this->errorMethodName]);
- }
+ $actionResult = $this->{$this->errorMethodName}();
}
if ($actionResult === null && $this->view instanceof ViewInterface) {
@@ -610,7 +597,7 @@ class ActionController extends AbstractController
if ($referringRequest !== null) {
$originalRequest = clone $this->request;
$this->request->setOriginalRequest($originalRequest);
- $this->request->setOriginalRequestMappingResults($this->arguments->getValidationResults());
+ $this->request->setOriginalRequestMappingResults($this->arguments->validate());
$this->forward(
$referringRequest->getControllerActionName(),
$referringRequest->getControllerName(),
diff --git a/typo3/sysext/extbase/Classes/Mvc/Controller/Argument.php b/typo3/sysext/extbase/Classes/Mvc/Controller/Argument.php
index e13efb9a3717..798ae6f529a6 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Controller/Argument.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Controller/Argument.php
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Extbase\Mvc\Controller;
* The TYPO3 project - inspiring people to share!
*/
+use TYPO3\CMS\Extbase\Error\Result;
use TYPO3\CMS\Extbase\Property\Exception\TargetNotFoundException;
use TYPO3\CMS\Extbase\Utility\TypeHandlingUtility;
@@ -88,7 +89,12 @@ class Argument
*
* @var \TYPO3\CMS\Extbase\Error\Result
*/
- protected $validationResults = null;
+ protected $validationResults;
+
+ /**
+ * @var bool
+ */
+ private $hasBeenValidated = false;
/**
* @param \TYPO3\CMS\Extbase\Property\PropertyMapper $propertyMapper
@@ -124,6 +130,8 @@ class Argument
}
$this->name = $name;
$this->dataType = TypeHandlingUtility::normalizeType($dataType);
+
+ $this->validationResults = new Result();
}
/**
@@ -274,12 +282,7 @@ class Argument
throw $e;
}
}
- $this->validationResults = $this->propertyMapper->getMessages();
- if ($this->validator !== null) {
- // @todo Validation API has also changed!!!
- $validationMessages = $this->validator->validate($this->value);
- $this->validationResults->merge($validationMessages);
- }
+ $this->validationResults->merge($this->propertyMapper->getMessages());
return $this;
}
@@ -312,18 +315,23 @@ class Argument
* @return bool TRUE if the argument is valid, FALSE otherwise
* @api
*/
- public function isValid()
+ public function isValid(): bool
{
- return !$this->validationResults->hasErrors();
+ return !$this->validate()->hasErrors();
}
/**
* @return \TYPO3\CMS\Extbase\Error\Result Validation errors which have occurred.
- * @api
+ * @deprecated
*/
public function getValidationResults()
{
- return $this->validationResults;
+ trigger_error(
+ 'Method ' . __METHOD__ . ' is deprecated and will be removed in TYPO3 v10.0.',
+ E_USER_DEPRECATED
+ );
+
+ return $this->validate();
}
/**
@@ -336,4 +344,23 @@ class Argument
{
return (string)$this->value;
}
+
+ /**
+ * @return \TYPO3\CMS\Extbase\Error\Result
+ * @api
+ */
+ public function validate(): \TYPO3\CMS\Extbase\Error\Result
+ {
+ if ($this->hasBeenValidated) {
+ return $this->validationResults;
+ }
+
+ if ($this->validator !== null) {
+ $validationMessages = $this->validator->validate($this->value);
+ $this->validationResults->merge($validationMessages);
+ }
+
+ $this->hasBeenValidated = true;
+ return $this->validationResults;
+ }
}
diff --git a/typo3/sysext/extbase/Classes/Mvc/Controller/Arguments.php b/typo3/sysext/extbase/Classes/Mvc/Controller/Arguments.php
index c70dc455ce75..e2936fe32ce1 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Controller/Arguments.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Controller/Arguments.php
@@ -275,13 +275,27 @@ class Arguments extends \ArrayObject
* Get all property mapping / validation errors
*
* @return \TYPO3\CMS\Extbase\Error\Result
+ * @deprecated
*/
public function getValidationResults()
+ {
+ trigger_error(
+ 'Method ' . __METHOD__ . ' is deprecated and will be removed in TYPO3 v10.0.',
+ E_USER_DEPRECATED
+ );
+
+ return $this->validate();
+ }
+
+ /**
+ * @return \TYPO3\CMS\Extbase\Error\Result
+ */
+ public function validate(): \TYPO3\CMS\Extbase\Error\Result
{
$results = new \TYPO3\CMS\Extbase\Error\Result();
/** @var Argument $argument */
foreach ($this as $argument) {
- $argumentValidationResults = $argument->getValidationResults();
+ $argumentValidationResults = $argument->validate();
if ($argumentValidationResults === null) {
continue;
}
diff --git a/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/ArgumentTest.php b/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/ArgumentTest.php
index 8ce709200328..165b479a03ad 100644
--- a/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/ArgumentTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Mvc/Controller/ArgumentTest.php
@@ -231,7 +231,7 @@ class ArgumentTest extends \TYPO3\TestingFramework\Core\Unit\UnitTestCase
$this->simpleValueArgument->setValidator($mockValidator);
$this->setupPropertyMapperAndSetValue();
$this->assertFalse($this->simpleValueArgument->isValid());
- $this->assertEquals([$error], $this->simpleValueArgument->getValidationResults()->getErrors());
+ $this->assertEquals([$error], $this->simpleValueArgument->validate()->getErrors());
}
/**
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
index af8dbf5e417c..1dc067d4eb2e 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
@@ -2249,4 +2249,18 @@ return [
'Deprecation-85005-DeprecateMethodsAndConstantsInValidatorResolver.rst',
],
],
+ 'TYPO3\CMS\Extbase\Mvc\Controller\Argument->getValidationResults' => [
+ 'numberOfMandatoryArguments' => 0,
+ 'maximumNumberOfArguments' => 0,
+ 'restFiles' => [
+ 'Deprecation-85012-OnlyValidateMethodParamsIfNeeded.rst',
+ ],
+ ],
+ 'TYPO3\CMS\Extbase\Mvc\Controller\Arguments->getValidationResults' => [
+ 'numberOfMandatoryArguments' => 0,
+ 'maximumNumberOfArguments' => 0,
+ 'restFiles' => [
+ 'Deprecation-85012-OnlyValidateMethodParamsIfNeeded.rst',
+ ],
+ ],
];
--
GitLab