Browse Source

Merge branch '3.x' into 3.next

ADmad 6 years ago
parent
commit
e36d17cc03

+ 1 - 7
.travis.yml

@@ -3,10 +3,7 @@ language: php
 dist: xenial
 
 php:
-  - 7.0
   - 5.6
-  - 7.2
-  - 7.3
   - 7.4
 
 env:
@@ -45,10 +42,7 @@ matrix:
 
 before_install:
   - echo cakephp version && tail -1 VERSION.txt
-  - |
-      if [[ ${TRAVIS_PHP_VERSION} != "7.4" ]]; then
-        phpenv config-rm xdebug.ini
-      fi
+  - phpenv config-rm xdebug.ini
 
   - if [ $DB = 'mysql' ]; then mysql -u root -e 'CREATE DATABASE cakephp_test;'; fi
   - if [ $DB = 'mysql' ]; then mysql -u root -e 'CREATE DATABASE cakephp_test2;'; fi

+ 1 - 1
Makefile

@@ -28,7 +28,7 @@ DASH_VERSION=$(shell echo $(VERSION) | sed -e s/\\./-/g)
 # correct tag in that repo.
 # For 3.1.x use 3.1.2
 # For 3.0.x use 3.0.5
-APP_VERSION:=master
+APP_VERSION:=3.x
 
 ALL: help
 .PHONY: help install test need-version bump-version tag-version

+ 11 - 5
src/Routing/Route/Route.php

@@ -753,6 +753,14 @@ class Route
         }
         $url += $hostOptions;
 
+        // Ensure controller/action keys are not null.
+        if (
+            (isset($keyNames['controller']) && !isset($url['controller'])) ||
+            (isset($keyNames['action']) && !isset($url['action']))
+        ) {
+            return false;
+        }
+
         return $this->_writeUrl($url, $pass, $query);
     }
 
@@ -803,12 +811,10 @@ class Route
 
         $search = $replace = [];
         foreach ($this->keys as $key) {
-            $string = null;
-            if (isset($params[$key])) {
-                $string = $params[$key];
-            } elseif (strpos($out, $key) != strlen($out) - strlen($key)) {
-                $key .= '/';
+            if (!array_key_exists($key, $params)) {
+                throw new InvalidArgumentException("Missing required route key `{$key}`");
             }
+            $string = $params[$key];
             if ($this->braceKeys) {
                 $search[] = "{{$key}}";
             } else {

+ 28 - 0
tests/TestCase/Routing/Route/RouteTest.php

@@ -1532,6 +1532,34 @@ class RouteTest extends TestCase
     }
 
     /**
+     * Test match handles optional keys
+     *
+     * @return void
+     */
+    public function testMatchNullValueOptionalKey()
+    {
+        $route = new Route('/path/:optional/fixed');
+        $this->assertSame('/path/fixed', $route->match(['optional' => null]));
+
+        $route = new Route('/path/{optional}/fixed');
+        $this->assertSame('/path/fixed', $route->match(['optional' => null]));
+    }
+
+    /**
+     * Test matching fails on required keys (controller/action)
+     *
+     * @return void
+     */
+    public function testMatchControllerRequiredKeys()
+    {
+        $route = new Route('/:controller/:action', ['action' => 'test']);
+        $this->assertFalse($route->match(['controller' => null]));
+
+        $route = new Route('/test/:action', ['controller' => 'thing']);
+        $this->assertFalse($route->match(['action' => null]));
+    }
+
+    /**
      * Test restructuring args with pass key
      *
      * @return void

+ 3 - 14
tests/TestCase/Routing/RouterTest.php

@@ -66,7 +66,7 @@ class RouterTest extends TestCase
     {
         $this->assertRegExp('/^http(s)?:\/\//', Router::url('/', true));
         $this->assertRegExp('/^http(s)?:\/\//', Router::url(null, true));
-        $this->assertRegExp('/^http(s)?:\/\//', Router::url(['_full' => true]));
+        $this->assertRegExp('/^http(s)?:\/\//', Router::url(['controller' => 'test', '_full' => true]));
     }
 
     /**
@@ -149,25 +149,14 @@ class RouterTest extends TestCase
     }
 
     /**
-     * testRouteDefaultParams method
-     *
-     * @return void
-     */
-    public function testRouteDefaultParams()
-    {
-        Router::connect('/:controller', ['controller' => 'posts']);
-        $this->assertEquals(Router::url(['action' => 'index']), '/');
-    }
-
-    /**
      * testRouteExists method
      *
      * @return void
      */
     public function testRouteExists()
     {
-        Router::connect('/:controller/:action', ['controller' => 'posts']);
-        $this->assertTrue(Router::routeExists(['action' => 'view']));
+        Router::connect('/posts/:action', ['controller' => 'posts']);
+        $this->assertTrue(Router::routeExists(['controller' => 'posts', 'action' => 'view']));
 
         $this->assertFalse(Router::routeExists(['action' => 'view', 'controller' => 'users', 'plugin' => 'test']));
     }

+ 4 - 7
tests/TestCase/View/Helper/HtmlHelperTest.php

@@ -119,6 +119,7 @@ class HtmlHelperTest extends TestCase
     public function testLink()
     {
         Router::reload();
+        Router::connect('/:controller', ['action' => 'index']);
         Router::connect('/:controller/:action/*');
 
         $this->View->setRequest($this->View->getRequest()->withAttribute('webroot', ''));
@@ -127,18 +128,14 @@ class HtmlHelperTest extends TestCase
         $expected = ['a' => ['href' => '/home'], 'preg:/\/home/', '/a'];
         $this->assertHtml($expected, $result);
 
-        $result = $this->Html->link(['action' => 'login', '<[You]>']);
+        $result = $this->Html->link(['controller' => 'users', 'action' => 'login', '<[You]>']);
         $expected = [
-            'a' => ['href' => '/login/%3C%5BYou%5D%3E'],
-            'preg:/\/login\/&lt;\[You\]&gt;/',
+            'a' => ['href' => '/users/login/%3C%5BYou%5D%3E'],
+            'preg:/\/users\/login\/&lt;\[You\]&gt;/',
             '/a',
         ];
         $this->assertHtml($expected, $result);
 
-        Router::reload();
-        Router::connect('/:controller', ['action' => 'index']);
-        Router::connect('/:controller/:action/*');
-
         $result = $this->Html->link('Posts', ['controller' => 'posts', 'action' => 'index', '_full' => true]);
         $expected = ['a' => ['href' => Router::fullBaseUrl() . '/posts'], 'Posts', '/a'];
         $this->assertHtml($expected, $result);

+ 4 - 0
tests/TestCase/View/Helper/PaginatorHelperTest.php

@@ -55,6 +55,9 @@ class PaginatorHelperTest extends TestCase
         $request = new ServerRequest([
             'url' => '/',
             'params' => [
+                'plugin' => null,
+                'controller' => '',
+                'action' => 'index',
                 'paging' => [
                     'Article' => [
                         'page' => 1,
@@ -76,6 +79,7 @@ class PaginatorHelperTest extends TestCase
         Router::reload();
         Router::connect('/:controller/:action/*');
         Router::connect('/:plugin/:controller/:action/*');
+        Router::pushRequest($request);
 
         $this->locale = I18n::getLocale();
     }