Merge pull request #1997 from soyuka/fix-metadata

fix #1980 user defined property metadata takes precedence
api-platform · Aug 23, 2018 · 3fdb58c · 3fdb58c
2 parents ebe842f + f7b9079
commit 3fdb58c
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 20 deletions.
diff --git 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
@@ -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
@@ -29,7 +29,7 @@
         </service>
         -->
 
-        <service id="api_platform.metadata.property.metadata_factory.annotation" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="40" class="ApiPlatform\Core\Metadata\Property\Factory\AnnotationPropertyMetadataFactory" public="false">
+        <service id="api_platform.metadata.property.metadata_factory.annotation" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="20" class="ApiPlatform\Core\Metadata\Property\Factory\AnnotationPropertyMetadataFactory" public="false">
             <argument type="service" id="annotation_reader" />
             <argument type="service" id="api_platform.metadata.property.metadata_factory.annotation.inner" />
         </service>

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/xml.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/xml.xml
@@ -23,7 +23,7 @@
         </service>
 
         <service id="api_platform.metadata.property.metadata_factory" alias="api_platform.metadata.property.metadata_factory.xml" />
-        <service id="api_platform.metadata.property.metadata_factory.xml" class="ApiPlatform\Core\Metadata\Property\Factory\ExtractorPropertyMetadataFactory" public="false">
+        <service id="api_platform.metadata.property.metadata_factory.xml" class="ApiPlatform\Core\Metadata\Property\Factory\ExtractorPropertyMetadataFactory" decoration-priority="20" public="false">
             <argument type="service" id="api_platform.metadata.extractor.xml" />
         </service>
     </services>

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml
@@ -22,7 +22,7 @@
             <argument type="service" id="api_platform.metadata.property.name_collection_factory.yaml.inner" />
         </service>
 
-        <service id="api_platform.metadata.property.metadata_factory.yaml" class="ApiPlatform\Core\Metadata\Property\Factory\ExtractorPropertyMetadataFactory" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="40" public="false">
+        <service id="api_platform.metadata.property.metadata_factory.yaml" class="ApiPlatform\Core\Metadata\Property\Factory\ExtractorPropertyMetadataFactory" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="20" public="false">
             <argument type="service" id="api_platform.metadata.extractor.yaml" />
             <argument type="service" id="api_platform.metadata.property.metadata_factory.yaml.inner" />
         </service>

diff --git a/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php b/src/Metadata/Property/Factory/AnnotationPropertyMetadataFactory.php
@@ -128,11 +128,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/ExtractorPropertyMetadataFactory.php b/src/Metadata/Property/Factory/ExtractorPropertyMetadataFactory.php
@@ -107,13 +107,17 @@ 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;
             }
 
             $propertyMetadata = $propertyMetadata->{'with'.ucfirst($metadataKey)}($metadata[$metadataKey]);
         }
 
+        if ($propertyMetadata->hasSubresource()) {
+            return $propertyMetadata;
+        }
+
         return $propertyMetadata->withSubresource($this->createSubresourceMetadata($metadata['subresource'], $propertyMetadata));
     }
 

diff --git a/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php b/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php
@@ -93,9 +93,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;
@@ -109,7 +106,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
@@ -121,8 +118,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
@@ -0,0 +1,62 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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
@@ -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
@@ -50,12 +50,16 @@ public function decoratedPropertyMetadataProvider()
             'description' => 'The dummy foo',
             'readable' => true,
             'writable' => true,
-            'readableLink' => true,
+            'readableLink' => false,
             'writableLink' => false,
             'required' => true,
             'identifier' => false,
-            'attributes' => ['Foo'],
-            'subresource' => new SubresourceMetadata('Foo', true, 1),
+            'attributes' => [
+                'foo' => ['Foo'],
+                'bar' => [['Bar'], 'baz' => 'Baz'],
+                'baz' => 'Baz',
+            ],
+            'subresource' => new SubresourceMetadata('Foo', false, null),
         ];
 
         return [[$this->getPropertyMetadata($metadata)]];