bug #13835 [PropertyAccess] stop overwriting once a reference is reached (3rd) (bananer)

This PR was squashed before being merged into the 2.6 branch (closes #13835).

Discussion
----------

[PropertyAccess] stop overwriting once a reference is reached (3rd)

I commited with a different email address in [PR 13831](https://github.com/symfony/symfony/pull/13831), so here is the third attempt. Also removed unnecessary test as suggested by @Tobion

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | [13731](https://github.com/symfony/symfony/issues/13731)
| License       | MIT
| Doc PR        | none

I added test cases for the scenario described in my issue. After looking through the PropertyAccessor source my conclusion was that there is no reason to overwrite parents of references, so I changed that. None of the previous test cases disagreed and my new tests also passed.

Or maybe I got it all wrong, I'm willing to learn.

Commits
-------

d733a88 [PropertyAccess] stop overwriting once a reference is reached (3rd)
This commit is contained in:
Tobias Schultze 2015-03-03 20:22:24 +01:00
commit 0cead0f17a
3 changed files with 35 additions and 21 deletions

View File

@ -70,7 +70,6 @@ class PropertyAccessor implements PropertyAccessorInterface
}
$propertyValues = & $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1);
$overwrite = true;
// Add the root object to the list
array_unshift($propertyValues, array(
@ -81,18 +80,19 @@ class PropertyAccessor implements PropertyAccessorInterface
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);
}
if ($propertyValues[$i][self::IS_REF]) {
return;
}
$value = & $objectOrArray;
$overwrite = !$propertyValues[$i][self::IS_REF];
}
}
@ -127,7 +127,6 @@ class PropertyAccessor implements PropertyAccessorInterface
try {
$propertyValues = $this->readPropertiesUntil($objectOrArray, $propertyPath, $propertyPath->getLength() - 1);
$overwrite = true;
// Add the root object to the list
array_unshift($propertyValues, array(
@ -138,21 +137,21 @@ class PropertyAccessor implements PropertyAccessorInterface
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;
}
}
return true;

View File

@ -25,6 +25,7 @@ class TestClass
private $publicAccessorWithMoreRequiredParameters;
private $publicIsAccessor;
private $publicHasAccessor;
private $publicGetter;
public function __construct($value)
{
@ -37,6 +38,7 @@ class TestClass
$this->publicAccessorWithMoreRequiredParameters = $value;
$this->publicIsAccessor = $value;
$this->publicHasAccessor = $value;
$this->publicGetter = $value;
}
public function setPublicAccessor($value)
@ -166,4 +168,9 @@ class TestClass
{
return 'foobar';
}
public function getPublicGetter()
{
return $this->publicGetter;
}
}

View File

@ -419,6 +419,14 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase
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'),
);
}