From e733757961a2bc6b96f675d58db3d3b8eeff43f3 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Mon, 4 Nov 2024 16:48:05 +1300 Subject: [PATCH] ENH Use FieldValidators for FormField validation --- .../FieldValidation/DateFieldValidator.php | 105 +++++++- src/Forms/CompositeField.php | 27 +- src/Forms/ConfirmedPasswordField.php | 252 +++++++++--------- src/Forms/CurrencyField.php | 30 ++- src/Forms/DateField.php | 82 +----- src/Forms/DatetimeField.php | 82 +----- src/Forms/EmailField.php | 28 +- src/Forms/FieldGroup.php | 2 +- src/Forms/FieldsValidator.php | 4 +- src/Forms/FileField.php | 69 +++-- src/Forms/FormField.php | 57 ++-- src/Forms/LookupField.php | 11 +- src/Forms/MoneyField.php | 51 ++-- src/Forms/MultiSelectField.php | 49 +--- src/Forms/NumericField.php | 46 ++-- src/Forms/OptionsetField.php | 12 - src/Forms/RequiredFields.php | 4 +- src/Forms/SearchableDropdownTrait.php | 8 - src/Forms/SelectField.php | 8 +- src/Forms/SelectionGroup_Item.php | 2 +- src/Forms/SingleLookupField.php | 8 +- src/Forms/SingleSelectField.php | 44 +-- src/Forms/TextField.php | 31 +-- src/Forms/TextareaField.php | 30 +-- src/Forms/TimeField.php | 36 +-- src/Forms/UrlField.php | 41 +-- src/ORM/FieldType/DBField.php | 2 - tests/php/Forms/FormFieldTest.php | 62 ++--- .../FieldValidationExtension.php | 15 +- 29 files changed, 485 insertions(+), 713 deletions(-) diff --git a/src/Core/Validation/FieldValidation/DateFieldValidator.php b/src/Core/Validation/FieldValidation/DateFieldValidator.php index 0a2a3de22b3..6be2c216cb7 100644 --- a/src/Core/Validation/FieldValidation/DateFieldValidator.php +++ b/src/Core/Validation/FieldValidation/DateFieldValidator.php @@ -2,18 +2,76 @@ namespace SilverStripe\Core\Validation\FieldValidation; +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, + ?int $minValue = null, + ?int $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) && $minTimestamp < $maxTimestamp) { + 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 +79,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) { $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 +122,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/Forms/CompositeField.php b/src/Forms/CompositeField.php index d4f2aef64ce..fff15876716 100644 --- a/src/Forms/CompositeField.php +++ b/src/Forms/CompositeField.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms; use SilverStripe\Dev\Debug; +use SilverStripe\Core\Validation\ValidationResult; /** * Base class for all fields that contain other fields. @@ -120,7 +121,7 @@ public function getChildren() * 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; @@ -523,18 +524,18 @@ public function debug(): string return $result; } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) + // TODO: replace with CompositeFieldValidator ? + public function validate(): ValidationResult { - $valid = true; - foreach ($this->children as $child) { - $valid = ($child && $child->validate($validator) && $valid); - } - return $this->extendValidationResult($valid, $validator); + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($result) { + foreach ($this->children as $child) { + if (!$child) { + continue; + } + $result->combineAnd($child->validate()); + } + }); + return $result->combineAnd(parent::validate()); } } diff --git a/src/Forms/ConfirmedPasswordField.php b/src/Forms/ConfirmedPasswordField.php index 1856a057eff..c1f731ae14a 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,144 @@ 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); + return; } - } - - 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" + ); + return; + } } - } - - // 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" + ); + return; + } } - - // 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); + return; + } + + // 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" + ); + } + + // 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..7f41bd306c0 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) + // TODO replace with DecimalFieldValidator? + 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" + ); + } + }); + return $result->combineAnd(parent::validate()); } public function getSchemaValidation() diff --git a/src/Forms/DateField.php b/src/Forms/DateField.php index 5649f8039e1..520dfeec5de 100644 --- a/src/Forms/DateField.php +++ b/src/Forms/DateField.php @@ -8,6 +8,7 @@ use SilverStripe\ORM\FieldType\DBDate; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\Core\Validation\FieldValidation\DateFieldValidator; /** * Form used for editing a date string @@ -56,6 +57,10 @@ */ class DateField extends TextField { + private static array $field_validators = [ + DateFieldValidator::class => ['getMinDate', 'getMaxDate', 'Value'], + ]; + protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DATE; /** @@ -361,83 +366,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 * diff --git a/src/Forms/DatetimeField.php b/src/Forms/DatetimeField.php index c822dbf7b42..7c00dfcded8 100644 --- a/src/Forms/DatetimeField.php +++ b/src/Forms/DatetimeField.php @@ -7,7 +7,7 @@ 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; /** * Form field used for editing date time strings. @@ -18,6 +18,9 @@ */ class DatetimeField extends TextField { + private static array $field_validators = [ + DatetimeFieldValidator::class => ['getMinDatetime', 'getMaxDatetime', 'Value'], + ]; /** * @var bool @@ -557,83 +560,6 @@ public function setMaxDatetime($maxDatetime) 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/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 b6f336b8972..8ab4602fd6e 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -16,6 +16,8 @@ use SilverStripe\View\SSViewer; use SilverStripe\Model\ModelData; use SilverStripe\ORM\DataObject; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationTrait; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface; /** * Represents a field in a form. @@ -41,10 +43,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'; @@ -428,7 +431,7 @@ public function getTemplateHelper() /** * Returns the field name. */ - public function getName() + public function getName(): string { return $this->name; } @@ -444,7 +447,28 @@ public function getInputType() } /** - * Returns the field value. + * Returns the field value + * + * Alias of Value() + */ + public function getValue(): mixed + { + // This alias is here to meet the FieldValidationInterface API + // While this is a newer method than Value(), this calls Value() rather than the other way + // around as there are many FormField subclasses that override Value() + return $this->Value(); + } + + /** + * Get the value of this field for field validation + */ + public function getValueForValidation(): mixed + { + return $this->getValue(); + } + + /** + * Returns the field value * * @see FormField::setSubmittedValue() * @return mixed @@ -1221,30 +1245,19 @@ 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 + * Determines whether the field is valid or not based on its value */ - protected function extendValidationResult(bool $result, Validator $validator): bool + public function validate(): ValidationResult { - $this->extend('updateValidationResult', $result, $validator); + $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/LookupField.php b/src/Forms/LookupField.php index ac388fc394b..6481a9a133f 100644 --- a/src/Forms/LookupField.php +++ b/src/Forms/LookupField.php @@ -4,6 +4,7 @@ use SilverStripe\Core\Convert; use SilverStripe\Core\ArrayLib; +use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; @@ -66,14 +67,12 @@ public function Field($properties = []) } /** - * Ignore validation as the field is readonly - * - * @param Validator $validator - * @return bool + * TODO: set any inherited $field_validators to null then remove this method */ - public function validate($validator) + public function validate(): ValidationResult { - return $this->extendValidationResult(true, $validator); + // This is just temporary code to make the signature valid + return new ValidationResult; } /** 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..2a8a2f3a885 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 @@ -223,49 +229,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..f87e6923b75 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 we value for validation will be cast to int or float + StringFieldValidator::class => null, + NumericFieldValidator::class, + ]; protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DECIMAL; @@ -113,6 +120,11 @@ private function clean(?string $value): string public function setSubmittedValue($value, $data = null) { + if (is_null($value)) { + $this->value = $value; + return $this; + } + // Save original value in case parse fails $value = $this->clean($value); $this->originalValue = $value; @@ -156,6 +168,11 @@ public function setValue($value, $data = null) return $this; } + public function getValueForValidation(): mixed + { + return $this->cast($this->value); + } + /** * Helper to cast non-localised strings to their native type * @@ -168,9 +185,9 @@ protected function cast($value) return null; } if ($this->getScale() === 0) { - return (int)$value; + return (int) $value; } - return (float)$value; + return (float) $value; } /** @@ -193,31 +210,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(); 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/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/SelectField.php b/src/Forms/SelectField.php index 22485590db9..346fe064236 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, @@ -130,9 +134,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); } /** 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..5fd68fab69a 100644 --- a/src/Forms/SingleLookupField.php +++ b/src/Forms/SingleLookupField.php @@ -3,6 +3,7 @@ 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; @@ -37,14 +38,17 @@ protected function valueToLabel() } /** + * TODO: set any inherited $field_validators to null then remove this method + * * Ignore validation as the field is readonly * * @param Validator $validator * @return bool */ - public function validate($validator) + public function validate(): ValidationResult { - return $this->extendValidationResult(true, $validator); + // this is just temporary code to make the signature pass + return new ValidationResult; } /** diff --git a/src/Forms/SingleSelectField.php b/src/Forms/SingleSelectField.php index 1dcd6b502ed..d7e6df87871 100644 --- a/src/Forms/SingleSelectField.php +++ b/src/Forms/SingleSelectField.php @@ -9,7 +9,6 @@ */ abstract class SingleSelectField extends SelectField { - /** * Show the first