From 69f40a6c88818c6fd9f31c53835533b70816eb85 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 27 Feb 2023 16:20:20 +1300 Subject: [PATCH] FIX Ensure getters and setters are respected --- src/ORM/FieldType/DBBoolean.php | 6 +- src/ORM/FieldType/DBComposite.php | 8 +- src/ORM/FieldType/DBDecimal.php | 8 +- src/ORM/FieldType/DBField.php | 6 +- src/ORM/FieldType/DBPercentage.php | 2 +- tests/php/ORM/DBFieldTest.php | 73 +++++++++++++++++++ tests/php/ORM/DBFieldTest/TestDataObject.php | 29 ++++++++ tests/php/ORM/DBFieldTest/TestDbField.php | 42 +++++++++++ tests/php/ORM/DataObjectTest.php | 11 +++ .../ORM/DataObjectTest/SettersAndGetters.php | 25 +++++++ 10 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 tests/php/ORM/DBFieldTest/TestDataObject.php create mode 100644 tests/php/ORM/DBFieldTest/TestDbField.php create mode 100644 tests/php/ORM/DataObjectTest/SettersAndGetters.php diff --git a/src/ORM/FieldType/DBBoolean.php b/src/ORM/FieldType/DBBoolean.php index a0b485f5d25..46374682cc1 100644 --- a/src/ORM/FieldType/DBBoolean.php +++ b/src/ORM/FieldType/DBBoolean.php @@ -46,7 +46,11 @@ public function saveInto($dataObject) { $fieldName = $this->name; if ($fieldName) { - $dataObject->setField($fieldName, $this->value ? 1 : 0); + if ($this->value instanceof DBField) { + $this->value->saveInto($dataObject); + } else { + $dataObject->__set($fieldName, $this->value ? 1 : 0); + } } else { $class = static::class; throw new \RuntimeException("DBField::saveInto() Called on a nameless '$class' object"); diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php index a4d936b2ebe..2447f3df952 100644 --- a/src/ORM/FieldType/DBComposite.php +++ b/src/ORM/FieldType/DBComposite.php @@ -221,8 +221,12 @@ public function saveInto($dataObject) { foreach ($this->compositeDatabaseFields() as $field => $spec) { // Save into record - $key = $this->getName() . $field; - $dataObject->setField($key, $this->getField($field)); + if ($this->value instanceof DBField) { + $this->value->saveInto($dataObject); + } else { + $key = $this->getName() . $field; + $dataObject->__set($key, $this->getField($field)); + } } } diff --git a/src/ORM/FieldType/DBDecimal.php b/src/ORM/FieldType/DBDecimal.php index a8c9df99b98..dcf2d7e8046 100644 --- a/src/ORM/FieldType/DBDecimal.php +++ b/src/ORM/FieldType/DBDecimal.php @@ -88,8 +88,12 @@ public function saveInto($dataObject) $fieldName = $this->name; if ($fieldName) { - $value = (float) preg_replace('/[^0-9.\-\+]/', '', $this->value ?? ''); - $dataObject->setField($fieldName, $value); + if ($this->value instanceof DBField) { + $this->value->saveInto($dataObject); + } else { + $value = (float) preg_replace('/[^0-9.\-\+]/', '', $this->value ?? ''); + $dataObject->__set($fieldName, $value); + } } else { throw new \UnexpectedValueException( "DBField::saveInto() Called on a nameless '" . static::class . "' object" diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index 0c96442fca9..520ce73d851 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -542,7 +542,11 @@ public function saveInto($dataObject) "DBField::saveInto() Called on a nameless '" . static::class . "' object" ); } - $dataObject->setField($fieldName, $this->value); + if ($this->value instanceof self) { + $this->value->saveInto($dataObject); + } else { + $dataObject->__set($fieldName, $this->value); + } } /** diff --git a/src/ORM/FieldType/DBPercentage.php b/src/ORM/FieldType/DBPercentage.php index dca55431cf7..1abf613a26a 100644 --- a/src/ORM/FieldType/DBPercentage.php +++ b/src/ORM/FieldType/DBPercentage.php @@ -45,7 +45,7 @@ public function saveInto($dataObject) $fieldName = $this->name; if ($fieldName && $dataObject->$fieldName > 1.0) { - $dataObject->setField($fieldName, 1.0); + $dataObject->__set($fieldName, 1.0); } } } diff --git a/tests/php/ORM/DBFieldTest.php b/tests/php/ORM/DBFieldTest.php index 4c106b7d993..8168137b540 100644 --- a/tests/php/ORM/DBFieldTest.php +++ b/tests/php/ORM/DBFieldTest.php @@ -27,6 +27,7 @@ use SilverStripe\ORM\FieldType\DBVarchar; use SilverStripe\ORM\FieldType\DBText; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\FieldType\DBYear; /** @@ -34,6 +35,9 @@ */ class DBFieldTest extends SapphireTest { + protected static $extra_dataobjects = [ + DBFieldTest\TestDataObject::class, + ]; /** * Test the nullValue() method on DBField. @@ -322,4 +326,73 @@ public function testStringFieldsWithMultibyteData() $this->assertEquals('

ÅÄÖ

', DBHTMLText::create_field('HTMLFragment', '

åäö

')->UpperCase()); $this->assertEquals('

åäö

', DBHTMLText::create_field('HTMLFragment', '

ÅÄÖ

')->LowerCase()); } + + public function testSaveInto() + { + $obj = new DBFieldTest\TestDataObject(); + /** @var DBField $field */ + $field = $obj->dbObject('Title'); + $field->setValue('New Value'); + $field->saveInto($obj); + + $this->assertEquals('New Value', $obj->getField('Title')); + $this->assertEquals(1, $field->saveIntoCalledCount); + $this->assertEquals(1, $obj->setFieldCalledCount); + } + + public function testSaveIntoNoRecursion() + { + $obj = new DBFieldTest\TestDataObject(); + /** @var DBField $field */ + $field = $obj->dbObject('Title'); + $value = new DBFieldTest\TestDbField('Title'); + $value->setValue('New Value'); + $field->setValue($value); + $field->saveInto($obj); + + $this->assertEquals('New Value', $obj->getField('Title')); + $this->assertEquals(1, $field->saveIntoCalledCount); + $this->assertEquals(1, $obj->setFieldCalledCount); + } + + public function testSaveIntoAsProperty() + { + $obj = new DBFieldTest\TestDataObject(); + /** @var DBField $field */ + $field = $obj->dbObject('Title'); + $field->setValue('New Value'); + $obj->Title = $field; + + $this->assertEquals('New Value', $obj->getField('Title')); + $this->assertEquals(1, $field->saveIntoCalledCount); + // Called twice because $obj->setField($field) => $field->saveInto() => $obj->setField('New Value') + $this->assertEquals(2, $obj->setFieldCalledCount); + } + + public function testSaveIntoNoRecursionAsProperty() + { + $obj = new DBFieldTest\TestDataObject(); + /** @var DBField $field */ + $field = $obj->dbObject('Title'); + $value = new DBFieldTest\TestDbField('Title'); + $value->setValue('New Value'); + $field->setValue($value); + $obj->Title = $field; + + $this->assertEquals('New Value', $obj->getField('Title')); + $this->assertEquals(1, $field->saveIntoCalledCount); + // Called twice because $obj->setField($field) => $field->saveInto() => $obj->setField('New Value') + $this->assertEquals(2, $obj->setFieldCalledCount); + } + + public function testSaveIntoRespectsSetters() + { + $obj = new DBFieldTest\TestDataObject(); + /** @var DBField $field */ + $field = $obj->dbObject('MyTestField'); + $field->setValue('New Value'); + $obj->MyTestField = $field; + + $this->assertEquals('new value', $obj->getField('MyTestField')); + } } diff --git a/tests/php/ORM/DBFieldTest/TestDataObject.php b/tests/php/ORM/DBFieldTest/TestDataObject.php new file mode 100644 index 00000000000..4d2efd37454 --- /dev/null +++ b/tests/php/ORM/DBFieldTest/TestDataObject.php @@ -0,0 +1,29 @@ + TestDbField::class, + 'MyTestField' => TestDbField::class, + ]; + + public $setFieldCalledCount = 0; + + public function setField($fieldName, $val) + { + $this->setFieldCalledCount++; + return parent::setField($fieldName, $val); + } + + public function setMyTestField($val) + { + return $this->setField('MyTestField', strtolower($val)); + } +} diff --git a/tests/php/ORM/DBFieldTest/TestDbField.php b/tests/php/ORM/DBFieldTest/TestDbField.php new file mode 100644 index 00000000000..deb9773c419 --- /dev/null +++ b/tests/php/ORM/DBFieldTest/TestDbField.php @@ -0,0 +1,42 @@ +get(MySQLDatabase::class, 'charset'); + $collation = Config::inst()->get(MySQLDatabase::class, 'collation'); + + $parts = [ + 'datatype' => 'varchar', + 'precision' => 255, + 'character set' => $charset, + 'collate' => $collation, + 'arrayValue' => $this->arrayValue + ]; + + $values = [ + 'type' => 'varchar', + 'parts' => $parts + ]; + + DB::require_field($this->tableName, $this->name, $values); + } + + public $saveIntoCalledCount = 0; + + public function saveInto($dataObject) + { + $this->saveIntoCalledCount++; + return parent::saveInto($dataObject); + } +} diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 493840301f9..4139291ca78 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -66,6 +66,7 @@ class DataObjectTest extends SapphireTest DataObjectTest\TreeNode::class, DataObjectTest\OverriddenDataObject::class, DataObjectTest\InjectedDataObject::class, + DataObjectTest\SettersAndGetters::class, ]; protected function setUp(): void @@ -2667,4 +2668,14 @@ public function testDBObjectEnum() $vals = ['25.25', '50.00', '75.00', '100.50']; $this->assertSame(array_combine($vals ?? [], $vals ?? []), $obj->dbObject('MyEnumWithDots')->enumValues()); } + + public function testSettersAndGettersAreRespected() + { + $obj = new DataObjectTest\SettersAndGetters(); + $obj->MyTestField = 'Some Value'; + // Setter overrides it with all lower case + $this->assertSame('some value', $obj->getField('MyTestField')); + // Getter overrides it with all upper case + $this->assertSame('SOME VALUE', $obj->MyTestField); + } } diff --git a/tests/php/ORM/DataObjectTest/SettersAndGetters.php b/tests/php/ORM/DataObjectTest/SettersAndGetters.php new file mode 100644 index 00000000000..7d6ff9059d3 --- /dev/null +++ b/tests/php/ORM/DataObjectTest/SettersAndGetters.php @@ -0,0 +1,25 @@ + 'Varchar(255)', + ]; + + public function setMyTestField($val) + { + $this->setField('MyTestField', strtolower($val)); + } + + public function getMyTestField() + { + return strtoupper($this->getField('MyTestField')); + } +}