From da500b2edce32a56d410f3466eab8194204bb84c Mon Sep 17 00:00:00 2001 From: Maxime Veber Date: Mon, 5 Mar 2018 17:10:15 +0100 Subject: [PATCH] Make resource constructor parameters writables This is motivated because other tools are already constructor friendly (ie Symfony serializer and Doctrine). Also using constructors must be recommended and not supporting them is a serious feature missing. --- ...erialize_objects_using_constructor.feature | 31 +++++ .../PropertyInfoPropertyMetadataFactory.php | 16 +++ .../Serializer/DocumentationNormalizer.php | 2 +- src/Metadata/Property/PropertyMetadata.php | 29 ++++- src/Serializer/AbstractItemNormalizer.php | 6 +- tests/Fixtures/DummyObjectWithConstructor.php | 29 +++++ .../DummyObjectWithoutConstructor.php | 32 +++++ .../Entity/DummyEntityWithConstructor.php | 112 ++++++++++++++++++ .../Property/PropertyMetadataTest.php | 4 + 9 files changed, 257 insertions(+), 4 deletions(-) create mode 100644 features/serializer/deserialize_objects_using_constructor.feature create mode 100644 tests/Fixtures/DummyObjectWithConstructor.php create mode 100644 tests/Fixtures/DummyObjectWithoutConstructor.php create mode 100644 tests/Fixtures/TestBundle/Entity/DummyEntityWithConstructor.php diff --git a/features/serializer/deserialize_objects_using_constructor.feature b/features/serializer/deserialize_objects_using_constructor.feature new file mode 100644 index 00000000000..624590f5ba2 --- /dev/null +++ b/features/serializer/deserialize_objects_using_constructor.feature @@ -0,0 +1,31 @@ +Feature: Resource with constructor deserializable + In order to build non anemic resource object + As a developer + I should be able to deserialize data into objects with constructors + + @createSchema + Scenario: post a resource built with constructor + When I add "Content-Type" header equal to "application/ld+json" + And I send a "POST" request to "/dummy_entity_with_constructors" with body: + """ + { + "foo": "hello", + "bar": "world" + } + """ + Then the response status code should be 201 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" + And the JSON should be equal to: + """ + { + "@context": "/contexts/DummyEntityWithConstructor", + "@id": "/dummy_entity_with_constructors/1", + "@type": "DummyEntityWithConstructor", + "id": 1, + "foo": "hello", + "bar": "world", + "baz": null + } + """ + diff --git a/src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyMetadataFactory.php b/src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyMetadataFactory.php index d606943f546..80b6f2f3d47 100644 --- a/src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyMetadataFactory.php +++ b/src/Bridge/Symfony/PropertyInfo/Metadata/Property/PropertyInfoPropertyMetadataFactory.php @@ -68,6 +68,22 @@ public function create(string $resourceClass, string $name, array $options = []) $propertyMetadata = $propertyMetadata->withWritable($writable); } + if (method_exists($this->propertyInfo, 'isInitializable')) { + if (null === $propertyMetadata->isInitializable() && null !== $initializable = $this->propertyInfo->isInitializable($resourceClass, $name, $options)) { + $propertyMetadata = $propertyMetadata->withInitializable($initializable); + } + } else { + // BC layer for Symfony < 4.2 + $ref = new \ReflectionClass($resourceClass); + if ($ref->isInstantiable() && $constructor = $ref->getConstructor()) { + foreach ($constructor->getParameters() as $constructorParameter) { + if ($constructorParameter->name === $name && null === $propertyMetadata->isInitializable()) { + $propertyMetadata = $propertyMetadata->withInitializable(true); + } + } + } + } + return $propertyMetadata; } } diff --git a/src/Hydra/Serializer/DocumentationNormalizer.php b/src/Hydra/Serializer/DocumentationNormalizer.php index c4957272d05..fd90ecfc02b 100644 --- a/src/Hydra/Serializer/DocumentationNormalizer.php +++ b/src/Hydra/Serializer/DocumentationNormalizer.php @@ -507,7 +507,7 @@ private function getProperty(PropertyMetadata $propertyMetadata, string $propert 'hydra:title' => $propertyName, 'hydra:required' => $propertyMetadata->isRequired(), 'hydra:readable' => $propertyMetadata->isReadable(), - 'hydra:writable' => $propertyMetadata->isWritable(), + 'hydra:writable' => $propertyMetadata->isWritable() || $propertyMetadata->isInitializable(), ]; if (null !== $range = $this->getRange($propertyMetadata)) { diff --git a/src/Metadata/Property/PropertyMetadata.php b/src/Metadata/Property/PropertyMetadata.php index 5d29af8bb87..1488f928578 100644 --- a/src/Metadata/Property/PropertyMetadata.php +++ b/src/Metadata/Property/PropertyMetadata.php @@ -34,8 +34,9 @@ final class PropertyMetadata private $childInherited; private $attributes; private $subresource; + private $initializable; - public function __construct(Type $type = null, string $description = null, bool $readable = null, bool $writable = null, bool $readableLink = null, bool $writableLink = null, bool $required = null, bool $identifier = null, string $iri = null, $childInherited = null, array $attributes = null, SubresourceMetadata $subresource = null) + public function __construct(Type $type = null, string $description = null, bool $readable = null, bool $writable = null, bool $readableLink = null, bool $writableLink = null, bool $required = null, bool $identifier = null, string $iri = null, $childInherited = null, array $attributes = null, SubresourceMetadata $subresource = null, bool $initializable = null) { $this->type = $type; $this->description = $description; @@ -49,6 +50,7 @@ public function __construct(Type $type = null, string $description = null, bool $this->childInherited = $childInherited; $this->attributes = $attributes; $this->subresource = $subresource; + $this->initializable = $initializable; } /** @@ -381,4 +383,29 @@ public function withSubresource(SubresourceMetadata $subresource = null): self return $metadata; } + + /** + * Is initializable? + * + * @return bool|null + */ + public function isInitializable() + { + return $this->initializable; + } + + /** + * Returns a new instance with the given initializable flag. + * + * @param bool $initializable + * + * @return self + */ + public function withInitializable(bool $initializable): self + { + $metadata = clone $this; + $metadata->initializable = $initializable; + + return $metadata; + } } diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 376fdaed169..22bbd1aa8ff 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -142,8 +142,10 @@ protected function getAllowedAttributes($classOrObject, array $context, $attribu if ( $this->isAllowedAttribute($classOrObject, $propertyName, null, $context) && - ((isset($context['api_normalize']) && $propertyMetadata->isReadable()) || - (isset($context['api_denormalize']) && $propertyMetadata->isWritable())) + ( + isset($context['api_normalize']) && $propertyMetadata->isReadable() || + isset($context['api_denormalize']) && ($propertyMetadata->isWritable() || !is_object($classOrObject) && $propertyMetadata->isInitializable()) + ) ) { $allowedAttributes[] = $propertyName; } diff --git a/tests/Fixtures/DummyObjectWithConstructor.php b/tests/Fixtures/DummyObjectWithConstructor.php new file mode 100644 index 00000000000..6f8ca93f609 --- /dev/null +++ b/tests/Fixtures/DummyObjectWithConstructor.php @@ -0,0 +1,29 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures; + +/** + * @author Maxime Veber + */ +class DummyObjectWithConstructor +{ + private $foo; + private $bar; + + public function __construct(string $foo, \stdClass $bar) + { + $this->foo = $foo; + $this->bar = $bar; + } +} diff --git a/tests/Fixtures/DummyObjectWithoutConstructor.php b/tests/Fixtures/DummyObjectWithoutConstructor.php new file mode 100644 index 00000000000..3bec226960e --- /dev/null +++ b/tests/Fixtures/DummyObjectWithoutConstructor.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures; + +/** + * @author Maxime Veber + */ +class DummyObjectWithoutConstructor +{ + private $foo; + + public function getFoo() + { + return $this->foo; + } + + public function setFoo($foo) + { + $this->foo = $foo; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/DummyEntityWithConstructor.php b/tests/Fixtures/TestBundle/Entity/DummyEntityWithConstructor.php new file mode 100644 index 00000000000..ad4da9e8202 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DummyEntityWithConstructor.php @@ -0,0 +1,112 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Serializer\Annotation\Groups; + +/** + * Dummy entity built with constructor. + * https://github.com/api-platform/core/issues/1747. + * + * @author Maxime Veber + * + * @ApiResource( + * itemOperations={ + * "get", + * "put"={"denormalization_context"={"groups"={"put"}}} + * } + * ) + * @ORM\Entity + */ +class DummyEntityWithConstructor +{ + /** + * @var int The id + * + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string + * + * @ORM\Column + */ + private $foo; + + /** + * @var string + * + * @ORM\Column + */ + private $bar; + + /** + * @var string + * + * @ORM\Column(nullable=true) + * @Groups({"put"}) + */ + private $baz; + + public function __construct(string $foo, string $bar) + { + $this->foo = $foo; + $this->bar = $bar; + } + + /** + * @return int + */ + public function getId(): int + { + return $this->id; + } + + /** + * @return string + */ + public function getFoo(): string + { + return $this->foo; + } + + /** + * @return string + */ + public function getBar(): string + { + return $this->bar; + } + + /** + * @return string + */ + public function getBaz() + { + return $this->baz; + } + + /** + * @param string $baz + */ + public function setBaz(string $baz) + { + $this->baz = $baz; + } +} diff --git a/tests/Metadata/Property/PropertyMetadataTest.php b/tests/Metadata/Property/PropertyMetadataTest.php index dcc1bf839f4..afee7f6adb6 100644 --- a/tests/Metadata/Property/PropertyMetadataTest.php +++ b/tests/Metadata/Property/PropertyMetadataTest.php @@ -78,6 +78,10 @@ public function testValueObject() $this->assertNotSame($metadata, $newMetadata); $this->assertEquals(['a' => 'b'], $newMetadata->getAttributes()); $this->assertEquals('b', $newMetadata->getAttribute('a')); + + $newMetadata = $metadata->withInitializable(true); + $this->assertNotSame($metadata, $newMetadata); + $this->assertTrue($newMetadata->isInitializable()); } public function testShouldReturnRequiredFalseWhenRequiredTrueIsSetButMaskedByWritableFalse()