From b53cda8de0211b2978b7c8ae9324ce20cdb97e2a Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Tue, 11 Jun 2024 16:49:27 +1200 Subject: [PATCH] FIX Respect explicit casting before casting arrays (#11271) --- src/Forms/Form.php | 4 +- src/Forms/FormField.php | 4 +- src/Forms/ReadonlyField.php | 4 +- src/ORM/DataObject.php | 4 +- src/ORM/FieldType/DBComposite.php | 4 +- src/View/ViewableData.php | 50 ++++++++++++++++---- tests/php/View/ViewableDataTest.php | 4 ++ tests/php/View/ViewableDataTest/Castable.php | 12 ++++- 8 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 111bbf120..ffd7732e6 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -521,13 +521,13 @@ class Form extends ViewableData implements HasRequestHandler return $this; } - public function castingHelper($field) + public function castingHelper($field, bool $useFallback = true) { // Override casting for field message if (strcasecmp($field ?? '', 'Message') === 0 && ($helper = $this->getMessageCastingHelper())) { return $helper; } - return parent::castingHelper($field); + return parent::castingHelper($field, $useFallback); } /** diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index c9cc22eb6..30fa2adc4 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -790,13 +790,13 @@ class FormField extends RequestHandler return $form->getSecurityToken()->isEnabled(); } - public function castingHelper($field) + public function castingHelper($field, bool $useFallback = true) { // Override casting for field message if (strcasecmp($field ?? '', 'Message') === 0 && ($helper = $this->getMessageCastingHelper())) { return $helper; } - return parent::castingHelper($field); + return parent::castingHelper($field, $useFallback); } /** diff --git a/src/Forms/ReadonlyField.php b/src/Forms/ReadonlyField.php index 8bbe0ba47..ef31f3d7b 100644 --- a/src/Forms/ReadonlyField.php +++ b/src/Forms/ReadonlyField.php @@ -56,7 +56,7 @@ class ReadonlyField extends FormField return 'readonly'; } - public function castingHelper($field) + public function castingHelper($field, bool $useFallback = true) { // Get dynamic cast for 'Value' field if (strcasecmp($field ?? '', 'Value') === 0) { @@ -64,7 +64,7 @@ class ReadonlyField extends FormField } // Fall back to default casting - return parent::castingHelper($field); + return parent::castingHelper($field, $useFallback); } public function getSchemaStateDefaults() diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 42b803190..bab495580 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -3015,7 +3015,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * {@inheritdoc} */ - public function castingHelper($field) + public function castingHelper($field, bool $useFallback = true) { $fieldSpec = static::getSchema()->fieldSpec(static::class, $field); if ($fieldSpec) { @@ -3033,7 +3033,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } } - return parent::castingHelper($field); + return parent::castingHelper($field, $useFallback); } /** diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php index c560a6098..1fa7001db 100644 --- a/src/ORM/FieldType/DBComposite.php +++ b/src/ORM/FieldType/DBComposite.php @@ -319,14 +319,14 @@ abstract class DBComposite extends DBField return $fieldObject; } - public function castingHelper($field) + public function castingHelper($field, bool $useFallback = true) { $fields = $this->compositeDatabaseFields(); if (isset($fields[$field])) { return $fields[$field]; } - return parent::castingHelper($field); + return parent::castingHelper($field, $useFallback); } public function getIndexSpecs() diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index e5a2c507c..06332f0c9 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -385,10 +385,11 @@ class ViewableData implements IteratorAggregate * for a field on this object. This helper will be a subclass of DBField. * * @param string $field - * @return string Casting helper As a constructor pattern, and may include arguments. + * @param bool $useFallback If true, fall back on the default casting helper if there isn't an explicit one. + * @return string|null Casting helper As a constructor pattern, and may include arguments. * @throws Exception */ - public function castingHelper($field) + public function castingHelper($field, bool $useFallback = true) { // Get casting if it has been configured. // DB fields and PHP methods are all case insensitive so we normalise casing before checking. @@ -399,20 +400,41 @@ class ViewableData implements IteratorAggregate } // If no specific cast is declared, fall back to failover. - // Note that if there is a failover, the default_cast will always - // be drawn from this object instead of the top level object. $failover = $this->getFailover(); if ($failover) { - $cast = $failover->castingHelper($field); + $cast = $failover->castingHelper($field, $useFallback); if ($cast) { return $cast; } } - // Fall back to default_cast + if ($useFallback) { + return $this->defaultCastingHelper($field); + } + + return null; + } + + /** + * Return the default "casting helper" for use when no explicit casting helper is defined. + * This helper will be a subclass of DBField. See castingHelper() + */ + protected function defaultCastingHelper(string $field): string + { + // If there is a failover, the default_cast will always + // be drawn from this object instead of the top level object. + $failover = $this->getFailover(); + if ($failover) { + $cast = $failover->defaultCastingHelper($field); + if ($cast) { + return $cast; + } + } + + // Fall back to raw default_cast $default = $this->config()->get('default_cast'); if (empty($default)) { - throw new Exception("No default_cast"); + throw new Exception('No default_cast'); } return $default; } @@ -559,15 +581,25 @@ class ViewableData implements IteratorAggregate $value = $this->$fieldName; } + // Try to cast object if we have an explicit cast set + if (!is_object($value)) { + $castingHelper = $this->castingHelper($fieldName, false); + if ($castingHelper !== null) { + $valueObject = Injector::inst()->create($castingHelper, $fieldName); + $valueObject->setValue($value, $this); + $value = $valueObject; + } + } + // Wrap list arrays in ViewableData so templates can handle them if (is_array($value) && array_is_list($value)) { $value = ArrayList::create($value); } - // Cast object + // Fallback on default casting if (!is_object($value)) { // Force cast - $castingHelper = $this->castingHelper($fieldName); + $castingHelper = $this->defaultCastingHelper($fieldName); $valueObject = Injector::inst()->create($castingHelper, $fieldName); $valueObject->setValue($value, $this); $value = $valueObject; diff --git a/tests/php/View/ViewableDataTest.php b/tests/php/View/ViewableDataTest.php index 1610fdb63..5b8ba205f 100644 --- a/tests/php/View/ViewableDataTest.php +++ b/tests/php/View/ViewableDataTest.php @@ -6,6 +6,7 @@ use ReflectionMethod; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\Dev\SapphireTest; use SilverStripe\ORM\ArrayList; +use SilverStripe\ORM\FieldType\DBText; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use SilverStripe\View\Tests\ViewableDataTest\ViewableDataTestExtension; @@ -59,6 +60,9 @@ class ViewableDataTest extends SapphireTest $this->assertInstanceOf(ViewableDataTest\RequiresCasting::class, $caster->obj('alwaysCasted')); $this->assertInstanceOf(ViewableDataTest\Caster::class, $caster->obj('noCastingInformation')); + + $this->assertInstanceOf(DBText::class, $caster->obj('arrayOne')); + $this->assertInstanceOf(ArrayList::class, $caster->obj('arrayTwo')); } public function testFailoverRequiresCasting() diff --git a/tests/php/View/ViewableDataTest/Castable.php b/tests/php/View/ViewableDataTest/Castable.php index 3e8a94a8b..e76966f18 100644 --- a/tests/php/View/ViewableDataTest/Castable.php +++ b/tests/php/View/ViewableDataTest/Castable.php @@ -7,13 +7,13 @@ use SilverStripe\View\ViewableData; class Castable extends ViewableData implements TestOnly { - private static $default_cast = Caster::class; private static $casting = [ 'alwaysCasted' => RequiresCasting::class, 'castedUnsafeXML' => UnescapedCaster::class, 'test' => 'Text', + 'arrayOne' => 'Text', ]; public $test = 'test'; @@ -25,6 +25,16 @@ class Castable extends ViewableData implements TestOnly return 'alwaysCasted'; } + public function arrayOne() + { + return ['value1', 'value2']; + } + + public function arrayTwo() + { + return ['value1', 'value2']; + } + public function noCastingInformation() { return 'noCastingInformation';