ENH Use FieldValidators for FormField validation

src/Core/Validation/FieldValidation/DateFieldValidator.php

@@ -2,29 +2,109 @@
 
 namespace SilverStripe\Core\Validation\FieldValidation;
 
-use SilverStripe\Core\Validation\FieldValidation\FieldValidator;
+use Error;
+use Exception;
+use InvalidArgumentException;
 use SilverStripe\Core\Validation\ValidationResult;
+use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator;
 
 /**
  * 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
+class DateFieldValidator extends StringFieldValidator
 {
+    /**
+     * Minimum value as a date string
+     */
+    private ?string $minValue = null;
+
+    /**
+     * Minimum value as a unix timestamp
+     */
+    private ?int $minTimestamp = null;
+
+    /**
+     * Maximum value as a date string
+     */
+    private ?string $maxValue = null;
+
+    /**
+     * Maximum value as a unix timestamp
+     */
+    private ?int $maxTimestamp = null;
+
+    /**
+     * 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
+        if (!is_null($minValue)) {
+            $this->minTimestamp = $this->dateToTimestamp($minValue);
+        }
+        if (!is_null($maxValue)) {
+            $this->maxTimestamp = $this->dateToTimestamp($maxValue);
+        }
+        if (!is_null($this->minTimestamp) && !is_null($this->maxTimestamp)
+            && $this->maxTimestamp < $this->minTimestamp
+        ) {
+            throw new InvalidArgumentException('maxValue cannot be less than minValue');
+        }
+        $this->minValue = $minValue;
+        $this->maxValue = $maxValue;
+        $this->converter = $converter;
+        parent::__construct($name, $value);
+    }
+
     protected function validateValue(): ValidationResult
     {
-        $result = ValidationResult::create();
-        // Allow empty strings
-        if ($this->value === '') {
+        $result = parent::validateValue();
+        // If the value is not a string, we can't validate it as a date
+        if (!$result->isValid()) {
             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
+        $timestamp = $this->dateToTimestamp($this->value ?? '');
+        if ($timestamp === false) {
             $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 older 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 newer than {maxValue}',
+                ['maxValue' => $maxValue]
+            );
+            $result->addFieldError($this->name, $message);
         }
         return $result;
     }
@@ -38,4 +118,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|false
+    {
+        // 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) {
+            return false;
+        }
+        return mktime($date['hour'], $date['minute'], $date['second'], $date['month'], $date['day'], $date['year']);
+    }
 }

src/Core/Validation/FieldValidation/DecimalFieldValidator.php

@@ -23,7 +23,7 @@
  * 1234.9 - 4 digits the before the decimal point
  * 999.999 - would be rounded to 1000.00 which exceeds 5 total digits
  */
-class DecimalFieldValidator extends NumericFieldValidator
+class DecimalFieldValidator extends NumericNonStringFieldValidator
 {
     /**
      * Whole number size e.g. For Decimal(9,2) this would be 9

src/Core/Validation/FieldValidation/EmailFieldValidator.php

@@ -19,6 +19,6 @@ class EmailFieldValidator extends StringFieldValidator implements SymfonyFieldVa
     public function getConstraint(): Constraint|array
     {
         $message =  _t(__CLASS__ . '.INVALID', 'Invalid email address');
-        return new Email(message: $message);
+        return new Email(message: $message, mode: Email::VALIDATION_MODE_STRICT);
     }
 }

src/Core/Validation/FieldValidation/FieldValidationTrait.php

@@ -21,13 +21,17 @@ trait FieldValidationTrait
      *
      * Each item in the array can be one of the following
      * a) MyFieldValidator::class,
-     * b) MyFieldValidator::class => [null, 'getSomething'],
-     * c) MyFieldValidator::class => null,
+     * b) MyFieldValidator::class => ['argNameA' => null, 'argNameB' => 'getSomething'],
+     * d) MyFieldValidator::class => null,
      *
      * a) Will create a MyFieldValidator and pass the name and value of the field as args to the constructor
      * b) Will create a MyFieldValidator and pass the name, value, and pass additional args, where each null values
      *    will be passed as null, and non-null values will call a method on the field e.g. will pass null for the first
      *    additional arg and call $field->getSomething() to get a value for the second additional arg
+     *    Keys are used to speicify the arg name, which is done to prevents duplicate
+     *    args being add to config when a subclass defines the same FieldValidator as a parent class.
+     *    Note that keys are not named args, they are simply arbitary keys - though best practice is
+     *    for the keys to match constructor argument names.
      * c) Will disable a previously set MyFieldValidator. This is useful to disable a FieldValidator that was set
      *    on a parent class
      *
@@ -49,6 +53,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
      */
@@ -63,10 +79,6 @@ private function getFieldValidators(): array
         /** @var FieldValidationInterface|Configurable $this */
         $name = $this->getName();
         $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 +134,9 @@ 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);
+            // Ensure that argCalls is a numerically indexed array
+            // as they may have been defined with string keys to prevent duplicate args
+            $argCalls = array_values($argCalls);
             $classes[$class] = $argCalls;
         }
         foreach (array_keys($disabledClasses) as $class) {

src/Core/Validation/FieldValidation/IntFieldValidator.php

@@ -8,7 +8,7 @@
 /**
  * Validates that a value is a 32-bit signed integer
  */
-class IntFieldValidator extends NumericFieldValidator
+class IntFieldValidator extends NumericNonStringFieldValidator
 {
     /**
      * The minimum value for a signed 32-bit integer.

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);
         }

src/Core/Validation/FieldValidation/NumericFieldValidator.php

@@ -26,7 +26,7 @@ public function __construct(
         string $name,
         mixed $value,
         ?int $minValue = null,
-        ?int $maxValue = null
+        ?int $maxValue = null,
     ) {
         if (!is_null($minValue) && !is_null($maxValue) && $maxValue < $minValue) {
             throw new InvalidArgumentException('maxValue cannot be less than minValue');
@@ -39,9 +39,8 @@ 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
-            $message = _t(__CLASS__ . '.WRONGTYPE', 'Must be numeric, and not a string');
+        if (!is_numeric($this->value)) {
+            $message = _t(__CLASS__ . '.NOTNUMERIC', 'Must be numeric');
             $result->addFieldError($this->name, $message);
         } elseif (!is_null($this->minValue) && $this->value < $this->minValue) {
             $result->addFieldError($this->name, $this->getTooSmallMessage());

src/Core/Validation/FieldValidation/NumericNonStringFieldValidator.php

@@ -0,0 +1,22 @@
+<?php
+
+namespace SilverStripe\Core\Validation\FieldValidation;
+
+use SilverStripe\Core\Validation\ValidationResult;
+use SilverStripe\Core\Validation\FieldValidation\NumericFieldValidator;
+
+/**
+ * Validates that a value is a numeric value and not a string
+ */
+class NumericNonStringFieldValidator extends NumericFieldValidator
+{
+    protected function validateValue(): ValidationResult
+    {
+        $result = parent::validateValue();
+        if (is_numeric($this->value) && is_string($this->value)) {
+            $message = _t(__CLASS__ . '.WRONGTYPE', 'Must be numeric and not a string');
+            $result->addFieldError($this->name, $message);
+        }
+        return $result;
+    }
+}

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');
+    }
 }

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 .= "</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);
-    }
 }