fix #241

AltoRouter.php

@@ -204,6 +204,8 @@ class AltoRouter
             $requestUrl = substr($requestUrl, 0, $strpos);
         }
 
+        $lastRequestUrlChar = $requestUrl[strlen($requestUrl)-1];
+
         // set Request Method if it isn't passed as a parameter
         if ($requestMethod === null) {
             $requestMethod = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : 'GET';
@@ -230,10 +232,12 @@ class AltoRouter
                 // No params in url, do string comparison
                 $match = strcmp($requestUrl, $route) === 0;
             } else {
-                // Compare longest non-param string with url
+                // Compare longest non-param string with url before moving on to regex
-                if (strncmp($requestUrl, $route, $position) !== 0) {
+				// Check if last character before param is a slash, because it could be optional if param is optional too (see https://github.com/dannyvankooten/AltoRouter/issues/241)
+                if (strncmp($requestUrl, $route, $position) !== 0 && ($lastRequestUrlChar === '/' || $route[$position-1] !== '/')) {
                     continue;
                 }
+
                 $regex = $this->compileRoute($route);
                 $match = preg_match($regex, $requestUrl, $params) === 1;
             }
@@ -254,6 +258,7 @@ class AltoRouter
                 ];
             }
         }
+
         return false;
     }
 

tests/AltoRouterTest.php

@@ -96,14 +96,14 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
         $route = '/[:controller]/[:action]';
         $target = static function () {
         };
-        
+
         $this->router->addRoutes([
         [$method, $route, $target],
         [$method, $route, $target, 'second_route']
         ]);
-        
+
         $routes = $this->router->getRoutes();
-        
+
         $this->assertEquals([$method, $route, $target, null], $routes[0]);
         $this->assertEquals([$method, $route, $target, 'second_route'], $routes[1]);
     }
@@ -115,15 +115,15 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     {
         $traversable = new SimpleTraversable();
         $this->router->addRoutes($traversable);
-        
+
         $traversable->rewind();
-        
+
         $first = $traversable->current();
         $traversable->next();
         $second = $traversable->current();
-        
+
         $routes = $this->router->getRoutes();
-        
+
         $this->assertEquals($first, $routes[0]);
         $this->assertEquals($second, $routes[1]);
     }
@@ -142,10 +142,10 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
      */
     public function testSetBasePath()
     {
-        $basePath = $this->router->setBasePath('/some/path');
+    	$this->router->setBasePath('/some/path');
         $this->assertEquals('/some/path', $this->router->getBasePath());
-        
+
-        $basePath = $this->router->setBasePath('/some/path');
+        $this->router->setBasePath('/some/path');
         $this->assertEquals('/some/path', $this->router->getBasePath());
     }
 
@@ -158,11 +158,11 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
         $route = '/[:controller]/[:action]';
         $target = static function () {
         };
-        
+
         $this->router->map($method, $route, $target);
-        
+
         $routes = $this->router->getRoutes();
-        
+
         $this->assertEquals([$method, $route, $target, null], $routes[0]);
     }
 
@@ -176,15 +176,15 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
         $target = static function () {
         };
         $name = 'myroute';
-        
+
         $this->router->map($method, $route, $target, $name);
-        
+
         $routes = $this->router->getRoutes();
         $this->assertEquals([$method, $route, $target, $name], $routes[0]);
-        
+
         $named_routes = $this->router->getNamedRoutes();
         $this->assertEquals($route, $named_routes[$name]);
-        
+
         try {
             $this->router->map($method, $route, $target, $name);
             $this->fail('Should not be able to add existing named route');
@@ -203,21 +203,21 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             'controller' => 'test',
             'action' => 'someaction'
         ];
-        
+
         $this->router->map('GET', '/[:controller]/[:action]', static function () {
         }, 'foo_route');
-        
+
         $this->assertEquals(
             '/test/someaction',
             $this->router->generate('foo_route', $params)
         );
-        
+
         $params = [
             'controller' => 'test',
             'action' => 'someaction',
             'type' => 'json'
         ];
-        
+
         $this->assertEquals(
             '/test/someaction',
             $this->router->generate('foo_route', $params)
@@ -231,23 +231,23 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     {
         $this->router->map('GET', '/[:controller]/[:action].[:type]?', static function () {
         }, 'bar_route');
-        
+
         $params = [
             'controller' => 'test',
             'action' => 'someaction'
         ];
-        
+
         $this->assertEquals(
             '/test/someaction',
             $this->router->generate('bar_route', $params)
         );
-        
+
         $params = [
             'controller' => 'test',
             'action' => 'someaction',
             'type' => 'json'
         ];
-        
+
         $this->assertEquals(
             '/test/someaction.json',
             $this->router->generate('bar_route', $params)
@@ -262,18 +262,18 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     {
         $this->router->map('GET', '/[i:page]?', static function () {
         }, 'bare_route');
-        
+
         $params = [
             'page' => 1
         ];
-        
+
         $this->assertEquals(
             '/1',
             $this->router->generate('bare_route', $params)
         );
-        
+
         $params = [];
-        
+
         $this->assertEquals(
             '/',
             $this->router->generate('bare_route', $params)
@@ -292,7 +292,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             $this->assertEquals("Route 'nonexisting_route' does not exist.", $e->getMessage());
         }
     }
-    
+
     /**
      * @covers AltoRouter::match
      * @covers AltoRouter::compileRoute
@@ -300,7 +300,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     public function testMatch()
     {
         $this->router->map('GET', '/foo/[:controller]/[:action]', 'foo_action', 'foo_route');
-        
+
         $this->assertEquals([
             'target' => 'foo_action',
             'params' => [
@@ -309,9 +309,9 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             ],
             'name' => 'foo_route'
         ], $this->router->match('/foo/test/do', 'GET'));
-        
+
         $this->assertFalse($this->router->match('/foo/test/do', 'POST'));
-        
+
         $this->assertEquals([
             'target' => 'foo_action',
             'params' => [
@@ -346,7 +346,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     public function testMatchWithFixedParamValues()
     {
         $this->router->map('POST', '/users/[i:id]/[delete|update:action]', 'usersController#doAction', 'users_do');
-        
+
         $this->assertEquals([
             'target' => 'usersController#doAction',
             'params' => [
@@ -355,7 +355,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             ],
             'name' => 'users_do'
         ], $this->router->match('/users/1/delete', 'POST'));
-        
+
         $this->assertFalse($this->router->match('/users/1/delete', 'GET'));
         $this->assertFalse($this->router->match('/users/abc/delete', 'POST'));
         $this->assertFalse($this->router->match('/users/1/create', 'GET'));
@@ -369,9 +369,9 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
         $router = $this->getMockBuilder('AltoRouterDebug')
             ->setMethods(['compileRoute'])
             ->getMock();
-        
+
         // this should prove that compileRoute is not called when the route doesn't
-        // have any params in it, but this doesn't work because compileRoute is private.
+        // have any params in it
         $router->expects($this->never())
             ->method('compileRoute');
 
@@ -384,8 +384,6 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             'name' => 'contact'
         ], $router->match('/contact', 'GET'));
 
-        $router->map('GET', '/page/[:id]', 'pages#show', 'page');
-
         // no prefix match, so no regex compilation necessary
         $this->assertFalse($router->match('/page1', 'GET'));
     }
@@ -396,10 +394,10 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     public function testMatchWithServerVars()
     {
         $this->router->map('GET', '/foo/[:controller]/[:action]', 'foo_action', 'foo_route');
-        
+
         $_SERVER['REQUEST_URI'] = '/foo/test/do';
         $_SERVER['REQUEST_METHOD'] = 'GET';
-        
+
         $this->assertEquals([
             'target' => 'foo_action',
             'params' => [
@@ -416,7 +414,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     public function testMatchWithOptionalUrlParts()
     {
         $this->router->map('GET', '/bar/[:controller]/[:action].[:type]?', 'bar_action', 'bar_route');
-        
+
         $this->assertEquals([
             'target' => 'bar_action',
             'params' => [
@@ -426,7 +424,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             ],
             'name' => 'bar_route'
         ], $this->router->match('/bar/test/do.json', 'GET'));
-        
+
         $this->assertEquals([
             'target' => 'bar_action',
             'params' => [
@@ -436,7 +434,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             'name' => 'bar_route'
         ], $this->router->match('/bar/test/do', 'GET'));
     }
-    
+
     /**
      * GitHub #98
      * @covers AltoRouter::match
@@ -444,7 +442,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     public function testMatchWithOptionalPartOnBareUrl()
     {
         $this->router->map('GET', '/[i:page]?', 'bare_action', 'bare_route');
-        
+
         $this->assertEquals([
             'target' => 'bare_action',
             'params' => [
@@ -452,7 +450,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             ],
             'name' => 'bare_route'
         ], $this->router->match('/1', 'GET'));
-        
+
         $this->assertEquals([
             'target' => 'bare_action',
             'params' => [],
@@ -467,7 +465,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     {
         $this->router->map('GET', '/a', 'foo_action', 'foo_route');
         $this->router->map('GET', '*', 'bar_action', 'bar_route');
-        
+
         $this->assertEquals([
             'target' => 'bar_action',
             'params' => [],
@@ -480,13 +478,13 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     public function testMatchWithCustomRegexp()
     {
         $this->router->map('GET', '@^/[a-z]*$', 'bar_action', 'bar_route');
-        
+
         $this->assertEquals([
             'target' => 'bar_action',
             'params' => [],
             'name' => 'bar_route'
         ], $this->router->match('/everything', 'GET'));
-        
+
         $this->assertFalse($this->router->match('/some-other-thing', 'GET'));
     }
     /**
@@ -505,9 +503,9 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
         // 'ZERO WIDTH NON-JOINER'
         $pattern .= '\x{200C}';
         $pattern .= ']+)';
-        
+
         $this->router->map('GET', '@' . $pattern, 'unicode_action', 'unicode_route');
-        
+
         $this->assertEquals([
             'target' => 'unicode_action',
             'name' => 'unicode_route',
@@ -515,10 +513,24 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
                 'path' => '大家好'
             ]
         ], $this->router->match('/大家好', 'GET'));
-        
+
         $this->assertFalse($this->router->match('/﷽‎', 'GET'));
     }
 
+    public function testMatchWithSlashBeforeOptionalPart()
+	{
+		$this->router->map('GET', '/archives/[lmin:category]?', 'Article#archives');
+		$expected =  [
+			'target' => 'Article#archives',
+			'params' => [],
+			'name' => null
+		];
+
+		$this->assertEquals($expected, $this->router->match('/archives/', 'GET'));
+		$this->assertEquals($expected, $this->router->match('/archives', 'GET'));
+
+	}
+
     /**
      * @covers AltoRouter::addMatchTypes
      */
@@ -526,7 +538,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
     {
         $this->router->addMatchTypes(['cId' => '[a-zA-Z]{2}[0-9](?:_[0-9]++)?']);
         $this->router->map('GET', '/bar/[cId:customId]', 'bar_action', 'bar_route');
-        
+
         $this->assertEquals([
             'target' => 'bar_action',
             'params' => [
@@ -542,7 +554,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
             ],
             'name' => 'bar_route'
         ], $this->router->match('/bar/AB1_0123456789', 'GET'));
-        
+
         $this->assertFalse($this->router->match('/some-other-thing', 'GET'));
     }
     /**
@@ -557,10 +569,10 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
         $pattern .= '\x{FE70}-\x{FEFF}';
         $pattern .= '\x{0750}-\x{077F}';
         $pattern .= ']+';
-        
+
         $this->router->addMatchTypes(['nonArabic' => $pattern]);
         $this->router->map('GET', '/bar/[nonArabic:string]', 'non_arabic_action', 'non_arabic_route');
-        
+
         $this->assertEquals([
             'target' => 'non_arabic_action',
             'name' => 'non_arabic_route',
@@ -568,7 +580,7 @@ class AltoRouterTest extends PHPUnit\Framework\TestCase
                 'string' => 'some-path'
             ]
         ], $this->router->match('/bar/some-path', 'GET'));
-        
+
         $this->assertFalse($this->router->match('/﷽‎', 'GET'));
     }
 }

tests/benchmark-parse-api.php

@@ -0,0 +1,106 @@
+<?php
+
+/**
+ * Benchmark Altorouter
+ *
+ * Usage: php ./tests/benchmark-parse-api.php <iterations>
+ *
+ * Options:
+ *
+ * <iterations>:
+ * The number of routes to map & match. Defaults to 1000.
+ */
+
+require __DIR__ . '/../vendor/autoload.php';
+$routes = [
+	["POST", "/1/classes/[a:className]"],
+	["GET", "/1/classes/[a:className]/[i:objectId]"],
+	["PUT", "/1/classes/[a:className]/[i:objectId]"],
+	["GET", "/1/classes/[a:className]"],
+	["DELETE", "/1/classes/[a:className]/[i:objectId]"],
+
+	// Users
+	["POST", "/1/users"],
+	["GET", "/1/login"],
+	["GET", "/1/users/[i:objectId]"],
+	["PUT", "/1/users/[i:objectId]"],
+	["GET", "/1/users"],
+	["DELETE", "/1/users/[i:objectId]"],
+	["POST", "/1/requestPasswordReset"],
+
+	// Roles
+	["POST", "/1/roles"],
+	["GET", "/1/roles/[i:objectId]"],
+	["PUT", "/1/roles/[i:objectId]"],
+	["GET", "/1/roles"],
+	["DELETE", "/1/roles/[i:objectId]"],
+
+	// Files
+	["POST", "/1/files/:fileName"],
+
+	// Analytics
+	["POST", "/1/events/[a:eventName]"],
+
+	// Push Notifications
+	["POST", "/1/push"],
+
+	// Installations
+	["POST", "/1/installations"],
+	["GET", "/1/installations/[i:objectId]"],
+	["PUT", "/1/installations/[i:objectId]"],
+	["GET", "/1/installations"],
+	["DELETE", "/1/installations/[i:objectId]"],
+
+	// Cloud Functions
+	["POST", "/1/functions"],
+];
+$total_time = 0;
+$router = new AltoRouter();
+
+// map requests
+$start = microtime(true);
+foreach ($routes as $r) {
+    $router->map($r[0], $r[1], '');
+}
+$end = microtime(true);
+$time = $end - $start;
+$total_time += $time;
+echo sprintf('Map time: %.3f ms', $time * 1000) . PHP_EOL;
+
+// match a static route
+$start = microtime(true);
+$router->match('/1/login', 'GET');
+$end = microtime(true);
+$time = $end - $start;
+$total_time += $time;
+echo sprintf('Match time (existing route, no params): %.3f ms', $time * 1000) . PHP_EOL;
+
+// match a route with 1 parameter
+$start = microtime(true);
+$res = $router->match('/1/classes/foo', 'GET');
+$end = microtime(true);
+$time = $end - $start;
+$total_time += $time;
+echo sprintf('Match time (existing route, 1 param): %.3f ms', $time * 1000) . PHP_EOL;
+
+// match a route with 2 parameters
+$start = microtime(true);
+$res = $router->match('/1/classes/foo/500', 'GET');
+$end = microtime(true);
+$time = $end - $start;
+$total_time += $time;
+echo sprintf('Match time (existing route, 2 params): %.3f ms', $time * 1000) . PHP_EOL;
+
+
+// match unexisting route
+$start = microtime(true);
+$router->match('/55-foo-bar', 'GET');
+$end = microtime(true);
+$time = $end - $start;
+$total_time += $time;
+echo sprintf('Match time (unexisting route): %.3f ms', $time * 1000) . PHP_EOL;
+
+// print totals
+echo sprintf('Total time: %.3f ms', $total_time * 1000) . PHP_EOL;
+echo sprintf('Memory usage: %d KB', round(memory_get_usage() / 1024)) . PHP_EOL;
+echo sprintf('Peak memory usage: %d KB', round(memory_get_peak_usage(true) / 1024)) . PHP_EOL;

tests/benchmark.php

@@ -1,92 +0,0 @@
-<?php
-
-/**
- * Benchmark Altorouter
- *
- * Usage: php ./tests/benchmark.php <iterations>
- *
- * Options:
- *
- * <iterations>:
- * The number of routes to map & match. Defaults to 1000.
- */
-
-require __DIR__ . '/../vendor/autoload.php';
-
-global $argv;
-$n = isset($argv[1]) ? intval($argv[1]) : 1000;
-
-// generates a random request url
-function random_request_url()
-{
-    $characters = 'abcdefghijklmnopqrstuvwxyz';
-    $charactersLength = strlen($characters);
-    $randomString = '/';
-
-    // create random path of 5-20 characters
-    for ($i = 0; $i < rand(5, 20); $i++) {
-        $randomString .= $characters[rand(0, $charactersLength - 1)];
-
-        if (rand(1, 10) === 1) {
-            $randomString .= '/';
-        }
-    }
-
-    // add dynamic route with 10% chance
-    if (rand(1, 10) === 1) {
-        $randomString = rtrim($randomString, '/') . '/[:part]';
-    }
-
-    return $randomString;
-}
-
-// generate a random request method
-function random_request_method()
-{
-    static $methods = [ 'GET', 'GET', 'POST', 'PUT', 'PATCH', 'DELETE' ];
-    $random_key = array_rand($methods);
-    return $methods[ $random_key ];
-}
-
-// prepare benchmark data
-$requests = [];
-for ($i=0; $i<$n; $i++) {
-    $requests[] = [
-        'method' => random_request_method(),
-        'url' => random_request_url(),
-    ];
-}
-
-$router = new AltoRouter();
-
-// map requests
-$start = microtime(true);
-foreach ($requests as $r) {
-    $router->map($r['method'], $r['url'], '');
-}
-$end = microtime(true);
-$map_time = ($end - $start) * 1000;
-echo sprintf('Map time: %.2f ms', $map_time) . PHP_EOL;
-
-
-// pick random route to match
-$r = $requests[array_rand($requests)];
-
-// match random known route
-$start = microtime(true);
-$router->match($r['url'], $r['method']);
-$end = microtime(true);
-$match_time_known_route = ($end - $start) * 1000;
-echo sprintf('Match time (known route): %.2f ms', $match_time_known_route) . PHP_EOL;
-
-// match unexisting route
-$start = microtime(true);
-$router->match('/55-foo-bar', 'GET');
-$end = microtime(true);
-$match_time_unknown_route = ($end - $start) * 1000;
-echo sprintf('Match time (unknown route): %.2f ms', $match_time_unknown_route) . PHP_EOL;
-
-// print totals
-echo sprintf('Total time: %.2f seconds', ($map_time + $match_time_known_route + $match_time_unknown_route)) . PHP_EOL;
-echo sprintf('Memory usage: %d KB', round(memory_get_usage() / 1024)) . PHP_EOL;
-echo sprintf('Peak memory usage: %d KB', round(memory_get_peak_usage(true) / 1024)) . PHP_EOL;