Merge d772268406 into 33929e2992

src/Core/Validation/FieldValidation/BigIntFieldValidator.php

@@ -21,6 +21,7 @@ class BigIntFieldValidator extends IntFieldValidator
     public function __construct(
         string $name,
         mixed $value,
+        bool $skipIfNull,
         ?int $minValue = null,
         ?int $maxValue = null
     ) {
@@ -31,6 +32,6 @@ class BigIntFieldValidator extends IntFieldValidator
         if (is_null($maxValue)) {
             $maxValue = (int) BigIntFieldValidator::MAX_64_BIT_INT;
         }
-        parent::__construct($name, $value, $minValue, $maxValue);
+        parent::__construct($name, $value, $skipIfNull, $minValue, $maxValue);
     }
 }

src/Core/Validation/FieldValidation/CompositeFieldValidator.php

@@ -9,11 +9,15 @@ use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface;
 
 class CompositeFieldValidator extends FieldValidator
 {
-    public function __construct(string $name, mixed $value)
+    public function __construct(string $name, mixed $value, bool $skipIfNull)
     {
-        parent::__construct($name, $value);
+        parent::__construct($name, $value, $skipIfNull);
         if (!is_iterable($value)) {
-            throw new InvalidArgumentException('Value must be iterable');
+            if (is_null($value) && $skipIfNull) {
+                $value = [];
+            } else {
+                throw new InvalidArgumentException('Value must be iterable');
+            }
         }
         foreach ($value as $child) {
             if (!is_a($child, FieldValidationInterface::class)) {

src/Core/Validation/FieldValidation/DateFieldValidator.php

@@ -9,17 +9,12 @@ 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
- * Emtpy values are allowed
  */
 class DateFieldValidator extends FieldValidator
 {
     protected function validateValue(): ValidationResult
     {
         $result = ValidationResult::create();
-        // Allow empty values
-        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) {

src/Core/Validation/FieldValidation/DecimalFieldValidator.php

@@ -17,9 +17,9 @@ class DecimalFieldValidator extends NumericFieldValidator
      */
     private int $decimalSize;
 
-    public function __construct(string $name, mixed $value, int $wholeSize, int $decimalSize)
+    public function __construct(string $name, mixed $value, bool $skipIfNull, int $wholeSize, int $decimalSize)
     {
-        parent::__construct($name, $value);
+        parent::__construct($name, $value, $skipIfNull);
         $this->wholeSize = $wholeSize;
         $this->decimalSize = $decimalSize;
     }

src/Core/Validation/FieldValidation/EnumFieldValidator.php

@@ -9,9 +9,9 @@ class EnumFieldValidator extends FieldValidator
 {
     protected array $allowedValues;
 
-    public function __construct(string $name, mixed $value, array $allowedValues)
+    public function __construct(string $name, mixed $value, bool $skipIfNull, array $allowedValues)
     {
-        parent::__construct($name, $value);
+        parent::__construct($name, $value, $skipIfNull);
         $this->allowedValues = $allowedValues;
     }
 

src/Core/Validation/FieldValidation/FieldValidationInterface.php

@@ -9,4 +9,6 @@ interface FieldValidationInterface extends ValidationInterface
     public function getName(): string;
 
     public function getValueForValidation(): mixed;
+
+    public function getSkipValidationIfNull(): bool;
 }

src/Core/Validation/FieldValidation/FieldValidator.php

@@ -12,11 +12,13 @@ abstract class FieldValidator implements ValidationInterface
 {
     protected string $name;
     protected mixed $value;
+    private bool $skipIfNull;
 
-    public function __construct(string $name, mixed $value)
+    public function __construct(string $name, mixed $value, bool $skipIfNull)
     {
         $this->name = $name;
         $this->value = $value;
+        $this->skipIfNull = $skipIfNull;
     }
 
     /**
@@ -25,6 +27,9 @@ abstract class FieldValidator implements ValidationInterface
     public function validate(): ValidationResult
     {
         $result = ValidationResult::create();
+        if (is_null($this->value) && $this->skipIfNull) {
+            return $result;
+        }
         $validationResult = $this->validateValue($result);
         if (!$validationResult->isValid()) {
             $result->combineAnd($validationResult);

src/Core/Validation/FieldValidation/FieldValidatorsTrait.php

@@ -29,6 +29,30 @@ trait FieldValidatorsTrait
      */
     private static array $field_validators = [];
 
+    /**
+     * Used by FieldValidator to skip validation if the field is null
+     */
+    protected bool $skipValidationIfNull = false;
+
+    /**
+     * Get whether this field should skip validation if it is null
+     * There is intentionally no setter for this
+     */
+    public function getSkipValidationIfNull(): bool
+    {
+        return $this->skipValidationIfNull;
+    }
+
+    /**
+     * Get the value of this field for field validation
+     * Override this method in your class to return the value you want to validate
+     * If it's different from what's normally returned in getValue();
+     */
+    public function getValueForValidation(): mixed
+    {
+        return $this->getValue();
+    }
+
     /**
      * Validate this field
      */
@@ -63,6 +87,7 @@ trait FieldValidatorsTrait
         /** @var FieldValidationInterface|Configurable $this */
         $name = $this->getName();
         $value = $this->getValueForValidation();
+        $skipIfNull = $this->getSkipValidationIfNull();
         // Field name is required for FieldValidators when called ValidationResult::addFieldMessage()
         if ($name === '') {
             throw new RuntimeException('Field name is blank');
@@ -100,7 +125,7 @@ trait FieldValidatorsTrait
             unset($classes[$class]);
         }
         foreach ($classes as $class => $argCalls) {
-            $args = [$name, $value];
+            $args = [$name, $value, $skipIfNull];
             foreach ($argCalls as $i => $argCall) {
                 if (!is_string($argCall) && !is_null($argCall)) {
                     throw new RuntimeException("argCall $i for FieldValidator $class is not a string or null");

src/Core/Validation/FieldValidation/IntFieldValidator.php

@@ -22,6 +22,7 @@ class IntFieldValidator extends NumericFieldValidator
     public function __construct(
         string $name,
         mixed $value,
+        bool $skipIfNull,
         ?int $minValue = null,
         ?int $maxValue = null
     ) {
@@ -31,7 +32,7 @@ class IntFieldValidator extends NumericFieldValidator
         if (is_null($maxValue)) {
             $maxValue = (int) IntFieldValidator::MAX_32_BIT_INT;
         }
-        parent::__construct($name, $value, $minValue, $maxValue);
+        parent::__construct($name, $value, $skipIfNull, $minValue, $maxValue);
     }
 
     protected function validateValue(): ValidationResult

src/Core/Validation/FieldValidation/MultiEnumFieldValidator.php

@@ -8,12 +8,12 @@ use SilverStripe\Core\Validation\FieldValidation\EnumFieldValidator;
 
 class MultiEnumFieldValidator extends EnumFieldValidator
 {
-    public function __construct(string $name, mixed $value, array $allowedValues)
+    public function __construct(string $name, mixed $value, bool $skipIfNull, array $allowedValues)
     {
         if (!is_array($value)) {
             throw new InvalidArgumentException('Value must be an array');
         }
-        parent::__construct($name, $value, $allowedValues);
+        parent::__construct($name, $value, $skipIfNull, $allowedValues);
     }
 
     protected function validateValue(): ValidationResult

src/Core/Validation/FieldValidation/NumericFieldValidator.php

@@ -20,12 +20,13 @@ class NumericFieldValidator extends FieldValidator
     public function __construct(
         string $name,
         mixed $value,
+        bool $skipIfNull,
         ?int $minValue = null,
         ?int $maxValue = null
     ) {
         $this->minValue = $minValue;
         $this->maxValue = $maxValue;
-        parent::__construct($name, $value);
+        parent::__construct($name, $value, $skipIfNull);
     }
 
     protected function validateValue(): ValidationResult

src/Core/Validation/FieldValidation/StringFieldValidator.php

@@ -24,10 +24,11 @@ class StringFieldValidator extends FieldValidator
     public function __construct(
         string $name,
         mixed $value,
+        bool $skipIfNull,
         ?int $minLength = null,
         ?int $maxLength = null
     ) {
-        parent::__construct($name, $value);
+        parent::__construct($name, $value, $skipIfNull);
         if ($minLength && $minLength < 0) {
             throw new InvalidArgumentException('minLength must be greater than or equal to 0');
         }

src/Forms/FormField.php

@@ -461,14 +461,6 @@ class FormField extends RequestHandler
         return $this->value;
     }
 
-    /**
-     * Get the value of this field for field validation
-     */
-    public function getValueForValidation(): mixed
-    {
-        return $this->getValue();
-    }
-
     /**
      * Method to save this form field into the given record.
      *

src/ORM/FieldType/DBBoolean.php

@@ -97,9 +97,7 @@ class DBBoolean extends DBField
 
     public function prepValueForDB(mixed $value): array|int|null
     {
-        $ret = $this->convertBooleanLikeValueToTinyInt($value);
-        // Ensure a tiny int is returned no matter what e.g. value is an
-        return $ret ? 1 : 0;
+        return $this->convertBooleanLikeValueToTinyInt($value);
     }
 
     private function convertBooleanLikeValueToTinyInt(mixed $value): mixed

src/ORM/FieldType/DBClassNameVarchar.php

@@ -3,7 +3,6 @@
 namespace SilverStripe\ORM\FieldType;
 
 use SilverStripe\ORM\FieldType\DBVarchar;
-use SilverStripe\Core\Validation\FieldValidation\EnumFieldValidator;
 
 /**
  * An alternative to DBClassName that stores the class name as a varchar instead of an enum
@@ -25,8 +24,4 @@ use SilverStripe\Core\Validation\FieldValidation\EnumFieldValidator;
 class DBClassNameVarchar extends DBVarchar
 {
     use DBClassNameTrait;
-
-    private static array $field_validators = [
-        EnumFieldValidator::class => ['getEnum'],
-    ];
 }

src/ORM/FieldType/DBComposite.php

@@ -30,6 +30,8 @@ abstract class DBComposite extends DBField
         CompositeFieldValidator::class,
     ];
 
+    protected bool $skipValidationIfNull = true;
+
     /**
      * Similar to {@link DataObject::$db},
      * holds an array of composite field names.
@@ -195,15 +197,6 @@ abstract class DBComposite extends DBField
         return $this;
     }
 
-    public function getValueForValidation(): mixed
-    {
-        $fields = [];
-        foreach (array_keys($this->compositeDatabaseFields()) as $fieldName) {
-            $fields[] = $this->dbObject($fieldName);
-        }
-        return $fields;
-    }
-
     /**
      * Bind this field to the model, and set the underlying table to that of the owner
      */

src/ORM/FieldType/DBDate.php

@@ -48,6 +48,8 @@ class DBDate extends DBField
         DateFieldValidator::class,
     ];
 
+    protected bool $skipValidationIfNull = true;
+
     public function setValue(mixed $value, null|array|ModelData $record = null, bool $markChanged = true): static
     {
         if ($value !== null) {

src/ORM/FieldType/DBEnum.php

@@ -22,6 +22,8 @@ class DBEnum extends DBString
         EnumFieldValidator::class => ['getEnum'],
     ];
 
+    protected bool $skipValidationIfNull = false;
+
     /**
      * List of enum values
      */

src/ORM/FieldType/DBField.php

@@ -195,18 +195,6 @@ abstract class DBField extends ModelData implements DBIndexable, FieldValidation
         return $this->value;
     }
 
-    /**
-     * Get the value of this field for field validation
-     */
-    public function getValueForValidation(): mixed
-    {
-        $value = $this->getValue();
-        if (is_null($value)) {
-            return $this->nullValue();
-        }
-        return $value;
-    }
-
     /**
      * Set the value of this field in various formats.
      * Used by {@link DataObject->getField()}, {@link DataObject->setCastedField()}

src/ORM/FieldType/DBMultiEnum.php

@@ -45,11 +45,7 @@ class DBMultiEnum extends DBEnum
 
     public function getValueForValidation(): array
     {
-        $value = parent::getValueForValidation();
-        if (is_array($value)) {
-            return $value;
-        }
-        return explode(',', (string) $value);
+        return explode(',', (string) $this->value);
     }
 
     public function requireField(): void

src/ORM/FieldType/DBString.php

@@ -16,6 +16,8 @@ abstract class DBString extends DBField
         'Plain' => 'Text',
     ];
 
+    protected bool $skipValidationIfNull = true;
+
     /**
      * Set the default value for "nullify empty"
      *
@@ -82,15 +84,6 @@ abstract class DBString extends DBField
         return $value || (is_string($value) && strlen($value ?? ''));
     }
 
-    public function getValueForValidation(): mixed
-    {
-        $value = parent::getValueForValidation();
-        if (is_null($value)) {
-            return '';
-        }
-        return $value;
-    }
-
     public function prepValueForDB(mixed $value): array|string|null
     {
         // Cast non-empty value

src/ORM/FieldType/DBTime.php

@@ -35,6 +35,8 @@ class DBTime extends DBField
         TimeFieldValidator::class,
     ];
 
+    protected bool $skipValidationIfNull = true;
+
     public function setValue(mixed $value, null|array|ModelData $record = null, bool $markChanged = true): static
     {
         $value = $this->parseTime($value);

tests/php/Core/Validation/FieldValidation/CompositeFieldValidatorTest.php

@@ -19,6 +19,15 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'fish',
                 'valueIsNull' => false,
+                'skipIfNull' => false,
+                'exception' => null,
+                'expected' => true,
+            ],
+            'valid-skip-null' => [
+                'valueBoolean' => true,
+                'valueString' => 'fish',
+                'valueIsNull' => true,
+                'skipIfNull' => true,
                 'exception' => null,
                 'expected' => true,
             ],
@@ -26,6 +35,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'not-iterable',
                 'valueIsNull' => false,
+                'skipIfNull' => false,
                 'exception' => InvalidArgumentException::class,
                 'expected' => true,
             ],
@@ -33,6 +43,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'no-field-validation',
                 'valueIsNull' => false,
+                'skipIfNull' => false,
                 'exception' => InvalidArgumentException::class,
                 'expected' => true,
             ],
@@ -40,6 +51,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'fish',
                 'valueIsNull' => true,
+                'skipIfNull' => false,
                 'exception' => InvalidArgumentException::class,
                 'expected' => true,
             ],
@@ -47,6 +59,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => 'dog',
                 'valueString' => 'fish',
                 'valueIsNull' => false,
+                'skipIfNull' => false,
                 'exception' => null,
                 'expected' => false,
             ],
@@ -54,6 +67,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 456.789,
                 'valueIsNull' => false,
+                'skipIfNull' => false,
                 'exception' => null,
                 'expected' => false,
             ],
@@ -65,6 +79,7 @@ class CompositeFieldValidatorTest extends SapphireTest
         mixed $valueBoolean,
         mixed $valueString,
         bool $valueIsNull,
+        bool $skipIfNull,
         ?string $exception,
         bool $expected
     ): void {
@@ -88,7 +103,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 $iterable = [$booleanField, $stringField];
             }
         }
-        $validator = new CompositeFieldValidator('MyField', $iterable);
+        $validator = new CompositeFieldValidator('MyField', $iterable, $skipIfNull);
         $result = $validator->validate();
         if (!$exception) {
             $this->assertSame($expected, $result->isValid());

tests/php/Core/Validation/FieldValidation/FieldValidatorTest.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace SilverStripe\Core\Tests\Validation\FieldValidation;
+
+use SilverStripe\Dev\SapphireTest;
+use PHPUnit\Framework\Attributes\DataProvider;
+use SilverStripe\Core\Validation\FieldValidation\FieldValidator;
+use SilverStripe\Core\Validation\ValidationResult;
+
+class FieldValidatorTest extends SapphireTest
+{
+    public static function provideSkipIfNull(): array
+    {
+        return [
+            'skip' => [
+                'skipIfNull' => true,
+                'expected' => true,
+            ],
+            'not-skip' => [
+                'skipIfNull' => false,
+                'expected' => false,
+            ],
+        ];
+    }
+
+    #[DataProvider('provideSkipIfNull')]
+    public function testSkipIfNull(bool $skipIfNull, bool $expected): void
+    {
+        $value = null;
+        $validator = new class ('MyField', $value, $skipIfNull) extends FieldValidator {
+            protected function validateValue(): ValidationResult
+            {
+                $result = ValidationResult::create();
+                if ($this->value === null) {
+                    $result->addFieldError('MyField', 'Disaster');
+                }
+                return $result;
+            }
+        };
+        $result = $validator->validate();
+        $this->assertSame($expected, $result->isValid());
+    }
+}

tests/php/ORM/DBCompositeTest.php

@@ -6,9 +6,6 @@ use SilverStripe\ORM\FieldType\DBMoney;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Dev\SapphireTest;
 use InvalidArgumentException;
-use PHPUnit\Framework\Attributes\DataProvider;
-use SilverStripe\ORM\FieldType\DBVarchar;
-use SilverStripe\ORM\FieldType\DBDecimal;
 
 class DBCompositeTest extends SapphireTest
 {
@@ -143,12 +140,4 @@ class DBCompositeTest extends SapphireTest
         // $this->assertSame($moneyField, $obj->dbObject('DoubleMoney'));
         // $this->assertEquals(20, $obj->dbObject('DoubleMoney')->getAmount());
     }
-
-    public function testGetValueForValidation(): void
-    {
-        $obj = DBCompositeTest\DBDoubleMoney::create();
-        $expected = [DBVarchar::class, DBDecimal::class];
-        $actual = array_map('get_class', $obj->getValueForValidation());
-        $this->assertSame($expected, $actual);
-    }
 }

tests/php/ORM/DBFieldTest.php

@@ -426,4 +426,62 @@ class DBFieldTest extends SapphireTest
             $this->assertSame($expected, $field->getValue(), $class);
         }
     }
+
+    public function testValidIfNull(): void
+    {
+        $expectedIfNotListed = false;
+        // Has skipValidationIfNull = true
+        $willSkipValidation = [
+            DBComposite::class,
+            DBDate::class,
+            DBString::class,
+            DBTime::class,
+        ];
+        // Subclass of something in $willSkipValidation, though has
+        // $skipValidationIfNull = false
+        $willNotSkipValidation = [
+            DBEnum::class,
+        ];
+        $validWithNullValue = [
+            // nullValue() returns 0
+            DBBoolean::class,
+            DBFloat::class,
+            DBInt::class,
+            // sets valid value in setValue()
+            DBCurrency::class,
+        ];
+        $classes = ClassInfo::subclassesFor(DBField::class);
+        foreach ($classes as $class) {
+            if (is_a($class, TestOnly::class, true)) {
+                continue;
+            }
+            $reflector = new ReflectionClass($class);
+            if ($reflector->isAbstract()) {
+                continue;
+            }
+            $expected = $expectedIfNotListed;
+            foreach ($willSkipValidation as $baseClass) {
+                if ($class === $baseClass || is_subclass_of($class, $baseClass)) {
+                    $expected = true;
+                    break;
+                }
+            }
+            foreach ($willNotSkipValidation as $baseClass) {
+                if ($class === $baseClass || is_subclass_of($class, $baseClass)) {
+                    $expected = false;
+                    break;
+                }
+            }
+            foreach ($validWithNullValue as $baseClass) {
+                if ($class === $baseClass || is_subclass_of($class, $baseClass)) {
+                    $expected = true;
+                    break;
+                }
+            }
+            $field = new $class('TestField');
+            $field->setValue(null);
+            $result = $field->validate();
+            $this->assertSame($expected, $result->isValid(), $class);
+        }
+    }
 }

tests/php/ORM/DBMultiEnumTest.php

@@ -1,44 +0,0 @@
-<?php
-
-namespace SilverStripe\ORM\Tests;
-
-use SilverStripe\Dev\SapphireTest;
-use SilverStripe\ORM\FieldType\DBMultiEnum;
-use PHPUnit\Framework\Attributes\DataProvider;
-
-class DBMultiEnumTest extends SapphireTest
-{
-    public static function provideGetValueForValidation(): array
-    {
-        return [
-            'array' => [
-                'value' => ['Red', 'Green'],
-                'expected' => ['Red', 'Green'],
-            ],
-            'string' => [
-                'value' => 'Red,Green',
-                'expected' => ['Red', 'Green'],
-            ],
-            'string-non-existant-value' => [
-                'value' => 'Red,Green,Purple',
-                'expected' => ['Red', 'Green', 'Purple'],
-            ],
-            'empty-string' => [
-                'value' => '',
-                'expected' => [''],
-            ],
-            'null' => [
-                'value' => null,
-                'expected' => [''],
-            ],
-        ];
-    }
-
-    #[DataProvider('provideGetValueForValidation')]
-    public function testGetValueForValidation(mixed $value, array $expected): void
-    {
-        $obj = new DBMultiEnum('TestField', ['Red', 'Green', 'Blue']);
-        $obj->setValue($value);
-        $this->assertSame($expected, $obj->getValueForValidation());
-    }
-}

tests/php/ORM/DBStringTest.php

@@ -7,7 +7,6 @@ use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\FieldType\DBString;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\ORM\Tests\DBStringTest\MyStringField;
-use PHPUnit\Framework\Attributes\DataProvider;
 
 class DBStringTest extends SapphireTest
 {
@@ -69,30 +68,4 @@ class DBStringTest extends SapphireTest
         $this->assertFalse(DBField::create_field(MyStringField::class, 0)->exists());
         $this->assertFalse(DBField::create_field(MyStringField::class, 0.0)->exists());
     }
-
-    public static function provideValueForValidation(): array
-    {
-        return [
-            'string' => [
-                'value' => 'fish',
-                'expected' => 'fish',
-            ],
-            'blank-string' => [
-                'value' => '',
-                'expected' => '',
-            ],
-            'null' => [
-                'value' => null,
-                'expected' => '',
-            ],
-        ];
-    }
-
-    #[DataProvider('provideValueForValidation')]
-    public function getValueForValidation(mixed $value, string $expected): void
-    {
-        $obj = new MyStringField('TestField');
-        $obj->setValue($value);
-        $this->assertSame($expected, $obj->getValueForValidation());
-    }
 }