From fa234378ffa9a361dbf429cb4d6464d8a6db1f35 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 27 Nov 2018 11:31:37 +0100 Subject: [PATCH] calculate cache keys for property setters depending on the value --- .../PropertyAccess/PropertyAccessor.php | 51 ++++++++++++------- .../Tests/PropertyAccessorTest.php | 24 ++++++++- .../Tests/TestPluralAdderRemoverAndSetter.php | 37 ++++++++++++++ ...rRemoverAndSetterSameSingularAndPlural.php | 28 ++++++++++ 4 files changed, 121 insertions(+), 19 deletions(-) create mode 100644 src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetter.php create mode 100644 src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetterSameSingularAndPlural.php diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 90e4c234fb..2e358f4586 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -687,7 +687,8 @@ class PropertyAccessor implements PropertyAccessorInterface */ private function getWriteAccessInfo($class, $property, $value) { - $key = str_replace('\\', '.', $class).'..'.$property; + $useAdderAndRemover = \is_array($value) || $value instanceof \Traversable; + $key = str_replace('\\', '.', $class).'..'.$property.'..'.(int) $useAdderAndRemover; if (isset($this->writePropertyCache[$key])) { return $this->writePropertyCache[$key]; @@ -707,6 +708,16 @@ class PropertyAccessor implements PropertyAccessorInterface $camelized = $this->camelize($property); $singulars = (array) Inflector::singularize($camelized); + if ($useAdderAndRemover) { + $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) @@ -728,22 +739,16 @@ class PropertyAccessor implements PropertyAccessorInterface $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC; $access[self::ACCESS_NAME] = $setter; } elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) { - 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) - ); - } + $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( @@ -783,6 +788,18 @@ class PropertyAccessor implements PropertyAccessorInterface $access = $this->getWriteAccessInfo(\get_class($object), $property, array()); + $isWritable = self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE] + || self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE] + || self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE] + || (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property)) + || self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE]; + + if ($isWritable) { + return true; + } + + $access = $this->getWriteAccessInfo(\get_class($object), $property, ''); + return self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE] || self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE] || self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE] diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index cf6152380d..894fbd5c0d 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -719,7 +719,27 @@ class PropertyAccessorTest extends TestCase $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()); + $this->assertEquals(array('test@email.com'), $object->getEmails()); + $this->assertNull($object->getEmail()); + } + + public function testAdderAndRemoverArePreferredOverSetter() + { + $object = new TestPluralAdderRemoverAndSetter(); + + $this->propertyAccessor->isWritable($object, 'emails'); //cache access info + $this->propertyAccessor->setValue($object, 'emails', array('test@email.com')); + + $this->assertEquals(array('test@email.com'), $object->getEmails()); + } + + public function testAdderAndRemoverArePreferredOverSetterForSameSingularAndPlural() + { + $object = new TestPluralAdderRemoverAndSetterSameSingularAndPlural(); + + $this->propertyAccessor->isWritable($object, 'aircraft'); //cache access info + $this->propertyAccessor->setValue($object, 'aircraft', array('aeroplane')); + + $this->assertEquals(array('aeroplane'), $object->getAircraft()); } } diff --git a/src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetter.php b/src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetter.php new file mode 100644 index 0000000000..ecb3f9b4a9 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetter.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\PropertyAccess\Tests; + +class TestPluralAdderRemoverAndSetter +{ + private $emails = array(); + + public function getEmails() + { + return $this->emails; + } + + public function setEmails(array $emails) + { + $this->emails = array('foo@email.com'); + } + + public function addEmail($email) + { + $this->emails[] = $email; + } + + public function removeEmail($email) + { + $this->emails = array_diff($this->emails, array($email)); + } +} diff --git a/src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetterSameSingularAndPlural.php b/src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetterSameSingularAndPlural.php new file mode 100644 index 0000000000..bb3b4f4688 --- /dev/null +++ b/src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetterSameSingularAndPlural.php @@ -0,0 +1,28 @@ +aircraft; + } + + public function setAircraft(array $aircraft) + { + $this->aircraft = array('plane'); + } + + public function addAircraft($aircraft) + { + $this->aircraft[] = $aircraft; + } + + public function removeAircraft($aircraft) + { + $this->aircraft = array_diff($this->aircraft, array($aircraft)); + } +}