Skip to content

Commit 9b9b64c

Browse files
xabbuhxabbuh
committed
calculate cache keys for property setters depending on the value
1 parent 0878006 commit 9b9b64c

File tree

3 files changed

+83
-19
lines changed

3 files changed

+83
-19
lines changed

‎src/Symfony/Component/PropertyAccess/PropertyAccessor.php

+34-17
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,8 @@ private function writeCollection($zval, $property, $collection, $addMethod, $rem
687687
*/
688688
private function getWriteAccessInfo($class, $property, $value)
689689
{
690-
$key = false !== strpbrk($key = $class.'..'.$property, '{}()/@:') ? rawurlencode($key) : $key;
690+
$useAdderAndRemover = \is_array($value) || $value instanceof \Traversable;
691+
$key = false !== strpbrk($key = $class.'..'.$property.''.var_export($useAdderAndRemover, true), '{}()/@:') ? rawurlencode($key) : $key;
691692

692693
if (isset($this->writePropertyCache[$key])) {
693694
return $this->writePropertyCache[$key];
@@ -707,6 +708,16 @@ private function getWriteAccessInfo($class, $property, $value)
707708
$camelized = $this->camelize($property);
708709
$singulars = (array) Inflector::singularize($camelized);
709710

711+
if ($useAdderAndRemover) {
712+
$methods = $this->findAdderAndRemover($reflClass, $singulars);
713+
714+
if (null !== $methods) {
715+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
716+
$access[self::ACCESS_ADDER] = $methods[0];
717+
$access[self::ACCESS_REMOVER] = $methods[1];
718+
}
719+
}
720+
710721
if (!isset($access[self::ACCESS_TYPE])) {
711722
$setter = 'set'.$camelized;
712723
$getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item)
@@ -728,22 +739,16 @@ private function getWriteAccessInfo($class, $property, $value)
728739
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC;
729740
$access[self::ACCESS_NAME] = $setter;
730741
} elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) {
731-
if (\is_array($value) || $value instanceof \Traversable) {
732-
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
733-
$access[self::ACCESS_ADDER] = $methods[0];
734-
$access[self::ACCESS_REMOVER] = $methods[1];
735-
} else {
736-
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
737-
$access[self::ACCESS_NAME] = sprintf(
738-
'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
739-
'the new value must be an array or an instance of \Traversable, '.
740-
'"%s" given.',
741-
$property,
742-
$reflClass->name,
743-
implode('()", "', $methods),
744-
\is_object($value) ? \get_class($value) : \gettype($value)
745-
);
746-
}
742+
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
743+
$access[self::ACCESS_NAME] = sprintf(
744+
'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
745+
'the new value must be an array or an instance of \Traversable, '.
746+
'"%s" given.',
747+
$property,
748+
$reflClass->name,
749+
implode('()", "', $methods),
750+
\is_object($value) ? \get_class($value) : \gettype($value)
751+
);
747752
} else {
748753
$access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
749754
$access[self::ACCESS_NAME] = sprintf(
@@ -783,6 +788,18 @@ private function isPropertyWritable($object, $property)
783788

784789
$access = $this->getWriteAccessInfo(\get_class($object), $property, array());
785790

791+
$isWritable = self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]
792+
|| self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]
793+
|| self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE]
794+
|| (!$access[self::ACCESS_HAS_PROPERTY] && property_exists($object, $property))
795+
|| self::ACCESS_TYPE_MAGIC === $access[self::ACCESS_TYPE];
796+
797+
if ($isWritable) {
798+
return true;
799+
}
800+
801+
$access = $this->getWriteAccessInfo(\get_class($object), $property, '');
802+
786803
return self::ACCESS_TYPE_METHOD === $access[self::ACCESS_TYPE]
787804
|| self::ACCESS_TYPE_PROPERTY === $access[self::ACCESS_TYPE]
788805
|| self::ACCESS_TYPE_ADDER_AND_REMOVER === $access[self::ACCESS_TYPE]

‎src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php

+12-2
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,17 @@ public function testWriteToPluralPropertyWhileSingularOneExists()
715715
$this->propertyAccessor->isWritable($object, 'emails'); //cache access info
716716
$this->propertyAccessor->setValue($object, 'emails', array('test@email.com'));
717717

718-
self::assertEquals(array('test@email.com'), $object->getEmails());
719-
self::assertNull($object->getEmail());
718+
$this->assertEquals(array('test@email.com'), $object->getEmails());
719+
$this->assertNull($object->getEmail());
720+
}
721+
722+
public function testAdderAndRemoverArePreferredOverSetter()
723+
{
724+
$object = new TestPluralAdderRemoverAndSetter();
725+
726+
$this->propertyAccessor->isWritable($object, 'emails'); //cache access info
727+
$this->propertyAccessor->setValue($object, 'emails', array('test@email.com'));
728+
729+
$this->assertEquals(array('test@email.com'), $object->getEmails());
720730
}
721731
}

‎src/Symfony/Component/PropertyAccess/Tests/TestPluralAdderRemoverAndSetter.php

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\PropertyAccess\Tests;
13+
14+
class TestPluralAdderRemoverAndSetter
15+
{
16+
private $emails = array();
17+
18+
public function getEmails()
19+
{
20+
return $this->emails;
21+
}
22+
23+
public function setEmails(array $emails)
24+
{
25+
$this->emails = array('foo@email.com');
26+
}
27+
28+
public function addEmail($email)
29+
{
30+
$this->emails[] = $email;
31+
}
32+
33+
public function removeEmail($email)
34+
{
35+
$this->emails = array_diff($this->emails, array($email));
36+
}
37+
}

0 commit comments

Comments
 (0)