From d7dd93f7a7c4835393aa94563c3fa4e63fca25b9 Mon Sep 17 00:00:00 2001 From: cpenny Date: Thu, 13 Feb 2020 12:51:56 +1300 Subject: [PATCH] Standardise getCMSValidator for DataObjects/Forms --- src/Forms/DefaultFormFactory.php | 11 +- src/Forms/Form.php | 10 +- src/Forms/GridField/GridFieldDetailForm.php | 7 +- src/Forms/RequiredFields.php | 8 + src/Forms/Validator.php | 13 ++ src/Forms/ValidatorList.php | 192 ++++++++++++++++++++ src/ORM/DataObject.php | 14 ++ 7 files changed, 235 insertions(+), 20 deletions(-) create mode 100644 src/Forms/ValidatorList.php diff --git a/src/Forms/DefaultFormFactory.php b/src/Forms/DefaultFormFactory.php index 24c6e3feb..f179803cc 100644 --- a/src/Forms/DefaultFormFactory.php +++ b/src/Forms/DefaultFormFactory.php @@ -5,6 +5,7 @@ namespace SilverStripe\Forms; use InvalidArgumentException; use SilverStripe\Control\RequestHandler; use SilverStripe\Core\Extensible; +use SilverStripe\ORM\DataObject; /** * Default form builder class. @@ -96,15 +97,11 @@ class DefaultFormFactory implements FormFactory */ protected function getFormValidator(RequestHandler $controller = null, $name, $context = []) { - $validator = null; - if ($context['Record']->hasMethod('getCMSValidator')) { - // @todo Deprecate or formalise support for getCMSValidator() - $validator = $context['Record']->getCMSValidator(); + if (!$context['Record'] instanceof DataObject) { + return null; } - // Extend validator - $this->invokeWithExtensions('updateFormValidator', $validator, $controller, $name, $context); - return $validator; + return $context['Record']->getCMSValidators(); } /** diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 1a59bb3ff..5c8309782 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -1871,14 +1871,6 @@ class Form extends ViewableData implements HasRequestHandler } // Don't cache if there are required fields, or some other complex validator - $validator = $this->getValidator(); - if ($validator instanceof RequiredFields) { - if (count($this->validator->getRequired())) { - return false; - } - } else { - return false; - } - return true; + return $this->getValidator()->canBeCached(); } } diff --git a/src/Forms/GridField/GridFieldDetailForm.php b/src/Forms/GridField/GridFieldDetailForm.php index 2bf2ba610..6f909e582 100644 --- a/src/Forms/GridField/GridFieldDetailForm.php +++ b/src/Forms/GridField/GridFieldDetailForm.php @@ -121,10 +121,9 @@ class GridFieldDetailForm implements GridField_URLHandler $gridField->getState(false)->setValue($gridStateStr); } - // if no validator has been set on the GridField and the record has a - // CMS validator, use that. - if (!$this->getValidator() && ClassInfo::hasMethod($record, 'getCMSValidator')) { - $this->setValidator($record->getCMSValidator()); + // if no validator has been set on the GridField then use the Validators from the record. + if (!$this->getValidator()) { + $this->setValidator($record->getCMSValidators()); } return $handler->handleRequest($request); diff --git a/src/Forms/RequiredFields.php b/src/Forms/RequiredFields.php index 2395b1455..fcfa84775 100644 --- a/src/Forms/RequiredFields.php +++ b/src/Forms/RequiredFields.php @@ -215,4 +215,12 @@ class RequiredFields extends Validator { return array_values($this->required); } + + /** + * @return bool + */ + public function canBeCached(): bool + { + return count($this->getRequired()) === 0; + } } diff --git a/src/Forms/Validator.php b/src/Forms/Validator.php index b51e39631..aac77551e 100644 --- a/src/Forms/Validator.php +++ b/src/Forms/Validator.php @@ -168,6 +168,19 @@ abstract class Validator return $this; } + /** + * When Validators are set on the form, it can affect whether or not the form cannot be cached. The base + * implementation always returns false (in order to match the current paradigm). + * + * @see RequiredFields for an example of when you might be able to cache your form. + * + * @return bool + */ + public function canBeCached(): bool + { + return false; + } + /** * Clear current result * diff --git a/src/Forms/ValidatorList.php b/src/Forms/ValidatorList.php new file mode 100644 index 000000000..57e123e94 --- /dev/null +++ b/src/Forms/ValidatorList.php @@ -0,0 +1,192 @@ +validators = ArrayList::create(); + + parent::__construct(); + } + + /** + * @param Form $form + * @return Validator + */ + public function setForm($form) + { + foreach ($this->getValidators() as $validator) { + $validator->setForm($form); + } + + return parent::setForm($form); + } + + /** + * Returns any errors there may be. This method considers the enabled status of the ValidatorList as a whole + * (exiting early if the List is disabled), as well as the enabled status of each individual Validator. + * + * @return ValidationResult + */ + public function validate() + { + $this->resetResult(); + + // This ValidatorList has been disabled in full + if (!$this->getEnabled()) { + return $this->result; + } + + $data = $this->form->getData(); + + foreach ($this->getValidators() as $validator) { + // Reset the validation results for this Validator + $validator->resetResult(); + + // This Validator has been disabled, so skip it + if (!$validator->getEnabled()) { + continue; + } + + // Run validation, and exit early if it's valid + if ($validator->php($data)) { + continue; + } + + // Validation result was invalid. Combine our ValidationResult messages + $this->getResult()->combineAnd($validator->getResult()); + } + + return $this->result; + } + + /** + * Returns whether the field in question is required. This will usually display '*' next to the + * field. + * + * @param string $fieldName + * + * @return bool + */ + public function fieldIsRequired($fieldName) + { + foreach ($this->getValidators() as $validator) { + if (!$validator->fieldIsRequired($fieldName)) { + continue; + } + + return true; + } + + return false; + } + + /** + * Note: The existing implementations for the php() method (@see RequiredFields) does not check whether the + * Validator is enabled or not, and it also does not reset the validation result - so, neither does this. + * + * @param array $data + * @return bool + */ + public function php($data) + { + foreach ($this->getValidators() as $validator) { + // Run validation, and exit early if it's valid + if ($validator->php($data)) { + continue; + } + + // Validation result was invalid. Combine our ValidationResult messages + $this->getResult()->combineAnd($validator->getResult()); + } + + // After collating results, return whether or not everything was valid + return $this->getResult()->isValid(); + } + + /** + * @param Validator $validator + * @return ValidatorList + */ + public function addValidator(Validator $validator): ValidatorList + { + $this->getValidators()->add($validator); + + return $this; + } + + /** + * @return ArrayList|Validator[] + */ + public function getValidators(): ArrayList + { + return $this->validators; + } + + /** + * @param string $className + * @return ArrayList|Validator[] + */ + public function getValidatorsByType(string $className): ArrayList + { + $validators = ArrayList::create(); + + foreach ($this->getValidators() as $validator) { + if (!$validator instanceof $className) { + continue; + } + + $validators->add($validator); + } + + return $validators; + } + + /** + * @param string $className + * @return ValidatorList + */ + public function removeByType(string $className): ValidatorList + { + foreach ($this->getValidators() as $validator) { + if (!$validator instanceof $className) { + continue; + } + + $this->getValidators()->remove($validator); + } + + return $this; + } + + /** + * @return bool + */ + public function canBeCached(): bool + { + foreach ($this->getValidators() as $validator) { + if ($validator->canBeCached()) { + continue; + } + + return false; + } + + return true; + } +} diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index b34cb9a86..028e1de32 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -15,6 +15,7 @@ use SilverStripe\Dev\Deprecation; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; +use SilverStripe\Forms\ValidatorList; use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18nEntityProvider; use SilverStripe\ORM\Connect\MySQLSchemaManager; @@ -2447,6 +2448,19 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity return $actions; } + public function getCMSValidators(): ValidatorList + { + $validatorList = new ValidatorList(); + + // Support for the old method during the deprecation period + if ($this->hasMethod('getCMSValidator')) { + $validatorList->addValidator($this->getCMSValidator()); + } + + $this->invokeWithExtensions('updateCMSValidators', $validatorList); + + return $validatorList; + } /** * Used for simple frontend forms without relation editing