From b95ffd1e5350932490a5c27472a8f499137a96d4 Mon Sep 17 00:00:00 2001 From: Philip Frank Date: Mon, 2 Mar 2015 21:56:10 +0100 Subject: [PATCH 1/4] [PropertyAccess] add test case for property with only getter, no setter --- .../Component/PropertyAccess/Tests/Fixtures/TestClass.php | 6 ++++++ .../PropertyAccess/Tests/PropertyAccessorTest.php | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php index 5cd605b164dd1..d4217e6fc976c 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php @@ -25,6 +25,7 @@ class TestClass private $publicAccessorWithMoreRequiredParameters; private $publicIsAccessor; private $publicHasAccessor; + private $publicGetter; public function __construct($value) { @@ -37,6 +38,7 @@ public function __construct($value) $this->publicAccessorWithMoreRequiredParameters = $value; $this->publicIsAccessor = $value; $this->publicHasAccessor = $value; + $this->publicGetter = $value; } public function setPublicAccessor($value) @@ -166,4 +168,8 @@ private function hasPrivateHasAccessor() { return 'foobar'; } + + public function getPublicGetter() { + return $this->publicGetter; + } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 58d4d74402084..9f704cc1a2e6a 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -430,4 +430,12 @@ public function testTicket5755() $this->assertEquals('foobar', $object->getProperty()); } + + public function testPublicGetter() + { + $this->propertyAccessor = new PropertyAccessor(true); + + $this->assertTrue($this->propertyAccessor->isReadable(new TestClass('foo'), 'publicGetter')); + $this->assertEquals('foo', $this->propertyAccessor->getValue(new TestClass('foo'), 'publicGetter')); + } } From 42ad722406ce1e7a7690b5f959a366cf53f0423e Mon Sep 17 00:00:00 2001 From: Philip Frank Date: Mon, 2 Mar 2015 22:01:54 +0100 Subject: [PATCH 2/4] [PropertyAccess] Add test cases for nested objects and arrays --- .../PropertyAccess/Tests/Fixtures/TestClass.php | 3 ++- .../Tests/PropertyAccessorTest.php | 16 ++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php index d4217e6fc976c..7b1b927529afe 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClass.php @@ -169,7 +169,8 @@ private function hasPrivateHasAccessor() return 'foobar'; } - public function getPublicGetter() { + public function getPublicGetter() + { return $this->publicGetter; } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 9f704cc1a2e6a..0c46467567444 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -419,6 +419,14 @@ public function getValidPropertyPaths() array(array('index' => array('%!@$§.' => 'Bernhard')), '[index][%!@$§.]', 'Bernhard'), array((object) array('%!@$§' => 'Bernhard'), '%!@$§', 'Bernhard'), array((object) array('property' => (object) array('%!@$§' => 'Bernhard')), 'property.%!@$§', 'Bernhard'), + + // nested objects and arrays + array(array('foo' => new TestClass('bar')), '[foo].publicGetSetter', 'bar'), + array(new TestClass(array('foo' => 'bar')), 'publicGetSetter[foo]', 'bar'), + array(new TestClass(new TestClass('bar')), 'publicGetter.publicGetSetter', 'bar'), + array(new TestClass(array('foo' => new TestClass('bar'))), 'publicGetter[foo].publicGetSetter', 'bar'), + array(new TestClass(new TestClass(new TestClass('bar'))), 'publicGetter.publicGetter.publicGetSetter', 'bar'), + array(new TestClass(array('foo' => array('baz' => new TestClass('bar')))), 'publicGetter[foo][baz].publicGetSetter', 'bar'), ); } @@ -430,12 +438,4 @@ public function testTicket5755() $this->assertEquals('foobar', $object->getProperty()); } - - public function testPublicGetter() - { - $this->propertyAccessor = new PropertyAccessor(true); - - $this->assertTrue($this->propertyAccessor->isReadable(new TestClass('foo'), 'publicGetter')); - $this->assertEquals('foo', $this->propertyAccessor->getValue(new TestClass('foo'), 'publicGetter')); - } } From 15b890f15b83e197149fbe3ce80cf12165f8f925 Mon Sep 17 00:00:00 2001 From: Philip Frank Date: Mon, 2 Mar 2015 22:02:48 +0100 Subject: [PATCH 3/4] [PropertyAccess] stop overwriting once a reference is reached --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index cef1c6e7f5a5a..1a764c457f13f 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -89,10 +89,11 @@ public function setValue(&$objectOrArray, $propertyPath, $value) } else { $this->writeProperty($objectOrArray, $property, $value); } + + $overwrite = !$propertyValues[$i][self::IS_REF]; } $value = & $objectOrArray; - $overwrite = !$propertyValues[$i][self::IS_REF]; } } @@ -150,9 +151,9 @@ public function isWritable($objectOrArray, $propertyPath) return false; } } - } - $overwrite = !$propertyValues[$i][self::IS_REF]; + $overwrite = !$propertyValues[$i][self::IS_REF]; + } } return true; From c3eedcbbe6232bd789a72b805c56f6da8b41e483 Mon Sep 17 00:00:00 2001 From: Philip Frank Date: Tue, 3 Mar 2015 12:27:57 +0100 Subject: [PATCH 4/4] [PropertyAccess] return as soon as possible from setValue and isWritable --- .../PropertyAccess/PropertyAccessor.php | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 1a764c457f13f..17f62bcc4556d 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -70,7 +70,6 @@ public function setValue(&$objectOrArray, $propertyPath, $value) } $propertyValues = & $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1); - $overwrite = true; // Add the root object to the list array_unshift($propertyValues, array( @@ -81,16 +80,16 @@ public function setValue(&$objectOrArray, $propertyPath, $value) for ($i = count($propertyValues) - 1; $i >= 0; --$i) { $objectOrArray = & $propertyValues[$i][self::VALUE]; - if ($overwrite) { - $property = $propertyPath->getElement($i); + $property = $propertyPath->getElement($i); - if ($propertyPath->isIndex($i)) { - $this->writeIndex($objectOrArray, $property, $value); - } else { - $this->writeProperty($objectOrArray, $property, $value); - } + if ($propertyPath->isIndex($i)) { + $this->writeIndex($objectOrArray, $property, $value); + } else { + $this->writeProperty($objectOrArray, $property, $value); + } - $overwrite = !$propertyValues[$i][self::IS_REF]; + if ($propertyValues[$i][self::IS_REF]) { + return; } $value = & $objectOrArray; @@ -128,7 +127,6 @@ public function isWritable($objectOrArray, $propertyPath) try { $propertyValues = $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1); - $overwrite = true; // Add the root object to the list array_unshift($propertyValues, array( @@ -139,20 +137,20 @@ public function isWritable($objectOrArray, $propertyPath) for ($i = count($propertyValues) - 1; $i >= 0; --$i) { $objectOrArray = $propertyValues[$i][self::VALUE]; - if ($overwrite) { - $property = $propertyPath->getElement($i); + $property = $propertyPath->getElement($i); - if ($propertyPath->isIndex($i)) { - if (!$objectOrArray instanceof \ArrayAccess && !is_array($objectOrArray)) { - return false; - } - } else { - if (!$this->isPropertyWritable($objectOrArray, $property)) { - return false; - } + if ($propertyPath->isIndex($i)) { + if (!$objectOrArray instanceof \ArrayAccess && !is_array($objectOrArray)) { + return false; } + } else { + if (!$this->isPropertyWritable($objectOrArray, $property)) { + return false; + } + } - $overwrite = !$propertyValues[$i][self::IS_REF]; + if ($propertyValues[$i][self::IS_REF]) { + return true; } }