Skip to content

Commit

Permalink
[PropertyAccess] Changed PropertyAccessor to continue searching when …
Browse files Browse the repository at this point in the history 
…a non-public method/property are found
  • Loading branch information
@webmozart
webmozart committed Apr 18, 2013
1 parent 26f20cc commit df67239
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 108 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CHANGELOG
=========

2.3.0
------

* [BC BREAK] changed PropertyAccessor to continue its search for a property or
method even if a non-public match was found. Before, a PropertyAccessDeniedException
was thrown in this case. Class PropertyAccessDeniedException was removed
now.
21 changes: 0 additions & 21 deletions Exception/PropertyAccessDeniedException.php
View file

This file was deleted.

115 changes: 55 additions & 60 deletions PropertyAccessor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Component\PropertyAccess;

use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
use Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException;
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;

/**
Expand Down Expand Up @@ -180,9 +179,8 @@ private function &readIndex(&$array, $index)
*
* @return mixed The value of the read property
*
* @throws NoSuchPropertyException If the property does not exist
* @throws PropertyAccessDeniedException If the property cannot be accessed due to
* access restrictions (private or protected)
* @throws NoSuchPropertyException If the property does not exist or is not
* public.
*/
private function &readProperty(&$object, $property)
{
Expand All @@ -202,41 +200,38 @@ private function &readProperty(&$object, $property)
$getter = 'get'.$camelProp;
$isser = 'is'.$camelProp;
$hasser = 'has'.$camelProp;
$classHasProperty = $reflClass->hasProperty($property);

if ($reflClass->hasMethod($getter)) {
if (!$reflClass->getMethod($getter)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $getter, $reflClass->name));
}

if ($reflClass->hasMethod($getter) && $reflClass->getMethod($getter)->isPublic()) {
$result[self::VALUE] = $object->$getter();
} elseif ($reflClass->hasMethod($isser)) {
if (!$reflClass->getMethod($isser)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $isser, $reflClass->name));
}

} elseif ($reflClass->hasMethod($isser) && $reflClass->getMethod($isser)->isPublic()) {
$result[self::VALUE] = $object->$isser();
} elseif ($reflClass->hasMethod($hasser)) {
if (!$reflClass->getMethod($hasser)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $hasser, $reflClass->name));
}

} elseif ($reflClass->hasMethod($hasser) && $reflClass->getMethod($hasser)->isPublic()) {
$result[self::VALUE] = $object->$hasser();
} elseif ($reflClass->hasMethod('__get')) {
// needed to support magic method __get
} elseif ($reflClass->hasMethod('__get') && $reflClass->getMethod('__get')->isPublic()) {
$result[self::VALUE] = $object->$property;
} elseif ($reflClass->hasProperty($property)) {
if (!$reflClass->getProperty($property)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s". Maybe you should create the method "%s()" or "%s()" or "%s()"?', $property, $reflClass->name, $getter, $isser, $hasser));
}

} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
$result[self::VALUE] =& $object->$property;
$result[self::IS_REF] = true;
} elseif (property_exists($object, $property)) {
// needed to support \stdClass instances
} elseif (!$classHasProperty && property_exists($object, $property)) {
// Needed to support \stdClass instances. We need to explicitly
// exclude $classHasProperty, otherwise if in the previous clause
// a *protected* property was found on the class, property_exists()
// returns true, consequently the following line will result in a
// fatal error.
$result[self::VALUE] =& $object->$property;
$result[self::IS_REF] = true;
} else {
throw new NoSuchPropertyException(sprintf('Neither property "%s" nor method "%s()" nor method "%s()" exists in class "%s"', $property, $getter, $isser, $reflClass->name));
throw new NoSuchPropertyException(sprintf(
'Neither the property "%s" nor one of the methods "%s()", '.
'"%s()", "%s()" or "__get()" exist and have public access in '.
'class "%s".',
$property,
$getter,
$isser,
$hasser,
$reflClass->name
));
}

// Objects are always passed around by reference
Expand Down Expand Up @@ -268,18 +263,17 @@ private function writeIndex(&$array, $index, $value)
/**
* Sets the value of the property at the given index in the path
*
* @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|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
*
* @throws NoSuchPropertyException If the property does not exist
* @throws PropertyAccessDeniedException If the property cannot be accessed due to
* access restrictions (private or protected)
* @throws NoSuchPropertyException If the property does not exist or is not
* public.
*/
private function writeProperty(&$object, $property, $singular, $value)
{
$adderRemoverError = null;
$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));
Expand Down Expand Up @@ -330,38 +324,39 @@ private function writeProperty(&$object, $property, $singular, $value)

return;
} else {
$adderRemoverError = ', nor could adders and removers be found based on the ';
if (null === $singular) {
// $adderRemoverError .= 'guessed singulars: '.implode(', ', $singulars).' (provide a singular by suffixing the property path with "|{singular}" to override the guesser)';
$adderRemoverError .= 'guessed singulars: '.implode(', ', $singulars);
} else {
$adderRemoverError .= 'passed singular: '.$singular;
}
// 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).'()", ';
}
}

$setter = 'set'.$this->camelize($property);
$classHasProperty = $reflClass->hasProperty($property);

if ($reflClass->hasMethod($setter)) {
if (!$reflClass->getMethod($setter)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $setter, $reflClass->name));
}

if ($reflClass->hasMethod($setter) && $reflClass->getMethod($setter)->isPublic()) {
$object->$setter($value);
} elseif ($reflClass->hasMethod('__set')) {
// needed to support magic method __set
} elseif ($reflClass->hasMethod('__set') && $reflClass->getMethod('__set')->isPublic()) {
$object->$property = $value;
} elseif ($reflClass->hasProperty($property)) {
if (!$reflClass->getProperty($property)->isPublic()) {
throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s"%s. Maybe you should create the method "%s()"?', $property, $reflClass->name, $adderRemoverError, $setter));
}

} elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) {
$object->$property = $value;
} elseif (property_exists($object, $property)) {
// needed to support \stdClass instances
} elseif (!$classHasProperty && property_exists($object, $property)) {
// Needed to support \stdClass instances. We need to explicitly
// exclude $classHasProperty, otherwise if in the previous clause
// a *protected* property was found on the class, property_exists()
// returns true, consequently the following line will result in a
// fatal error.
$object->$property = $value;
} else {
throw new NoSuchPropertyException(sprintf('Neither element "%s" nor method "%s()" exists in class "%s"%s', $property, $setter, $reflClass->name, $adderRemoverError));
throw new NoSuchPropertyException(sprintf(
'Neither the property "%s" nor one of the methods %s"%s()" or '.
'"__set()" exist and have public access in class "%s".',
$property,
$guessedAdders,
$setter,
$reflClass->name
));
}
}

Expand Down Expand Up @@ -402,7 +397,7 @@ private function findAdderAndRemover(\ReflectionClass $reflClass, array $singula

if ($addMethodFound xor $removeMethodFound) {
throw new NoSuchPropertyException(sprintf(
'Found the public method "%s", but did not find a public "%s" on class %s',
'Found the public method "%s()", but did not find a public "%s()" on class %s',
$addMethodFound ? $addMethod : $removeMethod,
$addMethodFound ? $removeMethod : $addMethod,
$reflClass->name
Expand Down
11 changes: 4 additions & 7 deletions PropertyAccessorInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@ 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
* @throws Exception\PropertyAccessDeniedException If a property cannot be accessed due to
* access restrictions (private or protected)
* @throws Exception\UnexpectedTypeException If a value within the path is neither object
* nor array
* @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);

Expand Down Expand Up @@ -77,8 +75,7 @@ public function setValue(&$objectOrArray, $propertyPath, $value);
*
* @return mixed The value at the end of the property path
*
* @throws Exception\NoSuchPropertyException If the property/getter does not exist
* @throws Exception\PropertyAccessDeniedException If the property/getter exists but is not public
* @throws Exception\NoSuchPropertyException If a property does not exist or is not public.
*/
public function getValue($objectOrArray, $propertyPath);
}
24 changes: 8 additions & 16 deletions Tests/PropertyAccessorCollectionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,10 @@ public function noAdderRemoverData()
$car = $this->getMock(__CLASS__.'_CarNoAdderAndRemover');
$propertyPath = 'axes';
$expectedMessage = sprintf(
'Neither element "axes" nor method "setAxes()" exists in class '
.'"%s", nor could adders and removers be found based on the '
.'guessed singulars: %s'
// .'(provide a singular by suffixing the '
// .'property path with "|{singular}" to override the guesser)'
,
get_class($car),
implode(', ', (array) $singulars = StringUtil::singularify('Axes'))
'Neither the property "axes" nor one of the methods "addAx()", '.
'"addAxe()", "addAxis()", "setAxes()" or "__set()" exist and have '.
'public access in class "%s".',
get_class($car)
);
$data[] = array($car, $propertyPath, $expectedMessage);

Expand All @@ -316,14 +312,10 @@ public function noAdderRemoverData()
$car = $this->getMock(__CLASS__.'_CarNoAdderAndRemoverWithProperty');
$propertyPath = 'axes';
$expectedMessage = sprintf(
'Property "axes" is not public in class "%s", nor could adders and '
.'removers be found based on the guessed singulars: %s'
// .' (provide a singular by suffixing the property path with '
// .'"|{singular}" to override the guesser)'
.'. Maybe you should '
.'create the method "setAxes()"?',
get_class($car),
implode(', ', (array) $singulars = StringUtil::singularify('Axes'))
'Neither the property "axes" nor one of the methods "addAx()", '.
'"addAxe()", "addAxis()", "setAxes()" or "__set()" exist and have '.
'public access in class "%s".',
get_class($car)
);
$data[] = array($car, $propertyPath, $expectedMessage);

Expand Down
8 changes: 4 additions & 4 deletions Tests/PropertyAccessorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public function testGetValueReadsPropertyWithCustomPropertyPath()
}

/**
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
*/
public function testGetValueThrowsExceptionIfPropertyIsNotPublic()
{
Expand All @@ -138,7 +138,7 @@ public function testGetValueCamelizesGetterNames()
}

/**
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
*/
public function testGetValueThrowsExceptionIfGetterIsNotPublic()
{
Expand Down Expand Up @@ -180,7 +180,7 @@ public function testGetValueReadsMagicGetThatReturnsConstant()
}

/**
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
*/
public function testGetValueThrowsExceptionIfIsserIsNotPublic()
{
Expand Down Expand Up @@ -295,7 +295,7 @@ public function testSetValueCamelizesSetterNames()
}

/**
* @expectedException \Symfony\Component\PropertyAccess\Exception\PropertyAccessDeniedException
* @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException
*/
public function testSetValueThrowsExceptionIfGetterIsNotPublic()
{
Expand Down

0 comments on commit df67239

Please sign in to comment.