Deserialize in a view listener #584

teohhanhui · 2016-06-27T04:55:58Z

I don't see the value of having this. It's far too easy to create an empty action, so there's no benefit of reuse here. This should just be PostCollectionAction, as before... And we will be free to change it maybe to do something later on without the name tying it down.

It allows to create custom operations triggering the event system with only config. It's why I've renamed it. Why not doing it now?

Why not go all the way and remove all other actions? 😆

So we will end up with an event listener which fetches data from the data provider, and another which performs the deserialization (if applicable).

What is the benefit of getting data from a listener (which is triggered at every request) instead of in an action?

You're right. We should probably not try to fight against how Symfony works.

The benefit is turtles all the way (kind of... at least, events all the way). And we sidestep the confusion of what the controller is supposed to do by consistently leaving it out of the picture (as a no-op).

Can we try that in another PR?

teohhanhui · 2016-06-27T06:29:00Z

http://symfony.com/doc/current/components/http_kernel/introduction.html#the-kernel-controller-event

A controller must return something. If a controller returns null, an exception will be thrown immediately.

Seems like the documentation is not right then?

Interesting, it doesn't throw an exception as highlighted by tests (so there is an error in the doc anyway). However maybe should we return another value than null, I honestly don't know.

I think this is the same dilemma as above. If we zoom out and look at the bigger picture it's a bigger dilemma, which might not have been obvious to us all this while.

The kernel.view event is the stage where some kind of domain object (or an array of them) is being turned into a Response. When we do the deserialization in kernel.view we seem to have gone against that expectation (but I'm not sure if we have any better option 😆). FWIW @ParamConverter uses the kernel.controller event (this should be the equivalent of our data provider step).

Yes, the kernel.view is badly named, but it do exactly what we want :)

-Original file line number
+Diff line change
@@ Expand Up / @@ -11,6 +11,7 @@ @@
     namespace ApiPlatform\Core\Action;
+    use ApiPlatform\Core\Api\RequestAttributesExtractor;
     use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
     use ApiPlatform\Core\DataProvider\PaginatorInterface;
     use ApiPlatform\Core\Exception\RuntimeException;
@@ Expand All / @@ -23,8 +24,6 @@ @@
      */
     final class GetCollectionAction
     {
-        use ActionUtilTrait;
         private $collectionDataProvider;
         public function __construct(CollectionDataProviderInterface $collectionDataProvider)
@@ Expand All @@
          */
         public function __invoke(Request $request)
         {
-            list($resourceClass, $operationName) = $this->extractAttributes($request);
+            list($resourceClass, $operationName) = RequestAttributesExtractor::extractAttributes($request);
             return $this->collectionDataProvider->getCollection($resourceClass, $operationName);
         }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -11,6 +11,7 @@ @@
     namespace ApiPlatform\Core\Action;
+    use ApiPlatform\Core\Api\RequestAttributesExtractor;
     use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
     use ApiPlatform\Core\Exception\RuntimeException;
     use Symfony\Component\HttpFoundation\Request;
@@ Expand All / @@ -23,8 +24,6 @@ @@
      */
     final class GetItemAction
     {
-        use ActionUtilTrait;
         private $itemDataProvider;
         public function __construct(ItemDataProviderInterface $itemDataProvider)
@@ Expand All @@
          */
         public function __invoke(Request $request, $id)
         {
-            list($resourceClass, , $operationName) = $this->extractAttributes($request);
+            list($resourceClass, , $operationName) = RequestAttributesExtractor::extractAttributes($request);
+            $data = $this->itemDataProvider->getItem($resourceClass, $id, $operationName, true);
+            if (!$data) {
+                throw new NotFoundHttpException('Not Found');
+            }
-            return $this->getItem($this->itemDataProvider, $resourceClass, $operationName, $id);
+            return $data;
         }
     }

-Original file line number
+Diff line change
@@ -0,0 +1,25 @@
+    <?php
+    /*
+     * This file is part of the API Platform project.
+     *
+     * (c) Kévin Dunglas <dunglas@gmail.com>
+     *
+     * For the full copyright and license information, please view the LICENSE
+     * file that was distributed with this source code.
+     */
+    namespace ApiPlatform\Core\Action;
+    /**
+     * Empty action. Useful to trigger the kernel.view event without doing anything specific in the action
+     * (e.g. the POST action).
+     *
+     * @author Kévin Dunglas <dunglas@gmail.com>
+     */
+    final class NullAction
+    {
+        public function __invoke()
+        {
+        }
+    }

-Original file line number
+Diff line change
@@ Expand Up / @@ -9,44 +9,20 @@ @@
      * file that was distributed with this source code.
      */
-    namespace ApiPlatform\Core\Action;
+    namespace ApiPlatform\Core\Api;
-    use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
     use ApiPlatform\Core\Exception\RuntimeException;
     use Symfony\Component\HttpFoundation\Request;
-    use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
     /**
-     * Checks if the request is properly configured.
+     * Extracts data used by the library form a Request instance.
      *
      * @author Kévin Dunglas <dunglas@gmail.com>
      */
-    trait ActionUtilTrait
+    final class RequestAttributesExtractor
     {
         /**
-         * Gets an item using the data provider. Throws a 404 error if not found.
-         *
-         * @param ItemDataProviderInterface $itemDataProvider
-         * @param string                    $resourceClass
-         * @param string                    $operationName
-         * @param string|int                $id
-         *
-         * @throws NotFoundHttpException
-         *
-         * @return object
-         */
-        private function getItem(ItemDataProviderInterface $itemDataProvider, string $resourceClass, string $operationName, $id)
-        {
-            $data = $itemDataProvider->getItem($resourceClass, $id, $operationName, true);
-            if (!$data) {
-                throw new NotFoundHttpException('Not Found');
-            }
-            return $data;
-        }
-        /**
-         * Extract resource class, operation name and format request attributes. Throws an exception if the request does not contain required
+         * Extracts resource class, operation name and format request attributes. Throws an exception if the request does not contain required
          * attributes.
          *
          * @param Request $request
@@ Expand All @@
          *
          * @return array
          */
-        private function extractAttributes(Request $request)
+        public static function extractAttributes(Request $request)
         {
             $resourceClass = $request->attributes->get('_resource_class');
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -58,6 +58,12 @@ @@
                 <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" />
             </service>
+            <service id="api_platform.listener.view.deserializer" class="ApiPlatform\Core\EventListener\DeserializerViewListener">
+                <argument type="service" id="api_platform.serializer" />
+                <tag name="kernel.event_listener" event="kernel.view" method="onKernelView" priority="30" />
+            </service>
             <service id="api_platform.listener.view.validation" class="ApiPlatform\Core\Bridge\Symfony\Validator\EventListener\ViewListener">
                 <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
                 <argument type="service" id="validator" />
@@ Expand All / @@ -71,18 +77,13 @@ @@
                 <argument type="service" id="api_platform.collection_data_provider" />
             </service>
-            <service id="api_platform.action.post_collection" class="ApiPlatform\Core\Action\PostCollectionAction">
-                <argument type="service" id="api_platform.serializer" />
-            </service>
+            <service id="api_platform.action.post_collection" class="ApiPlatform\Core\Action\NullAction" />
             <service id="api_platform.action.get_item" class="ApiPlatform\Core\Action\GetItemAction">
                 <argument type="service" id="api_platform.item_data_provider" />
             </service>
-            <service id="api_platform.action.put_item" class="ApiPlatform\Core\Action\PutItemAction">
-                <argument type="service" id="api_platform.item_data_provider" />
-                <argument type="service" id="api_platform.serializer" />
-            </service>
+            <service id="api_platform.action.put_item" alias="api_platform.action.get_item" />
             <service id="api_platform.action.delete_item" alias="api_platform.action.get_item" />
         </services>
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Deserialize in a view listener #584

Uh oh!

Diff view

Diff view

There are no files selected for viewing

teohhanhui Jun 27, 2016 •

edited

Loading

Uh oh!

dunglas Jun 27, 2016

Uh oh!

teohhanhui Jun 27, 2016

Uh oh!

dunglas Jun 27, 2016

Uh oh!

teohhanhui Jun 27, 2016

Uh oh!

teohhanhui Jun 27, 2016

Uh oh!

dunglas Jun 27, 2016

Uh oh!

teohhanhui Jun 27, 2016

Uh oh!

dunglas Jun 27, 2016

Uh oh!

teohhanhui Jun 27, 2016 •

edited

Loading

Uh oh!

dunglas Jun 27, 2016

Uh oh!

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -0,0 +1,63 @@
+    <?php
+    /*
+     * This file is part of the API Platform project.
+     *
+     * (c) Kévin Dunglas <dunglas@gmail.com>
+     *
+     * For the full copyright and license information, please view the LICENSE
+     * file that was distributed with this source code.
+     */
+    namespace ApiPlatform\Core\EventListener;
+    use ApiPlatform\Core\Api\RequestAttributesExtractor;
+    use ApiPlatform\Core\Exception\RuntimeException;
+    use Symfony\Component\HttpFoundation\Request;
+    use Symfony\Component\HttpFoundation\Response;
+    use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
+    use Symfony\Component\Serializer\SerializerInterface;
+    /**
+     * Updates the entity retrieved by the data provider with data contained in the request body.
+     *
+     * @author Kévin Dunglas <dunglas@gmail.com>
+     */
+    final class DeserializerViewListener
+    {
+        private $serializer;
+        public function __construct(SerializerInterface $serializer)
+        {
+            $this->serializer = $serializer;
+        }
+        public function onKernelView(GetResponseForControllerResultEvent $event)
+        {
+            $data = $event->getControllerResult();
+            $request = $event->getRequest();
+            if ($data instanceof Response || !in_array($request->getMethod(), [Request::METHOD_POST, Request::METHOD_PUT], true)) {
+                return;
+            }
+            try {
+                list($resourceClass, $collectionOperation, $itemOperation, $format) = RequestAttributesExtractor::extractAttributes($request);
+            } catch (RuntimeException $e) {
+                return;
+            }
+            $context = ['resource_class' => $resourceClass];
+            if ($collectionOperation) {
+                $context['collection_operation_name'] = $collectionOperation;
+            } else {
+                $context['item_operation_name'] = $itemOperation;
+            }
+            if (null !== $data) {
+                $context['object_to_populate'] = $data;
+            }
+            $event->setControllerResult($this->serializer->deserialize($request->getContent(), $resourceClass, $format, $context));
+        }
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,35 @@
+    <?php
+    /*
+     * This file is part of the API Platform project.
+     *
+     * (c) Kévin Dunglas <dunglas@gmail.com>
+     *
+     * For the full copyright and license information, please view the LICENSE
+     * file that was distributed with this source code.
+     */
+    namespace ApiPlatform\Core\Tests\Action;
+    use ApiPlatform\Core\Action\GetCollectionAction;
+    use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
+    use Symfony\Component\HttpFoundation\Request;
+    /**
+     * @author Kévin Dunglas <dunglas@gmail.com>
+     */
+    class GetCollectionActionTest extends \PHPUnit_Framework_TestCase
+    {
+        public function testGetCollection()
+        {
+            $result = new \stdClass();
+            $dataProviderProphecy = $this->prophesize(CollectionDataProviderInterface::class);
+            $dataProviderProphecy->getCollection('Foo', 'get')->willReturn($result);
+            $request = new Request([], [], ['_resource_class' => 'Foo', '_collection_operation_name' => 'get', '_api_format' => 'json']);
+            $action = new GetCollectionAction($dataProviderProphecy->reveal());
+            $this->assertSame($result, $action($request));
+        }
+    }

Uh oh!

Deserialize in a view listener #584

Uh oh!

Deserialize in a view listener #584

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

teohhanhui Jun 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teohhanhui Jun 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teohhanhui Jun 27, 2016 •

edited

Loading

teohhanhui Jun 27, 2016 •

edited

Loading