Fix return value from CRUDController::getRestMethod() respecting `R…

…equest::getHttpMethodParameterOverride()`
sonata-project · Aug 25, 2020 · fbd71fe · fbd71fe
1 parent 243f272
commit fbd71fe
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 6 deletions.
diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
@@ -1,6 +1,11 @@
 UPGRADE 3.x
 ===========
 
+## Deprecated `Sonata\AdminBundle\Controller\CRUDController::getRestMethod()` method
+
+`Sonata\AdminBundle\Controller\CRUDController::getRestMethod()` method is deprecated.
+Use `Symfony\Component\HttpFoundation\Request::getMethod()` instead.
+
 ## Deprecated `Sonata\AdminBundle\Model\ModelManagerInterface` collection-related methods.
 
 Use:

diff --git a/src/Controller/CRUDController.php b/src/Controller/CRUDController.php
@@ -1095,17 +1095,20 @@ protected function isXmlHttpRequest()
      * Returns the correct RESTful verb, given either by the request itself or
      * via the "_method" parameter.
      *
+     * @deprecated since sonata-project/admin-bundle 3.x, to be removed in 4.0. Use `Request::getMethod()` instead.
+     *
      * @return string HTTP method, either
      */
     protected function getRestMethod()
     {
-        $request = $this->getRequest();
-
-        if (Request::getHttpMethodParameterOverride() || !$request->request->has('_method')) {
-            return $request->getMethod();
-        }
+        @trigger_error(sprintf(
+            'Method "%s()" is deprecated since sonata-project/admin-bundle 3.x'
+            .', to be removed in 4.0. Use `%s::getMethod()` instead.',
+            __METHOD__,
+            Request::class
+        ), E_USER_DEPRECATED);
 
-        return $request->request->get('_method');
+        return $this->getRequest()->getMethod();
     }
 
     /**

diff --git a/tests/Controller/CRUDControllerTest.php b/tests/Controller/CRUDControllerTest.php
@@ -157,11 +157,17 @@ class CRUDControllerTest extends TestCase
      */
     private $logger;
 
+    /**
+     * @var bool
+     */
+    private $httpMethodParameterOverride = false;
+
     /**
      * {@inheritdoc}
      */
     protected function setUp(): void
     {
+        $this->httpMethodParameterOverride = Request::getHttpMethodParameterOverride();
         $this->container = new Container();
         $this->request = new Request();
         $this->pool = new Pool($this->container, 'title', 'logo.png');
@@ -367,6 +373,18 @@ static function (string $name, $object, array $parameters = []): string {
         }
     }
 
+    protected function tearDown(): void
+    {
+        parent::tearDown();
+
+        if (!$this->httpMethodParameterOverride && Request::getHttpMethodParameterOverride()) {
+            $disableHttpMethodParameterOverride = \Closure::bind(static function (): void {
+                self::$httpMethodParameterOverride = false;
+            }, null, Request::class);
+            $disableHttpMethodParameterOverride();
+        }
+    }
+
     public function testRenderJson1(): void
     {
         $data = ['example' => '123', 'foo' => 'bar'];
@@ -1095,11 +1113,14 @@ public function testDeleteActionAjaxSuccess2(): void
 
         $this->request->headers->set('X-Requested-With', 'XMLHttpRequest');
 
+        Request::enableHttpMethodParameterOverride();
+
         $response = $this->controller->deleteAction(1);
 
         $this->assertInstanceOf(Response::class, $response);
         $this->assertSame(json_encode(['result' => 'ok']), $response->getContent());
         $this->assertSame([], $this->session->getFlashBag()->all());
+        $this->assertSame(Request::METHOD_DELETE, $this->request->getMethod());
     }
 
     public function testDeleteActionAjaxError(): void
@@ -1226,11 +1247,14 @@ public function testDeleteActionSuccess2(string $expectedToStringValue, string $
 
         $this->request->request->set('_sonata_csrf_token', 'csrf-token-123_sonata.delete');
 
+        Request::enableHttpMethodParameterOverride();
+
         $response = $this->controller->deleteAction(1);
 
         $this->assertInstanceOf(RedirectResponse::class, $response);
         $this->assertSame(['flash_delete_success'], $this->session->getFlashBag()->get('sonata_flash_success'));
         $this->assertSame('list', $response->getTargetUrl());
+        $this->assertSame(Request::METHOD_DELETE, $this->request->getMethod());
     }
 
     /**
@@ -1261,11 +1285,14 @@ public function testDeleteActionSuccessNoCsrfTokenProvider(string $expectedToStr
         $this->request->setMethod(Request::METHOD_POST);
         $this->request->request->set('_method', Request::METHOD_DELETE);
 
+        Request::enableHttpMethodParameterOverride();
+
         $response = $this->controller->deleteAction(1);
 
         $this->assertInstanceOf(RedirectResponse::class, $response);
         $this->assertSame(['flash_delete_success'], $this->session->getFlashBag()->get('sonata_flash_success'));
         $this->assertSame('list', $response->getTargetUrl());
+        $this->assertSame(Request::METHOD_DELETE, $this->request->getMethod());
     }
 
     public function testDeleteActionWrongRequestMethod(): void
@@ -1284,6 +1311,8 @@ public function testDeleteActionWrongRequestMethod(): void
         //without POST request parameter "_method" should not be used as real REST method
         $this->request->query->set('_method', Request::METHOD_DELETE);
 
+        Request::enableHttpMethodParameterOverride();
+
         $this->assertInstanceOf(Response::class, $this->controller->deleteAction(1));
 
         $this->assertSame($this->admin, $this->parameters['admin']);
@@ -1296,6 +1325,7 @@ public function testDeleteActionWrongRequestMethod(): void
 
         $this->assertSame([], $this->session->getFlashBag()->all());
         $this->assertSame('@SonataAdmin/CRUD/delete.html.twig', $this->template);
+        $this->assertSame(Request::METHOD_GET, $this->request->getMethod());
     }
 
     /**
@@ -1350,12 +1380,16 @@ public function testDeleteActionInvalidCsrfToken(): void
         $this->request->request->set('_method', Request::METHOD_DELETE);
         $this->request->request->set('_sonata_csrf_token', 'CSRF-INVALID');
 
+        Request::enableHttpMethodParameterOverride();
+
         try {
             $this->controller->deleteAction(1);
         } catch (HttpException $e) {
             $this->assertSame('The csrf token is not valid, CSRF attack?', $e->getMessage());
             $this->assertSame(400, $e->getStatusCode());
         }
+
+        $this->assertSame(Request::METHOD_DELETE, $this->request->getMethod());
     }
 
     public function testEditActionNotFoundException(): void