Skip to content

Commit

Permalink
BUG Renable the ability to do dynamic assignment with DBField
Browse files Browse the repository at this point in the history
  • Loading branch information
Maxime Rainville committed Feb 21, 2019
1 parent a481d00 commit 9a59f2f
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 8 deletions.
12 changes: 8 additions & 4 deletions src/ORM/DataObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -1331,11 +1331,15 @@ protected function writeManipulation($baseTable, $now, $isNewRecord)
if (!isset($tableManipulation['fields'])) {
continue;
}
foreach ($tableManipulation['fields'] as $fieldValue) {
foreach ($tableManipulation['fields'] as $fieldName => $fieldValue) {
if (is_array($fieldValue)) {
throw new InvalidArgumentException(
'DataObject::writeManipulation: parameterised field assignments are disallowed'
);
$dbObject = $this->dbObject($fieldName);
// 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'
);
}
}
}
}
Expand Down
14 changes: 10 additions & 4 deletions src/ORM/ManyManyList.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ public function add($item, $extraFields = array())
);
}

/** @var DBField[] $fieldObjects */
$fieldObjects = [];
if ($extraFields && $this->extraFields) {
// Write extra field to manipluation in the same way
// that DataObject::prepareManipulationTable writes fields
Expand All @@ -288,6 +290,7 @@ public function add($item, $extraFields = array())
$fieldObject = Injector::inst()->create($fieldSpec, $fieldName);
$fieldObject->setValue($extraFields[$fieldName]);
$fieldObject->writeToManipulation($manipulation[$this->joinTable]);
$fieldObjects[$fieldName] = $fieldObject;
}
}
}
Expand All @@ -300,11 +303,14 @@ public function add($item, $extraFields = array())
if (!isset($tableManipulation['fields'])) {
continue;
}
foreach ($tableManipulation['fields'] as $fieldValue) {
foreach ($tableManipulation['fields'] as $fieldName => $fieldValue) {
if (is_array($fieldValue)) {
throw new InvalidArgumentException(
'ManyManyList::add: parameterised field assignments are disallowed'
);
// If the field allows non-scalar values we'll let it do dynamic assignments
if (isset($fieldObjects[$fieldName]) && $fieldObjects[$fieldName]->scalarValueOnly()) {
throw new InvalidArgumentException(
'ManyManyList::add: parameterised field assignments are disallowed'
);
}
}
}
}
Expand Down
31 changes: 31 additions & 0 deletions tests/php/ORM/DataObjectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class DataObjectTest extends SapphireTest
DataObjectTest\RelationParent::class,
DataObjectTest\RelationChildFirst::class,
DataObjectTest\RelationChildSecond::class,
DataObjectTest\MockDynamicAssignmentDataObject::class
);

public static function getExtraDataObjects()
Expand Down Expand Up @@ -2147,4 +2148,34 @@ public function testSetFieldWithArrayOnCompositeField()
$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();
}
}
54 changes: 54 additions & 0 deletions tests/php/ORM/DataObjectTest/MockDynamicAssignmentDBField.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

namespace SilverStripe\ORM\Tests\DataObjectTest;

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
{

private $scalarOnly;
private $dynamicAssignment;

/**
* @param $name
* @param $scalarOnly Whether our fake field should be scalar only
* @param $dynamicAssigment Wheter our fake field will try to do a dynamic assignement
*/
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 assignement
* @param $value
* @return array|int|mixed
*/
public function prepValueForDB($value)
{
if ($value) {
return $this->dynamicAssignment
? ['GREATEST(?, ?)' => [0, 1]]
: 1;
}

return 0;
}

public function scalarValueOnly()
{
return $this->scalarOnly;
}
}
52 changes: 52 additions & 0 deletions tests/php/ORM/DataObjectTest/MockDynamicAssignmentDataObject.php
Original file line number Diff line number Diff line change
@@ -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)',
]
];
}
37 changes: 37 additions & 0 deletions tests/php/ORM/ManyManyListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class ManyManyListTest extends SapphireTest
ManyManyListTest\Category::class,
ManyManyListTest\ExtraFieldsObject::class,
ManyManyListTest\Product::class,
DataObjectTest\MockDynamicAssignmentDataObject::class
];

public static function getExtraDataObjects()
Expand Down Expand Up @@ -378,4 +379,40 @@ public function testFilteringOnPreviouslyJoinedTable()
$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,
]);
}
}

0 comments on commit 9a59f2f

Please sign in to comment.