From 8238f167ada7fd5f0710f83cb098022edb4b0817 Mon Sep 17 00:00:00 2001 From: karser Date: Wed, 24 Oct 2018 13:47:17 +0300 Subject: [PATCH] [PropertyAccessor] Fix unable to write to singular property using setter while plural adder/remover exist --- .../PropertyAccess/PropertyAccessor.php | 36 +++++----- .../Fixtures/TestSingularAndPluralProps.php | 65 +++++++++++++++++++ .../Tests/PropertyAccessorTest.php | 23 +++++++ 3 files changed, 104 insertions(+), 20 deletions(-) create mode 100644 src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 33763a73c8..79f9cf6b2e 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -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( diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php new file mode 100644 index 0000000000..db17f3f612 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestSingularAndPluralProps.php @@ -0,0 +1,65 @@ + + * + * 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)); + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 07e5e2fb52..ef60e26dcd 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -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()); + } }