Skip to content

Commit

Permalink
ENH Use FieldValidators for FormField validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@emteknetnz
emteknetnz committed Nov 11, 2024
1 parent bbd8bb9 commit fc9d6db
Show file tree
Hide file tree
Showing 57 changed files with 877 additions and 1,090 deletions.
105 changes: 101 additions & 4 deletions src/Core/Validation/FieldValidation/DateFieldValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,113 @@

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,
?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) && $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();
// Allow empty strings
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;
}
Expand All @@ -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']);
}
}
27 changes: 21 additions & 6 deletions src/Core/Validation/FieldValidation/FieldValidationTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ private function getFieldValidators(): array
/** @var FieldValidationInterface|Configurable $this */
$name = $this->getName();
$value = $this->getValueForValidation();
$classes = $this->getClassesFromConfig();
// Field name is required for FieldValidators when called ValidationResult::addFieldMessage()
if ($name === '') {
if (count($classes) > 0 && $name === '') {
throw new RuntimeException('Field name is blank');
}
$classes = $this->getClassesFromConfig();
foreach ($classes as $class => $argCalls) {
$args = [$name, $value];
foreach ($argCalls as $i => $argCall) {
Expand Down Expand Up @@ -122,15 +122,30 @@ 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) {
unset($classes[$class]);
}
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;
}
}
10 changes: 8 additions & 2 deletions src/Core/Validation/FieldValidation/NumericFieldValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
40 changes: 20 additions & 20 deletions src/Forms/CompositeField.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

namespace SilverStripe\Forms;

use SilverStripe\Core\Validation\FieldValidation\CompositeFieldValidator;
use SilverStripe\Dev\Debug;
use SilverStripe\Core\Validation\ValidationResult;

/**
* Base class for all fields that contain other fields.
Expand All @@ -13,6 +15,9 @@
*/
class CompositeField extends FormField
{
private static array $field_validators = [
CompositeFieldValidator::class,
];

/**
* @var FieldList
Expand Down Expand Up @@ -52,6 +57,13 @@ class CompositeField extends FormField

protected $schemaComponent = 'CompositeField';

/**
* Iterator used to name the field so that FieldValidators won't complain that the field is not named.
*
* @internal
*/
private static int $nameCounter = 0;

public function __construct($children = null)
{
// Normalise $children to a FieldList
Expand All @@ -63,8 +75,8 @@ public function __construct($children = null)
$children = new FieldList($children);
}
$this->setChildren($children);

parent::__construct(null, false);
$name = 'CompositeField_' . self::$nameCounter++;

Check failure on line 78 in src/Forms/CompositeField.php

View workflow job for this annotation

GitHub Actions / CI / 8.3 mysql80 phplinting 

Can't use keyword 'self'. Use 'CompositeField' instead.
parent::__construct($name, false);
}

/**
Expand Down Expand Up @@ -115,12 +127,17 @@ public function getChildren()
return $this->children;
}

public function getValueForValidation(): mixed
{
return $this->getChildren();
}

/**
* 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;
Expand Down Expand Up @@ -266,8 +283,6 @@ public function setForm($form)
return $this;
}



public function setDisabled($disabled)
{
parent::setDisabled($disabled);
Expand Down Expand Up @@ -522,19 +537,4 @@ public function debug(): string
$result .= "</ul>";
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);
}
}
Loading

0 comments on commit fc9d6db

Please sign in to comment.