From 3f30da5155af6165627cfb6d7b6c19888fd58613 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Fri, 24 May 2024 12:51:14 +1200 Subject: [PATCH] ENH Looping through arrays in templates (#11244) --- src/ORM/ArrayList.php | 2 +- src/View/ArrayData.php | 2 +- src/View/SSTemplateParser.php | 7 +- src/View/SSViewer_Scope.php | 9 ++ src/View/ViewableData.php | 17 ++-- tests/php/View/SSViewerTest.php | 127 ++++++++++++++++++++++++++++ tests/php/View/ViewableDataTest.php | 28 ++++++ 7 files changed, 179 insertions(+), 13 deletions(-) diff --git a/src/ORM/ArrayList.php b/src/ORM/ArrayList.php index f953ffb6176..2d98d26f984 100644 --- a/src/ORM/ArrayList.php +++ b/src/ORM/ArrayList.php @@ -127,7 +127,7 @@ public function exists() public function getIterator(): Traversable { foreach ($this->items as $i => $item) { - if (is_array($item)) { + if (is_array($item) && !array_is_list($item)) { yield new ArrayData($item); } else { yield $item; diff --git a/src/View/ArrayData.php b/src/View/ArrayData.php index 1d166de5305..819ad8f75ac 100644 --- a/src/View/ArrayData.php +++ b/src/View/ArrayData.php @@ -77,7 +77,7 @@ public function toMap() public function getField($field) { $value = $this->array[$field]; - if (is_object($value) && !$value instanceof ViewableData) { + if (is_object($value) && !($value instanceof ViewableData) && !is_iterable($value)) { return new ArrayData($value); } elseif (ArrayLib::is_associative($value)) { return new ArrayData($value); diff --git a/src/View/SSTemplateParser.php b/src/View/SSTemplateParser.php index 9ca62eaa54b..bafe80e4be4 100644 --- a/src/View/SSTemplateParser.php +++ b/src/View/SSTemplateParser.php @@ -1886,8 +1886,6 @@ function PresenceCheck_Argument(&$res, $sub) $res['php'] .= '((bool)'.$sub['php'].')'; } else { $php = ($sub['ArgumentMode'] == 'default' ? $sub['lookup_php'] : $sub['php']); - // TODO: kinda hacky - maybe we need a way to pass state down the parse chain so - // Lookup_LastLookupStep and Argument_BareWord can produce hasValue instead of XML_val $res['php'] .= str_replace('$$FINAL', 'hasValue', $php ?? ''); } } @@ -5292,8 +5290,6 @@ function Text__finalise(&$res) $text = stripslashes($text ?? ''); $text = addcslashes($text ?? '', '\'\\'); - // TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this - // non-dynamically calculated $code = <<<'EOC' (\SilverStripe\View\SSViewer::getRewriteHashLinksDefault() ? \SilverStripe\Core\Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) ) @@ -5332,8 +5328,7 @@ public function compileString($string, $templateName = "", $includeDebuggingComm $this->includeDebuggingComments = $includeDebuggingComments; - // Ignore UTF8 BOM at beginning of string. TODO: Confirm this is needed, make sure SSViewer handles UTF - // (and other encodings) properly + // Ignore UTF8 BOM at beginning of string. if (substr($string ?? '', 0, 3) == pack("CCC", 0xef, 0xbb, 0xbf)) { $this->pos = 3; } diff --git a/src/View/SSViewer_Scope.php b/src/View/SSViewer_Scope.php index 3b0fe1a5efe..658756b659c 100644 --- a/src/View/SSViewer_Scope.php +++ b/src/View/SSViewer_Scope.php @@ -5,6 +5,7 @@ use ArrayIterator; use Countable; use Iterator; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\FieldType\DBBoolean; use SilverStripe\ORM\FieldType\DBText; use SilverStripe\ORM\FieldType\DBFloat; @@ -130,6 +131,12 @@ public function getItem() if (is_scalar($item)) { $item = $this->convertScalarToDBField($item); } + + // Wrap list arrays in ViewableData so templates can handle them + if (is_array($item) && array_is_list($item)) { + $item = ArrayList::create($item); + } + return $item; } @@ -308,6 +315,8 @@ public function next() // Item may be an array or a regular IteratorAggregate if (is_array($this->item)) { $this->itemIterator = new ArrayIterator($this->item); + } elseif ($this->item instanceof Iterator) { + $this->itemIterator = $this->item; } else { $this->itemIterator = $this->item->getIterator(); diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index f5f19ec8c0f..e5a2c507c4f 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -20,6 +20,7 @@ use SilverStripe\Dev\Debug; use SilverStripe\Dev\Deprecation; use SilverStripe\ORM\ArrayLib; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\View\SSViewer; @@ -537,7 +538,7 @@ protected function objCacheClear() * @param array $arguments * @param bool $cache Cache this object * @param string $cacheName a custom cache name - * @return Object|DBField + * @return object|DBField */ public function obj($fieldName, $arguments = [], $cache = false, $cacheName = null) { @@ -558,6 +559,11 @@ public function obj($fieldName, $arguments = [], $cache = false, $cacheName = nu $value = $this->$fieldName; } + // Wrap list arrays in ViewableData so templates can handle them + if (is_array($value) && array_is_list($value)) { + $value = ArrayList::create($value); + } + // Cast object if (!is_object($value)) { // Force cast @@ -601,7 +607,10 @@ public function cachedCall($fieldName, $arguments = [], $identifier = null) public function hasValue($field, $arguments = [], $cache = true) { $result = $this->obj($field, $arguments, $cache); - return $result->exists(); + if ($result instanceof ViewableData) { + return $result->exists(); + } + return (bool) $result; } /** @@ -671,10 +680,8 @@ public function getViewerTemplates($suffix = '') /** * When rendering some objects it is necessary to iterate over the object being rendered, to do this, you need * access to itself. - * - * @return ViewableData */ - public function Me() + public function Me(): static { return $this; } diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index b80b1c80949..4a9aa0a42f8 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -1025,6 +1025,44 @@ public function testIfBlocks() ); } + public function provideIfBlockWithIterable(): array + { + $scenarios = [ + 'empty array' => [ + 'iterable' => [], + 'inScope' => false, + ], + 'array' => [ + 'iterable' => [1, 2, 3], + 'inScope' => false, + ], + 'ArrayList' => [ + 'iterable' => new ArrayList([['Val' => 1], ['Val' => 2], ['Val' => 3]]), + 'inScope' => false, + ], + ]; + foreach ($scenarios as $name => $scenario) { + $scenario['inScope'] = true; + $scenarios[$name . ' in scope'] = $scenario; + } + return $scenarios; + } + + /** + * @dataProvider provideIfBlockWithIterable + */ + public function testIfBlockWithIterable(iterable $iterable, bool $inScope): void + { + $expected = count($iterable) ? 'has value' : 'no value'; + $data = new ArrayData(['Iterable' => $iterable]); + if ($inScope) { + $template = '<% with $Iterable %><% if $Me %>has value<% else %>no value<% end_if %><% end_with %>'; + } else { + $template = '<% if $Iterable %>has value<% else %>no value<% end_if %>'; + } + $this->assertEqualIgnoringWhitespace($expected, $this->render($template, $data)); + } + public function testBaseTagGeneration() { // XHTML will have a closed base tag @@ -1340,6 +1378,88 @@ public function testCastingHelpers() ); } + public function provideLoop(): array + { + return [ + 'nested array and iterator' => [ + 'iterable' => [ + [ + 'value 1', + 'value 2', + ], + new ArrayList([ + 'value 3', + 'value 4', + ]), + ], + 'template' => '<% loop $Iterable %><% loop $Me %>$Me<% end_loop %><% end_loop %>', + 'expected' => 'value 1 value 2 value 3 value 4', + ], + 'nested associative arrays' => [ + 'iterable' => [ + [ + 'Foo' => 'one', + ], + [ + 'Foo' => 'two', + ], + [ + 'Foo' => 'three', + ], + ], + 'template' => '<% loop $Iterable %>$Foo<% end_loop %>', + 'expected' => 'one two three', + ], + ]; + } + + /** + * @dataProvider provideLoop + */ + public function testLoop(iterable $iterable, string $template, string $expected): void + { + $data = new ArrayData(['Iterable' => $iterable]); + $this->assertEqualIgnoringWhitespace($expected, $this->render($template, $data)); + } + + public function provideCountIterable(): array + { + $scenarios = [ + 'empty array' => [ + 'iterable' => [], + 'inScope' => false, + ], + 'array' => [ + 'iterable' => [1, 2, 3], + 'inScope' => false, + ], + 'ArrayList' => [ + 'iterable' => new ArrayList([['Val' => 1], ['Val' => 2], ['Val' => 3]]), + 'inScope' => false, + ], + ]; + foreach ($scenarios as $name => $scenario) { + $scenario['inScope'] = true; + $scenarios[$name . ' in scope'] = $scenario; + } + return $scenarios; + } + + /** + * @dataProvider provideCountIterable + */ + public function testCountIterable(iterable $iterable, bool $inScope): void + { + $expected = count($iterable); + $data = new ArrayData(['Iterable' => $iterable]); + if ($inScope) { + $template = '<% with $Iterable %>$Count<% end_with %>'; + } else { + $template = '$Iterable.Count'; + } + $this->assertEqualIgnoringWhitespace($expected, $this->render($template, $data)); + } + public function testSSViewerBasicIteratorSupport() { $data = new ArrayData( @@ -2239,4 +2359,11 @@ public function testPrimitivesConvertedToDBFields() $this->render('<% loop $Foo %>$Me<% end_loop %>', $data) ); } + + public function testMe(): void + { + $mockArrayData = $this->getMockBuilder(ArrayData::class)->addMethods(['forTemplate'])->getMock(); + $mockArrayData->expects($this->once())->method('forTemplate'); + $this->render('$Me', $mockArrayData); + } } diff --git a/tests/php/View/ViewableDataTest.php b/tests/php/View/ViewableDataTest.php index 15ff7425290..1610fdb63ba 100644 --- a/tests/php/View/ViewableDataTest.php +++ b/tests/php/View/ViewableDataTest.php @@ -5,6 +5,7 @@ use ReflectionMethod; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\ArrayList; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use SilverStripe\View\Tests\ViewableDataTest\ViewableDataTestExtension; @@ -278,4 +279,31 @@ public function testDynamicData() $this->assertSame($obj, $viewableData->getDynamicData('abc')); $this->assertSame($obj, $viewableData->abc); } + + public function provideWrapArrayInObj(): array + { + return [ + 'empty array' => [ + 'arr' => [], + 'expectedClass' => ArrayList::class, + ], + 'fully indexed array' => [ + 'arr' => [ + 'value1', + 'value2', + ], + 'expectedClass' => ArrayList::class, + ], + ]; + } + + /** + * @dataProvider provideWrapArrayInObj + */ + public function testWrapArrayInObj(array $arr, string $expectedClass): void + { + $viewableData = new ViewableData(); + $viewableData->arr = $arr; + $this->assertInstanceOf($expectedClass, $viewableData->obj('arr')); + } }