FIX Ensure getters and setters are respected (#10708)

silverstripe · Mar 1, 2023 · e3a94b9 · e3a94b9
1 parent 981e20e
commit e3a94b9
Show file tree

Hide file tree

Showing 10 changed files with 203 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -27,13 +27,17 @@
 use SilverStripe\ORM\FieldType\DBVarchar;
 use SilverStripe\ORM\FieldType\DBText;
 use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\FieldType\DBYear;
 
 /**
  * Tests for DBField objects.
  */
 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('<P>ÅÄÖ</P>', DBHTMLText::create_field('HTMLFragment', '<p>åäö</p>')->UpperCase());
         $this->assertEquals('<p>åäö</p>', DBHTMLText::create_field('HTMLFragment', '<p>ÅÄÖ</p>')->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
@@ -0,0 +1,29 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DBFieldTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class TestDataObject extends DataObject implements TestOnly
+{
+    private static $table_name = 'DBFieldTest_TestDataObject';
+
+    private static $db = [
+        'Title' => 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
@@ -0,0 +1,42 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DBFieldTest;
+
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DB;
+use SilverStripe\ORM\FieldType\DBField;
+
+class TestDbField extends DBField implements TestOnly
+{
+    public function requireField()
+    {
+        // Basically the same as DBVarchar but we don't want to test with DBVarchar in case something
+        // changes in that class eventually.
+        $charset = Config::inst()->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
@@ -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
@@ -0,0 +1,25 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class SettersAndGetters extends DataObject implements TestOnly
+{
+    private static $table_name = 'DataObjectTest_SettersAndGetters';
+
+    private static $db = [
+        'MyTestField' => 'Varchar(255)',
+    ];
+
+    public function setMyTestField($val)
+    {
+        $this->setField('MyTestField', strtolower($val));
+    }
+
+    public function getMyTestField()
+    {
+        return strtoupper($this->getField('MyTestField'));
+    }
+}