From d68ec990ff7a4a018baf6c8b9714cc6220333d8f Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Wed, 2 May 2018 08:21:23 +0200 Subject: [PATCH 1/3] Do not use property metadata to get/set object values --- src/Accessor/DefaultAccessorStrategy.php | 53 +++++++++++++++++- src/Accessor/ExpressionAccessorStrategy.php | 59 --------------------- src/Metadata/ExpressionPropertyMetadata.php | 9 ---- src/Metadata/PropertyMetadata.php | 19 ------- src/Metadata/StaticPropertyMetadata.php | 5 -- src/Metadata/VirtualPropertyMetadata.php | 7 --- src/SerializerBuilder.php | 6 +-- tests/Metadata/ClassMetadataTest.php | 4 -- 8 files changed, 52 insertions(+), 110 deletions(-) delete mode 100644 src/Accessor/ExpressionAccessorStrategy.php diff --git a/src/Accessor/DefaultAccessorStrategy.php b/src/Accessor/DefaultAccessorStrategy.php index a23f2239e..0e008dea6 100644 --- a/src/Accessor/DefaultAccessorStrategy.php +++ b/src/Accessor/DefaultAccessorStrategy.php @@ -20,21 +20,70 @@ namespace JMS\Serializer\Accessor; +use JMS\Serializer\Exception\ExpressionLanguageRequiredException; +use JMS\Serializer\Expression\ExpressionEvaluatorInterface; +use JMS\Serializer\Metadata\ExpressionPropertyMetadata; use JMS\Serializer\Metadata\PropertyMetadata; +use JMS\Serializer\Metadata\StaticPropertyMetadata; /** * @author Asmir Mustafic */ final class DefaultAccessorStrategy implements AccessorStrategyInterface { + private $readAccessors = array(); + private $writeAccessors = array(); + + /** + * @var ExpressionEvaluatorInterface + */ + private $evaluator; + + public function __construct(ExpressionEvaluatorInterface $evaluator = null) + { + $this->evaluator = $evaluator; + } public function getValue(object $object, PropertyMetadata $metadata) { - return $metadata->getValue($object); + if ($metadata instanceof StaticPropertyMetadata) { + return $metadata->getValue(null); + } + + if ($metadata instanceof ExpressionPropertyMetadata) { + if ($this->evaluator === null) { + throw new ExpressionLanguageRequiredException(sprintf('The property %s on %s requires the expression accessor strategy to be enabled.', $metadata->name, $metadata->class)); + } + + return $this->evaluator->evaluate($metadata->expression, ['object' => $object]); + } + + if (null === $metadata->getter) { + if (!isset($this->readAccessors[$metadata->class])) { + $this->readAccessors[$metadata->class] = \Closure::bind(function ($o, $name) { + return $o->$name; + }, null, $metadata->class); + } + + return $this->readAccessors[$metadata->class]($object, $metadata->name); + } + + return $object->{$metadata->getter}(); } public function setValue(object $object, $value, PropertyMetadata $metadata): void { - $metadata->setValue($object, $value); + if (null === $metadata->setter) { + if (!isset($this->writeAccessors[$metadata->class])) { + $this->writeAccessors[$metadata->class] = \Closure::bind(function ($o, $name, $value) { + $o->$name = $value; + }, null, $metadata->class); + } + + $this->writeAccessors[$metadata->class]($object, $metadata->name, $value); + return; + } + + $object->{$metadata->setter}($value); } } diff --git a/src/Accessor/ExpressionAccessorStrategy.php b/src/Accessor/ExpressionAccessorStrategy.php deleted file mode 100644 index ec9dd00ab..000000000 --- a/src/Accessor/ExpressionAccessorStrategy.php +++ /dev/null @@ -1,59 +0,0 @@ - - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -namespace JMS\Serializer\Accessor; - -use JMS\Serializer\Expression\ExpressionEvaluatorInterface; -use JMS\Serializer\Metadata\ExpressionPropertyMetadata; -use JMS\Serializer\Metadata\PropertyMetadata; - -/** - * @author Asmir Mustafic - */ -final class ExpressionAccessorStrategy implements AccessorStrategyInterface -{ - /** - * @var AccessorStrategyInterface - */ - private $fallback; - /** - * @var ExpressionEvaluatorInterface - */ - private $evaluator; - - public function __construct(ExpressionEvaluatorInterface $evaluator, AccessorStrategyInterface $fallback) - { - $this->fallback = $fallback; - $this->evaluator = $evaluator; - } - - public function getValue(object $object, PropertyMetadata $metadata) - { - if ($metadata instanceof ExpressionPropertyMetadata) { - return $this->evaluator->evaluate($metadata->expression, ['object' => $object]); - } - return $this->fallback->getValue($object, $metadata); - } - - public function setValue(object $object, $value, PropertyMetadata $metadata): void - { - $this->fallback->setValue($object, $value, $metadata); - } -} diff --git a/src/Metadata/ExpressionPropertyMetadata.php b/src/Metadata/ExpressionPropertyMetadata.php index 700f7abf8..497bef964 100644 --- a/src/Metadata/ExpressionPropertyMetadata.php +++ b/src/Metadata/ExpressionPropertyMetadata.php @@ -48,15 +48,6 @@ public function setAccessor($type, $getter = null, $setter = null) { } - /** - * @param object $object - * @return mixed - */ - public function getValue($object) - { - throw new ExpressionLanguageRequiredException(sprintf('The property %s on %s requires the expression accessor strategy to be enabled.', $this->name, $this->class)); - } - public function setValue($obj, $value) { throw new LogicException('ExpressionPropertyMetadata is immutable.'); diff --git a/src/Metadata/PropertyMetadata.php b/src/Metadata/PropertyMetadata.php index 270bf9f88..3c57bca5e 100644 --- a/src/Metadata/PropertyMetadata.php +++ b/src/Metadata/PropertyMetadata.php @@ -53,14 +53,9 @@ class PropertyMetadata extends BasePropertyMetadata public $maxDepth = null; public $excludeIf = null; - private $closureAccessor; - public function __construct($class, $name) { parent::__construct($class, $name); - $this->closureAccessor = \Closure::bind(function ($o, $name) { - return $o->$name; - }, null, $class); } public function setAccessor($type, $getter = null, $setter = null) @@ -93,16 +88,6 @@ public function setAccessor($type, $getter = null, $setter = null) $this->setter = $setter; } - public function getValue($obj) - { - if (null === $this->getter) { - $accessor = $this->closureAccessor; - return $accessor($obj, $this->name); - } - - return $obj->{$this->getter}(); - } - public function setValue($obj, $value) { if (null === $this->setter) { @@ -190,9 +175,5 @@ public function unserialize($str) } parent::unserialize($parentStr); - - $this->closureAccessor = \Closure::bind(function ($o, $name) { - return $o->$name; - }, null, $this->class); } } diff --git a/src/Metadata/StaticPropertyMetadata.php b/src/Metadata/StaticPropertyMetadata.php index 5d0e777f5..50f0a91c4 100644 --- a/src/Metadata/StaticPropertyMetadata.php +++ b/src/Metadata/StaticPropertyMetadata.php @@ -41,11 +41,6 @@ public function getValue($obj) return $this->value; } - public function setValue($obj, $value) - { - throw new LogicException('StaticPropertyMetadata is immutable.'); - } - public function setAccessor($type, $getter = null, $setter = null) { } diff --git a/src/Metadata/VirtualPropertyMetadata.php b/src/Metadata/VirtualPropertyMetadata.php index 5304291cc..ba5482bec 100644 --- a/src/Metadata/VirtualPropertyMetadata.php +++ b/src/Metadata/VirtualPropertyMetadata.php @@ -20,8 +20,6 @@ namespace JMS\Serializer\Metadata; -use JMS\Serializer\Exception\LogicException; - class VirtualPropertyMetadata extends PropertyMetadata { public function __construct($class, $methodName) @@ -38,11 +36,6 @@ public function __construct($class, $methodName) $this->readOnly = true; } - public function setValue($obj, $value) - { - throw new LogicException('VirtualPropertyMetadata is immutable.'); - } - public function setAccessor($type, $getter = null, $setter = null) { } diff --git a/src/SerializerBuilder.php b/src/SerializerBuilder.php index 56905bdba..7d8bff09c 100644 --- a/src/SerializerBuilder.php +++ b/src/SerializerBuilder.php @@ -133,11 +133,7 @@ public function setAccessorStrategy(AccessorStrategyInterface $accessorStrategy) private function getAccessorStrategy(): AccessorStrategyInterface { if (!$this->accessorStrategy) { - $this->accessorStrategy = new DefaultAccessorStrategy(); - - if ($this->expressionEvaluator) { - $this->accessorStrategy = new ExpressionAccessorStrategy($this->expressionEvaluator, $this->accessorStrategy); - } + $this->accessorStrategy = new DefaultAccessorStrategy($this->expressionEvaluator); } return $this->accessorStrategy; } diff --git a/tests/Metadata/ClassMetadataTest.php b/tests/Metadata/ClassMetadataTest.php index 5cafee194..93878e26d 100644 --- a/tests/Metadata/ClassMetadataTest.php +++ b/tests/Metadata/ClassMetadataTest.php @@ -80,10 +80,6 @@ public function testAccessorTypePublicMethod($property, $getterInit, $setterInit self::assertEquals($getterName, $metadata->getter); self::assertEquals($setterName, $metadata->setter); - - $metadata->setValue($object, 'x'); - - self::assertEquals(sprintf('%1$s:%1$s:x', strtoupper($property)), $metadata->getValue($object)); } /** From 901d7b96f4bd1ee9b141e198a102e5bdf492fa3f Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Wed, 2 May 2018 08:59:44 +0200 Subject: [PATCH 2/3] better error handling --- src/Accessor/DefaultAccessorStrategy.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Accessor/DefaultAccessorStrategy.php b/src/Accessor/DefaultAccessorStrategy.php index 0e008dea6..bbab6d32a 100644 --- a/src/Accessor/DefaultAccessorStrategy.php +++ b/src/Accessor/DefaultAccessorStrategy.php @@ -21,10 +21,12 @@ namespace JMS\Serializer\Accessor; use JMS\Serializer\Exception\ExpressionLanguageRequiredException; +use JMS\Serializer\Exception\LogicException; use JMS\Serializer\Expression\ExpressionEvaluatorInterface; use JMS\Serializer\Metadata\ExpressionPropertyMetadata; use JMS\Serializer\Metadata\PropertyMetadata; use JMS\Serializer\Metadata\StaticPropertyMetadata; +use JMS\Serializer\Metadata\VirtualPropertyMetadata; /** * @author Asmir Mustafic @@ -73,6 +75,13 @@ public function getValue(object $object, PropertyMetadata $metadata) public function setValue(object $object, $value, PropertyMetadata $metadata): void { + if ($metadata instanceof StaticPropertyMetadata || $metadata instanceof VirtualPropertyMetadata) { + throw new LogicException(sprintf('%s on %s is immutable.'), $metadata->name, $metadata->class); + } + if ($metadata->readOnly) { + throw new LogicException(sprintf('%s on %s is read only.'), $metadata->name, $metadata->class); + } + if (null === $metadata->setter) { if (!isset($this->writeAccessors[$metadata->class])) { $this->writeAccessors[$metadata->class] = \Closure::bind(function ($o, $name, $value) { From 7b8f4ab5586a23e25fa061c5b1602852e8173de9 Mon Sep 17 00:00:00 2001 From: Asmir Mustafic Date: Wed, 2 May 2018 09:01:36 +0200 Subject: [PATCH 3/3] better error handling --- src/Accessor/DefaultAccessorStrategy.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Accessor/DefaultAccessorStrategy.php b/src/Accessor/DefaultAccessorStrategy.php index bbab6d32a..9064afa09 100644 --- a/src/Accessor/DefaultAccessorStrategy.php +++ b/src/Accessor/DefaultAccessorStrategy.php @@ -75,9 +75,6 @@ public function getValue(object $object, PropertyMetadata $metadata) public function setValue(object $object, $value, PropertyMetadata $metadata): void { - if ($metadata instanceof StaticPropertyMetadata || $metadata instanceof VirtualPropertyMetadata) { - throw new LogicException(sprintf('%s on %s is immutable.'), $metadata->name, $metadata->class); - } if ($metadata->readOnly) { throw new LogicException(sprintf('%s on %s is read only.'), $metadata->name, $metadata->class); }