From 0488389d95033a60b3de14340c5a532c6a6af18f Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Mar 2014 15:50:34 +0100 Subject: [PATCH 1/6] [PropertyAccess] Refactored PropertyAccessorTest --- .../PropertyAccess/PropertyAccessor.php | 11 +- .../PropertyAccess/Tests/Fixtures/Author.php | 71 --- .../Tests/Fixtures/Magician.php | 27 - .../Tests/Fixtures/MagicianCall.php | 28 -- .../Tests/Fixtures/TestClass.php | 115 +++++ .../Tests/Fixtures/TestClassMagicCall.php | 39 ++ .../Tests/Fixtures/TestClassMagicGet.php | 42 ++ .../Tests/PropertyAccessorTest.php | 467 ++++++------------ 8 files changed, 364 insertions(+), 436 deletions(-) delete mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/Author.php delete mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/Magician.php delete mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/MagicianCall.php create mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php create mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicCall.php create mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index d48891ef27..f5183a109e 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -116,7 +116,7 @@ class PropertyAccessor implements PropertyAccessorInterface * * @throws UnexpectedTypeException If a value within the path is neither object nor array. */ - private function &readPropertiesUntil(&$objectOrArray, PropertyPathInterface $propertyPath, $lastIndex, $throwExceptionOnNonexistantIndex = false) + private function &readPropertiesUntil(&$objectOrArray, PropertyPathInterface $propertyPath, $lastIndex, $throwExceptionOnInvalidIndex = false) { $propertyValues = array(); @@ -131,9 +131,10 @@ class PropertyAccessor implements PropertyAccessorInterface // Create missing nested arrays on demand if ($isIndex && $isArrayAccess && !isset($objectOrArray[$property])) { - if ($throwExceptionOnNonexistantIndex) { + if ($throwExceptionOnInvalidIndex) { throw new NoSuchIndexException(sprintf('Cannot read property "%s". Available properties are "%s"', $property, print_r(array_keys($objectOrArray), true))); } + $objectOrArray[$property] = $i + 1 < $propertyPath->getLength() ? array() : null; } @@ -412,8 +413,8 @@ class PropertyAccessor implements PropertyAccessorInterface $addMethod = 'add'.$singular; $removeMethod = 'remove'.$singular; - $addMethodFound = $this->isAccessible($reflClass, $addMethod, 1); - $removeMethodFound = $this->isAccessible($reflClass, $removeMethod, 1); + $addMethodFound = $this->isMethodAccessible($reflClass, $addMethod, 1); + $removeMethodFound = $this->isMethodAccessible($reflClass, $removeMethod, 1); if ($addMethodFound && $removeMethodFound) { return array($addMethod, $removeMethod); @@ -442,7 +443,7 @@ class PropertyAccessor implements PropertyAccessorInterface * @return Boolean Whether the method is public and has $parameters * required parameters */ - private function isAccessible(\ReflectionClass $class, $methodName, $parameters) + private function isMethodAccessible(\ReflectionClass $class, $methodName, $parameters) { if ($class->hasMethod($methodName)) { $method = $class->getMethod($methodName); diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/Author.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/Author.php deleted file mode 100644 index ed2331bab0..0000000000 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/Author.php +++ /dev/null @@ -1,71 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\PropertyAccess\Tests\Fixtures; - -class Author -{ - public $firstName; - private $lastName; - private $australian; - public $child; - private $readPermissions; - - private $privateProperty; - - public function setLastName($lastName) - { - $this->lastName = $lastName; - } - - public function getLastName() - { - return $this->lastName; - } - - private function getPrivateGetter() - { - return 'foobar'; - } - - public function setAustralian($australian) - { - $this->australian = $australian; - } - - public function isAustralian() - { - return $this->australian; - } - - public function setReadPermissions($bool) - { - $this->readPermissions = $bool; - } - - public function hasReadPermissions() - { - return $this->readPermissions; - } - - private function isPrivateIsser() - { - return true; - } - - public function getPrivateSetter() - { - } - - private function setPrivateSetter($data) - { - } -} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/Magician.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/Magician.php deleted file mode 100644 index 6faa5dbf7b..0000000000 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/Magician.php +++ /dev/null @@ -1,27 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\PropertyAccess\Tests\Fixtures; - -class Magician -{ - private $foobar; - - public function __set($property, $value) - { - $this->$property = $value; - } - - public function __get($property) - { - return isset($this->$property) ? $this->$property : null; - } -} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/MagicianCall.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/MagicianCall.php deleted file mode 100644 index 0508a71da1..0000000000 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/MagicianCall.php +++ /dev/null @@ -1,28 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\PropertyAccess\Tests\Fixtures; - -class MagicianCall -{ - private $foobar; - - public function __call($name, $args) - { - $property = lcfirst(substr($name, 3)); - if ('get' === substr($name, 0, 3)) { - return isset($this->$property) ? $this->$property : null; - } elseif ('set' === substr($name, 0, 3)) { - $value = 1 == count($args) ? $args[0] : null; - $this->$property = $value; - } - } -} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php new file mode 100644 index 0000000000..178d7f618a --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php @@ -0,0 +1,115 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests\Fixtures; + +class TestClass +{ + public $publicProperty; + protected $protectedProperty; + private $privateProperty; + + private $publicAccessor; + private $publicIsAccessor; + private $publicHasAccessor; + + public function __construct($value) + { + $this->publicProperty = $value; + $this->publicAccessor = $value; + $this->publicIsAccessor = $value; + $this->publicHasAccessor = $value; + } + + public function setPublicAccessor($value) + { + $this->publicAccessor = $value; + } + + public function getPublicAccessor() + { + return $this->publicAccessor; + } + + public function setPublicIsAccessor($value) + { + $this->publicIsAccessor = $value; + } + + public function isPublicIsAccessor() + { + return $this->publicIsAccessor; + } + + public function setPublicHasAccessor($value) + { + $this->publicHasAccessor = $value; + } + + public function hasPublicHasAccessor() + { + return $this->publicHasAccessor; + } + + protected function setProtectedAccessor($value) + { + } + + protected function getProtectedAccessor() + { + return 'foobar'; + } + + protected function setProtectedIsAccessor($value) + { + } + + protected function isProtectedIsAccessor() + { + return 'foobar'; + } + + protected function setProtectedHasAccessor($value) + { + } + + protected function hasProtectedHasAccessor() + { + return 'foobar'; + } + + private function setPrivateAccessor($value) + { + } + + private function getPrivateAccessor() + { + return 'foobar'; + } + + private function setPrivateIsAccessor($value) + { + } + + private function isPrivateIsAccessor() + { + return 'foobar'; + } + + private function setPrivateHasAccessor($value) + { + } + + private function hasPrivateHasAccessor() + { + return 'foobar'; + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicCall.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicCall.php new file mode 100644 index 0000000000..d49967abd1 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicCall.php @@ -0,0 +1,39 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests\Fixtures; + +class TestClassMagicCall +{ + private $magicCallProperty; + + public function __construct($value) + { + $this->magicCallProperty = $value; + } + + public function __call($method, array $args) + { + if ('getMagicCallProperty' === $method) { + return $this->magicCallProperty; + } + + if ('getConstantMagicCallProperty' === $method) { + return 'constant value'; + } + + if ('setMagicCallProperty' === $method) { + $this->magicCallProperty = reset($args); + } + + return null; + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php new file mode 100644 index 0000000000..fee8318966 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests\Fixtures; + +class TestClassMagicGet +{ + private $magicProperty; + + public function __construct($value) + { + $this->magicProperty = $value; + } + + public function __set($property, $value) + { + if ('magicProperty' === $property) { + $this->magicProperty = $value; + } + } + + public function __get($property) + { + if ('magicProperty' === $property) { + return $this->magicProperty; + } + + if ('constantMagicProperty' === $property) { + return 'constant value'; + } + + return null; + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 72fbfe428f..3c13e50bb2 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -12,228 +12,157 @@ namespace Symfony\Component\PropertyAccess\Tests; use Symfony\Component\PropertyAccess\PropertyAccessor; -use Symfony\Component\PropertyAccess\Tests\Fixtures\Author; +use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClass; use Symfony\Component\PropertyAccess\Tests\Fixtures\Magician; -use Symfony\Component\PropertyAccess\Tests\Fixtures\MagicianCall; -use Symfony\Component\PropertyAccess\PropertyAccessorBuilder; +use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicCall; +use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicGet; class PropertyAccessorTest extends \PHPUnit_Framework_TestCase { /** - * @var PropertyAccessorBuilder + * @var PropertyAccessor */ - private $propertyAccessorBuilder; + private $propertyAccessor; protected function setUp() { - $this->propertyAccessorBuilder = new PropertyAccessorBuilder(); + $this->propertyAccessor = new PropertyAccessor(); + } + + public function getValidPropertyPaths() + { + + return array( + array(array('Bernhard', 'Schussek'), '[0]', 'Bernhard'), + array(array('Bernhard', 'Schussek'), '[1]', 'Schussek'), + array(array('firstName' => 'Bernhard'), '[firstName]', 'Bernhard'), + array(array('index' => array('firstName' => 'Bernhard')), '[index][firstName]', 'Bernhard'), + array((object) array('firstName' => 'Bernhard'), 'firstName', 'Bernhard'), + array((object) array('property' => array('firstName' => 'Bernhard')), 'property[firstName]', 'Bernhard'), + array(array('index' => (object) array('firstName' => 'Bernhard')), '[index].firstName', 'Bernhard'), + array((object) array('property' => (object) array('firstName' => 'Bernhard')), 'property.firstName', 'Bernhard'), + + // Accessor methods + array(new TestClass('Bernhard'), 'publicProperty', 'Bernhard'), + array(new TestClass('Bernhard'), 'publicAccessor', 'Bernhard'), + array(new TestClass('Bernhard'), 'publicIsAccessor', 'Bernhard'), + array(new TestClass('Bernhard'), 'publicHasAccessor', 'Bernhard'), + + // Methods are camelized + array(new TestClass('Bernhard'), 'public_accessor', 'Bernhard'), + + // Missing indices + array(array('index' => array()), '[index][firstName]', null), + array(array('root' => array('index' => array())), '[root][index][firstName]', null), + + // Special chars + array(array('%!@$§.' => 'Bernhard'), '[%!@$§.]', 'Bernhard'), + array(array('index' => array('%!@$§.' => 'Bernhard')), '[index][%!@$§.]', 'Bernhard'), + array((object) array('%!@$§' => 'Bernhard'), '%!@$§', 'Bernhard'), + array((object) array('property' => (object) array('%!@$§' => 'Bernhard')), 'property.%!@$§', 'Bernhard'), + ); + } + + public function getPathsWithMissingProperty() + { + + return array( + array((object) array('firstName' => 'Bernhard'), 'lastName'), + array((object) array('property' => (object) array('firstName' => 'Bernhard')), 'property.lastName'), + array(array('index' => (object) array('firstName' => 'Bernhard')), '[index].lastName'), + array(new TestClass('Bernhard'), 'protectedProperty'), + array(new TestClass('Bernhard'), 'privateProperty'), + array(new TestClass('Bernhard'), 'protectedAccessor'), + array(new TestClass('Bernhard'), 'protectedIsAccessor'), + array(new TestClass('Bernhard'), 'protectedHasAccessor'), + array(new TestClass('Bernhard'), 'privateAccessor'), + array(new TestClass('Bernhard'), 'privateIsAccessor'), + array(new TestClass('Bernhard'), 'privateHasAccessor'), + + // Properties are not camelized + array(new TestClass('Bernhard'), 'public_property'), + ); + } + + public function getPathsWithMissingIndex() + { + + return array( + array(array('firstName' => 'Bernhard'), '[lastName]'), + array(array(), '[index][lastName]'), + array(array('index' => array()), '[index][lastName]'), + array(array('index' => array('firstName' => 'Bernhard')), '[index][lastName]'), + array((object) array('property' => array('firstName' => 'Bernhard')), 'property[lastName]'), + ); } /** - * Get PropertyAccessor configured - * - * @param string $withMagicCall - * @param string $throwExceptionOnInvalidIndex - * @return PropertyAccessorInterface + * @dataProvider getValidPropertyPaths */ - protected function getPropertyAccessor($withMagicCall = false, $throwExceptionOnInvalidIndex = false) + public function testGetValue($objectOrArray, $path, $value) { - if ($withMagicCall) { - $this->propertyAccessorBuilder->enableMagicCall(); - } else { - $this->propertyAccessorBuilder->disableMagicCall(); - } - - if ($throwExceptionOnInvalidIndex) { - $this->propertyAccessorBuilder->enableExceptionOnInvalidIndex(); - } else { - $this->propertyAccessorBuilder->disableExceptionOnInvalidIndex(); - } - - return $this->propertyAccessorBuilder->getPropertyAccessor(); - } - - public function testGetValueReadsArray() - { - $array = array('firstName' => 'Bernhard'); - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($array, '[firstName]')); + $this->assertSame($value, $this->propertyAccessor->getValue($objectOrArray, $path)); } /** + * @dataProvider getPathsWithMissingProperty * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException */ - public function testGetValueThrowsExceptionIfIndexNotationExpected() + public function testGetValueThrowsExceptionIfPropertyNotFound($objectOrArray, $path) { - $array = array('firstName' => 'Bernhard'); - - $this->getPropertyAccessor()->getValue($array, 'firstName'); - } - - public function testGetValueReadsZeroIndex() - { - $array = array('Bernhard'); - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($array, '[0]')); - } - - public function testGetValueReadsIndexWithSpecialChars() - { - $array = array('%!@$§.' => 'Bernhard'); - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($array, '[%!@$§.]')); - } - - public function testGetValueReadsNestedIndexWithSpecialChars() - { - $array = array('root' => array('%!@$§.' => 'Bernhard')); - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($array, '[root][%!@$§.]')); - } - - public function testGetValueReadsArrayWithCustomPropertyPath() - { - $array = array('child' => array('index' => array('firstName' => 'Bernhard'))); - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($array, '[child][index][firstName]')); - } - - public function testGetValueReadsArrayWithMissingIndexForCustomPropertyPath() - { - $array = array('child' => array('index' => array())); - - // No BC break - $this->assertNull($this->getPropertyAccessor()->getValue($array, '[child][index][firstName]')); - - try { - $this->getPropertyAccessor(false, true)->getValue($array, '[child][index][firstName]'); - $this->fail('Getting value on a nonexistent path from array should throw a Symfony\Component\PropertyAccess\Exception\NoSuchIndexException exception'); - } catch (\Exception $e) { - $this->assertInstanceof('Symfony\Component\PropertyAccess\Exception\NoSuchIndexException', $e, 'Getting value on a nonexistent path from array should throw a Symfony\Component\PropertyAccess\Exception\NoSuchIndexException exception'); - } - } - - public function testGetValueReadsProperty() - { - $object = new Author(); - $object->firstName = 'Bernhard'; - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($object, 'firstName')); - } - - public function testGetValueIgnoresSingular() - { - $this->markTestSkipped('This feature is temporarily disabled as of 2.1'); - - $object = (object) array('children' => 'Many'); - - $this->assertEquals('Many', $this->getPropertyAccessor()->getValue($object, 'children|child')); - } - - public function testGetValueReadsPropertyWithSpecialCharsExceptDot() - { - $array = (object) array('%!@$§' => 'Bernhard'); - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($array, '%!@$§')); - } - - public function testGetValueReadsPropertyWithSpecialCharsExceptDotNested() - { - $object = (object) array('nested' => (object) array('@child' => 'foo')); - - $this->assertEquals('foo', $this->getPropertyAccessor()->getValue($object, 'nested.@child')); - } - - public function testGetValueReadsPropertyWithCustomPropertyPath() - { - $object = new Author(); - $object->child = array(); - $object->child['index'] = new Author(); - $object->child['index']->firstName = 'Bernhard'; - - $this->assertEquals('Bernhard', $this->getPropertyAccessor()->getValue($object, 'child[index].firstName')); + $this->propertyAccessor->getValue($objectOrArray, $path); } /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @dataProvider getPathsWithMissingIndex */ - public function testGetValueThrowsExceptionIfPropertyIsNotPublic() + public function testGetValueThrowsNoExceptionIfIndexNotFound($objectOrArray, $path) { - $this->getPropertyAccessor()->getValue(new Author(), 'privateProperty'); - } - - public function testGetValueReadsGetters() - { - $object = new Author(); - $object->setLastName('Schussek'); - - $this->assertEquals('Schussek', $this->getPropertyAccessor()->getValue($object, 'lastName')); - } - - public function testGetValueCamelizesGetterNames() - { - $object = new Author(); - $object->setLastName('Schussek'); - - $this->assertEquals('Schussek', $this->getPropertyAccessor()->getValue($object, 'last_name')); + $this->assertNull($this->propertyAccessor->getValue($objectOrArray, $path)); } /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @dataProvider getPathsWithMissingIndex + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchIndexException */ - public function testGetValueThrowsExceptionIfGetterIsNotPublic() + public function testGetValueThrowsExceptionIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) { - $this->getPropertyAccessor()->getValue(new Author(), 'privateGetter'); - } - - public function testGetValueReadsIssers() - { - $object = new Author(); - $object->setAustralian(false); - - $this->assertFalse($this->getPropertyAccessor()->getValue($object, 'australian')); - } - - public function testGetValueReadHassers() - { - $object = new Author(); - $object->setReadPermissions(true); - - $this->assertTrue($this->getPropertyAccessor()->getValue($object, 'read_permissions')); + $this->propertyAccessor = new PropertyAccessor(false, true); + $this->propertyAccessor->getValue($objectOrArray, $path); } public function testGetValueReadsMagicGet() { - $object = new Magician(); - $object->__set('magicProperty', 'foobar'); - - $this->assertSame('foobar', $this->getPropertyAccessor()->getValue($object, 'magicProperty')); + $this->assertSame('Bernhard', $this->propertyAccessor->getValue(new TestClassMagicGet('Bernhard'), 'magicProperty')); } - /* - * https://github.com/symfony/symfony/pull/4450 - */ + // https://github.com/symfony/symfony/pull/4450 public function testGetValueReadsMagicGetThatReturnsConstant() { - $object = new Magician(); - - $this->assertNull($this->getPropertyAccessor()->getValue($object, 'magicProperty')); + $this->assertSame('constant value', $this->propertyAccessor->getValue(new TestClassMagicGet('Bernhard'), 'constantMagicProperty')); } /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException */ - public function testGetValueThrowsExceptionIfIsserIsNotPublic() + public function testGetValueDoesNotReadMagicCallByDefault() { - $this->getPropertyAccessor()->getValue(new Author(), 'privateIsser'); + $this->propertyAccessor->getValue(new TestClassMagicCall('Bernhard'), 'magicCallProperty'); } - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - */ - public function testGetValueThrowsExceptionIfPropertyDoesNotExist() + public function testGetValueReadsMagicCallIfEnabled() { - $this->getPropertyAccessor()->getValue(new Author(), 'foobar'); + $this->propertyAccessor = new PropertyAccessor(true); + + $this->assertSame('Bernhard', $this->propertyAccessor->getValue(new TestClassMagicCall('Bernhard'), 'magicCallProperty')); + } + + // https://github.com/symfony/symfony/pull/4450 + public function testGetValueReadsMagicCallThatReturnsConstant() + { + $this->propertyAccessor = new PropertyAccessor(true); + + $this->assertSame('constant value', $this->propertyAccessor->getValue(new TestClassMagicCall('Bernhard'), 'constantMagicCallProperty')); } /** @@ -241,7 +170,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase */ public function testGetValueThrowsExceptionIfNotObjectOrArray() { - $this->getPropertyAccessor()->getValue('baz', 'foobar'); + $this->propertyAccessor->getValue('baz', 'foobar'); } /** @@ -249,7 +178,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase */ public function testGetValueThrowsExceptionIfNull() { - $this->getPropertyAccessor()->getValue(null, 'foobar'); + $this->propertyAccessor->getValue(null, 'foobar'); } /** @@ -257,101 +186,77 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase */ public function testGetValueThrowsExceptionIfEmpty() { - $this->getPropertyAccessor()->getValue('', 'foobar'); + $this->propertyAccessor->getValue('', 'foobar'); } - public function testSetValueUpdatesArrays() + /** + * @dataProvider getValidPropertyPaths + */ + public function testSetValue($objectOrArray, $path) { - $array = array(); + $this->propertyAccessor->setValue($objectOrArray, $path, 'Updated'); - $this->getPropertyAccessor()->setValue($array, '[firstName]', 'Bernhard'); + $this->assertSame('Updated', $this->propertyAccessor->getValue($objectOrArray, $path)); + } - $this->assertEquals(array('firstName' => 'Bernhard'), $array); + /** + * @dataProvider getPathsWithMissingProperty + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + */ + public function testSetValueThrowsExceptionIfPropertyNotFound($objectOrArray, $path) + { + $this->propertyAccessor->setValue($objectOrArray, $path, 'Updated'); + } + + /** + * @dataProvider getPathsWithMissingIndex + */ + public function testSetValueThrowsNoExceptionIfIndexNotFound($objectOrArray, $path) + { + $this->propertyAccessor->setValue($objectOrArray, $path, 'Updated'); + + $this->assertSame('Updated', $this->propertyAccessor->getValue($objectOrArray, $path)); + } + + /** + * @dataProvider getPathsWithMissingIndex + */ + public function testSetValueThrowsNoExceptionIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) + { + $this->propertyAccessor = new PropertyAccessor(false, true); + $this->propertyAccessor->setValue($objectOrArray, $path, 'Updated'); + + $this->assertSame('Updated', $this->propertyAccessor->getValue($objectOrArray, $path)); + } + + public function testSetValueUpdatesMagicSet() + { + $author = new TestClassMagicGet('Bernhard'); + + $this->propertyAccessor->setValue($author, 'magicProperty', 'Updated'); + + $this->assertEquals('Updated', $author->__get('magicProperty')); } /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException */ - public function testSetValueThrowsExceptionIfIndexNotationExpected() + public function testSetValueDoesNotUpdateMagicCallByDefault() { - $array = array(); + $author = new TestClassMagicCall('Bernhard'); - $this->getPropertyAccessor()->setValue($array, 'firstName', 'Bernhard'); + $this->propertyAccessor->setValue($author, 'magicCallProperty', 'Updated'); } - public function testSetValueUpdatesArraysWithCustomPropertyPath() + public function testSetValueUpdatesMagicCallIfEnabled() { - $array = array(); + $this->propertyAccessor = new PropertyAccessor(true); - $this->getPropertyAccessor()->setValue($array, '[child][index][firstName]', 'Bernhard'); + $author = new TestClassMagicCall('Bernhard'); - $this->assertEquals(array('child' => array('index' => array('firstName' => 'Bernhard'))), $array); - } + $this->propertyAccessor->setValue($author, 'magicCallProperty', 'Updated'); - public function testSetValueUpdatesProperties() - { - $object = new Author(); - - $this->getPropertyAccessor()->setValue($object, 'firstName', 'Bernhard'); - - $this->assertEquals('Bernhard', $object->firstName); - } - - public function testSetValueUpdatesPropertiesWithCustomPropertyPath() - { - $object = new Author(); - $object->child = array(); - $object->child['index'] = new Author(); - - $this->getPropertyAccessor()->setValue($object, 'child[index].firstName', 'Bernhard'); - - $this->assertEquals('Bernhard', $object->child['index']->firstName); - } - - public function testSetValueUpdateMagicSet() - { - $object = new Magician(); - - $this->getPropertyAccessor()->setValue($object, 'magicProperty', 'foobar'); - - $this->assertEquals('foobar', $object->__get('magicProperty')); - } - - public function testSetValueUpdatesSetters() - { - $object = new Author(); - - $this->getPropertyAccessor()->setValue($object, 'lastName', 'Schussek'); - - $this->assertEquals('Schussek', $object->getLastName()); - } - - public function testSetValueCamelizesSetterNames() - { - $object = new Author(); - - $this->getPropertyAccessor()->setValue($object, 'last_name', 'Schussek'); - - $this->assertEquals('Schussek', $object->getLastName()); - } - - public function testSetValueWithSpecialCharsNested() - { - $object = new \stdClass(); - $person = new \stdClass(); - $person->{'@email'} = null; - $object->person = $person; - - $this->getPropertyAccessor()->setValue($object, 'person.@email', 'bar'); - $this->assertEquals('bar', $object->person->{'@email'}); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - */ - public function testSetValueThrowsExceptionIfGetterIsNotPublic() - { - $this->getPropertyAccessor()->setValue(new Author(), 'privateSetter', 'foobar'); + $this->assertEquals('Updated', $author->__call('getMagicCallProperty', array())); } /** @@ -361,7 +266,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase { $value = 'baz'; - $this->getPropertyAccessor()->setValue($value, 'foobar', 'bam'); + $this->propertyAccessor->setValue($value, 'foobar', 'bam'); } /** @@ -371,7 +276,7 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase { $value = null; - $this->getPropertyAccessor()->setValue($value, 'foobar', 'bam'); + $this->propertyAccessor->setValue($value, 'foobar', 'bam'); } /** @@ -381,54 +286,6 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase { $value = ''; - $this->getPropertyAccessor()->setValue($value, 'foobar', 'bam'); + $this->propertyAccessor->setValue($value, 'foobar', 'bam'); } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - */ - public function testSetValueFailsIfMagicCallDisabled() - { - $value = new MagicianCall(); - - $this->getPropertyAccessor()->setValue($value, 'foobar', 'bam'); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - */ - public function testGetValueFailsIfMagicCallDisabled() - { - $value = new MagicianCall(); - - $this->getPropertyAccessor()->getValue($value, 'foobar', 'bam'); - } - - public function testGetValueReadsMagicCall() - { - $propertyAccessor = new PropertyAccessor(true); - $object = new MagicianCall(); - $object->setMagicProperty('foobar'); - - $this->assertSame('foobar', $propertyAccessor->getValue($object, 'magicProperty')); - } - - public function testGetValueReadsMagicCallThatReturnsConstant() - { - $propertyAccessor = new PropertyAccessor(true); - $object = new MagicianCall(); - - $this->assertNull($propertyAccessor->getValue($object, 'MagicProperty')); - } - - public function testSetValueUpdatesMagicCall() - { - $propertyAccessor = new PropertyAccessor(true); - $object = new MagicianCall(); - - $propertyAccessor->setValue($object, 'magicProperty', 'foobar'); - - $this->assertEquals('foobar', $object->getMagicProperty()); - } - } From 20e6bf8f49e607325fa9b2df62de592ec404a1bf Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Mar 2014 17:07:20 +0100 Subject: [PATCH 2/6] [PropertyAccess] Refactored PropertyAccessorCollectionTest --- .../Component/PropertyAccess/CHANGELOG.md | 3 + .../PropertyAccess/PropertyAccessor.php | 8 +- .../Tests/PropertyAccessorCollectionTest.php | 150 +++--------------- .../Tests/PropertyAccessorTest.php | 19 ++- 4 files changed, 46 insertions(+), 134 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/CHANGELOG.md b/src/Symfony/Component/PropertyAccess/CHANGELOG.md index fb5ebfa550..bfe3d51459 100644 --- a/src/Symfony/Component/PropertyAccess/CHANGELOG.md +++ b/src/Symfony/Component/PropertyAccess/CHANGELOG.md @@ -5,6 +5,9 @@ CHANGELOG ------ * allowed non alpha numeric characters in second level and deeper object properties names + * [BC BREAK] when accessing an index on an object that does not implement + ArrayAccess, a NoSuchIndexException is now thrown instead of the + semantically wrong NoSuchPropertyException 2.3.0 ------ diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index f5183a109e..66671c5be0 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -160,12 +160,12 @@ class PropertyAccessor implements PropertyAccessorInterface * * @return mixed The value of the key * - * @throws NoSuchPropertyException If the array does not implement \ArrayAccess or it is not an array + * @throws NoSuchIndexException If the array does not implement \ArrayAccess or it is not an array */ private function &readIndex(&$array, $index) { if (!$array instanceof \ArrayAccess && !is_array($array)) { - throw new NoSuchPropertyException(sprintf('Index "%s" cannot be read from object of type "%s" because it doesn\'t implement \ArrayAccess', $index, get_class($array))); + throw new NoSuchIndexException(sprintf('Index "%s" cannot be read from object of type "%s" because it doesn\'t implement \ArrayAccess', $index, get_class($array))); } // Use an array instead of an object since performance is very crucial here @@ -271,12 +271,12 @@ class PropertyAccessor implements PropertyAccessorInterface * @param string|integer $index The index to write at * @param mixed $value The value to write * - * @throws NoSuchPropertyException If the array does not implement \ArrayAccess or it is not an array + * @throws NoSuchIndexException If the array does not implement \ArrayAccess or it is not an array */ private function writeIndex(&$array, $index, $value) { if (!$array instanceof \ArrayAccess && !is_array($array)) { - throw new NoSuchPropertyException(sprintf('Index "%s" cannot be modified in object of type "%s" because it doesn\'t implement \ArrayAccess', $index, get_class($array))); + throw new NoSuchIndexException(sprintf('Index "%s" cannot be modified in object of type "%s" because it doesn\'t implement \ArrayAccess', $index, get_class($array))); } $array[$index] = $value; diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index b0f75aa366..808c9e1449 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -47,19 +47,6 @@ class PropertyAccessorCollectionTest_Car } } -class PropertyAccessorCollectionTest_CarCustomSingular -{ - public function addFoo($axis) {} - - public function removeFoo($axis) {} - - public function getAxes() {} -} - -class PropertyAccessorCollectionTest_Engine -{ -} - class PropertyAccessorCollectionTest_CarOnlyAdder { public function addAxis($axis) {} @@ -79,13 +66,6 @@ class PropertyAccessorCollectionTest_CarNoAdderAndRemover public function getAxes() {} } -class PropertyAccessorCollectionTest_CarNoAdderAndRemoverWithProperty -{ - protected $axes = array(); - - public function getAxes() {} -} - class PropertyAccessorCollectionTest_CompositeCar { public function getStructure() {} @@ -116,52 +96,34 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas abstract protected function getCollection(array $array); - public function testGetValueReadsArrayAccess() + public function getValidPropertyPaths() { - $object = $this->getCollection(array('firstName' => 'Bernhard')); - - $this->assertEquals('Bernhard', $this->propertyAccessor->getValue($object, '[firstName]')); - } - - public function testGetValueReadsNestedArrayAccess() - { - $object = $this->getCollection(array('person' => array('firstName' => 'Bernhard'))); - - $this->assertEquals('Bernhard', $this->propertyAccessor->getValue($object, '[person][firstName]')); + return array( + array(array('firstName' => 'Bernhard'), '[firstName]', 'Bernhard'), + array(array('person' => array('firstName' => 'Bernhard')), '[person][firstName]', 'Bernhard'), + ); } /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @dataProvider getValidPropertyPaths */ - public function testGetValueThrowsExceptionIfArrayAccessExpected() + public function testGetValue(array $array, $path, $value) { - $this->propertyAccessor->getValue(new \stdClass(), '[firstName]'); - } + $collection = $this->getCollection($array); - public function testSetValueUpdatesArrayAccess() - { - $object = $this->getCollection(array()); - - $this->propertyAccessor->setValue($object, '[firstName]', 'Bernhard'); - - $this->assertEquals('Bernhard', $object['firstName']); - } - - public function testSetValueUpdatesNestedArrayAccess() - { - $object = $this->getCollection(array()); - - $this->propertyAccessor->setValue($object, '[person][firstName]', 'Bernhard'); - - $this->assertEquals('Bernhard', $object['person']['firstName']); + $this->assertSame($value, $this->propertyAccessor->getValue($collection, $path)); } /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @dataProvider getValidPropertyPaths */ - public function testSetValueThrowsExceptionIfArrayAccessExpected() + public function testSetValue(array $array, $path) { - $this->propertyAccessor->setValue(new \stdClass(), '[firstName]', 'Bernhard'); + $collection = $this->getCollection($array); + + $this->propertyAccessor->setValue($collection, $path, 'Updated'); + + $this->assertSame('Updated', $this->propertyAccessor->getValue($collection, $path)); } public function testSetValueCallsAdderAndRemoverForCollections() @@ -210,32 +172,9 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas $this->propertyAccessor->setValue($car, 'structure.axes', $axesAfter); } - public function testSetValueCallsCustomAdderAndRemover() - { - $this->markTestSkipped('This feature is temporarily disabled as of 2.1'); - - $car = $this->getMock(__CLASS__.'_CarCustomSingular'); - $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); - $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); - - $car->expects($this->at(0)) - ->method('getAxes') - ->will($this->returnValue($axesBefore)); - $car->expects($this->at(1)) - ->method('removeFoo') - ->with('fourth'); - $car->expects($this->at(2)) - ->method('addFoo') - ->with('first'); - $car->expects($this->at(3)) - ->method('addFoo') - ->with('third'); - - $this->propertyAccessor->setValue($car, 'axes|foo', $axesAfter); - } - /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessage Found the public method "addAxis()", but did not find a public "removeAxis()" on class Mock_PropertyAccessorCollectionTest_CarOnlyAdder */ public function testSetValueFailsIfOnlyAdderFound() { @@ -252,6 +191,7 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessage Found the public method "removeAxis()", but did not find a public "addAxis()" on class Mock_PropertyAccessorCollectionTest_CarOnlyRemover */ public function testSetValueFailsIfOnlyRemoverFound() { @@ -267,58 +207,14 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas } /** - * @dataProvider noAdderRemoverData + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException + * @expectedExceptionMessage Neither the property "axes" nor one of the methods "addAx()", "addAxe()", "addAxis()", "setAxes()", "__set()" or "__call()" exist and have public access in class "Mock_PropertyAccessorCollectionTest_CarNoAdderAndRemover */ - public function testNoAdderAndRemoverThrowsSensibleError($car, $path, $message) + public function testSetValueFailsIfNoAdderAndNoRemoverFound() { + $car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover'); $axes = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); - try { - $this->propertyAccessor->setValue($car, $path, $axes); - $this->fail('An expected exception was not thrown!'); - } catch (ExceptionInterface $e) { - $this->assertEquals($message, $e->getMessage()); - } - } - - public function noAdderRemoverData() - { - $data = array(); - - $car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover'); - $propertyPath = 'axes'; - $expectedMessage = sprintf( - 'Neither the property "axes" nor one of the methods "addAx()", '. - '"addAxe()", "addAxis()", "setAxes()", "__set()" or "__call()" exist and have '. - 'public access in class "%s".', - get_class($car) - ); - $data[] = array($car, $propertyPath, $expectedMessage); - - /* - Temporarily disabled in 2.1 - - $propertyPath = new PropertyPath('axes|boo'); - $expectedMessage = sprintf( - 'Neither element "axes" nor method "setAxes()" exists in class ' - .'"%s", nor could adders and removers be found based on the ' - .'passed singular: %s', - get_class($car), - 'boo' - ); - $data[] = array($car, $propertyPath, $expectedMessage); - */ - - $car = $this->getMock(__CLASS__.'_CarNoAdderAndRemoverWithProperty'); - $propertyPath = 'axes'; - $expectedMessage = sprintf( - 'Neither the property "axes" nor one of the methods "addAx()", '. - '"addAxe()", "addAxis()", "setAxes()", "__set()" or "__call()" exist and have '. - 'public access in class "%s".', - get_class($car) - ); - $data[] = array($car, $propertyPath, $expectedMessage); - - return $data; + $this->propertyAccessor->setValue($car, 'axes', $axes); } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 3c13e50bb2..2d8b97dc57 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -31,7 +31,6 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase public function getValidPropertyPaths() { - return array( array(array('Bernhard', 'Schussek'), '[0]', 'Bernhard'), array(array('Bernhard', 'Schussek'), '[1]', 'Schussek'), @@ -65,7 +64,6 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase public function getPathsWithMissingProperty() { - return array( array((object) array('firstName' => 'Bernhard'), 'lastName'), array((object) array('property' => (object) array('firstName' => 'Bernhard')), 'property.lastName'), @@ -86,7 +84,6 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase public function getPathsWithMissingIndex() { - return array( array(array('firstName' => 'Bernhard'), '[lastName]'), array(array(), '[index][lastName]'), @@ -131,6 +128,14 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->propertyAccessor->getValue($objectOrArray, $path); } + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchIndexException + */ + public function testGetValueThrowsExceptionIfNotArrayAccess() + { + $this->propertyAccessor->getValue(new \stdClass(), '[index]'); + } + public function testGetValueReadsMagicGet() { $this->assertSame('Bernhard', $this->propertyAccessor->getValue(new TestClassMagicGet('Bernhard'), 'magicProperty')); @@ -229,6 +234,14 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->assertSame('Updated', $this->propertyAccessor->getValue($objectOrArray, $path)); } + /** + * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchIndexException + */ + public function testSetValueThrowsExceptionIfNotArrayAccess() + { + $this->propertyAccessor->setValue(new \stdClass(), '[index]', 'Updated'); + } + public function testSetValueUpdatesMagicSet() { $author = new TestClassMagicGet('Bernhard'); From 6d2af217aafd03e3f1600ce0ebc9c30cf0a7fc70 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Mar 2014 17:21:37 +0100 Subject: [PATCH 3/6] [PropertyAccess] Added isReadable() and isWritable() --- UPGRADE-2.5.md | 43 +++++- .../Component/PropertyAccess/CHANGELOG.md | 1 + .../PropertyAccess/PropertyAccessor.php | 126 +++++++++++++++- .../PropertyAccessorInterface.php | 31 +++- .../Tests/PropertyAccessorCollectionTest.php | 54 ++++++- .../Tests/PropertyAccessorTest.php | 136 ++++++++++++++++++ 6 files changed, 382 insertions(+), 9 deletions(-) diff --git a/UPGRADE-2.5.md b/UPGRADE-2.5.md index e3b581b5b9..f1afa9495b 100644 --- a/UPGRADE-2.5.md +++ b/UPGRADE-2.5.md @@ -45,6 +45,47 @@ Form { ``` +PropertyAccess +-------------- + + * The methods `isReadable()` and `isWritable()` were added to + `PropertyAccessorInterface`. If you implemented this interface in your own + code, you should add these two methods. + + * The methods `getValue()` and `setValue()` now throw an + `NoSuchIndexException` instead of a `NoSuchPropertyException` when an index + is accessed on an object that does not implement `ArrayAccess`. If you catch + this exception in your code, you should adapt the catch statement: + + Before: + + ```php + $object = new \stdClass(); + + try { + $propertyAccessor->getValue($object, '[index]'); + $propertyAccessor->setValue($object, '[index]', 'New value'); + } catch (NoSuchPropertyException $e) { + // ... + } + ``` + + After: + + ```php + $object = new \stdClass(); + + try { + $propertyAccessor->getValue($object, '[index]'); + $propertyAccessor->setValue($object, '[index]', 'New value'); + } catch (NoSuchIndexException $e) { + // ... + } + ``` + + A `NoSuchPropertyException` is still thrown when a non-existing property is + accessed on an object or an array. + Validator --------- @@ -56,7 +97,7 @@ Validator After: - Default email validation is now done via a simple regex which may cause invalid emails (not RFC compilant) to be + Default email validation is now done via a simple regex which may cause invalid emails (not RFC compilant) to be valid. This is the default behaviour. Strict email validation has to be explicitly activated in the configuration file by adding diff --git a/src/Symfony/Component/PropertyAccess/CHANGELOG.md b/src/Symfony/Component/PropertyAccess/CHANGELOG.md index bfe3d51459..631c9d7256 100644 --- a/src/Symfony/Component/PropertyAccess/CHANGELOG.md +++ b/src/Symfony/Component/PropertyAccess/CHANGELOG.md @@ -8,6 +8,7 @@ CHANGELOG * [BC BREAK] when accessing an index on an object that does not implement ArrayAccess, a NoSuchIndexException is now thrown instead of the semantically wrong NoSuchPropertyException + * [BC BREAK] added isReadable() and isWritable() to PropertyAccessorInterface 2.3.0 ------ diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 66671c5be0..b21866bd68 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -105,6 +105,84 @@ class PropertyAccessor implements PropertyAccessorInterface } } + /** + * {@inheritdoc} + */ + public function isReadable($objectOrArray, $propertyPath) + { + if (is_string($propertyPath)) { + $propertyPath = new PropertyPath($propertyPath); + } elseif (!$propertyPath instanceof PropertyPathInterface) { + throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); + } + + try { + $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength(), $this->throwExceptionOnInvalidIndex); + + return true; + } catch (NoSuchIndexException $e) { + return false; + } catch (NoSuchPropertyException $e) { + return false; + } catch (UnexpectedTypeException $e) { + return false; + } + } + + /** + * {@inheritdoc} + */ + public function isWritable($objectOrArray, $propertyPath, $value) + { + if (is_string($propertyPath)) { + $propertyPath = new PropertyPath($propertyPath); + } elseif (!$propertyPath instanceof PropertyPathInterface) { + throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); + } + + try { + $propertyValues = $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1); + $overwrite = true; + + // Add the root object to the list + array_unshift($propertyValues, array( + self::VALUE => $objectOrArray, + self::IS_REF => true, + )); + + for ($i = count($propertyValues) - 1; $i >= 0; --$i) { + $objectOrArray = $propertyValues[$i][self::VALUE]; + + if ($overwrite) { + if (!is_object($objectOrArray) && !is_array($objectOrArray)) { + return false; + } + + $property = $propertyPath->getElement($i); + + if ($propertyPath->isIndex($i)) { + if (!$objectOrArray instanceof \ArrayAccess && !is_array($objectOrArray)) { + return false; + } + } else { + if (!$this->isPropertyWritable($objectOrArray, $property, $value)) { + return false; + } + } + } + + $value = $objectOrArray; + $overwrite = !$propertyValues[$i][self::IS_REF]; + } + + return true; + } catch (NoSuchIndexException $e) { + return false; + } catch (NoSuchPropertyException $e) { + return false; + } + } + /** * Reads the path from an object up to a given path index. * @@ -357,9 +435,9 @@ class PropertyAccessor implements PropertyAccessorInterface $setter = 'set'.$this->camelize($property); $classHasProperty = $reflClass->hasProperty($property); - if ($reflClass->hasMethod($setter) && $reflClass->getMethod($setter)->isPublic()) { + if ($this->isMethodAccessible($reflClass, $setter, 1)) { $object->$setter($value); - } elseif ($reflClass->hasMethod('__set') && $reflClass->getMethod('__set')->isPublic()) { + } elseif ($this->isMethodAccessible($reflClass, '__set', 2)) { $object->$property = $value; } elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) { $object->$property = $value; @@ -370,7 +448,7 @@ class PropertyAccessor implements PropertyAccessorInterface // returns true, consequently the following line will result in a // fatal error. $object->$property = $value; - } elseif ($this->magicCall && $reflClass->hasMethod('__call') && $reflClass->getMethod('__call')->isPublic()) { + } elseif ($this->magicCall && $this->isMethodAccessible($reflClass, '__call', 2)) { // we call the getter and hope the __call do the job $object->$setter($value); } else { @@ -385,6 +463,38 @@ class PropertyAccessor implements PropertyAccessorInterface } } + private function isPropertyWritable($object, $property, $value) + { + if (!is_object($object)) { + throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%s]" instead?', $property, $property)); + } + + $reflClass = new \ReflectionClass($object); + $plural = $this->camelize($property); + + // Any of the two methods is required, but not yet known + $singulars = (array) StringUtil::singularify($plural); + + if (is_array($value) || $value instanceof \Traversable) { + try { + if (null !== $this->findAdderAndRemover($reflClass, $singulars)) { + return true; + } + } catch (NoSuchPropertyException $e) { + return false; + } + } + + $setter = 'set'.$this->camelize($property); + $classHasProperty = $reflClass->hasProperty($property); + + return $this->isMethodAccessible($reflClass, $setter, 1) + || $this->isMethodAccessible($reflClass, '__set', 2) + || ($classHasProperty && $reflClass->getProperty($property)->isPublic()) + || (!$classHasProperty && property_exists($object, $property)) + || ($this->magicCall && $this->isMethodAccessible($reflClass, '__call', 2)); + } + /** * Camelizes a given string. * @@ -409,6 +519,8 @@ class PropertyAccessor implements PropertyAccessorInterface */ private function findAdderAndRemover(\ReflectionClass $reflClass, array $singulars) { + $exception = null; + foreach ($singulars as $singular) { $addMethod = 'add'.$singular; $removeMethod = 'remove'.$singular; @@ -420,8 +532,8 @@ class PropertyAccessor implements PropertyAccessorInterface return array($addMethod, $removeMethod); } - if ($addMethodFound xor $removeMethodFound) { - throw new NoSuchPropertyException(sprintf( + if ($addMethodFound xor $removeMethodFound && null === $exception) { + $exception = new NoSuchPropertyException(sprintf( 'Found the public method "%s()", but did not find a public "%s()" on class %s', $addMethodFound ? $addMethod : $removeMethod, $addMethodFound ? $removeMethod : $addMethod, @@ -430,6 +542,10 @@ class PropertyAccessor implements PropertyAccessorInterface } } + if (null !== $exception) { + throw $exception; + } + return null; } diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php b/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php index 1eed7c7b07..b9da0e4937 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php @@ -19,7 +19,7 @@ namespace Symfony\Component\PropertyAccess; interface PropertyAccessorInterface { /** - * Sets the value at the end of the property path of the object + * Sets the value at the end of the property path of the object graph. * * Example: * @@ -50,7 +50,7 @@ interface PropertyAccessorInterface public function setValue(&$objectOrArray, $propertyPath, $value); /** - * Returns the value at the end of the property path of the object + * Returns the value at the end of the property path of the object graph. * * Example: * @@ -78,4 +78,31 @@ interface PropertyAccessorInterface * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. */ public function getValue($objectOrArray, $propertyPath); + + /** + * Returns whether a value can be written at a given property path. + * + * Whenever this method returns true, {@link setValue()} is guaranteed not + * to throw an exception when called with the same arguments. + * + * @param object|array $objectOrArray The object or array to check + * @param string|PropertyPathInterface $propertyPath The property path to check + * @param mixed $value The value to set at the end of the property path + * + * @return Boolean Whether the value can be set + */ + public function isWritable($objectOrArray, $propertyPath, $value); + + /** + * Returns whether a property path can be read from an object graph. + * + * Whenever this method returns true, {@link getValue()} is guaranteed not + * to throw an exception when called with the same arguments. + * + * @param object|array $objectOrArray The object or array to check + * @param string|PropertyPathInterface $propertyPath The property path to check + * + * @return Boolean Whether the property path can be read + */ + public function isReadable($objectOrArray, $propertyPath); } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index 808c9e1449..cd51f2601c 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -210,11 +210,63 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException * @expectedExceptionMessage Neither the property "axes" nor one of the methods "addAx()", "addAxe()", "addAxis()", "setAxes()", "__set()" or "__call()" exist and have public access in class "Mock_PropertyAccessorCollectionTest_CarNoAdderAndRemover */ - public function testSetValueFailsIfNoAdderAndNoRemoverFound() + public function testSetValueFailsIfNoAdderNorRemoverFound() { $car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover'); $axes = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); $this->propertyAccessor->setValue($car, 'axes', $axes); } + + /** + * @dataProvider getValidPropertyPaths + */ + public function testIsReadable(array $array, $path) + { + $collection = $this->getCollection($array); + + $this->assertTrue($this->propertyAccessor->isReadable($collection, $path)); + } + + /** + * @dataProvider getValidPropertyPaths + */ + public function testIsWritable(array $array, $path) + { + $collection = $this->getCollection($array); + + $this->assertTrue($this->propertyAccessor->isWritable($collection, $path, 'Updated')); + } + + public function testIsWritableReturnsTrueIfAdderAndRemoverExists() + { + $car = $this->getMock(__CLASS__.'_Car'); + $axes = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third')); + + $this->assertTrue($this->propertyAccessor->isWritable($car, 'axes', $axes)); + } + + public function testIsWritableReturnsFalseIfOnlyAdderExists() + { + $car = $this->getMock(__CLASS__.'_CarOnlyAdder'); + $axes = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third')); + + $this->assertFalse($this->propertyAccessor->isWritable($car, 'axes', $axes)); + } + + public function testIsWritableReturnsFalseIfOnlyRemoverExists() + { + $car = $this->getMock(__CLASS__.'_CarOnlyRemover'); + $axes = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third')); + + $this->assertFalse($this->propertyAccessor->isWritable($car, 'axes', $axes)); + } + + public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists() + { + $car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover'); + $axes = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third')); + + $this->assertFalse($this->propertyAccessor->isWritable($car, 'axes', $axes)); + } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 2d8b97dc57..a40c2d9ffa 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -301,4 +301,140 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->propertyAccessor->setValue($value, 'foobar', 'bam'); } + + /** + * @dataProvider getValidPropertyPaths + */ + public function testIsReadable($objectOrArray, $path) + { + $this->assertTrue($this->propertyAccessor->isReadable($objectOrArray, $path)); + } + + /** + * @dataProvider getPathsWithMissingProperty + */ + public function testIsReadableReturnsFalseIfPropertyNotFound($objectOrArray, $path) + { + $this->assertFalse($this->propertyAccessor->isReadable($objectOrArray, $path)); + } + + /** + * @dataProvider getPathsWithMissingIndex + */ + public function testIsReadableReturnsTrueIfIndexNotFound($objectOrArray, $path) + { + // Non-existing indices can be read. In this case, null is returned + $this->assertTrue($this->propertyAccessor->isReadable($objectOrArray, $path)); + } + + /** + * @dataProvider getPathsWithMissingIndex + */ + public function testIsReadableReturnsFalseIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) + { + $this->propertyAccessor = new PropertyAccessor(false, true); + + // When exceptions are enabled, non-existing indices cannot be read + $this->assertFalse($this->propertyAccessor->isReadable($objectOrArray, $path)); + } + + public function testIsReadableRecognizesMagicGet() + { + $this->assertTrue($this->propertyAccessor->isReadable(new TestClassMagicGet('Bernhard'), 'magicProperty')); + } + + public function testIsReadableDoesNotRecognizeMagicCallByDefault() + { + $this->assertFalse($this->propertyAccessor->isReadable(new TestClassMagicCall('Bernhard'), 'magicCallProperty')); + } + + public function testIsReadableRecognizesMagicCallIfEnabled() + { + $this->propertyAccessor = new PropertyAccessor(true); + + $this->assertTrue($this->propertyAccessor->isReadable(new TestClassMagicCall('Bernhard'), 'magicCallProperty')); + } + + public function testIsReadableThrowsExceptionIfNotObjectOrArray() + { + $this->assertFalse($this->propertyAccessor->isReadable('baz', 'foobar')); + } + + public function testIsReadableThrowsExceptionIfNull() + { + $this->assertFalse($this->propertyAccessor->isReadable(null, 'foobar')); + } + + public function testIsReadableThrowsExceptionIfEmpty() + { + $this->assertFalse($this->propertyAccessor->isReadable('', 'foobar')); + } + + /** + * @dataProvider getValidPropertyPaths + */ + public function testIsWritable($objectOrArray, $path) + { + $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + } + + /** + * @dataProvider getPathsWithMissingProperty + */ + public function testIsWritableReturnsFalseIfPropertyNotFound($objectOrArray, $path) + { + $this->assertFalse($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + } + + /** + * @dataProvider getPathsWithMissingIndex + */ + public function testIsWritableReturnsTrueIfIndexNotFound($objectOrArray, $path) + { + // Non-existing indices can be written. Arrays are created on-demand. + $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + } + + /** + * @dataProvider getPathsWithMissingIndex + */ + public function testIsWritableReturnsTrueIfIndexNotFoundAndIndexExceptionsEnabled($objectOrArray, $path) + { + $this->propertyAccessor = new PropertyAccessor(false, true); + + // Non-existing indices can be written even if exceptions are enabled + $this->assertTrue($this->propertyAccessor->isWritable($objectOrArray, $path, 'Updated')); + } + + public function testIsWritableRecognizesMagicSet() + { + $this->assertTrue($this->propertyAccessor->isWritable(new TestClassMagicGet('Bernhard'), 'magicProperty', 'Updated')); + } + + public function testIsWritableDoesNotRecognizeMagicCallByDefault() + { + $this->assertFalse($this->propertyAccessor->isWritable(new TestClassMagicCall('Bernhard'), 'magicCallProperty', 'Updated')); + } + + public function testIsWritableRecognizesMagicCallIfEnabled() + { + $this->propertyAccessor = new PropertyAccessor(true); + + $this->assertTrue($this->propertyAccessor->isWritable(new TestClassMagicCall('Bernhard'), 'magicCallProperty', 'Updated')); + } + + public function testIsWritableThrowsExceptionIfNotObjectOrArray() + { + $this->assertFalse($this->propertyAccessor->isWritable('baz', 'foobar', 'Updated')); + } + + public function testIsWritableThrowsExceptionIfNull() + { + $this->assertFalse($this->propertyAccessor->isWritable(null, 'foobar', 'Updated')); + } + + public function testIsWritableThrowsExceptionIfEmpty() + { + $this->assertFalse($this->propertyAccessor->isWritable('', 'foobar', 'Updated')); + } } From 4262707e5aeee5b67448ba2fb422d2f6c8074445 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 28 Mar 2014 19:21:04 +0100 Subject: [PATCH 4/6] [PropertyAccess] Fixed CS and added missing documentation --- .../PropertyAccess/PropertyAccessor.php | 21 +++++++++++-------- .../Tests/PropertyAccessorCollectionTest.php | 2 -- .../Tests/PropertyAccessorTest.php | 1 - 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index b21866bd68..8c86d88303 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -33,7 +33,7 @@ class PropertyAccessor implements PropertyAccessorInterface /** * @var Boolean */ - private $throwExceptionOnInvalidIndex; + private $ignoreInvalidIndices; /** * Should not be used by application code. Use @@ -42,7 +42,7 @@ class PropertyAccessor implements PropertyAccessorInterface public function __construct($magicCall = false, $throwExceptionOnInvalidIndex = false) { $this->magicCall = $magicCall; - $this->throwExceptionOnInvalidIndex = $throwExceptionOnInvalidIndex; + $this->ignoreInvalidIndices = !$throwExceptionOnInvalidIndex; } /** @@ -56,7 +56,7 @@ class PropertyAccessor implements PropertyAccessorInterface throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); } - $propertyValues =& $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength(), $this->throwExceptionOnInvalidIndex); + $propertyValues =& $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength(), $this->ignoreInvalidIndices); return $propertyValues[count($propertyValues) - 1][self::VALUE]; } @@ -117,7 +117,7 @@ class PropertyAccessor implements PropertyAccessorInterface } try { - $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength(), $this->throwExceptionOnInvalidIndex); + $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength(), $this->ignoreInvalidIndices); return true; } catch (NoSuchIndexException $e) { @@ -186,15 +186,18 @@ class PropertyAccessor implements PropertyAccessorInterface /** * Reads the path from an object up to a given path index. * - * @param object|array $objectOrArray The object or array to read from - * @param PropertyPathInterface $propertyPath The property path to read - * @param integer $lastIndex The index up to which should be read + * @param object|array $objectOrArray The object or array to read from + * @param PropertyPathInterface $propertyPath The property path to read + * @param integer $lastIndex The index up to which should be read + * @param Boolean $ignoreInvalidIndices Whether to ignore invalid indices + * or throw an exception * * @return array The values read in the path. * * @throws UnexpectedTypeException If a value within the path is neither object nor array. + * @throws NoSuchIndexException If a non-existing index is accessed */ - private function &readPropertiesUntil(&$objectOrArray, PropertyPathInterface $propertyPath, $lastIndex, $throwExceptionOnInvalidIndex = false) + private function &readPropertiesUntil(&$objectOrArray, PropertyPathInterface $propertyPath, $lastIndex, $ignoreInvalidIndices = true) { $propertyValues = array(); @@ -209,7 +212,7 @@ class PropertyAccessor implements PropertyAccessorInterface // Create missing nested arrays on demand if ($isIndex && $isArrayAccess && !isset($objectOrArray[$property])) { - if ($throwExceptionOnInvalidIndex) { + if (!$ignoreInvalidIndices) { throw new NoSuchIndexException(sprintf('Cannot read property "%s". Available properties are "%s"', $property, print_r(array_keys($objectOrArray), true))); } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index cd51f2601c..18211c822b 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -11,9 +11,7 @@ namespace Symfony\Component\PropertyAccess\Tests; -use Symfony\Component\PropertyAccess\Exception\ExceptionInterface; use Symfony\Component\PropertyAccess\PropertyAccessor; -use Symfony\Component\PropertyAccess\StringUtil; class PropertyAccessorCollectionTest_Car { diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index a40c2d9ffa..a6b09fa0ab 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -13,7 +13,6 @@ namespace Symfony\Component\PropertyAccess\Tests; use Symfony\Component\PropertyAccess\PropertyAccessor; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClass; -use Symfony\Component\PropertyAccess\Tests\Fixtures\Magician; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicCall; use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicGet; From 9aee2ad999c78bddf3b5eae52c8a3fc918cb2c48 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Sun, 30 Mar 2014 18:19:15 +0200 Subject: [PATCH 5/6] [PropertyAccess] Removed the argument $value from isWritable() To keep isWritable() and setValue() consistent, the exception thrown when only an adder was present, but no remover (or vice versa), was removed. --- .../Exception/InvalidArgumentException.php | 21 ++ .../PropertyAccess/PropertyAccessor.php | 205 ++++++++++-------- .../PropertyAccessorInterface.php | 17 +- .../Tests/PropertyAccessorCollectionTest.php | 36 +-- 4 files changed, 144 insertions(+), 135 deletions(-) create mode 100644 src/Symfony/Component/PropertyAccess/Exception/InvalidArgumentException.php diff --git a/src/Symfony/Component/PropertyAccess/Exception/InvalidArgumentException.php b/src/Symfony/Component/PropertyAccess/Exception/InvalidArgumentException.php new file mode 100644 index 0000000000..47bc7e150d --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Exception/InvalidArgumentException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Exception; + +/** + * Base InvalidArgumentException for the PropertyAccess component. + * + * @author Bernhard Schussek + */ +class InvalidArgumentException extends \InvalidArgumentException implements ExceptionInterface +{ +} diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 8c86d88303..c52b598cb9 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -11,6 +11,7 @@ namespace Symfony\Component\PropertyAccess; +use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException; use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException; use Symfony\Component\PropertyAccess\Exception\NoSuchIndexException; use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException; @@ -53,7 +54,12 @@ class PropertyAccessor implements PropertyAccessorInterface if (is_string($propertyPath)) { $propertyPath = new PropertyPath($propertyPath); } elseif (!$propertyPath instanceof PropertyPathInterface) { - throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); + throw new InvalidArgumentException(sprintf( + 'The property path should be a string or an instance of '. + '"Symfony\Component\PropertyAccess\PropertyPathInterface". '. + 'Got: "%s"', + is_object($propertyPath) ? get_class($propertyPath) : gettype($propertyPath) + )); } $propertyValues =& $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength(), $this->ignoreInvalidIndices); @@ -69,7 +75,12 @@ class PropertyAccessor implements PropertyAccessorInterface if (is_string($propertyPath)) { $propertyPath = new PropertyPath($propertyPath); } elseif (!$propertyPath instanceof PropertyPathInterface) { - throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); + throw new InvalidArgumentException(sprintf( + 'The property path should be a string or an instance of '. + '"Symfony\Component\PropertyAccess\PropertyPathInterface". '. + 'Got: "%s"', + is_object($propertyPath) ? get_class($propertyPath) : gettype($propertyPath) + )); } $propertyValues =& $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1); @@ -90,13 +101,11 @@ class PropertyAccessor implements PropertyAccessorInterface } $property = $propertyPath->getElement($i); - //$singular = $propertyPath->singulars[$i]; - $singular = null; if ($propertyPath->isIndex($i)) { $this->writeIndex($objectOrArray, $property, $value); } else { - $this->writeProperty($objectOrArray, $property, $singular, $value); + $this->writeProperty($objectOrArray, $property, $value); } } @@ -113,7 +122,12 @@ class PropertyAccessor implements PropertyAccessorInterface if (is_string($propertyPath)) { $propertyPath = new PropertyPath($propertyPath); } elseif (!$propertyPath instanceof PropertyPathInterface) { - throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); + throw new InvalidArgumentException(sprintf( + 'The property path should be a string or an instance of '. + '"Symfony\Component\PropertyAccess\PropertyPathInterface". '. + 'Got: "%s"', + is_object($propertyPath) ? get_class($propertyPath) : gettype($propertyPath) + )); } try { @@ -132,12 +146,17 @@ class PropertyAccessor implements PropertyAccessorInterface /** * {@inheritdoc} */ - public function isWritable($objectOrArray, $propertyPath, $value) + public function isWritable($objectOrArray, $propertyPath) { if (is_string($propertyPath)) { $propertyPath = new PropertyPath($propertyPath); } elseif (!$propertyPath instanceof PropertyPathInterface) { - throw new UnexpectedTypeException($propertyPath, 'string or Symfony\Component\PropertyAccess\PropertyPathInterface'); + throw new InvalidArgumentException(sprintf( + 'The property path should be a string or an instance of '. + '"Symfony\Component\PropertyAccess\PropertyPathInterface". '. + 'Got: "%s"', + is_object($propertyPath) ? get_class($propertyPath) : gettype($propertyPath) + )); } try { @@ -165,13 +184,12 @@ class PropertyAccessor implements PropertyAccessorInterface return false; } } else { - if (!$this->isPropertyWritable($objectOrArray, $property, $value)) { + if (!$this->isPropertyWritable($objectOrArray, $property)) { return false; } } } - $value = $objectOrArray; $overwrite = !$propertyValues[$i][self::IS_REF]; } @@ -346,7 +364,7 @@ class PropertyAccessor implements PropertyAccessorInterface } /** - * Sets the value of the property at the given index in the path + * Sets the value of an index in a given array-accessible value. * * @param \ArrayAccess|array $array An array or \ArrayAccess object to write to * @param string|integer $index The index to write at @@ -364,74 +382,33 @@ class PropertyAccessor implements PropertyAccessorInterface } /** - * Sets the value of the property at the given index in the path + * Sets the value of a property in the given object * - * @param object|array $object The object or array to write to - * @param string $property The property to write - * @param string|null $singular The singular form of the property name or null - * @param mixed $value The value to write + * @param object $object The object to write to + * @param string $property The property to write + * @param mixed $value The value to write * * @throws NoSuchPropertyException If the property does not exist or is not * public. */ - private function writeProperty(&$object, $property, $singular, $value) + private function writeProperty(&$object, $property, $value) { - $guessedAdders = ''; - if (!is_object($object)) { throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%s]" instead?', $property, $property)); } $reflClass = new \ReflectionClass($object); $plural = $this->camelize($property); - - // Any of the two methods is required, but not yet known - $singulars = null !== $singular ? array($singular) : (array) StringUtil::singularify($plural); + $singulars = (array) StringUtil::singularify($plural); if (is_array($value) || $value instanceof \Traversable) { $methods = $this->findAdderAndRemover($reflClass, $singulars); + // Use addXxx() and removeXxx() to write the collection if (null !== $methods) { - // At this point the add and remove methods have been found - // Use iterator_to_array() instead of clone in order to prevent side effects - // see https://github.com/symfony/symfony/issues/4670 - $itemsToAdd = is_object($value) ? iterator_to_array($value) : $value; - $itemToRemove = array(); - $propertyValue = $this->readProperty($object, $property); - $previousValue = $propertyValue[self::VALUE]; - - if (is_array($previousValue) || $previousValue instanceof \Traversable) { - foreach ($previousValue as $previousItem) { - foreach ($value as $key => $item) { - if ($item === $previousItem) { - // Item found, don't add - unset($itemsToAdd[$key]); - - // Next $previousItem - continue 2; - } - } - - // Item not found, add to remove list - $itemToRemove[] = $previousItem; - } - } - - foreach ($itemToRemove as $item) { - call_user_func(array($object, $methods[1]), $item); - } - - foreach ($itemsToAdd as $item) { - call_user_func(array($object, $methods[0]), $item); - } + $this->writeCollection($object, $property, $value, $methods[0], $methods[1]); return; - } else { - // It is sufficient to include only the adders in the error - // message. If the user implements the adder but not the remover, - // an exception will be thrown in findAdderAndRemover() that - // the remover has to be implemented as well. - $guessedAdders = '"add'.implode('()", "add', $singulars).'()", '; } } @@ -459,43 +436,98 @@ class PropertyAccessor implements PropertyAccessorInterface 'Neither the property "%s" nor one of the methods %s"%s()", '. '"__set()" or "__call()" exist and have public access in class "%s".', $property, - $guessedAdders, + implode('', array_map(function ($singular) { + return '"add'.$singular.'()"/"remove'.$singular.'()", '; + }, $singulars)), $setter, $reflClass->name )); } } - private function isPropertyWritable($object, $property, $value) + /** + * Adjusts a collection-valued property by calling add*() and remove*() + * methods. + * + * @param object $object The object to write to + * @param string $property The property to write + * @param array|\Traversable $collection The collection to write + * @param string $addMethod The add*() method + * @param string $removeMethod The remove*() method + */ + private function writeCollection($object, $property, $collection, $addMethod, $removeMethod) + { + // At this point the add and remove methods have been found + // Use iterator_to_array() instead of clone in order to prevent side effects + // see https://github.com/symfony/symfony/issues/4670 + $itemsToAdd = is_object($collection) ? iterator_to_array($collection) : $collection; + $itemToRemove = array(); + $propertyValue = $this->readProperty($object, $property); + $previousValue = $propertyValue[self::VALUE]; + + if (is_array($previousValue) || $previousValue instanceof \Traversable) { + foreach ($previousValue as $previousItem) { + foreach ($collection as $key => $item) { + if ($item === $previousItem) { + // Item found, don't add + unset($itemsToAdd[$key]); + + // Next $previousItem + continue 2; + } + } + + // Item not found, add to remove list + $itemToRemove[] = $previousItem; + } + } + + foreach ($itemToRemove as $item) { + call_user_func(array($object, $removeMethod), $item); + } + + foreach ($itemsToAdd as $item) { + call_user_func(array($object, $addMethod), $item); + } + } + + /** + * Returns whether a property is writable in the given object. + * + * @param object $object The object to write to + * @param string $property The property to write + * + * @return Boolean Whether the property is writable + */ + private function isPropertyWritable($object, $property) { if (!is_object($object)) { - throw new NoSuchPropertyException(sprintf('Cannot write property "%s" to an array. Maybe you should write the property path as "[%s]" instead?', $property, $property)); + return false; } $reflClass = new \ReflectionClass($object); + + $setter = 'set'.$this->camelize($property); + $classHasProperty = $reflClass->hasProperty($property); + + if ($this->isMethodAccessible($reflClass, $setter, 1) + || $this->isMethodAccessible($reflClass, '__set', 2) + || ($classHasProperty && $reflClass->getProperty($property)->isPublic()) + || (!$classHasProperty && property_exists($object, $property)) + || ($this->magicCall && $this->isMethodAccessible($reflClass, '__call', 2))) { + return true; + } + $plural = $this->camelize($property); // Any of the two methods is required, but not yet known $singulars = (array) StringUtil::singularify($plural); - if (is_array($value) || $value instanceof \Traversable) { - try { - if (null !== $this->findAdderAndRemover($reflClass, $singulars)) { - return true; - } - } catch (NoSuchPropertyException $e) { - return false; - } + if (null !== $this->findAdderAndRemover($reflClass, $singulars)) { + return true; } - $setter = 'set'.$this->camelize($property); - $classHasProperty = $reflClass->hasProperty($property); - - return $this->isMethodAccessible($reflClass, $setter, 1) - || $this->isMethodAccessible($reflClass, '__set', 2) - || ($classHasProperty && $reflClass->getProperty($property)->isPublic()) - || (!$classHasProperty && property_exists($object, $property)) - || ($this->magicCall && $this->isMethodAccessible($reflClass, '__call', 2)); + return false; } /** @@ -517,8 +549,6 @@ class PropertyAccessor implements PropertyAccessorInterface * @param array $singulars The singular form of the property name or null * * @return array|null An array containing the adder and remover when found, null otherwise - * - * @throws NoSuchPropertyException If the property does not exist */ private function findAdderAndRemover(\ReflectionClass $reflClass, array $singulars) { @@ -534,19 +564,6 @@ class PropertyAccessor implements PropertyAccessorInterface if ($addMethodFound && $removeMethodFound) { return array($addMethod, $removeMethod); } - - if ($addMethodFound xor $removeMethodFound && null === $exception) { - $exception = new NoSuchPropertyException(sprintf( - 'Found the public method "%s()", but did not find a public "%s()" on class %s', - $addMethodFound ? $addMethod : $removeMethod, - $addMethodFound ? $removeMethod : $addMethod, - $reflClass->name - )); - } - } - - if (null !== $exception) { - throw $exception; } return null; diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php b/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php index b9da0e4937..7e1cd24ae8 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php @@ -43,9 +43,10 @@ interface PropertyAccessorInterface * @param string|PropertyPathInterface $propertyPath The property path to modify * @param mixed $value The value to set at the end of the property path * - * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. - * @throws Exception\UnexpectedTypeException If a value within the path is neither object - * nor array + * @throws Exception\InvalidArgumentException If the property path is invalid + * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. + * @throws Exception\UnexpectedTypeException If a value within the path is neither object + * nor array */ public function setValue(&$objectOrArray, $propertyPath, $value); @@ -75,7 +76,8 @@ interface PropertyAccessorInterface * * @return mixed The value at the end of the property path * - * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. + * @throws Exception\InvalidArgumentException If the property path is invalid + * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. */ public function getValue($objectOrArray, $propertyPath); @@ -87,11 +89,12 @@ interface PropertyAccessorInterface * * @param object|array $objectOrArray The object or array to check * @param string|PropertyPathInterface $propertyPath The property path to check - * @param mixed $value The value to set at the end of the property path * * @return Boolean Whether the value can be set + * + * @throws Exception\InvalidArgumentException If the property path is invalid */ - public function isWritable($objectOrArray, $propertyPath, $value); + public function isWritable($objectOrArray, $propertyPath); /** * Returns whether a property path can be read from an object graph. @@ -103,6 +106,8 @@ interface PropertyAccessorInterface * @param string|PropertyPathInterface $propertyPath The property path to check * * @return Boolean Whether the property path can be read + * + * @throws Exception\InvalidArgumentException If the property path is invalid */ public function isReadable($objectOrArray, $propertyPath); } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index 18211c822b..7e4a49247c 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -172,41 +172,7 @@ abstract class PropertyAccessorCollectionTest extends \PHPUnit_Framework_TestCas /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - * @expectedExceptionMessage Found the public method "addAxis()", but did not find a public "removeAxis()" on class Mock_PropertyAccessorCollectionTest_CarOnlyAdder - */ - public function testSetValueFailsIfOnlyAdderFound() - { - $car = $this->getMock(__CLASS__.'_CarOnlyAdder'); - $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); - $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); - - $car->expects($this->any()) - ->method('getAxes') - ->will($this->returnValue($axesBefore)); - - $this->propertyAccessor->setValue($car, 'axes', $axesAfter); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - * @expectedExceptionMessage Found the public method "removeAxis()", but did not find a public "addAxis()" on class Mock_PropertyAccessorCollectionTest_CarOnlyRemover - */ - public function testSetValueFailsIfOnlyRemoverFound() - { - $car = $this->getMock(__CLASS__.'_CarOnlyRemover'); - $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth')); - $axesAfter = $this->getCollection(array(0 => 'first', 1 => 'second', 2 => 'third')); - - $car->expects($this->any()) - ->method('getAxes') - ->will($this->returnValue($axesBefore)); - - $this->propertyAccessor->setValue($car, 'axes', $axesAfter); - } - - /** - * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - * @expectedExceptionMessage Neither the property "axes" nor one of the methods "addAx()", "addAxe()", "addAxis()", "setAxes()", "__set()" or "__call()" exist and have public access in class "Mock_PropertyAccessorCollectionTest_CarNoAdderAndRemover + * @expectedExceptionMessage Neither the property "axes" nor one of the methods "addAx()"/"removeAx()", "addAxe()"/"removeAxe()", "addAxis()"/"removeAxis()", "setAxes()", "__set()" or "__call()" exist and have public access in class "Mock_PropertyAccessorCollectionTest_CarNoAdderAndRemover */ public function testSetValueFailsIfNoAdderNorRemoverFound() { From f7fb855f46a35be018de01bdf57ec22ed5d4dd26 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 31 Mar 2014 10:36:17 +0200 Subject: [PATCH 6/6] [PropertyAccess] Added missing exceptions to phpdoc --- .../Component/PropertyAccess/PropertyAccessorInterface.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php b/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php index 7e1cd24ae8..42e0d1e95d 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessorInterface.php @@ -44,7 +44,7 @@ interface PropertyAccessorInterface * @param mixed $value The value to set at the end of the property path * * @throws Exception\InvalidArgumentException If the property path is invalid - * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. + * @throws Exception\AccessException If a property/index does not exist or is not public * @throws Exception\UnexpectedTypeException If a value within the path is neither object * nor array */ @@ -77,7 +77,9 @@ interface PropertyAccessorInterface * @return mixed The value at the end of the property path * * @throws Exception\InvalidArgumentException If the property path is invalid - * @throws Exception\NoSuchPropertyException If a property does not exist or is not public. + * @throws Exception\AccessException If a property/index does not exist or is not public + * @throws Exception\UnexpectedTypeException If a value within the path is neither object + * nor array */ public function getValue($objectOrArray, $propertyPath);