bug #28966 [PropertyAccessor] Fix unable to write to singular property using setter while plural adder/remover exist (karser)

This PR was merged into the 3.4 branch.

Discussion
----------

[PropertyAccessor] Fix unable to write to singular property using setter while plural adder/remover exist

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28961
| License       | MIT

Please take a look at the tests I added - they describe the issue. It has to do with the priorities: `findAdderAndRemover('User', 'email')` is called earlier than `$this->isMethodAccessible('User', 'setEmail', 1)`

Commits
-------

8238f167ad [PropertyAccessor] Fix unable to write to singular property using setter while plural adder/remover exist
This commit is contained in:
Nicolas Grekas 2018-11-15 13:12:43 +01:00
commit 1ac042b7b9
3 changed files with 104 additions and 20 deletions

View File

@ -707,16 +707,6 @@ class PropertyAccessor implements PropertyAccessorInterface
$camelized = $this->camelize($property);
$singulars = (array) Inflector::singularize($camelized);
if (\is_array($value) || $value instanceof \Traversable) {
$methods = $this->findAdderAndRemover($reflClass, $singulars);
if (null !== $methods) {
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
$access[self::ACCESS_ADDER] = $methods[0];
$access[self::ACCESS_REMOVER] = $methods[1];
}
}
if (!isset($access[self::ACCESS_TYPE])) {
$setter = 'set'.$camelized;
$getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item)
@ -738,16 +728,22 @@ class PropertyAccessor implements PropertyAccessorInterface
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC;
$access[self::ACCESS_NAME] = $setter;
} elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) {
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
$access[self::ACCESS_NAME] = sprintf(
'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
'the new value must be an array or an instance of \Traversable, '.
'"%s" given.',
$property,
$reflClass->name,
implode('()", "', $methods),
\is_object($value) ? \get_class($value) : \gettype($value)
);
if (\is_array($value) || $value instanceof \Traversable) {
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
$access[self::ACCESS_ADDER] = $methods[0];
$access[self::ACCESS_REMOVER] = $methods[1];
} else {
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
$access[self::ACCESS_NAME] = sprintf(
'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
'the new value must be an array or an instance of \Traversable, '.
'"%s" given.',
$property,
$reflClass->name,
implode('()", "', $methods),
\is_object($value) ? \get_class($value) : \gettype($value)
);
}
} else {
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
$access[self::ACCESS_NAME] = sprintf(

View File

@ -0,0 +1,65 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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;
/**
* Notice we don't have getter/setter for emails
* because we count on adder/remover.
*/
class TestSingularAndPluralProps
{
/** @var string|null */
private $email;
/** @var array */
private $emails = array();
/**
* @return string|null
*/
public function getEmail()
{
return $this->email;
}
/**
* @param string|null $email
*/
public function setEmail($email)
{
$this->email = $email;
}
/**
* @return array
*/
public function getEmails()
{
return $this->emails;
}
/**
* @param string $email
*/
public function addEmail($email)
{
$this->emails[] = $email;
}
/**
* @param string $email
*/
public function removeEmail($email)
{
$this->emails = array_diff($this->emails, array($email));
}
}

View File

@ -22,6 +22,7 @@ use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicCall;
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassMagicGet;
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassSetValue;
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestClassTypeErrorInsideCall;
use Symfony\Component\PropertyAccess\Tests\Fixtures\TestSingularAndPluralProps;
use Symfony\Component\PropertyAccess\Tests\Fixtures\Ticket5775Object;
use Symfony\Component\PropertyAccess\Tests\Fixtures\TypeHinted;
@ -686,4 +687,26 @@ class PropertyAccessorTest extends TestCase
$this->propertyAccessor->setValue($object, 'name', 'foo');
}
public function testWriteToSingularPropertyWhilePluralOneExists()
{
$object = new TestSingularAndPluralProps();
$this->propertyAccessor->isWritable($object, 'email'); //cache access info
$this->propertyAccessor->setValue($object, 'email', 'test@email.com');
self::assertEquals('test@email.com', $object->getEmail());
self::assertEmpty($object->getEmails());
}
public function testWriteToPluralPropertyWhileSingularOneExists()
{
$object = new TestSingularAndPluralProps();
$this->propertyAccessor->isWritable($object, 'emails'); //cache access info
$this->propertyAccessor->setValue($object, 'emails', array('test@email.com'));
self::assertEquals(array('test@email.com'), $object->getEmails());
self::assertNull($object->getEmail());
}
}