From b52087105c72abee4fea8b5e57942767e7318d38 Mon Sep 17 00:00:00 2001 From: CheeseSucker Date: Tue, 18 Jun 2013 22:11:32 +0300 Subject: [PATCH 1/3] FIX: ViewableData::obj() would sometimes return an empty object For instance, this happens when these criteria are met: 1) No casting has been specified for a method in $casting. 2) A template accesses the field without any casting 3) Any casts by the template will now yield an empty object. After a brief look at the commit history, it can seem like this bug is several years old, unless it is a side-effect of other changes in the code. == Steps to reproduce == Add two methods to be accessed by a template. Make sure you do not define an entry in $casting for them: public function Testus() { return "Tet1"; } public function Testus2() { return "Tet2"; } Add this to a template:

First access:
"$Testus" : "$Testus.XML"
"$Testus2.XML" : "$Testus2"

Second access:
"$Testus" : "$Testus.XML"
"$Testus2.XML" : "$Testus2"

Open the page in a browser, and you will get: First access: "Tet1" : "" "Tet2" : "Tet2" Second access: "Tet1" : "" "" : "Tet2" We see that any cast can yield an empty string. --- view/ViewableData.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/view/ViewableData.php b/view/ViewableData.php index dc428d24f..7a6cf5b4c 100644 --- a/view/ViewableData.php +++ b/view/ViewableData.php @@ -383,7 +383,9 @@ class ViewableData extends Object implements IteratorAggregate { if(!is_object($value) && $forceReturnedObject) { $default = Config::inst()->get('ViewableData', 'default_cast', Config::FIRST_SET); - $value = new $default($fieldName); + $castedValue = new $default($fieldName); + $castedValue->setValue($value); + $value = $castedValue; } return $value; From 761eec773646f3ec0f62bec2827b806d77520a4c Mon Sep 17 00:00:00 2001 From: CheeseSucker Date: Wed, 19 Jun 2013 00:47:47 +0200 Subject: [PATCH 2/3] Unit test for bugfix in ViewableData::obj(). --- tests/view/ViewableDataTest.php | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/view/ViewableDataTest.php b/tests/view/ViewableDataTest.php index a90c165fe..8de0d715d 100644 --- a/tests/view/ViewableDataTest.php +++ b/tests/view/ViewableDataTest.php @@ -121,6 +121,28 @@ class ViewableDataTest extends SapphireTest { ); } } + + public function testObjWithCachedStringValueReturnsValidObject() { + $obj = new ViewableDataTest_NoCastingInformation(); + + // Save a literal string into cache + $cache = true; + $uncastedData = $obj->obj('noCastingInformation', null, false, $cache); + + // Fetch the cached string as an object + $forceReturnedObject = true; + $castedData = $obj->obj('noCastingInformation', null, $forceReturnedObject); + + // Uncasted data should always be the nonempty string + $this->assertNotEmpty($uncastedData, 'Uncasted data was empty.'); + $this->assertTrue(is_string($uncastedData), 'Uncasted data should be a string.'); + + // Casted data should be the string wrapped in a DBField-object. + $this->assertNotEmpty($castedData, 'Casted data was empty.'); + $this->assertInstanceOf('DBField', $castedData, 'Casted data should be instance of DBField.'); + + $this->assertEquals($uncastedData, $castedData->getValue(), 'Casted and uncasted strings are not equal.'); + } } /**#@+ @@ -212,4 +234,10 @@ class ViewableDataTest_CastingClass extends ViewableData { ); } +class ViewableDataTest_NoCastingInformation extends ViewableData { + public function noCastingInformation() { + return "No casting information"; + } +} + /**#@-*/ From e6bfabfd6cabcb1f0f79fb11b1e712597f60fe9c Mon Sep 17 00:00:00 2001 From: Jeremy Thomerson Date: Fri, 21 Jun 2013 16:19:06 +0000 Subject: [PATCH 3/3] TEST: additional test for ViewableData not wrapping cached strings --- tests/view/ViewableDataTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/view/ViewableDataTest.php b/tests/view/ViewableDataTest.php index 8de0d715d..855520691 100644 --- a/tests/view/ViewableDataTest.php +++ b/tests/view/ViewableDataTest.php @@ -81,6 +81,19 @@ class ViewableDataTest extends SapphireTest { $this->assertEquals('casted', $newViewableData->forTemplate()); } + public function testDefaultValueWrapping() { + $data = new ArrayData(array('Title' => 'SomeTitleValue')); + // this results in a cached raw string in ViewableData: + $this->assertTrue($data->hasValue('Title')); + $this->assertFalse($data->hasValue('SomethingElse')); + // this should cast the raw string to a StringField since we are + // passing true as the third argument: + $obj = $data->obj('Title', null, true); + $this->assertTrue(is_object($obj)); + // and the string field should have the value of the raw string: + $this->assertEquals('SomeTitleValue', $obj->forTemplate()); + } + public function testRAWVal() { $data = new ViewableDataTest_Castable(); $data->test = 'This & This';