Compare commits

..

2 Commits

Author SHA1 Message Date
Steve Boyd
a380d7f758
Merge b052f45a802a5fc40cc8d5bb3d8557284d101894 into 6bb9a0b33d4ceab145a7effc2e4ce16d6eedc877 2024-10-15 02:48:16 +00:00
Steve Boyd
b052f45a80 NEW Validate DBFields 2024-10-15 15:48:10 +13:00
18 changed files with 181 additions and 71 deletions

View File

@ -23,7 +23,7 @@ class DateFieldValidator extends FieldValidator
// Not using symfony/validator because it was allowing d-m-Y format strings // Not using symfony/validator because it was allowing d-m-Y format strings
$date = date_parse_from_format($this->getFormat(), $this->value ?? ''); $date = date_parse_from_format($this->getFormat(), $this->value ?? '');
if ($date === false || $date['error_count'] > 0 || $date['warning_count'] > 0) { if ($date === false || $date['error_count'] > 0 || $date['warning_count'] > 0) {
$result->addFieldError($this->name, $this->getMessage()); $result->addFieldError($this->name, $this->getMessage(), value: $this->value);
} }
return $result; return $result;
} }

View File

@ -8,5 +8,7 @@ interface FieldValidationInterface extends ValidationInterface
{ {
public function getName(): string; public function getName(): string;
public function getValue(): mixed;
public function getValueForValidation(): mixed; public function getValueForValidation(): mixed;
} }

View File

@ -35,6 +35,10 @@ trait FieldValidatorsTrait
public function validate(): ValidationResult public function validate(): ValidationResult
{ {
$result = ValidationResult::create(); $result = ValidationResult::create();
// Skip validation if the field value is null
if ($this->getValue() === null) {
return $result;
}
$fieldValidators = $this->getFieldValidators(); $fieldValidators = $this->getFieldValidators();
foreach ($fieldValidators as $fieldValidator) { foreach ($fieldValidators as $fieldValidator) {
$validationResult = $fieldValidator->validate(); $validationResult = $fieldValidator->validate();

View File

@ -43,6 +43,7 @@ class StringFieldValidator extends FieldValidator
$result->addFieldError($this->name, $message, value: $this->value); $result->addFieldError($this->name, $message, value: $this->value);
return $result; return $result;
} }
// TODO this seems non-sensical - should enforce minLength??
// Blank strings are valid, even if there's a minLength requirement // Blank strings are valid, even if there's a minLength requirement
if ($this->value === '') { if ($this->value === '') {
return $result; return $result;

View File

@ -1257,6 +1257,10 @@ class FormField extends RequestHandler
{ {
$isValid = true; $isValid = true;
$result = ValidationResult::create(); $result = ValidationResult::create();
if ($this->getValue() === null) {
// Skip field validation if the value is null
return $this->extendValidationResult($isValid, $validator);
}
$fieldValidators = $this->getFieldValidators(); $fieldValidators = $this->getFieldValidators();
foreach ($fieldValidators as $fieldValidator) { foreach ($fieldValidators as $fieldValidator) {
$validationResult = $fieldValidator->validate(); $validationResult = $fieldValidator->validate();

View File

@ -57,11 +57,6 @@ class DBDate extends DBField
return $this; return $this;
} }
public function nullValue(): string
{
return '';
}
/** /**
* Parse timestamp or iso8601-ish date into standard iso8601 format * Parse timestamp or iso8601-ish date into standard iso8601 format
* *

View File

@ -60,7 +60,7 @@ class DBDatetime extends DBDate implements TemplateGlobalProvider
/** /**
* Flag idicating if this field is considered immutable * Flag idicating if this field is considered immutable
* when this is enabled setting the value of this field will return a new field instance * when this is enabled setting the value of this field will return a new field instance
* instead updating the old one * instead updatin the old one
*/ */
protected bool $immutable = false; protected bool $immutable = false;

View File

@ -176,11 +176,6 @@ class DBEnum extends DBString
return $this->enum; return $this->enum;
} }
public function nullValue(): string
{
return '';
}
/** /**
* Get the list of enum values, including obsolete values still present in the database * Get the list of enum values, including obsolete values still present in the database
* *

View File

@ -90,11 +90,6 @@ abstract class DBString extends DBField
return $value || (is_string($value) && strlen($value ?? '')); return $value || (is_string($value) && strlen($value ?? ''));
} }
public function nullValue(): string
{
return '';
}
public function prepValueForDB(mixed $value): array|string|null public function prepValueForDB(mixed $value): array|string|null
{ {
// Cast non-empty value // Cast non-empty value

View File

@ -44,16 +44,13 @@ class DBTime extends DBField
/** /**
* Parse timestamp or iso8601-ish date into standard iso8601 format * Parse timestamp or iso8601-ish date into standard iso8601 format
*
* @return string|null|false Formatted time, null if empty but valid, or false if invalid
*/ */
protected function parseTime(mixed $value): string|null|false protected function parseTime(mixed $value): mixed
{ {
// Skip empty values // Return empty dates as-is, they will get caught by validator later
if (empty($value) && !is_numeric($value)) { if (empty($value) && !is_numeric($value)) {
return null; return $value;
} }
// Determine value to parse // Determine value to parse
if (is_array($value)) { if (is_array($value)) {
$source = $value; // parse array $source = $value; // parse array
@ -63,14 +60,16 @@ class DBTime extends DBField
// Convert using strtotime // Convert using strtotime
$source = strtotime($value ?? ''); $source = strtotime($value ?? '');
} }
if ($value === false) {
return false;
}
// Format as iso8601 // Format as iso8601
$formatter = $this->getFormatter(); $formatter = $this->getFormatter();
$formatter->setPattern($this->getISOFormat()); $formatter->setPattern($this->getISOFormat());
return $formatter->format($source); $formatted = $formatter->format($source);
// If it failed to format, return the original value so that it can
// be caught by the validator
if ($formatted === false) {
return $value;
}
return $formatted;
} }
/** /**

View File

@ -66,11 +66,6 @@ class DBYear extends DBField
return $this; return $this;
} }
public function nullValue(): int
{
return 0;
}
public function getMinYear(): int public function getMinYear(): int
{ {
return DBYear::MIN_YEAR; return DBYear::MIN_YEAR;

View File

@ -444,8 +444,8 @@ class Member extends DataObject
if (!$this->PasswordExpiry) { if (!$this->PasswordExpiry) {
return false; return false;
} }
$expired = strtotime(date('Y-m-d')) >= strtotime($this->PasswordExpiry ?? '');
return strtotime(date('Y-m-d')) >= strtotime($this->PasswordExpiry ?? ''); return $expired;
} }
/** /**

View File

@ -39,6 +39,7 @@ use SilverStripe\Security\PermissionCheckboxSetField_Readonly;
use SilverStripe\Forms\SearchableMultiDropdownField; use SilverStripe\Forms\SearchableMultiDropdownField;
use SilverStripe\Forms\SearchableDropdownField; use SilverStripe\Forms\SearchableDropdownField;
use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\DataProvider;
use SilverStripe\ORM\FieldType\DBInt;
class FormFieldTest extends SapphireTest class FormFieldTest extends SapphireTest
{ {

View File

@ -181,9 +181,21 @@ class DBDateTest extends SapphireTest
); );
} }
public static function provideSetNullAndZeroValues() public static function provideSetValue()
{ {
return [ return [
'date' => [
'value' => '1998-04-05',
'expected' => '1998-04-05'
],
'int' => [
'value' => 978797878,
'expected' => '2001-01-06'
],
'int-string' => [
'value' => '978797878',
'expected' => '2001-01-06'
],
'blank-string' => [ 'blank-string' => [
'value' => '', 'value' => '',
'expected' => '' 'expected' => ''
@ -208,9 +220,9 @@ class DBDateTest extends SapphireTest
'value' => 0, 'value' => 0,
'expected' => '1970-01-01' 'expected' => '1970-01-01'
], ],
'zero-datetime' => [ 'zero-date' => [
'value' => '0000-00-00 00:00:00', 'value' => '0000-00-00',
'expected' => '0000-00-00 00:00:00' 'expected' => '0000-00-00'
], ],
'zero-date-slashes' => [ 'zero-date-slashes' => [
'value' => '00/00/0000', 'value' => '00/00/0000',
@ -227,11 +239,12 @@ class DBDateTest extends SapphireTest
]; ];
} }
#[DataProvider('provideSetNullAndZeroValues')] #[DataProvider('provideSetValue')]
public function testSetNullAndZeroValues(mixed $value, mixed $expected) public function testSetValue(mixed $value, mixed $expected)
{ {
$date = DBField::create_field('Date', $value); $field = new DBDate('MyField');
$this->assertSame($expected, $date->getValue()); $field->setValue($value);
$this->assertSame($expected, $field->getValue());
} }
public function testDayOfMonth() public function testDayOfMonth()

View File

@ -330,4 +330,50 @@ class DBDatetimeTest extends SapphireTest
$after = (new DBDateTime())->setValue($timeAfter); $after = (new DBDateTime())->setValue($timeAfter);
$this->assertSame($expected, DBDatetime::getTimeBetween($before, $after)); $this->assertSame($expected, DBDatetime::getTimeBetween($before, $after));
} }
public static function provideSetValue(): array
{
return [
'int' => [
'value' => 800001234,
'expected' => '1995-05-09 06:33:54',
],
'string-int' => [
'value' => '800001234',
'expected' => '1995-05-09 06:33:54',
],
'zero' => [
'value' => 0,
'expected' => '1970-01-01 00:00:00',
],
'zeroed' => [
'value' => '0000-00-00 00:00:00',
'expected' => '0000-00-00 00:00:00',
],
'non-date-string' => [
'value' => 'fish',
'expected' => 'fish',
],
'null' => [
'value' => null,
'expected' => null,
],
'true' => [
'value' => true,
'expected' => true,
],
'false' => [
'value' => false,
'expected' => false,
],
];
}
#[DataProvider('provideSetValue')]
public function testSetValue(mixed $value, mixed $expected): void
{
$field = new DBDatetime('MyField');
$field->setValue($value);
$this->assertSame($expected, $field->getValue());
}
} }

View File

@ -3,7 +3,6 @@
namespace SilverStripe\ORM\Tests; namespace SilverStripe\ORM\Tests;
use Exception; use Exception;
use SilverStripe\Assets\Image;
use SilverStripe\ORM\FieldType\DBBigInt; use SilverStripe\ORM\FieldType\DBBigInt;
use SilverStripe\ORM\FieldType\DBBoolean; use SilverStripe\ORM\FieldType\DBBoolean;
use SilverStripe\ORM\FieldType\DBCurrency; use SilverStripe\ORM\FieldType\DBCurrency;
@ -56,7 +55,6 @@ use SilverStripe\ORM\FieldType\DBPolymorphicRelationAwareForeignKey;
use SilverStripe\ORM\FieldType\DBIp; use SilverStripe\ORM\FieldType\DBIp;
use SilverStripe\ORM\FieldType\DBEmail; use SilverStripe\ORM\FieldType\DBEmail;
use SilverStripe\Core\Validation\FieldValidation\DatetimeFieldValidator; use SilverStripe\Core\Validation\FieldValidation\DatetimeFieldValidator;
use SilverStripe\Assets\Storage\DBFile;
use SilverStripe\ORM\FieldType\DBClassNameVarchar; use SilverStripe\ORM\FieldType\DBClassNameVarchar;
/** /**
@ -522,7 +520,6 @@ class DBFieldTest extends SapphireTest
CompositeFieldValidator::class, CompositeFieldValidator::class,
], ],
DBMultiEnum::class => [ DBMultiEnum::class => [
StringFieldValidator::class,
MultiEnumFieldValidator::class, MultiEnumFieldValidator::class,
], ],
DBPercentage::class => [ DBPercentage::class => [
@ -578,4 +575,22 @@ class DBFieldTest extends SapphireTest
// that haven't been tested // that haven't been tested
$this->assertSame(29, $count); $this->assertSame(29, $count);
} }
public function testSkipValidateIfNull()
{
$field = new DBInt('MyField');
$field->setValue(null);
// assert value isn't getting changed on setValue();
$this->assertNull($field->getValue());
// assert that field validators were not called
$this->assertTrue($field->validate()->isValid());
// assert that IntFieldValidator was applied to the field
$method = new ReflectionMethod(DBInt::class, 'getFieldValidators');
$method->setAccessible(true);
$actual = array_map('get_class', $method->invoke($field));
$this->assertSame([IntFieldValidator::class], $actual);
// assert that IntFieldValidator considers null as invalid
$validator = new IntFieldValidator('Test', null);
$this->assertFalse($validator->validate()->isValid());
}
} }

View File

@ -17,33 +17,78 @@ class DBTimeTest extends SapphireTest
i18n::set_locale('en_NZ'); i18n::set_locale('en_NZ');
} }
public static function dataTestParse() public static function provideSetValue()
{ {
return [ return [
// Test am-pm conversion 'time-11pm' => [
['11:01 pm', '23:01:00'], 'value' => '11:01 pm',
['11:01 am', '11:01:00'], 'expected' => '23:01:00'
['12:01 pm', '12:01:00'], ],
['12:01 am', '00:01:00'], 'time-11am' => [
// Test seconds 'value' => '11:01 am',
['11:01.01 pm', '23:01:01'], 'expected' => '11:01:00'
['12:01.01', '12:01:01'], ],
'time-12am' => [
'value' => '12:01 am',
'expected' => '00:01:00'
],
'time-12pm' => [
'value' => '12:01 pm',
'expected' => '12:01:00'
],
'time-11pm-seconds' => [
'value' => '11:01.01 pm',
'expected' => '23:01:01'
],
'time-12-seconds' => [
'value' => '12:01.01',
'expected' => '12:01:01'
],
'wrong-format-works' => [
'value' => '12.34.56',
'expected' => '12:34:56',
],
'int' => [
'value' => 6789,
'expected' => '01:53:09'
],
'int-string' => [
'value' => '6789',
'expected' => '01:53:09'
],
'zero-string' => [
'value' => '0',
'expected' => '00:00:00'
],
'zero-int' => [
'value' => 0,
'expected' => '00:00:00'
],
'blank-string' => [
'value' => '',
'expected' => ''
],
'null' => [
'value' => null,
'expected' => null
],
'false' => [
'value' => false,
'expected' => false
],
'empty-array' => [
'value' => [],
'expected' => []
],
]; ];
} }
/** #[DataProvider('provideSetValue')]
* @param string $input public function testSetValue(mixed $value, mixed $expected)
* @param string $expected
*/
#[DataProvider('dataTestParse')]
public function testParse($input, $expected)
{ {
$time = DBField::create_field('Time', $input); $field = new DBTime('MyField');
$this->assertEquals( $field->setValue($value);
$expected, $this->assertSame($expected, $field->getValue());
$time->getValue(),
"Date parsed from {$input} should be {$expected}"
);
} }
public function testNice() public function testNice()

View File

@ -30,9 +30,9 @@ class DecimalTest extends SapphireTest
public function testDefaultValue() public function testDefaultValue()
{ {
$this->assertSame( $this->assertSame(
0, 0.0,
$this->testDataObject->MyDecimal1, $this->testDataObject->MyDecimal1,
'Database default for Decimal type is 0' 'Database default for Decimal type is 0.0'
); );
} }
@ -53,9 +53,9 @@ class DecimalTest extends SapphireTest
public function testSpecifiedDefaultValueInDefaultsArray() public function testSpecifiedDefaultValueInDefaultsArray()
{ {
$this->assertEquals( $this->assertSame(
$this->testDataObject->MyDecimal4, $this->testDataObject->MyDecimal4,
4, 4.0,
'Default value for Decimal type is set to 4' 'Default value for Decimal type is set to 4'
); );
} }