From 2bdcfb67435fcfbc590346ce57b4643190ec6855 Mon Sep 17 00:00:00 2001 From: SpacePossum Date: Fri, 18 Nov 2016 11:38:22 +0100 Subject: [PATCH] Optimize usage of Traversable/Iterator --- lib/Twig/Extension/Core.php | 40 ++++- test/Twig/Tests/Extension/CoreTest.php | 225 +++++++++++++++++++++++-- 2 files changed, 248 insertions(+), 17 deletions(-) diff --git a/lib/Twig/Extension/Core.php b/lib/Twig/Extension/Core.php index 0a05a0490..60a113060 100644 --- a/lib/Twig/Extension/Core.php +++ b/lib/Twig/Extension/Core.php @@ -652,7 +652,7 @@ function twig_array_merge($arr1, $arr2) function twig_slice(Twig_Environment $env, $item, $start, $length = null, $preserveKeys = false) { if ($item instanceof Traversable) { - if ($item instanceof IteratorAggregate) { + while ($item instanceof IteratorAggregate) { $item = $item->getIterator(); } @@ -821,7 +821,27 @@ function _twig_default_filter($value, $default = '') function twig_get_array_keys_filter($array) { if ($array instanceof Traversable) { - return array_keys(iterator_to_array($array)); + while ($array instanceof IteratorAggregate) { + $array = $array->getIterator(); + } + + if ($array instanceof Iterator) { + $keys = array(); + $array->rewind(); + while ($array->valid()) { + $keys[] = $array->key(); + $array->next(); + } + + return $keys; + } + + $keys = array(); + foreach ($array as $key => $item) { + $keys[] = $key; + } + + return $keys; } if (!is_array($array)) { @@ -901,7 +921,21 @@ function twig_in_filter($value, $compare) } elseif (is_string($compare) && (is_string($value) || is_int($value) || is_float($value))) { return '' === $value || false !== strpos($compare, (string) $value); } elseif ($compare instanceof Traversable) { - return in_array($value, iterator_to_array($compare, false), is_object($value) || is_resource($value)); + if (is_object($value) || is_resource($value)) { + foreach ($compare as $item) { + if ($item === $value) { + return true; + } + } + } else { + foreach ($compare as $item) { + if ($item == $value) { + return true; + } + } + } + + return false; } return false; diff --git a/test/Twig/Tests/Extension/CoreTest.php b/test/Twig/Tests/Extension/CoreTest.php index 89937b785..70312c0b9 100644 --- a/test/Twig/Tests/Extension/CoreTest.php +++ b/test/Twig/Tests/Extension/CoreTest.php @@ -115,14 +115,24 @@ class Twig_Tests_Extension_CoreTest extends PHPUnit_Framework_TestCase $this->assertEquals($output, 'éÄ'); } - public function testCustomEscaper() + /** + * @dataProvider provideCustomEscaperCases + */ + public function testCustomEscaper($expected, $string, $strategy) { $twig = new Twig_Environment($this->getMockBuilder('Twig_LoaderInterface')->getMock()); $twig->getExtension('Twig_Extension_Core')->setEscaper('foo', 'foo_escaper_for_test'); - $this->assertEquals('fooUTF-8', twig_escape_filter($twig, 'foo', 'foo')); - $this->assertEquals('UTF-8', twig_escape_filter($twig, null, 'foo')); - $this->assertEquals('42UTF-8', twig_escape_filter($twig, 42, 'foo')); + $this->assertSame($expected, twig_escape_filter($twig, $string, $strategy)); + } + + public function provideCustomEscaperCases() + { + return array( + array('fooUTF-8', 'foo', 'foo'), + array('UTF-8', null, 'foo'), + array('42UTF-8', 42, 'foo'), + ); } /** @@ -133,22 +143,129 @@ class Twig_Tests_Extension_CoreTest extends PHPUnit_Framework_TestCase twig_escape_filter(new Twig_Environment($this->getMockBuilder('Twig_LoaderInterface')->getMock()), 'foo', 'bar'); } - public function testTwigFirst() + /** + * @dataProvider provideTwigFirstCases + */ + public function testTwigFirst($expected, $input) { $twig = new Twig_Environment($this->getMockBuilder('Twig_LoaderInterface')->getMock()); - $this->assertEquals('a', twig_first($twig, 'abc')); - $this->assertEquals(1, twig_first($twig, array(1, 2, 3))); - $this->assertSame('', twig_first($twig, null)); - $this->assertSame('', twig_first($twig, '')); + $this->assertSame($expected, twig_first($twig, $input)); } - public function testTwigLast() + public function provideTwigFirstCases() + { + $i = array(1 => 'a', 2 => 'b', 3 => 'c'); + + return array( + array('a', 'abc'), + array(1, array(1, 2, 3)), + array('', null), + array('', ''), + array('a', new CoreTestIterator($i, array_keys($i), true, 3)), + ); + } + + /** + * @dataProvider provideTwigLastCases + */ + public function testTwigLast($expected, $input) { $twig = new Twig_Environment($this->getMockBuilder('Twig_LoaderInterface')->getMock()); - $this->assertEquals('c', twig_last($twig, 'abc')); - $this->assertEquals(3, twig_last($twig, array(1, 2, 3))); - $this->assertSame('', twig_last($twig, null)); - $this->assertSame('', twig_last($twig, '')); + $this->assertSame($expected, twig_last($twig, $input)); + } + + public function provideTwigLastCases() + { + $i = array(1 => 'a', 2 => 'b', 3 => 'c'); + + return array( + array('c', 'abc'), + array(3, array(1, 2, 3)), + array('', null), + array('', ''), + array('c', new CoreTestIterator($i, array_keys($i), true)), + ); + } + + /** + * @dataProvider provideArrayKeyCases + */ + public function testArrayKeysFilter(array $expected, $input) + { + $this->assertSame($expected, twig_get_array_keys_filter($input)); + } + + public function provideArrayKeyCases() + { + $array = array('a' => 'a1', 'b' => 'b1', 'c' => 'c1'); + $keys = array_keys($array); + + return array( + array($keys, $array), + array($keys, new CoreTestIterator($array, $keys)), + array($keys, new CoreTestIteratorAggregate($array, $keys)), + array($keys, new CoreTestIteratorAggregateAggregate($array, $keys)), + array(array(), null), + array(array('a'), new SimpleXMLElement('')), + ); + } + + /** + * @dataProvider provideInFilterCases + */ + public function testInFilter($expected, $value, $compare) + { + $this->assertSame($expected, twig_in_filter($value, $compare)); + } + + public function provideInFilterCases() + { + $array = array(1, 2, 'a' => 3, 5, 6, 7); + $keys = array_keys($array); + + return array( + array(true, 1, $array), + array(true, '3', $array), + array(true, '3', 'abc3def'), + array(true, 1, new CoreTestIterator($array, $keys, true, 1)), + array(true, '3', new CoreTestIterator($array, $keys, true, 3)), + array(true, '3', new CoreTestIteratorAggregateAggregate($array, $keys, true, 3)), + array(false, 4, $array), + array(false, 4, new CoreTestIterator($array, $keys, true)), + array(false, 4, new CoreTestIteratorAggregateAggregate($array, $keys, true)), + array(false, 1, 1), + array(true, 'b', new SimpleXMLElement('b')), + ); + } + + /** + * @dataProvider provideSliceFilterCases + */ + public function testSliceFilter($expected, $input, $start, $length = null, $preserveKeys = false) + { + $twig = new Twig_Environment($this->getMockBuilder('Twig_LoaderInterface')->getMock()); + $this->assertSame($expected, twig_slice($twig, $input, $start, $length, $preserveKeys)); + } + + public function provideSliceFilterCases() + { + $i = array('a' => 1, 'b' => 2, 'c' => 3, 'd' => 4); + $keys = array_keys($i); + + return array( + array(array('a' => 1), $i, 0, 1, true), + array(array('a' => 1), $i, 0, 1, false), + array(array('b' => 2, 'c' => 3), $i, 1, 2), + array(array(1), array(1, 2, 3, 4), 0, 1), + array(array(2, 3), array(1, 2, 3, 4), 1, 2), + array(array(2, 3), new CoreTestIterator($i, $keys, true), 1, 2), + array(array('c' => 3, 'd' => 4), new CoreTestIteratorAggregate($i, $keys, true), 2, null, true), + array($i, new CoreTestIterator($i, $keys, true), 0, count($keys) + 10, true), + array(array(), new CoreTestIterator($i, $keys, true), count($keys) + 10), + array('de', 'abcdef', 3, 2), + array(array(), new SimpleXMLElement('12'), 3), + array(array(), new ArrayIterator(array(1, 2)), 3) + ); } } @@ -156,3 +273,83 @@ function foo_escaper_for_test(Twig_Environment $env, $string, $charset) { return $string.$charset; } + +final class CoreTestIteratorAggregate implements IteratorAggregate +{ + private $iterator; + + public function __construct(array $array, array $keys, $allowAccess = false, $maxPosition = false) + { + $this->iterator = new CoreTestIterator($array, $keys, $allowAccess, $maxPosition); + } + + public function getIterator() + { + return $this->iterator; + } +} + +final class CoreTestIteratorAggregateAggregate implements IteratorAggregate +{ + private $iterator; + + public function __construct(array $array, array $keys, $allowValueAccess = false, $maxPosition = false) + { + $this->iterator = new CoreTestIteratorAggregate($array, $keys, $allowValueAccess, $maxPosition); + } + + public function getIterator() + { + return $this->iterator; + } +} + +final class CoreTestIterator implements Iterator +{ + private $position; + private $array; + private $arrayKeys; + private $allowValueAccess; + private $maxPosition; + + public function __construct(array $values, array $keys, $allowValueAccess = false, $maxPosition = false) + { + $this->array = $values; + $this->arrayKeys = $keys; + $this->position = 0; + $this->allowValueAccess = $allowValueAccess; + $this->maxPosition = false === $maxPosition ? count($values) + 1 : $maxPosition; + } + + public function rewind() + { + $this->position = 0; + } + + public function current() + { + if ($this->allowValueAccess) { + return $this->array[$this->key()]; + } + + throw new \LogicException('Code should only use the keys, not the values provided by iterator.'); + } + + public function key() + { + return $this->arrayKeys[$this->position]; + } + + public function next() + { + ++$this->position; + if ($this->position === $this->maxPosition) { + throw new \LogicException(sprintf('Code should not iterate beyond %d.', $this->maxPosition)); + } + } + + public function valid() + { + return isset($this->arrayKeys[$this->position]); + } +}