Merge pull request #8815 from open-sausages/pulls/4.0/restore-dynamic-field-assigment

BUG Renable the ability to do dynamic assignment with DBField

src/ORM/DataObject.php

@@ -1331,11 +1331,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
             if (!isset($tableManipulation['fields'])) {
                 continue;
             }
-            foreach ($tableManipulation['fields'] as $fieldValue) {
+            foreach ($tableManipulation['fields'] as $fieldName => $fieldValue) {
                 if (is_array($fieldValue)) {
-                    throw new InvalidArgumentException(
+                    $dbObject = $this->dbObject($fieldName);
-                        'DataObject::writeManipulation: parameterised field assignments are disallowed'
+                    // If the field allows non-scalar values we'll let it do dynamic assignments
-                    );
+                    if ($dbObject && $dbObject->scalarValueOnly()) {
+                        throw new InvalidArgumentException(
+                            'DataObject::writeManipulation: parameterised field assignments are disallowed'
+                        );
+                    }
                 }
             }
         }

src/ORM/ManyManyList.php

@@ -279,6 +279,8 @@ class ManyManyList extends RelationList
                 );
             }
 
+            /** @var DBField[] $fieldObjects */
+            $fieldObjects = [];
             if ($extraFields && $this->extraFields) {
                 // Write extra field to manipluation in the same way
                 // that DataObject::prepareManipulationTable writes fields
@@ -288,6 +290,7 @@ class ManyManyList extends RelationList
                         $fieldObject = Injector::inst()->create($fieldSpec, $fieldName);
                         $fieldObject->setValue($extraFields[$fieldName]);
                         $fieldObject->writeToManipulation($manipulation[$this->joinTable]);
+                        $fieldObjects[$fieldName] = $fieldObject;
                     }
                 }
             }
@@ -300,11 +303,14 @@ class ManyManyList extends RelationList
                 if (!isset($tableManipulation['fields'])) {
                     continue;
                 }
-                foreach ($tableManipulation['fields'] as $fieldValue) {
+                foreach ($tableManipulation['fields'] as $fieldName => $fieldValue) {
                     if (is_array($fieldValue)) {
-                        throw new InvalidArgumentException(
+                        // If the field allows non-scalar values we'll let it do dynamic assignments
-                            'ManyManyList::add: parameterised field assignments are disallowed'
+                        if (isset($fieldObjects[$fieldName]) && $fieldObjects[$fieldName]->scalarValueOnly()) {
-                        );
+                            throw new InvalidArgumentException(
+                                'ManyManyList::add: parameterised field assignments are disallowed'
+                            );
+                        }
                     }
                 }
             }

tests/php/ORM/DataObjectTest.php

@@ -57,6 +57,7 @@ class DataObjectTest extends SapphireTest
         DataObjectTest\RelationParent::class,
         DataObjectTest\RelationChildFirst::class,
         DataObjectTest\RelationChildSecond::class,
+        DataObjectTest\MockDynamicAssignmentDataObject::class
     );
 
     public static function getExtraDataObjects()
@@ -2147,4 +2148,34 @@ class DataObjectTest extends SapphireTest
         $do->SalaryCap = array('Amount' => 123456, 'Currency' => 'CAD');
         $this->assertNotEmpty($do->SalaryCap);
     }
+
+    public function testWriteManipulationWithNonScalarValuesAllowed()
+    {
+        $do = DataObjectTest\MockDynamicAssignmentDataObject::create();
+        $do->write();
+
+        $do->StaticScalarOnlyField = true;
+        $do->DynamicScalarOnlyField = false;
+        $do->DynamicField = true;
+
+        $do->write();
+
+        $this->assertTrue($do->StaticScalarOnlyField);
+        $this->assertFalse($do->DynamicScalarOnlyField);
+        $this->assertTrue($do->DynamicField);
+    }
+
+    public function testWriteManipulationWithNonScalarValuesDisallowed()
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        $do = DataObjectTest\MockDynamicAssignmentDataObject::create();
+        $do->write();
+
+        $do->StaticScalarOnlyField = false;
+        $do->DynamicScalarOnlyField = true;
+        $do->DynamicField = false;
+
+        $do->write();
+    }
 }

tests/php/ORM/DataObjectTest/MockDynamicAssignmentDBField.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\FieldType\DBBoolean;
+use SilverStripe\ORM\FieldType\DBField;
+
+/**
+ * This is a fake DB field specifically design to test dynamic value assignment. You can set `scalarValueOnly` in
+ * the constructor. You can control whetever the field will try to do a dynamic assignment by specifing
+ * `$dynamicAssignment` in nthe consturctor.
+ *
+ * If the field is set to false, it will try to do a plain assignment. This is so you can save the initial value no
+ * matter what. If the field is set to true, it will try to do a dynamic assignment.
+ */
+class MockDynamicAssignmentDBField extends DBBoolean implements TestOnly
+{
+
+    private $scalarOnly;
+    private $dynamicAssignment;
+
+    /**
+     * @param string $name
+     * @param boolean $scalarOnly Whether our fake field should be scalar only.
+     * @param boolean $dynamicAssignment Whether our fake field will try to do a dynamic assignment.
+     */
+    public function __construct($name = '', $scalarOnly = false, $dynamicAssignment = false)
+    {
+        $this->scalarOnly = $scalarOnly;
+        $this->dynamicAssignment = $dynamicAssignment;
+        parent::__construct($name);
+    }
+
+    /**
+     * If the field value and $dynamicAssignment are true, we'll try to do a dynamic assignment.
+     * @param $value
+     * @return array|int
+     */
+    public function prepValueForDB($value)
+    {
+        if ($value) {
+            return $this->dynamicAssignment
+                ? ['ABS(?)' => [1]]
+                : 1;
+        }
+
+        return 0;
+    }
+
+    public function scalarValueOnly()
+    {
+        return $this->scalarOnly;
+    }
+}

tests/php/ORM/DataObjectTest/MockDynamicAssignmentDataObject.php

@@ -0,0 +1,52 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\ManyManyList;
+
+/**
+ * This is a fake DB field specifically design to test dynamic value assignment
+ * @property boolean $StaticScalarOnlyField
+ * @property boolean $DynamicScalarOnlyField
+ * @property boolean $DynamicField
+ * @method ManyManyList MockManyMany
+ */
+class MockDynamicAssignmentDataObject extends DataObject implements TestOnly
+{
+
+    private static $table_name = 'MockDynamicAssignmentDataObject';
+
+    private static $db = [
+        // This field only emits scalar value and will save
+        'StaticScalarOnlyField' => MockDynamicAssignmentDBField::class . '(1,0)',
+
+        // This field tries to emit dynamic assignment but will fail because of scalar only
+        'DynamicScalarOnlyField' => MockDynamicAssignmentDBField::class . '(1,1)',
+
+        // This field does dynamic assignement and will pass
+        'DynamicField' => MockDynamicAssignmentDBField::class . '(0,1)',
+    ];
+
+    private static $many_many = [
+        'MockManyMany' => self::class,
+    ];
+
+    private static $belongs_many_many = [
+        'MockBelongsManyMany' => self::class,
+    ];
+
+    private static $many_many_extraFields = [
+        'MockManyMany' => [
+            // This field only emits scalar value and will save
+            'ManyManyStaticScalarOnlyField' => MockDynamicAssignmentDBField::class . '(1,0)',
+
+            // This field tries to emit dynamic assignment but will fail because of scalar only
+            'ManyManyDynamicScalarOnlyField' => MockDynamicAssignmentDBField::class . '(1,1)',
+
+            // This field does dynamic assignement and will pass
+            'ManyManyDynamicField' => MockDynamicAssignmentDBField::class . '(0,1)',
+        ]
+    ];
+}

tests/php/ORM/ManyManyListTest.php

@@ -10,6 +10,7 @@ use SilverStripe\ORM\Tests\DataObjectTest\Player;
 use SilverStripe\ORM\Tests\DataObjectTest\Team;
 use SilverStripe\ORM\Tests\ManyManyListTest\ExtraFieldsObject;
 use SilverStripe\ORM\Tests\ManyManyListTest\Product;
+use InvalidArgumentException;
 
 class ManyManyListTest extends SapphireTest
 {
@@ -20,6 +21,7 @@ class ManyManyListTest extends SapphireTest
         ManyManyListTest\Category::class,
         ManyManyListTest\ExtraFieldsObject::class,
         ManyManyListTest\Product::class,
+        DataObjectTest\MockDynamicAssignmentDataObject::class
     ];
 
     public static function getExtraDataObjects()
@@ -378,4 +380,40 @@ class ManyManyListTest extends SapphireTest
         $productsRelatedToProductB = $category->Products()->filter('RelatedProducts.Title', 'Product A');
         $this->assertEquals(1, $productsRelatedToProductB->count());
     }
+
+    public function testWriteManipulationWithNonScalarValuesAllowed()
+    {
+        $left = DataObjectTest\MockDynamicAssignmentDataObject::create();
+        $left->write();
+        $right = DataObjectTest\MockDynamicAssignmentDataObject::create();
+        $right->write();
+
+        $left->MockManyMany()->add($right, [
+            'ManyManyStaticScalarOnlyField' => true,
+            'ManyManyDynamicScalarOnlyField' => false,
+            'ManyManyDynamicField' => true,
+        ]);
+
+        $pivot = $left->MockManyMany()->first();
+
+        $this->assertNotFalse($pivot->ManyManyStaticScalarOnlyField);
+        $this->assertNotTrue($pivot->ManyManyDynamicScalarOnlyField);
+        $this->assertNotFalse($pivot->ManyManyDynamicField);
+    }
+
+    public function testWriteManipulationWithNonScalarValuesDisallowed()
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        $left = DataObjectTest\MockDynamicAssignmentDataObject::create();
+        $left->write();
+        $right = DataObjectTest\MockDynamicAssignmentDataObject::create();
+        $right->write();
+
+        $left->MockManyMany()->add($right, [
+            'ManyManyStaticScalarOnlyField' => false,
+            'ManyManyDynamicScalarOnlyField' => true,
+            'ManyManyDynamicField' => false,
+        ]);
+    }
 }