Merge pull request #10698 from creative-commoners/pulls/5.0/dynamic-data

NEW Access dynamic data inside ViewableData
silverstripe · Feb 21, 2023 · 469bac8 · 469bac8
2 parents bddad3a + 0075bf6
commit 469bac8
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 8 deletions.
diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\ORM\FieldType;
 
+use InvalidArgumentException;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
@@ -270,11 +271,11 @@ public function setField($field, $value, $markChanged = true)
     {
         $this->objCacheClear();
 
-        // Non-db fields get assigned as normal properties
         if (!$this->hasField($field)) {
-            parent::setField($field, $value);
-
-            return $this;
+            throw new InvalidArgumentException(implode(' ', [
+                "Field $field does not exist.",
+                'If this was accessed via a dynamic property then call setDynamicData() instead.'
+            ]));
         }
 
         // Set changed

diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php
@@ -73,7 +73,7 @@ class ViewableData implements IteratorAggregate
     /**
      * Acts as a PHP 8.2+ compliant replacement for dynamic properties
      */
-    private array $data = [];
+    private array $dynamicData = [];
 
     // -----------------------------------------------------------------------------------------------------------------
 
@@ -203,7 +203,7 @@ public function getFailover()
      */
     public function hasField($field)
     {
-        return property_exists($this, $field) || isset($this->data[$field]);
+        return property_exists($this, $field) || $this->hasDynamicData($field);
     }
 
     /**
@@ -217,7 +217,7 @@ public function getField($field)
         if ($this->isAccessibleProperty($field)) {
             return $this->$field;
         }
-        return $this->data[$field];
+        return $this->getDynamicData($field);
     }
 
     /**
@@ -236,10 +236,25 @@ public function setField($field, $value)
         if ($this->isAccessibleProperty($field)) {
             $this->$field = $value;
         }
-        $this->data[$field] = $value;
+        return $this->setDynamicData($field, $value);
+    }
+
+    public function getDynamicData(string $field): mixed
+    {
+        return $this->hasDynamicData($field) ? $this->dynamicData[$field] : null;
+    }
+
+    public function setDynamicData(string $field, mixed $value): static
+    {
+        $this->dynamicData[$field] = $value;
         return $this;
     }
 
+    public function hasDynamicData(string $field): bool
+    {
+        return array_key_exists($field, $this->dynamicData);
+    }
+
     /**
      * Returns true if a method exists for the current class which isn't private.
      * Also returns true for private methods if $this is ViewableData (not a subclass)

diff --git a/tests/php/ORM/DBCompositeTest.php b/tests/php/ORM/DBCompositeTest.php
@@ -5,6 +5,7 @@
 use SilverStripe\ORM\FieldType\DBMoney;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\Dev\SapphireTest;
+use InvalidArgumentException;
 
 class DBCompositeTest extends SapphireTest
 {
@@ -108,4 +109,15 @@ public function testInheritedTables()
         $this->assertEquals('DBCompositeTest_SubclassedDBFieldObject', $object2->dbObject('OtherMoney')->getTable());
         $this->assertEquals('DBCompositeTest_SubclassedDBFieldObject', $object2->dbObject('OverriddenMoney')->getTable());
     }
+
+    public function testSetFieldDynamicPropertyException()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(implode(' ', [
+            'Field abc does not exist.',
+            'If this was accessed via a dynamic property then call setDynamicData() instead.'
+        ]));
+        $object = new DBCompositeTest\TestObject();
+        $object->MyMoney->abc = 'def';
+    }
 }
diff --git a/tests/php/View/ViewableDataTest.php b/tests/php/View/ViewableDataTest.php
@@ -267,4 +267,15 @@ public function testIsAccessibleProperty()
         $output = $reflectionMethod->invokeArgs(new ViewableData(), ['objCache']);
         $this->assertTrue($output, 'Property should be accessible');
     }
+
+    public function testDynamicData()
+    {
+        $obj = (object) ['SomeField' => [1, 2, 3]];
+        $viewableData = new ViewableData();
+        $this->assertFalse($viewableData->hasDynamicData('abc'));
+        $viewableData->setDynamicData('abc', $obj);
+        $this->assertTrue($viewableData->hasDynamicData('abc'));
+        $this->assertSame($obj, $viewableData->getDynamicData('abc'));
+        $this->assertSame($obj, $viewableData->abc);
+    }
 }