Merge pull request biig-io#28 from swagindustries/feature/api-request…

…-handler Add new ApiRequest handler and fix biig-io#3
Nek- · Aug 18, 2020 · d812db2 · d812db2
2 parents ff2cc74 + 11423f0
commit d812db2
Show file tree

Hide file tree

Showing 8 changed files with 151 additions and 140 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - BC Break: on CRUD classes. It's big changes time. So we made the security optional for crud controllers, this has a
   consequence on their constructor
 - BC Break: `CrudableModelInterface` is now `MelodiiaModel`
+- The ApiType now uses a custom request handler. It makes handling api request easier and removes some code
+  in controllers. You should now use `handleRequest($request)` on your form. Here is a list of other changes:
+    - An api form cannot be "not submitted" anymore after a call to `handleRequest`
+    - In case the input data is invalid, a form error is add to the form
+    - There is a new optional option on the ApiType: `clear_missing` but its value is automatically guessed by default
+      to follow HTTP verbs.
 
 ## [0.6.0] 2020-06-01
 ### Added

diff --git a/composer.json b/composer.json
@@ -8,7 +8,7 @@
         "post-install-cmd": "make hooks.install"
     },
     "require": {
-        "php": ">=7.2",
+        "php": ">=7.3",
         "zircote/swagger-php": "^3.0.3",
         "nekland/tools": "^2.5.1",
         "zendframework/zend-json": "^3.0",

diff --git a/src/Bridge/Symfony/Form/Type/ApiType.php b/src/Bridge/Symfony/Form/Type/ApiType.php
@@ -4,6 +4,7 @@
 
 use SwagIndustries\Melodiia\Bridge\Symfony\Form\DomainObjectDataMapperInterface;
 use SwagIndustries\Melodiia\Bridge\Symfony\Form\DomainObjectsDataMapper;
+use SwagIndustries\Melodiia\Form\ApiRequestHandler;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\DataMapperInterface;
 use Symfony\Component\Form\FormBuilderInterface;
@@ -12,6 +13,8 @@
 
 class ApiType extends AbstractType
 {
+    public const CLEAR_MISSING_OPTION = 'clear_missing';
+
     /** @var DataMapperInterface */
     private $dataMapper;
 
@@ -31,6 +34,8 @@ public function buildForm(FormBuilderInterface $builder, array $options)
         if ($options['value_object']) {
             $builder->setDataMapper($dataMapper);
         }
+
+        $builder->setRequestHandler(new ApiRequestHandler());
     }
 
     public function configureOptions(OptionsResolver $resolver)
@@ -40,6 +45,10 @@ public function configureOptions(OptionsResolver $resolver)
             'value_object' => false,
             'melodiiaDataMapper' => null,
 
+            // If set to false, all fields will be required, in the request, otherwise some may be removed.
+            // By default is null, but it will be automatically guess by the request handler.
+            self::CLEAR_MISSING_OPTION => null,
+
             /*
              * Creates data object just like standard form would do
              * but used constructor with given data.

diff --git a/src/Crud/Controller/BaseCrudController.php b/src/Crud/Controller/BaseCrudController.php
@@ -5,13 +5,10 @@
 use SwagIndustries\Melodiia\Bridge\Symfony\Response\FormErrorResponse;
 use SwagIndustries\Melodiia\Crud\CrudControllerInterface;
 use SwagIndustries\Melodiia\Response\ApiResponse;
-use SwagIndustries\Melodiia\Response\WrongDataInput;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
-use Zend\Json\Exception\RuntimeException;
-use Zend\Json\Json;
 
 abstract class BaseCrudController implements CrudControllerInterface
 {
@@ -28,23 +25,11 @@ public function __construct(EventDispatcherInterface $eventDispatcher)
      */
     protected function decodeInputData(FormFactoryInterface $formFactory, string $form, Request $request, bool $clearMissing = null, object $data = null)
     {
-        if (null === $clearMissing) {
-            $clearMissing = !in_array($request->getMethod(), ['POST', 'PUT']);
-        }
-        try {
-            $form = $formFactory->createNamed('', $form, $data);
-            $inputData = Json::decode($request->getContent(), Json::TYPE_ARRAY);
-            $form->submit($inputData, $clearMissing);
-
-            if (!$form->isSubmitted()) {
-                return new WrongDataInput();
-            }
-
-            if (!$form->isValid()) {
-                return new FormErrorResponse($form);
-            }
-        } catch (RuntimeException $e) {
-            return new WrongDataInput();
+        $form = $formFactory->createNamed('', $form, $data, ['clear_missing' => $clearMissing]);
+        $form->handleRequest($request);
+
+        if (!$form->isValid()) {
+            return new FormErrorResponse($form);
         }
 
         return $form;

diff --git a/src/Form/ApiRequestHandler.php b/src/Form/ApiRequestHandler.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace SwagIndustries\Melodiia\Form;
+
+use SwagIndustries\Melodiia\Bridge\Symfony\Form\Type\ApiType;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
+use Symfony\Component\Form\FormError;
+use Symfony\Component\Form\FormInterface;
+use Symfony\Component\Form\RequestHandlerInterface;
+use Symfony\Component\HttpFoundation\Request;
+
+class ApiRequestHandler implements RequestHandlerInterface
+{
+    public function handleRequest(FormInterface $form, $request = null)
+    {
+        if (!$request instanceof Request) {
+            throw new UnexpectedTypeException($request, 'Symfony\Component\HttpFoundation\Request');
+        }
+
+        $clearMissing = $form->getConfig()->getOption(ApiType::CLEAR_MISSING_OPTION);
+        if (null === $clearMissing) {
+            $clearMissing = !in_array($request->getMethod(), ['POST', 'PUT']);
+        }
+
+        try {
+            $payload = \json_decode($request->getContent(), true, 512, JSON_THROW_ON_ERROR);
+
+            $form->submit($payload, $clearMissing);
+        } catch (\JsonException $e) {
+            $form->addError(new FormError(
+                'Invalid request content',
+            ));
+            $form->submit(null, false);
+        }
+    }
+
+    /**
+     * Notice: this impacts allow_file_upload field.
+     * {@inheritdoc}
+     */
+    public function isFileUpload($data)
+    {
+        return false;
+    }
+}
diff --git a/tests/Melodiia/Crud/Controller/CreateTest.php b/tests/Melodiia/Crud/Controller/CreateTest.php
@@ -66,8 +66,7 @@ public function setUp()
         $this->attributes->get(CrudControllerInterface::SECURITY_CHECK, null)->willReturn(null);
         $this->attributes->getBoolean(CrudControllerInterface::FORM_CLEAR_MISSING, true)->willReturn(true);
         $this->request->attributes = $this->attributes->reveal();
-        $this->request->getContent()->willReturn('{"awesome":"json"}');
-        $this->form->submit(['awesome' => 'json'], true)->willReturn();
+        $this->form->handleRequest($this->request)->willReturn();
         $this->formFactory->createNamed('', Argument::cetera())->willReturn($this->form);
 
         $this->controller = new Create(
@@ -78,17 +77,6 @@ public function setUp()
         );
     }
 
-    public function testItReturn400OnNotSubmittedForm()
-    {
-        $this->form->isSubmitted()->willReturn(false);
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal());
-
-        $this->assertInstanceOf(ApiResponse::class, $res);
-        $this->assertEquals(400, $res->httpStatus());
-    }
-
     public function testItReturn400OnInvalidForm()
     {
         $this->form->isSubmitted()->willReturn(true);
@@ -101,22 +89,8 @@ public function testItReturn400OnInvalidForm()
         $this->assertEquals(400, $res->httpStatus());
     }
 
-    /**
-     * Issue #28.
-     */
-    public function testItReturnProperlyOnWrongInput()
-    {
-        $this->request->getContent()->willReturn('{"awesome":json"}'); // Wrong JSON
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal(), 'id');
-        $this->assertInstanceOf(ApiResponse::class, $res);
-        $this->assertEquals(400, $res->httpStatus());
-    }
-
     public function testItCreateMelodiiaObject()
     {
-        $this->form->isSubmitted()->willReturn(true);
         $this->form->isValid()->willReturn(true);
 
         $this->form->getData()->willReturn(new FakeMelodiiaModel());
@@ -133,15 +107,14 @@ public function testItCreateMelodiiaObject()
 
     public function testICanChangeTheClearSubmitParam()
     {
-        $this->form->isSubmitted()->willReturn(true);
         $this->form->isValid()->willReturn(true);
 
         $this->form->getData()->willReturn(new FakeMelodiiaModel());
         $this->mockDispatch($this->dispatcher, Argument::type(CrudEvent::class), Create::EVENT_PRE_CREATE)->shouldBeCalled();
         $this->mockDispatch($this->dispatcher, Argument::type(CustomResponseEvent::class), Create::EVENT_POST_CREATE)->shouldBeCalled();
         $this->dataStore->save(Argument::type(FakeMelodiiaModel::class))->shouldBeCalled();
         $this->attributes->getBoolean(CrudControllerInterface::FORM_CLEAR_MISSING, true)->willReturn(false);
-        $this->form->submit(['awesome' => 'json'], false)->willReturn()->shouldBeCalled();
+        $this->form->handleRequest($this->request)->willReturn()->shouldBeCalled();
         /** @var ApiResponse $res */
         $res = ($this->controller)($this->request->reveal());
 

diff --git a/tests/Melodiia/Crud/Controller/UpdateTest.php b/tests/Melodiia/Crud/Controller/UpdateTest.php
@@ -68,9 +68,7 @@ public function setUp()
         $this->attributes->has(CrudControllerInterface::FORM_CLEAR_MISSING)->willReturn(false);
         $this->attributes->getBoolean(CrudControllerInterface::FORM_CLEAR_MISSING, false)->willReturn(false);
         $this->request->attributes = $this->attributes->reveal();
-        $this->request->getMethod()->willReturn('POST');
-        $this->request->getContent()->willReturn('{"awesome":"json"}');
-        $this->form->submit(['awesome' => 'json'], false)->willReturn();
+        $this->form->handleRequest($this->request)->willReturn();
         $this->formFactory->createNamed('', Argument::cetera())->willReturn($this->form);
 
         $this->dataStore->find(FakeMelodiiaModel::class, 'id')->willReturn(new \stdClass());
@@ -87,74 +85,6 @@ public function setUp()
         );
     }
 
-    public function testItReturn400OnNotSubmittedForm()
-    {
-        $this->form->isSubmitted()->willReturn(false);
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal());
-
-        $this->assertInstanceOf(ApiResponse::class, $res);
-        $this->assertEquals(400, $res->httpStatus());
-    }
-
-    /**
-     * Issue #54.
-     */
-    public function testItClearMissingWhileNullGivenAndMethodPatch()
-    {
-        // Mocking
-        $this->form->isSubmitted()->willReturn(true);
-        $this->form->isValid()->willReturn(true);
-        $this->form->getData()->willReturn(new FakeMelodiiaModel());
-        $this->dataStore->save(Argument::type(FakeMelodiiaModel::class))->shouldBeCalled();
-        $this->mockDispatch($this->dispatcher, Argument::type(CrudEvent::class), Argument::type('string'));
-
-        // Most important check
-        $this->form->submit(['awesome' => 'json'], true)->willReturn()->shouldBeCalled();
-
-        $this->request->getMethod()->willReturn('PATCH');
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal());
-
-        $this->assertInstanceOf(ApiResponse::class, $res);
-        $this->assertEquals(200, $res->httpStatus());
-    }
-
-    public function testItClearMissingWhenEnforcedToTrue()
-    {
-        // Mocking
-        $this->form->isSubmitted()->willReturn(true);
-        $this->form->isValid()->willReturn(true);
-        $this->form->getData()->willReturn(new FakeMelodiiaModel());
-        $this->attributes->has(CrudControllerInterface::FORM_CLEAR_MISSING)->willReturn(true);
-        $this->attributes->getBoolean(CrudControllerInterface::FORM_CLEAR_MISSING)->willReturn(true);
-        $this->dataStore->save(Argument::type(FakeMelodiiaModel::class))->shouldBeCalled();
-        $this->mockDispatch($this->dispatcher, Argument::type(CrudEvent::class), Argument::type('string'));
-
-        // Most important check
-        $this->form->submit(['awesome' => 'json'], true)->willReturn()->shouldBeCalled();
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal());
-        $this->assertInstanceOf(ApiResponse::class, $res);
-        $this->assertEquals(200, $res->httpStatus());
-    }
-
-    /**
-     * Issue #28.
-     */
-    public function testItReturnProperlyOnWrongInput()
-    {
-        $this->request->getContent()->willReturn('{"awesome":json"}'); // Wrong JSON
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal());
-        $this->assertInstanceOf(ApiResponse::class, $res);
-        $this->assertEquals(400, $res->httpStatus());
-    }
-
     public function testItReturn400OnInvalidForm()
     {
         $this->form->isSubmitted()->willReturn(true);
@@ -183,25 +113,6 @@ public function testItUpdateMelodiiaObject()
         $this->assertInstanceOf(OkContent::class, $res);
     }
 
-    public function testICanChangeClearMissingOption()
-    {
-        $this->form->isSubmitted()->willReturn(true);
-        $this->form->isValid()->willReturn(true);
-
-        $this->form->getData()->willReturn(new FakeMelodiiaModel());
-        $this->attributes->has(CrudControllerInterface::FORM_CLEAR_MISSING)->willReturn(true);
-        $this->attributes->getBoolean(CrudControllerInterface::FORM_CLEAR_MISSING)->willReturn(true);
-        $this->form->submit(['awesome' => 'json'], true)->willReturn()->shouldBeCalled();
-        $this->mockDispatch($this->dispatcher, Argument::type(CrudEvent::class), Update::EVENT_PRE_UPDATE)->shouldBeCalled();
-        $this->mockDispatch($this->dispatcher, Argument::type(CustomResponseEvent::class), Update::EVENT_POST_UPDATE)->shouldBeCalled();
-        $this->dataStore->save(Argument::type(FakeMelodiiaModel::class))->shouldBeCalled();
-
-        /** @var ApiResponse $res */
-        $res = ($this->controller)($this->request->reveal());
-
-        $this->assertInstanceOf(OkContent::class, $res);
-    }
-
     /**
      * @expectedException \Symfony\Component\Security\Core\Exception\AccessDeniedException
      */