10000 [Serializer] Use PropertyAccessor to access / mutate values in AbstractObjectNormalizer by Korbeil · Pull Request #35498 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Serializer] Use PropertyAccessor to access / mutate values in AbstractObjectNormalizer #35498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rebase, fix tests, review & update CHANGELOG
  • Loading branch information
Korbeil committed Jan 28, 2020
commit 0a92dab7532aef0f6cc1fd5f20509f291d8eec55
5 changes: 5 additions & 0 deletions src/Symfony/Component/PropertyAccess/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

5.1.0
-----

* Linking to PropertyInfo extractor to remove a lot of duplicate code

4.4.0
-----

Expand Down
81 changes: 30 additions & 51 deletions src/Symfony/Component/PropertyAccess/PropertyAccessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,14 @@ class PropertyAccessor implements PropertyAccessorInterface
* Should not be used by application code. Use
* {@link PropertyAccess::createPropertyAccessor()} instead.
*/
public function __construct(bool $magicCall = false, bool $throwExceptionOnInvalidIndex = false, CacheItemPoolInterface $cacheItemPool = null, bool $throwExceptionOnInvalidPropertyPath = true)
public function __construct(bool $magicCall = false, bool $throwExceptionOnInvalidIndex = false, CacheItemPoolInterface $cacheItemPool = null, bool $throwExceptionOnInvalidPropertyPath = true, PropertyReadInfoExtractorInterface $readInfoExtractor = null, PropertyWriteInfoExtractorInterface $writeInfoExtractor = null)
{
$this->magicCall = $magicCall;
$this->ignoreInvalidIndices = !$throwExceptionOnInvalidIndex;
$this->cacheItemPool = $cacheItemPool instanceof NullAdapter ? null : $cacheItemPool; // Replace the NullAdapter by the null value
$this->ignoreInvalidProperty = !$throwExceptionOnInvalidPropertyPath;
$this->readInfoExtractor = $this->writeInfoExtractor = new ReflectionExtractor(
['set'],
['get', 'is', 'has', 'can'],
['add', 'remove'],
false,
ReflectionExtractor::ALLOW_PUBLIC
);
$this->readInfoExtractor = $readInfoExtractor ?? new ReflectionExtractor([], null, null, false);
$this->writeInfoExtractor = $writeInfoExtractor ?? new ReflectionExtractor(['set'], null, null, false);
}

/**
Expand Down Expand Up @@ -391,34 +386,25 @@ private function readProperty(array $zval, string $property, bool $ignoreInvalid
$access = $this->getReadInfo($class, $property);

if (null !== $access) {
if (PropertyReadInfo::TYPE_METHOD === $access->getType()) {
$result[self::VALUE] = $object->{$access->getName()}();
}
$name = $access->getName();
$type = $access->getType();

if (PropertyReadInfo::TYPE_PROPERTY === $access->getType()) {
$result[self::VALUE] = $object->{$access->getName()};
if (PropertyReadInfo::TYPE_METHOD === $type) {
$result[self::VALUE] = $object->$name();
} elseif (PropertyReadInfo::TYPE_PROPERTY === $type) {
$result[self::VALUE] = $object->$name;

if (isset($zval[self::REF]) && $access->canBeReference()) {
$result[self::REF] = &$object->{$access->getName()};
$result[self::REF] = &$object->$name;
}
}
} elseif ($object instanceof \stdClass && property_exists($object, $property)) {
// Needed to support \stdClass instances. We need to explicitly
// exclude $access[self::ACCESS_HAS_PROPERTY], otherwise if
// a *protected* property was found on the class, property_exists()
// returns true, consequently the following line will result in a
// fatal error.

$result[self::VALUE] = $object->$property;
if (isset($zval[self::REF])) {
$result[self::REF] = &$object->$property;
}
} elseif (!$ignoreInvalidProperty) {
throw new NoSuchPropertyException(sprintf(
'Can get a way to read the property "%s" in class "%s".',
$property,
$class
));
throw new NoSuchPropertyException(sprintf('Can\'t get a way to read the property "%s" in class "%s".', $property, $class));
}

// Objects are always passed around by reference
Expand Down Expand Up @@ -494,46 +480,39 @@ private function writeProperty(array $zval, string $property, $value)
$class = \get_class($object);
$mutator = $this->getWriteInfo($class, $property, $value);

if (null !== $mutator) {
if (PropertyWriteInfo::TYPE_METHOD === $mutator->getType()) {
$object->{$mutator->getName()}($value);
}
if (PropertyWriteInfo::TYPE_NONE !== $mutator->getType()) {
$type = $mutator->getType();

if (PropertyWriteInfo::TYPE_PROPERTY === $mutator->getType()) {
if (PropertyWriteInfo::TYPE_METHOD === $type) {
$object->{$mutator->getName()}($value);
} elseif (PropertyWriteInfo::TYPE_PROPERTY === $type) {
$object->{$mutator->getName()} = $value;
}

if (PropertyWriteInfo::TYPE_ADDER_AND_REMOVER === $mutator->getType()) {
} elseif (PropertyWriteInfo::TYPE_ADDER_AND_REMOVER === $type) {
$this->writeCollection($zval, $property, $value, $mutator->getAdderInfo(), $mutator->getRemoverInfo());
}
} elseif ($object instanceof \stdClass && property_exists($object, $property)) {
// Needed to support \stdClass instances. We need to explicitly
// exclude $access[self::ACCESS_HAS_PROPERTY], otherwise if
// a *protected* property was found on the class, property_exists()
// returns true, consequently the following line will result in a
// fatal error.

$object->$property = $value;
} else {
} elseif (!$this->ignoreInvalidProperty) {
if ($mutator->hasErrors()) {
throw new NoSuchPropertyException(implode('. ', $mutator->getErrors()).'.');
}

throw new NoSuchPropertyException(sprintf('Could not determine access type for property "%s" in class "%s".', $property, \get_class($object)));
}
}

/**
* Adjusts a collection-valued property by calling add*() and remove*() methods.
*
* @param array $zval The array containing the object to write to
* @param string $property The property to write
* @param iterable $collection The collection to write
* @param PropertyWriteInfo $addMethod The add*() method
* @param PropertyWriteInfo $removeMethod The remove*() method
*/
private function writeCollection(array $zval, string $property, iterable $collection, PropertyWriteInfo $addMethod, PropertyWriteInfo $removeMethod)
{
// At this point the add and remove methods have been found
$previousValue = $this->readProperty($zval, $property);
$previousValue = $previousValue[self::VALUE];

$removeMethodName = $removeMethod->getName() E864 ;
$addMethodName = $addMethod->getName();

if ($previousValue instanceof \Traversable) {
$previousValue = iterator_to_array($previousValue);
}
Expand All @@ -544,7 +523,7 @@ private function writeCollection(array $zval, string $property, iterable $collec
foreach ($previousValue as $key => $item) {
if (!\in_array($item, $collection, true)) {
unset($previousValue[$key]);
$zval[self::VALUE]->{$removeMethod->getName()}($item);
$zval[self::VALUE]->$removeMethodName($item);
}
}
} else {
Expand All @@ -553,12 +532,12 @@ private function writeCollection(array $zval, string $property, iterable $collec

foreach ($collection as $item) {
if (!$previousValue || !\in_array($item, $previousValue, true)) {
$zval[self::VALUE]->{$addMethod->getName()}($item);
$zval[self::VALUE]->$addMethodName($item);
}
}
}

private function getWriteInfo(string $class, string $property, $value): ?PropertyWriteInfo
private function getWriteInfo(string $class, string $property, $value): PropertyWriteInfo
{
$useAdderAndRemover = \is_array($value) || $value instanceof \Traversable;
$key = str_replace('\\', '.', $class).'..'.$property.'..'.(int) $useAdderAndRemover;
Expand Down Expand Up @@ -601,13 +580,13 @@ private function isPropertyWritable($object, string $property): bool

$mutatorForArray = $this->getWriteInfo(\get_class($object), $property, []);

if (null !== $mutatorForArray || ($object instanceof \stdClass && property_exists($object, $property))) {
if (PropertyWriteInfo::TYPE_NONE !== $mutatorForArray->getType() || ($object instanceof \stdClass && property_exists($object, $property))) {
return true;
}

$mutator = $this->getWriteInfo(\get_class($object), $property, '');

return null !== $mutator || ($object instanceof \stdClass && property_exists($object, $property));
return PropertyWriteInfo::TYPE_NONE !== $mutator->getType() || ($object instanceof \stdClass && property_exists($object, $property));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public function testIsWritableReturnsFalseIfNoAdderNorRemoverExists()
public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable()
{
$this->expectException('Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException');
$this->expectExceptionMessageRegExp('/Could not determine access type for property "axes" in class "Symfony\\\\Component\\\\PropertyAccess\\\\Tests\\\\PropertyAccessorCollectionTest_Car[^"]*": The property "axes" in class "Symfony\\\\Component\\\\PropertyAccess\\\\Tests\\\\PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis\(\)", "removeAxis\(\)" but the new value must be an array or an instance of \\\\Traversable, "string" given./');
$this->expectExceptionMessageRegExp('/The property "axes" in class "Symfony\\\Component\\\PropertyAccess\\\Tests\\\PropertyAccessorCollectionTest_Car" can be defined with the methods "addAxis\(\)", "removeAxis\(\)" but the new value must be an array or an instance of \\\Traversable\./');
$car = new PropertyAccessorCollectionTest_Car();

$this->propertyAccessor->setValue($car, 'axes', 'Not an array or Traversable');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ public function testRemoverWithoutAdder()
public function testAdderAndRemoveNeedsTheExactParametersDefined()
{
$this->expectException('Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException');
$this->expectExceptionMessageRegExp('/.*The method "addFoo" in class "Symfony\\\Component\\\PropertyAccess\\\Tests\\\Fixtures\\\TestAdderRemoverInvalidArgumentLength" requires 0 arguments, but should accept only 1\. The method "removeFoo" in class "Symfony\\\Component\\\PropertyAccess\\\Tests\\\Fixtures\\\TestAdderRemoverInvalidArgumentLength" requires 2 arguments, but should accept only 1\./');
$this->expectExceptionMessageRegExp('/.*The method "addFoo" in class "Symfony\\\Component\\\PropertyAccess\\\Tests\\\Fixtures\\\TestAdderRemoverInvalidArgumentLength" requires 0 arguments, but should accept only 1\./');
$object = new TestAdderRemoverInvalidArgumentLength();
$this->propertyAccessor->setValue($object, 'foo', [1, 2]);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/PropertyAccess/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"require": {
"php": "^7.2.5",
"symfony/inflector": "^4.4|^5.0",
"symfony/property-info": "^4.4|^5.0"
"symfony/property-info": "^5.1"
},
"require-dev": {
"symfony/cache": "^4.4|^5.0"
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/PropertyInfo/CHANGELOG.md
42E0
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

5.1.0
-----

* Add support for extracting accessor and mutator via PHP Reflection

4.3.0
-----

Expand Down
Loading
0