From 22535af11be3d514017bc2d176914946bfd232ae Mon Sep 17 00:00:00 2001 From: abluchet Date: Tue, 5 Jun 2018 09:37:30 +0200 Subject: [PATCH] fix #1980 user defined property metadata takes precedence Change priority of user-defined metadata services to 20 User-defined metadata always overrides previous metadata --- features/jsonld/context.feature | 5 +- features/main/operation.feature | 24 +++++++ .../Resources/config/metadata/annotation.xml | 2 +- .../Bundle/Resources/config/metadata/xml.xml | 2 +- .../Bundle/Resources/config/metadata/yaml.xml | 2 +- .../AnnotationPropertyMetadataFactory.php | 5 -- .../Factory/CachedPropertyMetadataFactory.php | 3 +- .../ExtractorPropertyMetadataFactory.php | 2 +- .../SerializerPropertyMetadataFactory.php | 14 +++-- .../Entity/ReadableOnlyProperty.php | 62 +++++++++++++++++++ .../AnnotationPropertyMetadataFactoryTest.php | 2 +- ...leConfigurationMetadataFactoryProvider.php | 8 ++- 12 files changed, 111 insertions(+), 20 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Entity/ReadableOnlyProperty.php diff --git a/features/jsonld/context.feature b/features/jsonld/context.feature index c38cec92121..cfc7e75c375 100644 --- a/features/jsonld/context.feature +++ b/features/jsonld/context.feature @@ -27,7 +27,10 @@ Feature: JSON-LD contexts generation "description": "https://schema.org/description", "dummy": "Dummy/dummy", "dummyBoolean": "Dummy/dummyBoolean", - "dummyDate": "Dummy/dummyDate", + "dummyDate": { + "@id": "Dummy/dummyDate", + "@type": "@id" + }, "dummyFloat": "Dummy/dummyFloat", "dummyPrice": "Dummy/dummyPrice", "relatedDummy": { diff --git a/features/main/operation.feature b/features/main/operation.feature index 89117bfe0d8..5a8aed2549c 100644 --- a/features/main/operation.feature +++ b/features/main/operation.feature @@ -3,6 +3,30 @@ Feature: Operation support As an API developer I need to be able to add custom operations and remove built-in ones + @createSchema + @dropSchema + Scenario: Can not write readonly property + When I add "Content-Type" header equal to "application/ld+json" + And I send a "POST" request to "/readable_only_properties" with body: + """ + { + "name": "My Dummy" + } + """ + 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/ReadableOnlyProperty", + "@id": "/readable_only_properties/1", + "@type": "ReadableOnlyProperty", + "id": 1, + "name": "Read only" + } + """ + Scenario: Access custom operations When I send a "GET" request to "/relation_embedders/42/custom" Then the response status code should be 200 diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml index aa3d2334db7..70556d4f8bc 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml @@ -29,7 +29,7 @@ --> - + diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/xml.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/xml.xml index cac0c2724ae..2662d22ba22 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/metadata/xml.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/metadata/xml.xml @@ -23,7 +23,7 @@ - + diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml index 586bcf79df7..bf172a11930 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml @@ -22,7 +22,7 @@ - + diff --git a/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php b/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php index 3923ded17fa..ed98e5ad71c 100644 --- a/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php +++ b/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php @@ -133,11 +133,6 @@ private function createMetadata(ApiProperty $annotation, PropertyMetadata $paren private function createWith(PropertyMetadata $propertyMetadata, array $property, $value): PropertyMetadata { - $getter = $property[0].ucfirst($property[1]); - if (null !== $propertyMetadata->$getter()) { - return $propertyMetadata; - } - $wither = 'with'.ucfirst($property[1]); return $propertyMetadata->$wither($value); diff --git a/src/Metadata/Property/Factory/CachedPropertyMetadataFactory.php b/src/Metadata/Property/Factory/CachedPropertyMetadataFactory.php index 8d1b9915e59..a67002aa89b 100644 --- a/src/Metadata/Property/Factory/CachedPropertyMetadataFactory.php +++ b/src/Metadata/Property/Factory/CachedPropertyMetadataFactory.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Cache\CachedTrait; use ApiPlatform\Core\Metadata\Property\PropertyMetadata; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; use Psr\Cache\CacheItemPoolInterface; /** @@ -44,7 +45,7 @@ public function create(string $resourceClass, string $property, array $options = $cacheKey = self::CACHE_KEY_PREFIX.md5(serialize([$resourceClass, $property, $options])); return $this->getCached($cacheKey, function () use ($resourceClass, $property, $options) { - return $this->decorated->create($resourceClass, $property, $options); + return $this->decorated->create($resourceClass, $property, $options); }); } } diff --git a/src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php b/src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php index 3a0ef775c32..6548ffdddf2 100644 --- a/src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php +++ b/src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php @@ -117,7 +117,7 @@ private function update(PropertyMetadata $propertyMetadata, array $metadata): Pr ]; foreach ($metadataAccessors as $metadataKey => $accessorPrefix) { - if (null === $metadata[$metadataKey] || null !== $propertyMetadata->{$accessorPrefix.ucfirst($metadataKey)}()) { + if (null === $metadata[$metadataKey]) { continue; } diff --git a/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php b/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php index 47f26cac1b3..0ffd2a5b7b4 100644 --- a/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php +++ b/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php @@ -101,9 +101,6 @@ private function transformReadWrite(PropertyMetadata $propertyMetadata, string $ */ private function transformLinkStatus(PropertyMetadata $propertyMetadata, array $normalizationGroups = null, array $denormalizationGroups = null): PropertyMetadata { - $propertyMetadata = $propertyMetadata->withReadableLink(true); - $propertyMetadata = $propertyMetadata->withWritableLink(true); - // No need to check link status if property is not readable and not writable if (false === $propertyMetadata->isReadable() && false === $propertyMetadata->isWritable()) { return $propertyMetadata; @@ -117,7 +114,7 @@ private function transformLinkStatus(PropertyMetadata $propertyMetadata, array $ $relatedClass = $type->isCollection() && ($collectionValueType = $type->getCollectionValueType()) ? $collectionValueType->getClassName() : $type->getClassName(); if (null === $relatedClass) { - return $propertyMetadata; + return $propertyMetadata->withReadableLink(true)->withWritableLink(true); } // No need to check link status if related class is not a resource @@ -129,8 +126,13 @@ private function transformLinkStatus(PropertyMetadata $propertyMetadata, array $ $relatedGroups = $this->getResourceSerializerGroups($relatedClass); - $propertyMetadata = $propertyMetadata->withReadableLink(null !== $normalizationGroups && !empty(array_intersect($normalizationGroups, $relatedGroups))); - $propertyMetadata = $propertyMetadata->withWritableLink(null !== $denormalizationGroups && !empty(array_intersect($denormalizationGroups, $relatedGroups))); + if (null === $propertyMetadata->isReadableLink()) { + $propertyMetadata = $propertyMetadata->withReadableLink(null !== $normalizationGroups && !empty(array_intersect($normalizationGroups, $relatedGroups))); + } + + if (null === $propertyMetadata->isWritableLink()) { + $propertyMetadata = $propertyMetadata->withWritableLink(null !== $denormalizationGroups && !empty(array_intersect($denormalizationGroups, $relatedGroups))); + } return $propertyMetadata; } diff --git a/tests/Fixtures/TestBundle/Entity/ReadableOnlyProperty.php b/tests/Fixtures/TestBundle/Entity/ReadableOnlyProperty.php new file mode 100644 index 00000000000..f9f883fc5c5 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/ReadableOnlyProperty.php @@ -0,0 +1,62 @@ + + * + * 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\ApiProperty; +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ApiResource + * @ORM\Entity + */ +class ReadableOnlyProperty +{ + /** + * @var int The id + * + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string The foo name + * + * @ORM\Column + * @ApiProperty(writable=false) + */ + private $name; + + public function __construct() + { + $this->name = 'Read only'; + } + + public function getId() + { + return $this->id; + } + + public function setName($name) + { + throw new \Exception('Can not write name.'); + } + + public function getName() + { + return $this->name; + } +} diff --git a/tests/Metadata/Property/Factory/AnnotationPropertyMetadataFactoryTest.php b/tests/Metadata/Property/Factory/AnnotationPropertyMetadataFactoryTest.php index 326518adcd3..021a2a6d1d8 100644 --- a/tests/Metadata/Property/Factory/AnnotationPropertyMetadataFactoryTest.php +++ b/tests/Metadata/Property/Factory/AnnotationPropertyMetadataFactoryTest.php @@ -83,7 +83,7 @@ public function getDependencies() [$propertyReaderProphecy, null, 'description'], [$getterReaderProphecy, $decoratedThrowNotFoundProphecy, 'description'], [$setterReaderProphecy, $decoratedThrowNotFoundProphecy, 'description'], - [$setterReaderProphecy, $decoratedReturnProphecy, 'Hi'], + [$setterReaderProphecy, $decoratedReturnProphecy, 'description'], ]; } diff --git a/tests/Metadata/Property/Factory/FileConfigurationMetadataFactoryProvider.php b/tests/Metadata/Property/Factory/FileConfigurationMetadataFactoryProvider.php index 1144f54e231..462cb2b98f3 100644 --- a/tests/Metadata/Property/Factory/FileConfigurationMetadataFactoryProvider.php +++ b/tests/Metadata/Property/Factory/FileConfigurationMetadataFactoryProvider.php @@ -50,11 +50,15 @@ public function decoratedPropertyMetadataProvider() 'description' => 'The dummy foo', 'readable' => true, 'writable' => true, - 'readableLink' => true, + 'readableLink' => false, 'writableLink' => false, 'required' => true, 'identifier' => false, - 'attributes' => ['Foo'], + 'attributes' => [ + 'foo' => ['Foo'], + 'bar' => [['Bar'], 'baz' => 'Baz'], + 'baz' => 'Baz', + ], 'subresource' => new SubresourceMetadata('Foo', true, 1), ];