From 51a2dbf13f6b62ea970e10d0bf1e17433bc9e155 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 14 Nov 2024 16:59:38 +1300 Subject: [PATCH] ENH Use FieldValidators for FormField validation --- .../FieldValidation/DateFieldValidator.php | 106 +++++- .../FieldValidation/FieldValidationTrait.php | 43 ++- .../MultiOptionFieldValidator.php | 10 +- .../FieldValidation/NumericFieldValidator.php | 10 +- .../FieldValidation/OptionFieldValidator.php | 8 +- src/Forms/CompositeField.php | 29 +- src/Forms/ConfirmedPasswordField.php | 249 ++++++------- src/Forms/CurrencyField.php | 30 +- src/Forms/DateField.php | 131 ++----- src/Forms/DateField_Disabled.php | 35 +- src/Forms/DatetimeField.php | 131 ++----- src/Forms/EmailField.php | 28 +- src/Forms/FieldGroup.php | 2 +- .../FieldValidatorConverterInterface.php | 10 + .../FieldValidatorConverterTrait.php | 24 ++ src/Forms/FieldsValidator.php | 4 +- src/Forms/FileField.php | 69 ++-- src/Forms/FormField.php | 80 ++-- src/Forms/ListboxField.php | 14 +- src/Forms/LookupField.php | 16 +- src/Forms/MoneyField.php | 51 +-- src/Forms/MultiSelectField.php | 68 ++-- src/Forms/NumericField.php | 70 ++-- src/Forms/OptionsetField.php | 12 - src/Forms/RequiredFields.php | 4 +- src/Forms/SearchableDropdownField.php | 11 + src/Forms/SearchableDropdownTrait.php | 8 - src/Forms/SearchableMultiDropdownField.php | 5 + src/Forms/SelectField.php | 23 +- src/Forms/SelectionGroup_Item.php | 2 +- src/Forms/SingleLookupField.php | 18 +- src/Forms/SingleSelectField.php | 60 +-- src/Forms/TextField.php | 31 +- src/Forms/TextareaField.php | 30 +- src/Forms/TimeField.php | 93 ++--- src/Forms/UrlField.php | 41 ++- src/ORM/FieldType/DBField.php | 13 - .../DateFieldValidatorTest.php | 148 +++++++- .../MultiOptionFieldValidatorTest.php | 23 +- .../NumericFieldValidatorTest.php | 97 +++-- tests/php/Forms/CheckboxFieldTest.php | 9 +- tests/php/Forms/CheckboxSetFieldTest.php | 58 +-- tests/php/Forms/CompositeFieldTest.php | 16 +- .../php/Forms/ConfirmedPasswordFieldTest.php | 78 ++-- tests/php/Forms/CurrencyFieldTest.php | 43 +-- tests/php/Forms/DateFieldTest.php | 103 +++++- tests/php/Forms/DatetimeFieldTest.php | 115 ++++-- tests/php/Forms/DropdownFieldTest.php | 31 +- tests/php/Forms/EmailFieldTest.php | 5 +- tests/php/Forms/FileFieldTest.php | 3 +- tests/php/Forms/FormFieldTest.php | 343 ++++++++++++++++-- .../FieldValidationExtension.php | 15 +- tests/php/Forms/FormTest.php | 19 +- tests/php/Forms/GroupedDropdownFieldTest.php | 31 +- tests/php/Forms/ListboxFieldTest.php | 139 +++---- tests/php/Forms/LookupFieldTest.php | 27 ++ tests/php/Forms/MoneyFieldTest.php | 15 +- tests/php/Forms/MultiSelectFieldTest.php | 72 ++++ tests/php/Forms/NumericFieldTest.php | 72 +++- tests/php/Forms/OptionsetFieldTest.php | 21 +- .../php/Forms/SearchableDropdownFieldTest.php | 43 +++ .../php/Forms/SearchableDropdownTraitTest.php | 1 + tests/php/Forms/SelectFieldTest.php | 86 +++++ tests/php/Forms/SingleLookupFieldTest.php | 24 ++ tests/php/Forms/SingleSelectFieldTest.php | 101 ++++++ tests/php/Forms/TextFieldTest.php | 42 ++- tests/php/Forms/TextareaFieldTest.php | 42 ++- tests/php/Forms/TimeFieldTest.php | 60 ++- tests/php/Forms/UrlFieldTest.php | 4 +- .../php/Security/MemberTest/ValidatorForm.php | 3 +- 70 files changed, 2171 insertions(+), 1287 deletions(-) create mode 100644 src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php create mode 100644 src/Forms/FieldValidatorConverter/FieldValidatorConverterTrait.php create mode 100644 tests/php/Forms/MultiSelectFieldTest.php create mode 100644 tests/php/Forms/SearchableDropdownFieldTest.php create mode 100644 tests/php/Forms/SelectFieldTest.php create mode 100644 tests/php/Forms/SingleSelectFieldTest.php diff --git a/src/Core/Validation/FieldValidation/DateFieldValidator.php b/src/Core/Validation/FieldValidation/DateFieldValidator.php index 0a2a3de22b3..11b8c1e6dd4 100644 --- a/src/Core/Validation/FieldValidation/DateFieldValidator.php +++ b/src/Core/Validation/FieldValidation/DateFieldValidator.php @@ -2,18 +2,77 @@ namespace SilverStripe\Core\Validation\FieldValidation; +use Error; +use Exception; +use InvalidArgumentException; use SilverStripe\Core\Validation\FieldValidation\FieldValidator; use SilverStripe\Core\Validation\ValidationResult; /** * Validates that a value is a valid date, which means that it follows the equivalent formats: * - PHP date format Y-m-d - * - SO format y-MM-dd i.e. DBDate::ISO_DATE + * - ISO format y-MM-dd i.e. DBDate::ISO_DATE * * Blank string values are allowed */ class DateFieldValidator extends FieldValidator { + /** + * Minimum value as a date string + */ + private ?string $minValue; + + /** + * Minimum value as a unix timestamp + */ + private ?int $minTimestamp; + + /** + * Maximum value as a date string + */ + private ?string $maxValue; + + /** + * Maximum value as a unix timestamp + */ + private ?int $maxTimestamp; + + /** + * Converter to convert date strings to another format for display in error messages + * + * @var callable + */ + private $converter; + + public function __construct( + string $name, + mixed $value, + ?string $minValue = null, + ?string $maxValue = null, + ?callable $converter = null, + ) { + // Convert Y-m-d args to timestamps + // Intermiediate variables are used to prevent "must not be accessed before initialization" PHP errors + // when reading properties in the constructor + $minTimestamp = null; + $maxTimestamp = null; + if (!is_null($minValue)) { + $minTimestamp = $this->dateToTimestamp($minValue); + } + if (!is_null($maxValue)) { + $maxTimestamp = $this->dateToTimestamp($maxValue); + } + if (!is_null($minTimestamp) && !is_null($maxTimestamp) && $maxTimestamp < $minTimestamp) { + throw new InvalidArgumentException('maxValue cannot be less than minValue'); + } + $this->minValue = $minValue; + $this->maxValue = $maxValue; + $this->minTimestamp = $minTimestamp; + $this->maxTimestamp = $maxTimestamp; + $this->converter = $converter; + parent::__construct($name, $value); + } + protected function validateValue(): ValidationResult { $result = ValidationResult::create(); @@ -21,10 +80,36 @@ protected function validateValue(): ValidationResult if ($this->value === '') { return $result; } - // Not using symfony/validator because it was allowing d-m-Y format strings - $date = date_parse_from_format($this->getFormat(), $this->value ?? ''); - if ($date === false || $date['error_count'] > 0 || $date['warning_count'] > 0) { + // Validate value is a valid date + try { + $timestamp = $this->dateToTimestamp($this->value ?? ''); + } catch (Exception|Error) { $result->addFieldError($this->name, $this->getMessage()); + return $result; + } + // Validate value is within range + if (!is_null($this->minTimestamp) && $timestamp < $this->minTimestamp) { + $minValue = $this->minValue; + if (!is_null($this->converter)) { + $minValue = call_user_func($this->converter, $this->minValue) ?: $this->minValue; + } + $message = _t( + __CLASS__ . '.TOOSMALL', + 'Value cannot be less than {minValue}', + ['minValue' => $minValue] + ); + $result->addFieldError($this->name, $message); + } elseif (!is_null($this->maxTimestamp) && $timestamp > $this->maxTimestamp) { + $maxValue = $this->maxValue; + if (!is_null($this->converter)) { + $maxValue = call_user_func($this->converter, $this->maxValue) ?: $this->maxValue; + } + $message = _t( + __CLASS__ . '.TOOLARGE', + 'Value cannot be greater than {maxValue}', + ['maxValue' => $maxValue] + ); + $result->addFieldError($this->name, $message); } return $result; } @@ -38,4 +123,17 @@ protected function getMessage(): string { return _t(__CLASS__ . '.INVALID', 'Invalid date'); } + + /** + * Parse a date string into a unix timestamp using the format specified by getFormat() + */ + private function dateToTimestamp(string $date): int + { + // Not using symfony/validator because it was allowing d-m-Y format strings + $date = date_parse_from_format($this->getFormat(), $date); + if ($date === false || $date['error_count'] > 0 || $date['warning_count'] > 0) { + throw new InvalidArgumentException('Invalid date'); + } + return mktime($date['hour'], $date['minute'], $date['second'], $date['month'], $date['day'], $date['year']); + } } diff --git a/src/Core/Validation/FieldValidation/FieldValidationTrait.php b/src/Core/Validation/FieldValidation/FieldValidationTrait.php index 649c350e64e..f5a3302339c 100644 --- a/src/Core/Validation/FieldValidation/FieldValidationTrait.php +++ b/src/Core/Validation/FieldValidation/FieldValidationTrait.php @@ -49,6 +49,18 @@ public function validate(): ValidationResult return $result; } + /** + * Get the value of this field for use in validation via FieldValidators + * + * Intended to be overridden in subclasses when there is a need to provide something different + * from the value of the field itself, for instance DBComposite and CompositeField which need to + * provide a value that is a combination of the values of their children + */ + public function getValueForValidation(): mixed + { + return $this->getValue(); + } + /** * Get instantiated FieldValidators based on `field_validators` configuration */ @@ -61,12 +73,10 @@ private function getFieldValidators(): array throw new RuntimeException($message); } /** @var FieldValidationInterface|Configurable $this */ - $name = $this->getName(); + // Use a default name if the field doesn't have one rather than throwing an exception + // because some fields may not have a name, i.e. CompositeField + $name = $this->getName() ?: _t(__CLASS__ . '.UNNAMEDFIELD', 'Unnamed field'); $value = $this->getValueForValidation(); - // Field name is required for FieldValidators when called ValidationResult::addFieldMessage() - if ($name === '') { - throw new RuntimeException('Field name is blank'); - } $classes = $this->getClassesFromConfig(); foreach ($classes as $class => $argCalls) { $args = [$name, $value]; @@ -122,10 +132,7 @@ private function getClassesFromConfig(): array if (!is_array($argCalls)) { throw new RuntimeException("argCalls for FieldValidator $class is not an array"); } - // array_unique() is used to dedupe any cases where a subclass defines the same FieldValidator - // this config can happens when a subclass defines a FieldValidator that was already defined on a parent - // class, though it calls different methods - $argCalls = array_unique($argCalls); + $argCalls = $this->makeUnique($argCalls); $classes[$class] = $argCalls; } foreach (array_keys($disabledClasses) as $class) { @@ -133,4 +140,22 @@ private function getClassesFromConfig(): array } return $classes; } + + /** + * Dedupe any cases where a subclass defines the same FieldValidator + * his config can happens when a subclass defines a FieldValidator that was already defined + * on a parent class, though it calls different methods + */ + private function makeUnique(array $argCalls): array + { + // there may be multiple null values, which we need to retain so make them unqiue first + // note that we can't use a short function in combination with $n++ in array_map as $n will always be 0 + $n = 0; + $argCalls = array_map(function ($v) use (&$n) { + return is_null($v) ? '{null}-' . $n++ : $v; + }, $argCalls); + $argCalls = array_unique($argCalls); + $argCalls = array_map(fn($v) => str_contains($v, '{null}-') ? null : $v, $argCalls); + return $argCalls; + } } diff --git a/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php b/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php index dd770784dfa..28b25bf139f 100644 --- a/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php +++ b/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php @@ -2,7 +2,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; -use InvalidArgumentException; use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; @@ -12,19 +11,20 @@ class MultiOptionFieldValidator extends OptionFieldValidator { /** - * @param mixed $value - an array of values to be validated + * @param mixed $value - an iterable set of values to be validated */ public function __construct(string $name, mixed $value, array $options) { - if (!is_iterable($value) && !is_null($value)) { - throw new InvalidArgumentException('Value must be iterable'); - } parent::__construct($name, $value, $options); } protected function validateValue(): ValidationResult { $result = ValidationResult::create(); + if (!is_iterable($this->value) && !is_null($this->value)) { + $result->addFieldError($this->name, $this->getMessage()); + return $result; + } foreach ($this->value as $value) { $this->checkValueInOptions($value, $result); } diff --git a/src/Core/Validation/FieldValidation/NumericFieldValidator.php b/src/Core/Validation/FieldValidation/NumericFieldValidator.php index 398d9d7234c..2ba9d81cd09 100644 --- a/src/Core/Validation/FieldValidation/NumericFieldValidator.php +++ b/src/Core/Validation/FieldValidation/NumericFieldValidator.php @@ -39,8 +39,14 @@ public function __construct( protected function validateValue(): ValidationResult { $result = ValidationResult::create(); - if (!is_numeric($this->value) || is_string($this->value)) { - // Must be a numeric value, though not as a numeric string + if (!is_numeric($this->value)) { + $message = _t(__CLASS__ . '.NOTNUMERIC', 'Must be numeric'); + $result->addFieldError($this->name, $message); + } elseif (is_numeric($this->value) && is_string($this->value)) { + // This is a separate check from the the one above because for DBField the type is important + // though for FormField form submissions values will usually have a string type + // though should cast to the correct int or float type before validation + // i.e. we don't want to tell CMS users to not use a string when they're using a TextField $message = _t(__CLASS__ . '.WRONGTYPE', 'Must be numeric, and not a string'); $result->addFieldError($this->name, $message); } elseif (!is_null($this->minValue) && $this->value < $this->minValue) { diff --git a/src/Core/Validation/FieldValidation/OptionFieldValidator.php b/src/Core/Validation/FieldValidation/OptionFieldValidator.php index dd514570743..09460fdee8c 100644 --- a/src/Core/Validation/FieldValidation/OptionFieldValidator.php +++ b/src/Core/Validation/FieldValidation/OptionFieldValidator.php @@ -31,8 +31,12 @@ protected function validateValue(): ValidationResult protected function checkValueInOptions(mixed $value, ValidationResult $result): void { if (!in_array($value, $this->options, true)) { - $message = _t(__CLASS__ . '.NOTALLOWED', 'Not an allowed value'); - $result->addFieldError($this->name, $message); + $result->addFieldError($this->name, $this->getMessage()); } } + + protected function getMessage(): string + { + return _t(__CLASS__ . '.NOTALLOWED', 'Not an allowed value'); + } } diff --git a/src/Forms/CompositeField.php b/src/Forms/CompositeField.php index d4f2aef64ce..11c48c34544 100644 --- a/src/Forms/CompositeField.php +++ b/src/Forms/CompositeField.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms; +use SilverStripe\Core\Validation\FieldValidation\CompositeFieldValidator; use SilverStripe\Dev\Debug; /** @@ -13,6 +14,9 @@ */ class CompositeField extends FormField { + private static array $field_validators = [ + CompositeFieldValidator::class, + ]; /** * @var FieldList @@ -63,7 +67,6 @@ public function __construct($children = null) $children = new FieldList($children); } $this->setChildren($children); - parent::__construct(null, false); } @@ -115,12 +118,17 @@ public function getChildren() return $this->children; } + public function getValueForValidation(): mixed + { + return $this->getChildren()->toArray(); + } + /** * Returns the name (ID) for the element. * If the CompositeField doesn't have a name, but we still want the ID/name to be set. * This code generates the ID from the nested children. */ - public function getName() + public function getName(): string { if ($this->name) { return $this->name; @@ -266,8 +274,6 @@ public function setForm($form) return $this; } - - public function setDisabled($disabled) { parent::setDisabled($disabled); @@ -522,19 +528,4 @@ public function debug(): string $result .= ""; return $result; } - - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - $valid = true; - foreach ($this->children as $child) { - $valid = ($child && $child->validate($validator) && $valid); - } - return $this->extendValidationResult($valid, $validator); - } } diff --git a/src/Forms/ConfirmedPasswordField.php b/src/Forms/ConfirmedPasswordField.php index 1856a057eff..2b889709edd 100644 --- a/src/Forms/ConfirmedPasswordField.php +++ b/src/Forms/ConfirmedPasswordField.php @@ -12,6 +12,7 @@ use Closure; use SilverStripe\Core\Validation\ConstraintValidator; use Symfony\Component\Validator\Constraints\PasswordStrength; +use SilverStripe\Core\Validation\ValidationResult; /** * Two masked input fields, checks for matching passwords. @@ -420,155 +421,141 @@ public function isSaveable() || ($this->showOnClick && $this->hiddenField && $this->hiddenField->Value()); } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) + public function validate(): ValidationResult { - $name = $this->name; - + $result = ValidationResult::create(); // if field isn't visible, don't validate if (!$this->isSaveable()) { - return $this->extendValidationResult(true, $validator); - } - - $this->getPasswordField()->setValue($this->value); - $this->getConfirmPasswordField()->setValue($this->confirmValue); - $value = $this->getPasswordField()->Value(); - - // both password-fields should be the same - if ($value != $this->getConfirmPasswordField()->Value()) { - $validator->validationError( - $name, - _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSDONTMATCH', "Passwords don't match"), - "validation" - ); - - return $this->extendValidationResult(false, $validator); - } - - if (!$this->canBeEmpty) { - // both password-fields shouldn't be empty - if (!$value || !$this->getConfirmPasswordField()->Value()) { - $validator->validationError( - $name, - _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"), - "validation" - ); - - return $this->extendValidationResult(false, $validator); - } + return $result; } - - // lengths - $minLength = $this->getMinLength(); - $maxLength = $this->getMaxLength(); - if ($minLength || $maxLength) { - $errorMsg = null; - $limit = null; - if ($minLength && $maxLength) { - $limit = "{{$minLength},{$maxLength}}"; - $errorMsg = _t( - __CLASS__ . '.BETWEEN', - 'Passwords must be {min} to {max} characters long.', - ['min' => $minLength, 'max' => $maxLength] - ); - } elseif ($minLength) { - $limit = "{{$minLength}}.*"; - $errorMsg = _t( - __CLASS__ . '.ATLEAST', - 'Passwords must be at least {min} characters long.', - ['min' => $minLength] - ); - } elseif ($maxLength) { - $limit = "{0,{$maxLength}}"; - $errorMsg = _t( - __CLASS__ . '.MAXIMUM', - 'Passwords must be at most {max} characters long.', - ['max' => $maxLength] - ); - } - $limitRegex = '/^.' . $limit . '$/'; - if (!empty($value) && !preg_match($limitRegex ?? '', $value ?? '')) { - $validator->validationError( + $this->beforeExtending('updateValidate', function () use ($result) { + $name = $this->name; + + $this->getPasswordField()->setValue($this->value); + $this->getConfirmPasswordField()->setValue($this->confirmValue); + $value = $this->getPasswordField()->Value(); + + // both password-fields should be the same + if ($value != $this->getConfirmPasswordField()->Value()) { + $result->addFieldError( $name, - $errorMsg, + _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSDONTMATCH', "Passwords don't match"), "validation" ); - - return $this->extendValidationResult(false, $validator); } - } - - if ($this->getRequireStrongPassword()) { - $strongEnough = ConstraintValidator::validate( - $value, - new PasswordStrength(minScore: $this->getMinPasswordStrength()) - )->isValid(); - if (!$strongEnough) { - $validator->validationError( - $name, - _t( - __CLASS__ . '.VALIDATIONSTRONGPASSWORD', - 'The password strength is too low. Please use a stronger password.' - ), - 'validation' - ); - - return $this->extendValidationResult(false, $validator); + + if (!$this->canBeEmpty) { + // both password-fields shouldn't be empty + if (!$value || !$this->getConfirmPasswordField()->Value()) { + $result->addFieldError( + $name, + _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"), + "validation" + ); + } } - } - - // Check if current password is valid - if (!empty($value) && $this->getRequireExistingPassword()) { - if (!$this->currentPasswordValue) { - $validator->validationError( - $name, - _t( - __CLASS__ . '.CURRENT_PASSWORD_MISSING', - 'You must enter your current password.' - ), - "validation" - ); - return $this->extendValidationResult(false, $validator); + + // lengths + $minLength = $this->getMinLength(); + $maxLength = $this->getMaxLength(); + if ($minLength || $maxLength) { + $errorMsg = null; + $limit = null; + if ($minLength && $maxLength) { + $limit = "{{$minLength},{$maxLength}}"; + $errorMsg = _t( + __CLASS__ . '.BETWEEN', + 'Passwords must be {min} to {max} characters long.', + ['min' => $minLength, 'max' => $maxLength] + ); + } elseif ($minLength) { + $limit = "{{$minLength}}.*"; + $errorMsg = _t( + __CLASS__ . '.ATLEAST', + 'Passwords must be at least {min} characters long.', + ['min' => $minLength] + ); + } elseif ($maxLength) { + $limit = "{0,{$maxLength}}"; + $errorMsg = _t( + __CLASS__ . '.MAXIMUM', + 'Passwords must be at most {max} characters long.', + ['max' => $maxLength] + ); + } + $limitRegex = '/^.' . $limit . '$/'; + if (!empty($value) && !preg_match($limitRegex ?? '', $value ?? '')) { + $result->addFieldError( + $name, + $errorMsg, + "validation" + ); + } } - - // Check this password is valid for the current user - $member = Security::getCurrentUser(); - if (!$member) { - $validator->validationError( - $name, - _t( - __CLASS__ . '.LOGGED_IN_ERROR', - "You must be logged in to change your password." - ), - "validation" - ); - return $this->extendValidationResult(false, $validator); + + if ($this->getRequireStrongPassword()) { + $strongEnough = ConstraintValidator::validate( + $value, + new PasswordStrength(minScore: $this->getMinPasswordStrength()) + )->isValid(); + if (!$strongEnough) { + $result->addFieldError( + $name, + _t( + __CLASS__ . '.VALIDATIONSTRONGPASSWORD', + 'The password strength is too low. Please use a stronger password.' + ), + 'validation' + ); + } } - - // With a valid user and password, check the password is correct - $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD); - foreach ($authenticators as $authenticator) { - $checkResult = $authenticator->checkPassword($member, $this->currentPasswordValue); - if (!$checkResult->isValid()) { - $validator->validationError( + + // Check if current password is valid + if (!empty($value) && $this->getRequireExistingPassword()) { + if (!$this->currentPasswordValue) { + $result->addFieldError( $name, _t( - __CLASS__ . '.CURRENT_PASSWORD_ERROR', - "The current password you have entered is not correct." + __CLASS__ . '.CURRENT_PASSWORD_MISSING', + 'You must enter your current password.' ), "validation" ); - return $this->extendValidationResult(false, $validator); + } + + // Check this password is valid for the current user + $member = Security::getCurrentUser(); + if (!$member) { + $result->addFieldError( + $name, + _t( + __CLASS__ . '.LOGGED_IN_ERROR', + "You must be logged in to change your password." + ), + "validation" + ); + return; + } + + // With a valid user and password, check the password is correct + $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD); + foreach ($authenticators as $authenticator) { + $checkResult = $authenticator->checkPassword($member, $this->currentPasswordValue); + if (!$checkResult->isValid()) { + $result->addFieldError( + $name, + _t( + __CLASS__ . '.CURRENT_PASSWORD_ERROR', + "The current password you have entered is not correct." + ), + "validation" + ); + return; + } } } - } - - return $this->extendValidationResult(true, $validator); + }); + return $result->combineAnd(parent::validate()); } /** diff --git a/src/Forms/CurrencyField.php b/src/Forms/CurrencyField.php index 5d36cf8edb3..f0a00fa3fd3 100644 --- a/src/Forms/CurrencyField.php +++ b/src/Forms/CurrencyField.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms; use SilverStripe\ORM\FieldType\DBCurrency; +use SilverStripe\Core\Validation\ValidationResult; /** * Renders a text field, validating its input as a currency. @@ -54,21 +55,22 @@ public function performReadonlyTransformation() return $this->castedCopy(CurrencyField_Readonly::class); } - public function validate($validator) + public function validate(): ValidationResult { - $result = true; - $currencySymbol = preg_quote(DBCurrency::config()->uninherited('currency_symbol') ?? ''); - $regex = '/^\s*(\-?' . $currencySymbol . '?|' . $currencySymbol . '\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/'; - if (!empty($this->value) && !preg_match($regex ?? '', $this->value ?? '')) { - $validator->validationError( - $this->name, - _t('SilverStripe\\Forms\\Form.VALIDCURRENCY', "Please enter a valid currency"), - "validation" - ); - $result = false; - } - - return $this->extendValidationResult($result, $validator); + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($result) { + $currencySymbol = preg_quote(DBCurrency::config()->uninherited('currency_symbol') ?? ''); + $regex = '/^\s*(\-?' . $currencySymbol . '?|' . $currencySymbol . '\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/'; + if (!empty($this->value) && !preg_match($regex ?? '', $this->value ?? '')) { + $result->addFieldError( + $this->name, + _t('SilverStripe\\Forms\\Form.VALIDCURRENCY', "Please enter a valid currency"), + "validation" + ); + $result = false; + } + }); + return $result->combineAnd(parent::validate()); } public function getSchemaValidation() diff --git a/src/Forms/DateField.php b/src/Forms/DateField.php index 4bbc9ed4d5d..701c5754285 100644 --- a/src/Forms/DateField.php +++ b/src/Forms/DateField.php @@ -8,6 +8,10 @@ use SilverStripe\ORM\FieldType\DBDate; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\Core\Validation\FieldValidation\DateFieldValidator; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterInterface; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterTrait; +use PHPUnit\Framework\Attributes\DataProvider; /** * Form used for editing a date string @@ -54,8 +58,14 @@ * @see https://unicode-org.github.io/icu/userguide/format_parse/datetime * @see http://api.jqueryui.com/datepicker/#utility-formatDate */ -class DateField extends TextField +class DateField extends TextField implements FieldValidatorConverterInterface { + use FieldValidatorConverterTrait; + + private static array $field_validators = [ + DateFieldValidator::class => ['getMinDate', 'getMaxDate', 'getFieldValidatorConverter'], + ]; + protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DATE; /** @@ -96,15 +106,6 @@ class DateField extends TextField */ protected $maxDate = null; - /** - * Unparsed value, used exclusively for comparing with internal value - * to detect invalid values. - * - * @var mixed - * @deprecated 5.4.0 Use $value instead - */ - protected $rawValue = null; - /** * Use HTML5-based input fields (and force ISO 8601 date formats). * @@ -307,9 +308,6 @@ public function Type() */ public function setSubmittedValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - // Null case if (!$value) { $this->value = null; @@ -332,17 +330,13 @@ public function setSubmittedValue($value, $data = null) */ public function setValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - - // Null case - if (!$value) { + if (is_null($value)) { $this->value = null; return $this; } // Re-run through formatter to tidy up (e.g. remove time component) - $this->value = $this->tidyInternal($value); + $this->value = $this->tidyInternal($value, false); return $this; } @@ -362,83 +356,6 @@ public function performReadonlyTransformation() return $field; } - /** - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - // Don't validate empty fields - if (empty($this->rawValue)) { - return $this->extendValidationResult(true, $validator); - } - - // We submitted a value, but it couldn't be parsed - if (empty($this->value)) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\DateField.VALIDDATEFORMAT2', - "Please enter a valid date format ({format})", - ['format' => $this->getDateFormat()] - ) - ); - return $this->extendValidationResult(false, $validator); - } - - // Check min date - $min = $this->getMinDate(); - if ($min) { - $oops = strtotime($this->value ?? '') < strtotime($min ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\DateField.VALIDDATEMINDATE', - "Your date has to be newer or matching the minimum allowed date ({date})", - [ - 'date' => sprintf( - '', - $min, - $this->internalToFrontend($min) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - // Check max date - $max = $this->getMaxDate(); - if ($max) { - $oops = strtotime($this->value ?? '') > strtotime($max ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\DateField.VALIDDATEMAXDATE', - "Your date has to be older or matching the maximum allowed date ({date})", - [ - 'date' => sprintf( - '', - $max, - $this->internalToFrontend($max) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - return $this->extendValidationResult(true, $validator); - } - /** * Get locale to use for this field * @@ -488,7 +405,7 @@ public function getMinDate() */ public function setMinDate($minDate) { - $this->minDate = $this->tidyInternal($minDate); + $this->minDate = $this->tidyInternal($minDate, true); return $this; } @@ -506,7 +423,7 @@ public function getMaxDate() */ public function setMaxDate($maxDate) { - $this->maxDate = $this->tidyInternal($maxDate); + $this->maxDate = $this->tidyInternal($maxDate, true); return $this; } @@ -536,13 +453,12 @@ protected function frontendToInternal($date) * as defined by {@link $dateFormat}. With $html5=true, the frontend date will also be * in ISO 8601. * - * @param string $date * @return string The formatted date, or null if not a valid date */ - protected function internalToFrontend($date) + public function internalToFrontend(mixed $value): ?string { - $date = $this->tidyInternal($date); - if (!$date) { + $date = $this->tidyInternal($value, true); + if (is_null($date)) { return null; } $fromFormatter = $this->getInternalFormatter(); @@ -558,12 +474,13 @@ protected function internalToFrontend($date) * Tidy up the internal date representation (ISO 8601), * and fall back to strtotime() if there's parsing errors. * - * @param string $date Date in ISO 8601 or approximate form - * @return string ISO 8601 date, or null if not valid + * @param mixed $date Date in ISO 8601 or approximate form + * @param bool $returnNullOnFailure return null if the date is not valid, or the original date + * @return null|string ISO 8601 date or null */ - protected function tidyInternal($date) + protected function tidyInternal(mixed $date, bool $returnNullOnFailure): ?string { - if (!$date) { + if (is_null($date)) { return null; } // Re-run through formatter to tidy up (e.g. remove time component) @@ -573,7 +490,7 @@ protected function tidyInternal($date) // Fallback to strtotime $timestamp = strtotime($date ?? '', DBDatetime::now()->getTimestamp()); if ($timestamp === false) { - return null; + return $returnNullOnFailure ? null : $date; } } return $formatter->format($timestamp); diff --git a/src/Forms/DateField_Disabled.php b/src/Forms/DateField_Disabled.php index 6bdc64f882e..be56d92ea5a 100644 --- a/src/Forms/DateField_Disabled.php +++ b/src/Forms/DateField_Disabled.php @@ -22,26 +22,27 @@ public function Field($properties = []) $value = $this->dataValue(); if ($value) { - $value = $this->tidyInternal($value); + $value = $this->tidyInternal($value, true); $df = new DBDate($this->name); $df->setValue($value); - - if ($df->IsToday()) { - // e.g. 2018-06-01 (today) - $format = '%s (%s)'; - $infoComplement = _t('SilverStripe\\Forms\\DateField.TODAY', 'today'); - } else { - // e.g. 2018-06-01, 5 days ago - $format = '%s, %s'; - $infoComplement = $df->Ago(); + // if the date is not valid, $df->getValue() will return false + if ($df->getValue()) { + if ($df->IsToday()) { + // e.g. 2018-06-01 (today) + $format = '%s (%s)'; + $infoComplement = _t('SilverStripe\\Forms\\DateField.TODAY', 'today'); + } else { + // e.g. 2018-06-01, 5 days ago + $format = '%s, %s'; + $infoComplement = $df->Ago(); + } + // Render the display value with some complement of info + $displayValue = Convert::raw2xml(sprintf( + $format ?? '', + $this->Value(), + $infoComplement + )); } - - // Render the display value with some complement of info - $displayValue = Convert::raw2xml(sprintf( - $format ?? '', - $this->Value(), - $infoComplement - )); } return sprintf( diff --git a/src/Forms/DatetimeField.php b/src/Forms/DatetimeField.php index b8e4a7b0985..e12b713abbf 100644 --- a/src/Forms/DatetimeField.php +++ b/src/Forms/DatetimeField.php @@ -7,7 +7,9 @@ use SilverStripe\i18n\i18n; use SilverStripe\ORM\FieldType\DBDate; use SilverStripe\ORM\FieldType\DBDatetime; -use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\Core\Validation\FieldValidation\DatetimeFieldValidator; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterTrait; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterInterface; /** * Form field used for editing date time strings. @@ -16,8 +18,13 @@ * Data is passed on via {@link dataValue()} with whitespace separators. * The {@link $value} property is always in ISO 8601 format, in the server timezone. */ -class DatetimeField extends TextField +class DatetimeField extends TextField implements FieldValidatorConverterInterface { + use FieldValidatorConverterTrait; + + private static array $field_validators = [ + DatetimeFieldValidator::class => ['getMinDatetime', 'getMaxDatetime', 'getFieldValidatorConverter'], + ]; /** * @var bool @@ -70,15 +77,6 @@ class DatetimeField extends TextField */ protected $timeLength = null; - /** - * Unparsed value, used exclusively for comparing with internal value - * to detect invalid values. - * - * @var mixed - * @deprecated 5.4.0 Use $value instead - */ - protected $rawValue = null; - /** * @inheritDoc */ @@ -159,9 +157,6 @@ public function setHTML5($bool) */ public function setSubmittedValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - // Null case if (!$value) { $this->value = null; @@ -325,11 +320,7 @@ protected function getInternalFormatter($timezone = null) */ public function setValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - - // Empty value - if (empty($value)) { + if (is_null($value)) { $this->value = null; return $this; } @@ -347,6 +338,7 @@ public function setValue($value, $data = null) } if ($timestamp === false) { + $this->value = $value; return $this; } @@ -376,12 +368,11 @@ public function Value() * as defined by {@link $dateFormat}. With $html5=true, the frontend date will also be * in ISO 8601. * - * @param string $datetime - * @return string The formatted date and time, or null if not a valid date and time + * @return null|string The formatted date and time, or null if not a valid date and time */ - public function internalToFrontend($datetime) + public function internalToFrontend(mixed $value): ?string { - $datetime = $this->tidyInternal($datetime); + $datetime = $this->tidyInternal($value, true); if (!$datetime) { return null; } @@ -399,22 +390,23 @@ public function internalToFrontend($datetime) * Tidy up the internal date representation (ISO 8601), * and fall back to strtotime() if there's parsing errors. * - * @param string $datetime Date in ISO 8601 or approximate form - * @return string ISO 8601 date, or null if not valid + * @param mixed $datetime Date in ISO 8601 or approximate form + * @param bool $returnNullOnFailure return null if the datetime is not valid, or the original datetime + * @return null|string ISO 8601 date or null */ - public function tidyInternal($datetime) + public function tidyInternal(mixed $datetime, bool $returnNullOnFailure): ?string { - if (!$datetime) { + if (is_null($datetime)) { return null; } - // Re-run through formatter to tidy up (e.g. remove time component) + // Re-run through formatter to tidy up $formatter = $this->getInternalFormatter(); $timestamp = $formatter->parse($datetime); if ($timestamp === false) { // Fallback to strtotime $timestamp = strtotime($datetime ?? '', DBDatetime::now()->getTimestamp()); if ($timestamp === false) { - return null; + return $returnNullOnFailure ? null : $datetime; } } return $formatter->format($timestamp); @@ -536,7 +528,7 @@ public function getMinDatetime() */ public function setMinDatetime($minDatetime) { - $this->minDatetime = $this->tidyInternal($minDatetime); + $this->minDatetime = $this->tidyInternal($minDatetime, true); return $this; } @@ -554,87 +546,10 @@ public function getMaxDatetime() */ public function setMaxDatetime($maxDatetime) { - $this->maxDatetime = $this->tidyInternal($maxDatetime); + $this->maxDatetime = $this->tidyInternal($maxDatetime, true); return $this; } - /** - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - // Don't validate empty fields - if (empty($this->rawValue)) { - return $this->extendValidationResult(true, $validator); - } - - // We submitted a value, but it couldn't be parsed - if (empty($this->value)) { - $validator->validationError( - $this->name, - _t( - __CLASS__ . '.VALIDDATETIMEFORMAT', - "Please enter a valid date and time format ({format})", - ['format' => $this->getDatetimeFormat()] - ) - ); - return $this->extendValidationResult(false, $validator); - } - - // Check min date (in server timezone) - $min = $this->getMinDatetime(); - if ($min) { - $oops = strtotime($this->value ?? '') < strtotime($min ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - __CLASS__ . '.VALIDDATETIMEMINDATE', - "Your date has to be newer or matching the minimum allowed date and time ({datetime})", - [ - 'datetime' => sprintf( - '', - $min, - $this->internalToFrontend($min) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - // Check max date (in server timezone) - $max = $this->getMaxDatetime(); - if ($max) { - $oops = strtotime($this->value ?? '') > strtotime($max ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - __CLASS__ . '.VALIDDATEMAXDATETIME', - "Your date has to be older or matching the maximum allowed date and time ({datetime})", - [ - 'datetime' => sprintf( - '', - $max, - $this->internalToFrontend($max) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - return $this->extendValidationResult(true, $validator); - } - public function performReadonlyTransformation() { $field = clone $this; diff --git a/src/Forms/EmailField.php b/src/Forms/EmailField.php index 4426d67f8c6..752f047756b 100644 --- a/src/Forms/EmailField.php +++ b/src/Forms/EmailField.php @@ -2,14 +2,17 @@ namespace SilverStripe\Forms; -use SilverStripe\Core\Validation\ConstraintValidator; -use Symfony\Component\Validator\Constraints\Email as EmailConstraint; +use SilverStripe\Core\Validation\FieldValidation\EmailFieldValidator; /** * Text input field with validation for correct email format according to the relevant RFC. */ class EmailField extends TextField { + private static array $field_validators = [ + EmailFieldValidator::class, + ]; + protected $inputType = 'email'; public function Type() @@ -17,27 +20,6 @@ public function Type() return 'email text'; } - /** - * Validates for RFC compliant email addresses. - * - * @param Validator $validator - */ - public function validate($validator) - { - $this->value = trim($this->value ?? ''); - - $message = _t('SilverStripe\\Forms\\EmailField.VALIDATION', 'Please enter an email address'); - $result = ConstraintValidator::validate( - $this->value, - new EmailConstraint(message: $message, mode: EmailConstraint::VALIDATION_MODE_STRICT), - $this->getName() - ); - $validator->getResult()->combineAnd($result); - $isValid = $result->isValid(); - - return $this->extendValidationResult($isValid, $validator); - } - public function getSchemaValidation() { $rules = parent::getSchemaValidation(); diff --git a/src/Forms/FieldGroup.php b/src/Forms/FieldGroup.php index c61de2136b9..7e1e77b04d6 100644 --- a/src/Forms/FieldGroup.php +++ b/src/Forms/FieldGroup.php @@ -106,7 +106,7 @@ public function __construct($titleOrField = null, $otherFields = null) * In some cases the FieldGroup doesn't have a title, but we still want * the ID / name to be set. This code, generates the ID from the nested children */ - public function getName() + public function getName(): string { if ($this->name) { return $this->name; diff --git a/src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php b/src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php new file mode 100644 index 00000000000..29fd085602e --- /dev/null +++ b/src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php @@ -0,0 +1,10 @@ +internalToFrontend($value); + }; + } +} diff --git a/src/Forms/FieldsValidator.php b/src/Forms/FieldsValidator.php index 0de96225fdf..6ff9b7317a8 100644 --- a/src/Forms/FieldsValidator.php +++ b/src/Forms/FieldsValidator.php @@ -13,7 +13,9 @@ public function php($data): bool $fields = $this->form->Fields(); foreach ($fields as $field) { - $valid = ($field->validate($this) && $valid); + $result = $field->validate(); + $valid = $result->isValid() && $valid; + $this->result->combineAnd($result); } return $valid; diff --git a/src/Forms/FileField.php b/src/Forms/FileField.php index 8040d1fd594..b0ab8352adc 100644 --- a/src/Forms/FileField.php +++ b/src/Forms/FileField.php @@ -7,6 +7,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; +use SilverStripe\Core\Validation\ValidationResult; /** * Represents a file type which can be added to a form. @@ -176,59 +177,49 @@ public function Value() return isset($_FILES[$this->getName()]) ? $_FILES[$this->getName()] : null; } - public function validate($validator) + public function validate(): ValidationResult { - // FileField with the name multi_file_syntax[] or multi_file_syntax[key] will have the brackets trimmed in - // $_FILES super-global so it will be stored as $_FILES['mutli_file_syntax'] - // multi-file uploads, which are not officially supported by Silverstripe, though may be - // implemented in custom code, so we should still ensure they are at least validated - $isMultiFileUpload = strpos($this->name ?? '', '[') !== false; - $fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name ?? ''); - - if (!isset($_FILES[$fieldName])) { - return $this->extendValidationResult(true, $validator); - } - - if ($isMultiFileUpload) { - $isValid = true; - foreach (array_keys($_FILES[$fieldName]['name'] ?? []) as $key) { - $fileData = [ - 'name' => $_FILES[$fieldName]['name'][$key], - 'type' => $_FILES[$fieldName]['type'][$key], - 'tmp_name' => $_FILES[$fieldName]['tmp_name'][$key], - 'error' => $_FILES[$fieldName]['error'][$key], - 'size' => $_FILES[$fieldName]['size'][$key], - ]; - if (!$this->validateFileData($validator, $fileData)) { - $isValid = false; + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($result) { + // FileField with the name multi_file_syntax[] or multi_file_syntax[key] will have the brackets trimmed in + // $_FILES super-global so it will be stored as $_FILES['mutli_file_syntax'] + // multi-file uploads, which are not officially supported by Silverstripe, though may be + // implemented in custom code, so we should still ensure they are at least validated + $isMultiFileUpload = strpos($this->name ?? '', '[') !== false; + $fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name ?? ''); + if (!isset($_FILES[$fieldName])) { + return; + } + if ($isMultiFileUpload) { + foreach (array_keys($_FILES[$fieldName]['name'] ?? []) as $key) { + $fileData = [ + 'name' => $_FILES[$fieldName]['name'][$key], + 'type' => $_FILES[$fieldName]['type'][$key], + 'tmp_name' => $_FILES[$fieldName]['tmp_name'][$key], + 'error' => $_FILES[$fieldName]['error'][$key], + 'size' => $_FILES[$fieldName]['size'][$key], + ]; + $this->validateFileData($result, $fileData); } + return; } - return $this->extendValidationResult($isValid, $validator); - } - - // regular single-file upload - $result = $this->validateFileData($validator, $_FILES[$this->name]); - return $this->extendValidationResult($result, $validator); + // regular single-file upload + $this->validateFileData($result, $_FILES[$this->name]); + }); + return $result->combineAnd(parent::validate()); } - /** - * @param Validator $validator - * @param array $fileData - * @return bool - */ - private function validateFileData($validator, array $fileData): bool + private function validateFileData(ValidationResult $result, array $fileData): void { $valid = $this->upload->validate($fileData); if (!$valid) { $errors = $this->upload->getErrors(); if ($errors) { foreach ($errors as $error) { - $validator->validationError($this->name, $error, "validation"); + $result->addFieldError($this->name, $error, "validation"); } } - return false; } - return true; } /** diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index d5a08e39a95..d69f07aeb53 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -17,6 +17,9 @@ use SilverStripe\Model\ModelData; use SilverStripe\ORM\DataObject; use SilverStripe\Dev\Deprecation; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationTrait; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface; +use InvalidArgumentException; /** * Represents a field in a form. @@ -42,10 +45,11 @@ * including both structure (name, id, attributes, etc.) and state (field value). * Can be used by for JSON data which is consumed by a front-end application. */ -class FormField extends RequestHandler +class FormField extends RequestHandler implements FieldValidationInterface { use AttributesHTML; use FormMessage; + use FieldValidationTrait; /** @see $schemaDataType */ const SCHEMA_DATA_TYPE_STRING = 'String'; @@ -429,7 +433,7 @@ public function getTemplateHelper() /** * Returns the field name. */ - public function getName() + public function getName(): string { return $this->name; } @@ -445,9 +449,17 @@ public function getInputType() } /** - * Returns the field value. + * Returns the internal value of the field + */ + public function getValue(): mixed + { + return $this->value; + } + + /** + * Returns the value of the field which may be modified for display purposes + * for instance to add localisation or formatting. * - * @see FormField::setSubmittedValue() * @return mixed */ public function Value() @@ -455,6 +467,16 @@ public function Value() return $this->value; } + /** + * Returns the value of the field suitable for insertion into the database. + * + * @return mixed + */ + public function dataValue() + { + return $this->value; + } + /** * Method to save this form field into the given record. * @@ -483,16 +505,6 @@ public function saveInto(DataObjectInterface $record) } } - /** - * Returns the field value suitable for insertion into the data object. - * @see Formfield::setValue() - * @return mixed - */ - public function dataValue() - { - return $this->value; - } - /** * Returns the field label - used by templates. * @@ -1222,32 +1234,30 @@ public function Type() } /** - * Utility method to call an extension hook which allows the result of validate() calls to be adjusted - * - * @param bool $result - * @param Validator $validator - * @return bool - * @deprecated 5.4.0 Use extend() directly instead + * Determines whether the field is valid or not based on its value */ - protected function extendValidationResult(bool $result, Validator $validator): bool + public function validate(): ValidationResult { - Deprecation::notice('5.4.0', 'Use extend() directly instead'); - $this->extend('updateValidationResult', $result, $validator); + if (count(func_get_args()) !== 0) { + // In prior versions, FormField::validate() had a method signature of + // validate(Validator $validator): bool + // Old code may still call this method this without throwing an error, however the + // return type is now ValidationResult which may be mistakenly interpreted as truthy + // While we cannot stop people from interpreting the return value as a boolean, we can + // at least throw an exception so they might notice something is wrong. + $message = 'validate() does not accept any arguments.' + . ' Note that the return type is ValidationResult, not boolean.'; + throw new InvalidArgumentException($message); + } + $result = ValidationResult::create(); + $fieldValidators = $this->getFieldValidators(); + foreach ($fieldValidators as $fieldValidator) { + $result->combineAnd($fieldValidator->validate()); + } + $this->extend('updateValidate', $result); return $result; } - /** - * Abstract method each {@link FormField} subclass must implement, determines whether the field - * is valid or not based on the value. - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - return $this->extendValidationResult(true, $validator); - } - /** * Describe this field, provide help text for it. * diff --git a/src/Forms/ListboxField.php b/src/Forms/ListboxField.php index 5ff18afff0c..60c608e4d2f 100644 --- a/src/Forms/ListboxField.php +++ b/src/Forms/ListboxField.php @@ -4,6 +4,7 @@ use SilverStripe\Model\List\ArrayList; use SilverStripe\Model\ArrayData; +use SilverStripe\ORM\DataObject; /** * Multi-line listbox field, created from a select tag. @@ -165,6 +166,17 @@ public function getDisabledItems() return $this->disabledItems; } + public function getValueForValidation(): mixed + { + // Unlike other MultiSelectField's, ListboxField allows setting a single value + // Wrap single values in an array so that the validation logic can treat it as a multi-select + $value = $this->getValue(); + if (!is_array($value) && !is_null($value)) { + $value = [$value]; + } + return $value; + } + /** * Provide ListboxField data to the JSON schema for the frontend component * @@ -256,7 +268,7 @@ public function getValueArray() $replaced = []; foreach ($value as $item) { if (!is_array($item)) { - $item = json_decode($item, true); + $item = json_decode($item ?? '', true); } if ($targetType === gettype($item)) { diff --git a/src/Forms/LookupField.php b/src/Forms/LookupField.php index ac388fc394b..1cb0b3e256d 100644 --- a/src/Forms/LookupField.php +++ b/src/Forms/LookupField.php @@ -6,6 +6,7 @@ use SilverStripe\Core\ArrayLib; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\Core\Validation\FieldValidation\MultiOptionFieldValidator; /** * Read-only complement of {@link MultiSelectField}. @@ -15,6 +16,10 @@ */ class LookupField extends MultiSelectField { + private static $field_validators = [ + MultiOptionFieldValidator::class => null, + ]; + protected $schemaComponent = 'LookupField'; /** @@ -65,17 +70,6 @@ public function Field($properties = []) return parent::Field($properties); } - /** - * Ignore validation as the field is readonly - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - return $this->extendValidationResult(true, $validator); - } - /** * Stubbed so invalid data doesn't save into the DB * diff --git a/src/Forms/MoneyField.php b/src/Forms/MoneyField.php index 1d8e4688911..325c2764dca 100644 --- a/src/Forms/MoneyField.php +++ b/src/Forms/MoneyField.php @@ -4,8 +4,10 @@ use InvalidArgumentException; use SilverStripe\Core\ArrayLib; +use SilverStripe\Core\Validation\FieldValidation\CompositeFieldValidator; use SilverStripe\ORM\FieldType\DBMoney; use SilverStripe\ORM\DataObjectInterface; +use SilverStripe\Core\Validation\ValidationResult; /** * A form field that can save into a {@link Money} database field. @@ -16,6 +18,9 @@ */ class MoneyField extends FormField { + private static array $field_validators = [ + CompositeFieldValidator::class, + ]; protected $schemaDataType = 'MoneyField'; @@ -321,32 +326,30 @@ public function getLocale() return $this->fieldAmount->getLocale(); } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) + public function getValueForValidation(): mixed { - // Validate currency - $currencies = $this->getAllowedCurrencies(); - $currency = $this->fieldCurrency->dataValue(); - if ($currency && $currencies && !in_array($currency, $currencies ?? [])) { - $validator->validationError( - $this->getName(), - _t( - __CLASS__ . '.INVALID_CURRENCY', - 'Currency {currency} is not in the list of allowed currencies', - ['currency' => $currency] - ) - ); - return $this->extendValidationResult(false, $validator); - } + return [$this->getAmountField(), $this->getCurrencyField()]; + } - // Field-specific validation - $result = $this->fieldAmount->validate($validator) && $this->fieldCurrency->validate($validator); - return $this->extendValidationResult($result, $validator); + public function validate(): ValidationResult + { + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($result) { + // Validate currency + $currencies = $this->getAllowedCurrencies(); + $currency = $this->fieldCurrency->dataValue(); + if ($currency && $currencies && !in_array($currency, $currencies ?? [])) { + $result->addFieldError( + $this->getName(), + _t( + __CLASS__ . '.INVALID_CURRENCY', + 'Currency {currency} is not in the list of allowed currencies', + ['currency' => $currency] + ) + ); + } + }); + return $result->combineAnd(parent::validate()); } public function setForm($form) diff --git a/src/Forms/MultiSelectField.php b/src/Forms/MultiSelectField.php index 7fe0a9b5999..1e0a0bd179c 100644 --- a/src/Forms/MultiSelectField.php +++ b/src/Forms/MultiSelectField.php @@ -6,6 +6,8 @@ use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBMultiEnum; use SilverStripe\ORM\Relation; +use SilverStripe\Core\Validation\FieldValidation\MultiOptionFieldValidator; +use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; /** * Represents a SelectField that may potentially have multiple selections, and may have @@ -13,6 +15,10 @@ */ abstract class MultiSelectField extends SelectField { + private static array $field_validators = [ + OptionFieldValidator::class => null, + MultiOptionFieldValidator::class => ['getValidValues'], + ]; /** * List of items to mark as checked, and may not be unchecked @@ -72,11 +78,28 @@ public function setValue($value, $obj = null) if ($obj instanceof DataObject) { $this->loadFrom($obj); } else { - parent::setValue($value); + parent::setValue($value, $obj); } return $this; } + public function setSubmittedValue($value, $data = null) + { + // When no value is selected and a regular edit form is submitted, + // usually an empty string will be POSTed e.g. MyField[]: "" + if ($value === [""]) { + $value = null; + } + if (is_array($value) && $this->getAllSourceValuesAreIntType()) { + foreach ($value as $key => $val) { + if (is_numeric($val) && is_string($val)) { + $value[$key] = (int) $val; + } + } + } + return parent::setSubmittedValue($value, $data); + } + /** * Load the value from the dataobject into this field * @@ -223,49 +246,6 @@ protected function csvDecode($value) return preg_split('/\s*,\s*/', trim($value ?? '')); } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - $values = $this->getValueArray(); - $validValues = $this->getValidValues(); - - // Filter out selected values not in the data source - $self = $this; - $invalidValues = array_filter( - $values ?? [], - function ($userValue) use ($self, $validValues) { - foreach ($validValues as $formValue) { - if ($self->isSelectedValue($formValue, $userValue)) { - return false; - } - } - return true; - } - ); - - $result = true; - if (!empty($invalidValues)) { - $result = false; - // List invalid items - $validator->validationError( - $this->getName(), - _t( - 'SilverStripe\\Forms\\MultiSelectField.SOURCE_VALIDATION', - "Please select values within the list provided. Invalid option(s) {value} given", - ['value' => implode(',', $invalidValues)] - ), - "validation" - ); - } - - return $this->extendValidationResult($result, $validator); - } - /** * Transforms the source data for this CheckboxSetField * into a comma separated list of values. diff --git a/src/Forms/NumericField.php b/src/Forms/NumericField.php index 2e93e3e9c30..0cd37b05389 100644 --- a/src/Forms/NumericField.php +++ b/src/Forms/NumericField.php @@ -3,7 +3,9 @@ namespace SilverStripe\Forms; use NumberFormatter; +use SilverStripe\Core\Validation\FieldValidation\NumericFieldValidator; use SilverStripe\i18n\i18n; +use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; /** * Text input field with validation for numeric values. Supports validating @@ -12,6 +14,11 @@ */ class NumericField extends TextField { + private static array $field_validators = [ + // Disable parent validator as value for validation will be cast to int or float + StringFieldValidator::class => null, + NumericFieldValidator::class, + ]; protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DECIMAL; @@ -48,6 +55,14 @@ class NumericField extends TextField */ protected $scale = 0; + public function __construct($name, $title = null, $value = null, $maxLength = null, $form = null) + { + // This constructor has a default value of null for the $value param, as opposed to the parent + // TextField constructor which has a default value of blank string. This is done to prevent passing + // a blank string to the NumericFieldValidator which which will cause a validation error. + parent::__construct($name, $title, $value, $maxLength, $form); + } + /** * Get number formatter for localising this field * @@ -104,7 +119,6 @@ protected function getNumberType() /** * In some cases and locales, validation expects non-breaking spaces. * This homogenises regular, narrow and thin non-breaking spaces to a regular space character. - * */ private function clean(?string $value): string { @@ -113,6 +127,11 @@ private function clean(?string $value): string public function setSubmittedValue($value, $data = null) { + if (is_null($value)) { + $this->value = null; + return $this; + } + // Save original value in case parse fails $value = $this->clean($value); $this->originalValue = $value; @@ -142,7 +161,7 @@ public function setSubmittedValue($value, $data = null) public function Value() { // Show invalid value back to user in case of error - if ($this->value === null || $this->value === false) { + if (!is_int($this->value) && !is_float($this->value)) { return $this->originalValue; } $formatter = $this->getFormatter(); @@ -156,21 +175,30 @@ public function setValue($value, $data = null) return $this; } + public function getValueForValidation(): mixed + { + $value = $this->value; + if (is_numeric($value) && is_string($value)) { + return $this->cast($value); + } + return $value; + } + /** * Helper to cast non-localised strings to their native type * * @param string $value - * @return float|int + * @return mixed */ protected function cast($value) { - if (strlen($value ?? '') === 0) { - return null; + if (!is_string($value) || strlen($value) === 0) { + return $value; } if ($this->getScale() === 0) { - return (int)$value; + return (int) $value; } - return (float)$value; + return (float) $value; } /** @@ -193,31 +221,6 @@ public function getAttributes() return $attributes; } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - $result = true; - // false signifies invalid value due to failed parse() - if ($this->value === false) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\NumericField.VALIDATION', - "'{value}' is not a number, only numbers can be accepted for this field", - ['value' => $this->originalValue] - ) - ); - $result = false; - } - - return $this->extendValidationResult($result, $validator); - } - public function getSchemaValidation() { $rules = parent::getSchemaValidation(); @@ -232,6 +235,9 @@ public function getSchemaValidation() */ public function dataValue() { + if ($this->value === '') { + return null; + } return $this->cast($this->value); } diff --git a/src/Forms/OptionsetField.php b/src/Forms/OptionsetField.php index 72655646c36..a966aebe375 100644 --- a/src/Forms/OptionsetField.php +++ b/src/Forms/OptionsetField.php @@ -134,18 +134,6 @@ public function Field($properties = []) return FormField::Field($properties); } - /** - * {@inheritdoc} - */ - public function validate($validator) - { - if (!$this->Value()) { - return $this->extendValidationResult(true, $validator); - } - - return parent::validate($validator); - } - public function getAttributes() { $attributes = array_merge( diff --git a/src/Forms/RequiredFields.php b/src/Forms/RequiredFields.php index 395c18a9647..6e2068aba82 100644 --- a/src/Forms/RequiredFields.php +++ b/src/Forms/RequiredFields.php @@ -87,7 +87,9 @@ public function php($data) $fields = $this->form->Fields(); foreach ($fields as $field) { - $valid = ($field->validate($this) && $valid); + $result = $field->validate(); + $valid = $result->isValid() && $valid; + $this->result->combineAnd($result); } if (!$this->required) { diff --git a/src/Forms/SearchableDropdownField.php b/src/Forms/SearchableDropdownField.php index eb7c7d5beb6..5f459ad6634 100644 --- a/src/Forms/SearchableDropdownField.php +++ b/src/Forms/SearchableDropdownField.php @@ -38,4 +38,15 @@ public function setEmptyString($string) Deprecation::notice('5.2.0', 'Use setPlaceholder() instead'); return parent::setEmptyString($string); } + + public function getValueForValidation(): mixed + { + $arr = $this->getValueArray(); + if (count($arr) === 0) { + return null; + } elseif (count($arr) === 1) { + return $arr[0]; + } + return $arr; + } } diff --git a/src/Forms/SearchableDropdownTrait.php b/src/Forms/SearchableDropdownTrait.php index fa751c49a2f..f49d0e878a8 100644 --- a/src/Forms/SearchableDropdownTrait.php +++ b/src/Forms/SearchableDropdownTrait.php @@ -417,14 +417,6 @@ public function saveInto(DataObjectInterface $record): void } } - /** - * @param Validator $validator - */ - public function validate($validator): bool - { - return $this->extendValidationResult(true, $validator); - } - public function getSchemaDataType(): string { if ($this->isMultiple) { diff --git a/src/Forms/SearchableMultiDropdownField.php b/src/Forms/SearchableMultiDropdownField.php index 90608a40de5..541c114a7fd 100644 --- a/src/Forms/SearchableMultiDropdownField.php +++ b/src/Forms/SearchableMultiDropdownField.php @@ -25,4 +25,9 @@ public function __construct( $this->addExtraClass('ss-searchable-dropdown-field'); $this->setIsMultiple(true); } + + public function getValueForValidation(): mixed + { + return $this->getValueArray(); + } } diff --git a/src/Forms/SelectField.php b/src/Forms/SelectField.php index 22485590db9..abdae9c5302 100644 --- a/src/Forms/SelectField.php +++ b/src/Forms/SelectField.php @@ -5,12 +5,16 @@ use SilverStripe\Model\List\SS_List; use SilverStripe\Model\List\Map; use ArrayAccess; +use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; /** * Represents a field that allows users to select one or more items from a list */ abstract class SelectField extends FormField { + private static array $field_validators = [ + OptionFieldValidator::class => ['getValidValues'], + ]; /** * Associative or numeric array of all dropdown items, @@ -121,6 +125,16 @@ protected function getSourceValues() return array_keys($this->getSource() ?? []); } + /** + * Whether all source values are typed as int + */ + protected function getAllSourceValuesAreIntType(): bool + { + $sourceValues = $this->getSourceValues(); + $intCount = count(array_filter(array_map('is_int', $sourceValues))); + return $intCount === count($sourceValues); + } + /** * Gets all valid values for this field. * @@ -130,9 +144,9 @@ protected function getSourceValues() */ public function getValidValues() { - $valid = array_diff($this->getSourceValues() ?? [], $this->getDisabledItems()); + $values = array_diff($this->getSourceValues() ?? [], $this->getDisabledItems()); // Renumber indexes from 0 - return array_values($valid ?? []); + return array_values($values); } /** @@ -203,7 +217,10 @@ protected function getListValues($values) return $values->column('ID'); } - return [trim($values ?? '')]; + if (is_string($values)) { + $values = trim($values); + } + return is_array($values) ? $values : [$values]; } /** diff --git a/src/Forms/SelectionGroup_Item.php b/src/Forms/SelectionGroup_Item.php index 55d021859e7..ab23c0cc83d 100644 --- a/src/Forms/SelectionGroup_Item.php +++ b/src/Forms/SelectionGroup_Item.php @@ -43,7 +43,7 @@ function setTitle($title) return $this; } - function getValue() + function getValue(): mixed { return $this->value; } diff --git a/src/Forms/SingleLookupField.php b/src/Forms/SingleLookupField.php index 5590574caa1..1bf54b8f24e 100644 --- a/src/Forms/SingleLookupField.php +++ b/src/Forms/SingleLookupField.php @@ -3,9 +3,12 @@ namespace SilverStripe\Forms; use SilverStripe\Core\Convert; +use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\Model\List\Map; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; +use PHPUnit\Framework\Attributes\DataProvider; /** * Read-only complement of {@link DropdownField}. @@ -15,6 +18,10 @@ */ class SingleLookupField extends SingleSelectField { + private static $field_validators = [ + OptionFieldValidator::class => null, + ]; + /** * @var bool */ @@ -36,17 +43,6 @@ protected function valueToLabel() return null; } - /** - * Ignore validation as the field is readonly - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - return $this->extendValidationResult(true, $validator); - } - /** * Stubbed so invalid data doesn't save into the DB * diff --git a/src/Forms/SingleSelectField.php b/src/Forms/SingleSelectField.php index 1dcd6b502ed..b2d6f81c229 100644 --- a/src/Forms/SingleSelectField.php +++ b/src/Forms/SingleSelectField.php @@ -9,7 +9,6 @@ */ abstract class SingleSelectField extends SelectField { - /** * Show the first