FIX Ensure getters and setters are respected

silverstripe · Feb 27, 2023 · 145fb40 · 145fb40
1 parent 469bac8
commit 145fb40
Show file tree

Hide file tree

Showing 7 changed files with 107 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/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);
+    }
 }