Merge e449186c2e into 33929e2992

src/Core/Validation/FieldValidation/BigIntFieldValidator.php

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

src/Core/Validation/FieldValidation/CompositeFieldValidator.php

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

src/Core/Validation/FieldValidation/DateFieldValidator.php

@@ -9,12 +9,17 @@ 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, bool $skipIfNull, int $wholeSize, int $decimalSize)
+    public function __construct(string $name, mixed $value, int $wholeSize, int $decimalSize)
     {
-        parent::__construct($name, $value, $skipIfNull);
+        parent::__construct($name, $value);
         $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, bool $skipIfNull, array $allowedValues)
+    public function __construct(string $name, mixed $value, array $allowedValues)
     {
-        parent::__construct($name, $value, $skipIfNull);
+        parent::__construct($name, $value);
         $this->allowedValues = $allowedValues;
     }
 

src/Core/Validation/FieldValidation/FieldValidationInterface.php

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

src/Core/Validation/FieldValidation/FieldValidator.php

@@ -12,13 +12,11 @@ abstract class FieldValidator implements ValidationInterface
 {
     protected string $name;
     protected mixed $value;
-    private bool $skipIfNull;
 
-    public function __construct(string $name, mixed $value, bool $skipIfNull)
+    public function __construct(string $name, mixed $value)
     {
         $this->name = $name;
         $this->value = $value;
-        $this->skipIfNull = $skipIfNull;
     }
 
     /**
@@ -27,9 +25,6 @@ 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,30 +29,6 @@ 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
      */
@@ -87,7 +63,6 @@ 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');
@@ -125,7 +100,7 @@ trait FieldValidatorsTrait
             unset($classes[$class]);
         }
         foreach ($classes as $class => $argCalls) {
-            $args = [$name, $value, $skipIfNull];
+            $args = [$name, $value];
             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,7 +22,6 @@ class IntFieldValidator extends NumericFieldValidator
     public function __construct(
         string $name,
         mixed $value,
-        bool $skipIfNull,
         ?int $minValue = null,
         ?int $maxValue = null
     ) {
@@ -32,7 +31,7 @@ class IntFieldValidator extends NumericFieldValidator
         if (is_null($maxValue)) {
             $maxValue = (int) IntFieldValidator::MAX_32_BIT_INT;
         }
-        parent::__construct($name, $value, $skipIfNull, $minValue, $maxValue);
+        parent::__construct($name, $value, $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, bool $skipIfNull, array $allowedValues)
+    public function __construct(string $name, mixed $value, array $allowedValues)
     {
         if (!is_array($value)) {
             throw new InvalidArgumentException('Value must be an array');
         }
-        parent::__construct($name, $value, $skipIfNull, $allowedValues);
+        parent::__construct($name, $value, $allowedValues);
     }
 
     protected function validateValue(): ValidationResult

src/Core/Validation/FieldValidation/NumericFieldValidator.php

@@ -20,13 +20,12 @@ 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, $skipIfNull);
+        parent::__construct($name, $value);
     }
 
     protected function validateValue(): ValidationResult

src/Core/Validation/FieldValidation/StringFieldValidator.php

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

src/Forms/FormField.php

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

src/ORM/FieldType/DBClassNameVarchar.php

@@ -3,6 +3,7 @@
 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
@@ -24,4 +25,8 @@ use SilverStripe\ORM\FieldType\DBVarchar;
 class DBClassNameVarchar extends DBVarchar
 {
     use DBClassNameTrait;
+
+    private static array $field_validators = [
+        EnumFieldValidator::class => ['getEnum'],
+    ];
 }

src/ORM/FieldType/DBComposite.php

@@ -30,8 +30,6 @@ abstract class DBComposite extends DBField
         CompositeFieldValidator::class,
     ];
 
-    protected bool $skipValidationIfNull = true;
-
     /**
      * Similar to {@link DataObject::$db},
      * holds an array of composite field names.
@@ -197,6 +195,15 @@ 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,8 +48,6 @@ 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,8 +22,6 @@ class DBEnum extends DBString
         EnumFieldValidator::class => ['getEnum'],
     ];
 
-    protected bool $skipValidationIfNull = false;
-
     /**
      * List of enum values
      */

src/ORM/FieldType/DBField.php

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

src/ORM/FieldType/DBString.php

@@ -16,8 +16,6 @@ abstract class DBString extends DBField
         'Plain' => 'Text',
     ];
 
-    protected bool $skipValidationIfNull = true;
-
     /**
      * Set the default value for "nullify empty"
      *
@@ -84,6 +82,15 @@ 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,8 +35,6 @@ 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,15 +19,6 @@ 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,
             ],
@@ -35,7 +26,6 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'not-iterable',
                 'valueIsNull' => false,
-                'skipIfNull' => false,
                 'exception' => InvalidArgumentException::class,
                 'expected' => true,
             ],
@@ -43,7 +33,6 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'no-field-validation',
                 'valueIsNull' => false,
-                'skipIfNull' => false,
                 'exception' => InvalidArgumentException::class,
                 'expected' => true,
             ],
@@ -51,7 +40,6 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 'fish',
                 'valueIsNull' => true,
-                'skipIfNull' => false,
                 'exception' => InvalidArgumentException::class,
                 'expected' => true,
             ],
@@ -59,7 +47,6 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => 'dog',
                 'valueString' => 'fish',
                 'valueIsNull' => false,
-                'skipIfNull' => false,
                 'exception' => null,
                 'expected' => false,
             ],
@@ -67,7 +54,6 @@ class CompositeFieldValidatorTest extends SapphireTest
                 'valueBoolean' => true,
                 'valueString' => 456.789,
                 'valueIsNull' => false,
-                'skipIfNull' => false,
                 'exception' => null,
                 'expected' => false,
             ],
@@ -79,7 +65,6 @@ class CompositeFieldValidatorTest extends SapphireTest
         mixed $valueBoolean,
         mixed $valueString,
         bool $valueIsNull,
-        bool $skipIfNull,
         ?string $exception,
         bool $expected
     ): void {
@@ -103,7 +88,7 @@ class CompositeFieldValidatorTest extends SapphireTest
                 $iterable = [$booleanField, $stringField];
             }
         }
-        $validator = new CompositeFieldValidator('MyField', $iterable, $skipIfNull);
+        $validator = new CompositeFieldValidator('MyField', $iterable);
         $result = $validator->validate();
         if (!$exception) {
             $this->assertSame($expected, $result->isValid());

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

@@ -1,43 +0,0 @@
-<?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,6 +6,9 @@ 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
 {
@@ -140,4 +143,12 @@ 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,62 +426,4 @@ 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

@@ -0,0 +1,44 @@
+<?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,6 +7,7 @@ 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
 {
@@ -68,4 +69,30 @@ 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());
+    }
 }