Permalink
Browse files

merged branch bschussek/issue4450 (PR #4806)

Commits
-------

1345360 [Form] Fixed PropertyPath handling of offsetGet() that returns a constant value
6e1462e [Form] Fixed PropertyPath handling of __get() method that returns a constant

Discussion
----------

[Form] Fixed "Indirect modification.." exceptions in PropertyPath

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4450, #4535?, #4612
Todo: -
  • Loading branch information...
fabpot committed Jul 9, 2012
2 parents 48b2a26 + 1426041 commit b7357efb8a3e8a5b148f586d27c5ef23da7de621
Showing with 59 additions and 42 deletions.
  1. +39 −0 Tests/Util/PropertyPathCollectionTest.php
  2. +12 −40 Tests/Util/PropertyPathTest.php
  3. +8 −2 Util/PropertyPath.php
@@ -96,6 +96,45 @@ public function getAxes() {}
{
abstract protected function getCollection(array $array);
+ public function testGetValueReadsArrayAccess()
+ {
+ $object = $this->getCollection(array('firstName' => 'Bernhard'));
+
+ $path = new PropertyPath('[firstName]');
+
+ $this->assertEquals('Bernhard', $path->getValue($object));
+ }
+
+ /**
+ * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
+ */
+ public function testGetValueThrowsExceptionIfArrayAccessExpected()
+ {
+ $path = new PropertyPath('[firstName]');
+
+ $path->getValue(new \stdClass());
+ }
+
+ public function testSetValueUpdatesArrayAccess()
+ {
+ $object = $this->getCollection(array());
+
+ $path = new PropertyPath('[firstName]');
+ $path->setValue($object, 'Bernhard');
+
+ $this->assertEquals('Bernhard', $object['firstName']);
+ }
+
+ /**
+ * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
+ */
+ public function testSetValueThrowsExceptionIfArrayAccessExpected()
+ {
+ $path = new PropertyPath('[firstName]');
+
+ $path->setValue(new \stdClass(), 'Bernhard');
+ }
+
public function testSetValueCallsAdderAndRemoverForCollections()
{
$axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth', 4 => 'fifth'));
@@ -123,26 +123,6 @@ public function testGetValueReadsPropertyWithCustomPropertyPath()
$this->assertEquals('Bernhard', $path->getValue($object));
}
- public function testGetValueReadsArrayAccess()
- {
- $object = new \ArrayObject();
- $object['firstName'] = 'Bernhard';
-
- $path = new PropertyPath('[firstName]');
-
- $this->assertEquals('Bernhard', $path->getValue($object));
- }
-
- /**
- * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
- */
- public function testGetValueThrowsExceptionIfArrayAccessExpected()
- {
- $path = new PropertyPath('[firstName]');
-
- $path->getValue(new Author());
- }
-
/**
* @expectedException Symfony\Component\Form\Exception\PropertyAccessDeniedException
*/
@@ -213,6 +193,18 @@ public function testGetValueReadsMagicGet()
$this->assertSame('foobar', $path->getValue($object));
}
+ /*
+ * https://github.com/symfony/symfony/pull/4450
+ */
+ public function testGetValueReadsMagicGetThatReturnsConstant()
+ {
+ $path = new PropertyPath('magicProperty');
+
+ $object = new Magician();
+
+ $this->assertNull($path->getValue($object));
+ }
+
/**
* @expectedException Symfony\Component\Form\Exception\PropertyAccessDeniedException
*/
@@ -316,16 +308,6 @@ public function testSetValueUpdatesPropertiesWithCustomPropertyPath()
$this->assertEquals('Bernhard', $object->child['index']->firstName);
}
- public function testSetValueUpdatesArrayAccess()
- {
- $object = new \ArrayObject();
-
- $path = new PropertyPath('[firstName]');
- $path->setValue($object, 'Bernhard');
-
- $this->assertEquals('Bernhard', $object['firstName']);
- }
-
public function testSetValueUpdateMagicSet()
{
$object = new Magician();
@@ -336,16 +318,6 @@ public function testSetValueUpdateMagicSet()
$this->assertEquals('foobar', $object->__get('magicProperty'));
}
- /**
- * @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
- */
- public function testSetValueThrowsExceptionIfArrayAccessExpected()
- {
- $path = new PropertyPath('[firstName]');
-
- $path->setValue(new Author(), 'Bernhard');
- }
-
public function testSetValueUpdatesSetters()
{
$object = new Author();
View
@@ -365,6 +365,8 @@ protected function &readPropertyAt(&$objectOrArray, $index)
*/
protected function &readProperty(&$objectOrArray, $property, $isIndex)
{
+ // The local variable (instead of immediate returns) is necessary
+ // because we want to return by reference!
$result = null;
if ($isIndex) {
@@ -373,7 +375,11 @@ protected function &readProperty(&$objectOrArray, $property, $isIndex)
}
if (isset($objectOrArray[$property])) {
- $result =& $objectOrArray[$property];
+ if (is_array($objectOrArray)) {
+ $result =& $objectOrArray[$property];
+ } else {
+ $result = $objectOrArray[$property];
+ }
}
} elseif (is_object($objectOrArray)) {
$camelProp = $this->camelize($property);
@@ -402,7 +408,7 @@ protected function &readProperty(&$objectOrArray, $property, $isIndex)
$result = $objectOrArray->$hasser();
} elseif ($reflClass->hasMethod('__get')) {
// needed to support magic method __get
- $result =& $objectOrArray->$property;
+ $result = $objectOrArray->$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()"?', $property, $reflClass->name, $getter, $isser));

0 comments on commit b7357ef

Please sign in to comment.