Browse Source

Add deprecation warning for global state usage.

I missed this earlier but found it because
ServerRequest::createFromGlobals() is also deprecated.
Mark Story 8 years ago
parent
commit
8e746c6c7a

+ 6 - 1
src/Routing/Route/Route.php

@@ -15,6 +15,7 @@
 namespace Cake\Routing\Route;
 
 use Cake\Http\ServerRequest;
+use Cake\Http\ServerRequestFactory;
 use Cake\Routing\Router;
 use InvalidArgumentException;
 use Psr\Http\Message\ServerRequestInterface;
@@ -437,8 +438,12 @@ class Route
 
         if (isset($this->defaults['_method'])) {
             if (empty($method)) {
+                deprecationWarning(
+                    'Extracting the request method from global state when parsing routes is deprecated. ' .
+                    'Instead adopt Route::parseRequest() which extracts the method from the passed request.'
+                );
                 // Deprecated reading the global state is deprecated and will be removed in 4.x
-                $request = Router::getRequest(true) ?: ServerRequest::createFromGlobals();
+                $request = Router::getRequest(true) ?: ServerRequestFactory::fromGlobals();
                 $method = $request->getMethod();
             }
             if (!in_array($method, (array)$this->defaults['_method'], true)) {

+ 30 - 3
tests/TestCase/Routing/Route/RouteTest.php

@@ -1166,8 +1166,6 @@ class RouteTest extends TestCase
         ]);
         $this->assertFalse($route->parse('/sample', 'GET'));
 
-        // Test for deprecated behavior
-        $_SERVER['REQUEST_METHOD'] = 'POST';
         $expected = [
             'controller' => 'posts',
             'action' => 'index',
@@ -1175,7 +1173,36 @@ class RouteTest extends TestCase
             '_method' => ['PUT', 'POST'],
             '_matchedRoute' => '/sample'
         ];
-        $this->assertEquals($expected, $route->parse('/sample'));
+        $this->assertEquals($expected, $route->parse('/sample', 'POST'));
+    }
+
+    /**
+     * Test deprecated globals reading for method matching
+     *
+     * @group deprecated
+     * @return void
+     */
+    public function testParseWithMultipleHttpMethodDeprecated()
+    {
+        $this->deprecated(function () {
+            $_SERVER['REQUEST_METHOD'] = 'GET';
+            $route = new Route('/sample', [
+                'controller' => 'posts',
+                'action' => 'index',
+                '_method' => ['PUT', 'POST']
+            ]);
+            $this->assertFalse($route->parse('/sample'));
+
+            $_SERVER['REQUEST_METHOD'] = 'POST';
+            $expected = [
+                'controller' => 'posts',
+                'action' => 'index',
+                'pass' => [],
+                '_method' => ['PUT', 'POST'],
+                '_matchedRoute' => '/sample'
+            ];
+            $this->assertEquals($expected, $route->parse('/sample'));
+        });
     }
 
     /**

+ 31 - 9
tests/TestCase/Routing/RouteBuilderTest.php

@@ -639,6 +639,32 @@ class RouteBuilderTest extends TestCase
     /**
      * Test resource parsing.
      *
+     * @group deprecated
+     * @return void
+     */
+    public function testResourcesParsingReadGlobals()
+    {
+        $this->deprecated(function () {
+            $routes = new RouteBuilder($this->collection, '/');
+            $routes->resources('Articles');
+
+            $_SERVER['REQUEST_METHOD'] = 'GET';
+            $result = $this->collection->parse('/articles');
+            $this->assertEquals('Articles', $result['controller']);
+            $this->assertEquals('index', $result['action']);
+            $this->assertEquals([], $result['pass']);
+
+            $_SERVER['REQUEST_METHOD'] = 'POST';
+            $result = $this->collection->parse('/articles');
+            $this->assertEquals('Articles', $result['controller']);
+            $this->assertEquals('add', $result['action']);
+            $this->assertEquals([], $result['pass']);
+        });
+    }
+
+    /**
+     * Test resource parsing.
+     *
      * @return void
      */
     public function testResourcesParsing()
@@ -646,31 +672,27 @@ class RouteBuilderTest extends TestCase
         $routes = new RouteBuilder($this->collection, '/');
         $routes->resources('Articles');
 
-        $_SERVER['REQUEST_METHOD'] = 'GET';
-        $result = $this->collection->parse('/articles');
+        $result = $this->collection->parse('/articles', 'GET');
         $this->assertEquals('Articles', $result['controller']);
         $this->assertEquals('index', $result['action']);
         $this->assertEquals([], $result['pass']);
 
-        $result = $this->collection->parse('/articles/1');
+        $result = $this->collection->parse('/articles/1', 'GET');
         $this->assertEquals('Articles', $result['controller']);
         $this->assertEquals('view', $result['action']);
         $this->assertEquals([1], $result['pass']);
 
-        $_SERVER['REQUEST_METHOD'] = 'POST';
-        $result = $this->collection->parse('/articles');
+        $result = $this->collection->parse('/articles', 'POST');
         $this->assertEquals('Articles', $result['controller']);
         $this->assertEquals('add', $result['action']);
         $this->assertEquals([], $result['pass']);
 
-        $_SERVER['REQUEST_METHOD'] = 'PUT';
-        $result = $this->collection->parse('/articles/1');
+        $result = $this->collection->parse('/articles/1', 'PUT');
         $this->assertEquals('Articles', $result['controller']);
         $this->assertEquals('edit', $result['action']);
         $this->assertEquals([1], $result['pass']);
 
-        $_SERVER['REQUEST_METHOD'] = 'DELETE';
-        $result = $this->collection->parse('/articles/1');
+        $result = $this->collection->parse('/articles/1', 'DELETE');
         $this->assertEquals('Articles', $result['controller']);
         $this->assertEquals('delete', $result['action']);
         $this->assertEquals([1], $result['pass']);