From 4e11c0710b601c1f6808aeaf80fd509c96b16aa6 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Wed, 18 Feb 2015 21:52:22 +0100 Subject: [PATCH 1/2] [PropertyAccess] refactor type checks to remove duplicate logic and thus improve performance --- .../PropertyAccess/PropertyAccessor.php | 17 ++--- .../Tests/PropertyAccessorTest.php | 64 +++++++------------ 2 files changed, 31 insertions(+), 50 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 92caf1e826..a71789f7e7 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -75,10 +75,6 @@ class PropertyAccessor implements PropertyAccessorInterface $objectOrArray = & $propertyValues[$i][self::VALUE]; if ($overwrite) { - if (!is_object($objectOrArray) && !is_array($objectOrArray)) { - throw new UnexpectedTypeException($objectOrArray, 'object or array'); - } - $property = $propertyPath->getElement($i); //$singular = $propertyPath->singulars[$i]; $singular = null; @@ -108,13 +104,13 @@ class PropertyAccessor implements PropertyAccessorInterface */ private function &readPropertiesUntil(&$objectOrArray, PropertyPathInterface $propertyPath, $lastIndex) { + if (!is_object($objectOrArray) && !is_array($objectOrArray)) { + throw new UnexpectedTypeException($objectOrArray, 'object or array'); + } + $propertyValues = array(); for ($i = 0; $i < $lastIndex; ++$i) { - if (!is_object($objectOrArray) && !is_array($objectOrArray)) { - throw new UnexpectedTypeException($objectOrArray, 'object or array'); - } - $property = $propertyPath->getElement($i); $isIndex = $propertyPath->isIndex($i); @@ -137,6 +133,11 @@ class PropertyAccessor implements PropertyAccessorInterface $objectOrArray = & $propertyValue[self::VALUE]; + // the final value of the path must not be validated + if ($i + 1 < $propertyPath->getLength() && !is_object($objectOrArray) && !is_array($objectOrArray)) { + throw new UnexpectedTypeException($objectOrArray, 'object or array'); + } + $propertyValues[] = & $propertyValue; } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index ca49be6012..fec4e14b09 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -29,6 +29,20 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->propertyAccessor = new PropertyAccessor(); } + public function getPathsWithUnexpectedType() + { + return array( + array('', 'foobar'), + array('foo', 'foobar'), + array(null, 'foobar'), + array(123, 'foobar'), + array((object) array('prop' => null), 'prop.foobar'), + array((object) array('prop' => (object) array('subProp' => null)), 'prop.subProp.foobar'), + array(array('index' => null), '[index][foobar]'), + array(array('index' => array('subIndex' => null)), '[index][subIndex][foobar]'), + ); + } + public function testGetValueReadsArray() { $array = array('firstName' => 'Bernhard'); @@ -198,27 +212,13 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase } /** + * @dataProvider getPathsWithUnexpectedType * @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException + * @expectedExceptionMessage Expected argument of type "object or array" */ - public function testGetValueThrowsExceptionIfNotObjectOrArray() + public function testGetValueThrowsExceptionIfNotObjectOrArray($objectOrArray, $path) { - $this->propertyAccessor->getValue('baz', 'foobar'); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException - */ - public function testGetValueThrowsExceptionIfNull() - { - $this->propertyAccessor->getValue(null, 'foobar'); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException - */ - public function testGetValueThrowsExceptionIfEmpty() - { - $this->propertyAccessor->getValue('', 'foobar'); + $this->propertyAccessor->getValue($objectOrArray, $path); } public function testGetValueWhenArrayValueIsNull() @@ -311,33 +311,13 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase } /** + * @dataProvider getPathsWithUnexpectedType * @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException + * @expectedExceptionMessage Expected argument of type "object or array" */ - public function testSetValueThrowsExceptionIfNotObjectOrArray() + public function testSetValueThrowsExceptionIfNotObjectOrArray($objectOrArray, $path) { - $value = 'baz'; - - $this->propertyAccessor->setValue($value, 'foobar', 'bam'); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException - */ - public function testSetValueThrowsExceptionIfNull() - { - $value = null; - - $this->propertyAccessor->setValue($value, 'foobar', 'bam'); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException - */ - public function testSetValueThrowsExceptionIfEmpty() - { - $value = ''; - - $this->propertyAccessor->setValue($value, 'foobar', 'bam'); + $this->propertyAccessor->setValue($objectOrArray, $path, 'value'); } /** From 9cacecbf790b563a60e464c622b4364b7e8af9c7 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Thu, 19 Feb 2015 13:40:39 +0100 Subject: [PATCH 2/2] [PropertyAccess] the property path constructor already implements the type check --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index a71789f7e7..2aeb1a8a6e 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -40,10 +40,8 @@ class PropertyAccessor implements PropertyAccessorInterface */ public function getValue($objectOrArray, $propertyPath) { - if (is_string($propertyPath)) { + if (!$propertyPath instanceof PropertyPathInterface) { $propertyPath = new PropertyPath($propertyPath); - } elseif (!$propertyPath instanceof PropertyPathInterface) { - throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); } $propertyValues = & $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength()); @@ -56,10 +54,8 @@ class PropertyAccessor implements PropertyAccessorInterface */ public function setValue(&$objectOrArray, $propertyPath, $value) { - if (is_string($propertyPath)) { + if (!$propertyPath instanceof PropertyPathInterface) { $propertyPath = new PropertyPath($propertyPath); - } elseif (!$propertyPath instanceof PropertyPathInterface) { - throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); } $propertyValues = & $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1);