Skip to content
This repository

Support request method in route matching (close #535). #794

Closed
wants to merge 1 commit into from

3 participants

Simon JAILLET Nate Abele eLod
Simon JAILLET
Collaborator

No description provided.

Nate Abele
Owner

Okay, what are the implications, if any, for generating URLs with this? And what if you want your default to be for routes to accept any HTTP method (like I normally want)?

Simon JAILLET jails closed this January 22, 2013
Simon JAILLET
Collaborator

explanation in #535.

eLod
eLod commented March 11, 2013

i think there's an even 'lighter' fix:

        public function compile() {
                parent::compile();
                unset($this->_match['http:method']);
        }

e.g. after compiling the route just unset the 'http:method' from _match. i noticed that a route with a param (e.g. /foo/{:id:\d+}) will cause that key removed from _match anyway (see Route::compile() and its last row, however 'simple' routes would return from compile() sooner).

with 'http:method' removed all the matching would work as before and you still can add http:method (which will be checked against a request in Route::parse()). i think this makes sense e.g. when generating an url (e.g. in a template) it's inconvenient to pass http-method.

if we agree that this is a bug in lithium (the current implementation itself isn't consistent, because you need to pass http-method if and only if you don't have a param in your route) i think the only thing needs to be modified is that in Route::compile() the $this->_match = $this->_params; line should go after the for loop that cleans params (this is also done by this PR). however i'm not sure about the logic, whether it should match http-method explicitly or not, should GET be a default, etc. (as i said i prefer matching without http-method when generating routes, but dispatching should check http-method).

(please note i'm using 0.11 right now)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Jan 23, 2013
Simon JAILLET Support request method in route matching (close #535). 10520a1
This page is out of date. Refresh to see the latest.
11  net/http/Route.php
@@ -256,7 +256,7 @@ public function parse($request, array $options = array()) {
256 256
 	 * @return mixed
257 257
 	 */
258 258
 	public function match(array $options = array(), $context = null) {
259  
-		$defaults = array('action' => 'index');
  259
+		$defaults = array('action' => 'index', 'http:method' => 'GET');
260 260
 		$query = null;
261 261
 
262 262
 		if (!$this->_config['continue']) {
@@ -269,6 +269,11 @@ public function match(array $options = array(), $context = null) {
269 269
 			}
270 270
 		}
271 271
 
  272
+		if (isset($this->_meta['http:method']) && $options['http:method'] != $this->_meta['http:method']) {
  273
+			return false;
  274
+		}
  275
+		unset($options['http:method']);
  276
+
272 277
 		if (!$options = $this->_matchKeys($options)) {
273 278
 			return false;
274 279
 		}
@@ -398,8 +403,6 @@ public function export() {
398 403
 	 * @return void
399 404
 	 */
400 405
 	public function compile() {
401  
-		$this->_match = $this->_params;
402  
-
403 406
 		foreach ($this->_params as $key => $value) {
404 407
 			if (!strpos($key, ':')) {
405 408
 				continue;
@@ -408,6 +411,8 @@ public function compile() {
408 411
 			$this->_meta[$key] = $value;
409 412
 		}
410 413
 
  414
+		$this->_match = $this->_params;
  415
+
411 416
 		if ($this->_template === '/' || $this->_template === '') {
412 417
 			$this->_pattern = '@^/*$@';
413 418
 			return;
95  tests/cases/net/http/RouteTest.php
@@ -544,6 +544,101 @@ public function testMatchingEmptyRoute() {
544 544
 	}
545 545
 
546 546
 	/**
  547
+	 * Test route matching for routes with specified request method (http:method)
  548
+	 */
  549
+	public function testMatchWithRequestMethod() {
  550
+		$parameters = array('controller' => 'resource', 'action' => 'create');
  551
+
  552
+		$route = new Route(array(
  553
+			'template' => '/resource',
  554
+			'params' => $parameters + array('http:method' => 'POST')
  555
+		));
  556
+
  557
+		$result = $route->match(array(
  558
+			'controller' => 'resource', 'action' => 'create', 'http:method' => 'POST'
  559
+		));
  560
+		$this->assertEqual('/resource', $result);
  561
+
  562
+		$result = $route->match(array('controller' => 'resource', 'action' => 'create'));
  563
+		$this->assertEqual(false, $result);
  564
+	}
  565
+
  566
+	/**
  567
+	 * Test route matching for routes with specified request method (http:method) and a param
  568
+	 */
  569
+	public function testMatchWithRequestMethodWithParam() {
  570
+		$parameters = array('controller' => 'resource', 'action' => 'create');
  571
+
  572
+		$route = new Route(array(
  573
+			'template' => '/{:param}',
  574
+			'params' => $parameters + array('http:method' => 'POST')
  575
+		));
  576
+
  577
+		$result = $route->match(array(
  578
+			'controller' => 'resource',
  579
+			'action' => 'create',
  580
+			'param' => 'value',
  581
+			'http:method' => 'POST'
  582
+		));
  583
+		$this->assertEqual('/value', $result);
  584
+
  585
+		$result = $route->match(array(
  586
+			'controller' => 'resource', 'action' => 'create', 'param' => 'value'
  587
+		));
  588
+		$this->assertEqual(false, $result);
  589
+	}
  590
+
  591
+	/**
  592
+	 * Test route matching for routes with no request method (http:method)
  593
+	 */
  594
+	public function testMatchWithNoRequestMethod() {
  595
+		$parameters = array('controller' => 'resource', 'action' => 'create');
  596
+
  597
+		$route = new Route(array(
  598
+			'template' => '/resource',
  599
+			'params' => $parameters
  600
+		));
  601
+
  602
+		$result = $route->match(array('controller' => 'resource', 'action' => 'create'));
  603
+		$this->assertEqual('/resource', $result);
  604
+		$result = $route->match(array(
  605
+			'controller' => 'resource', 'action' => 'create', 'http:method' => 'GET'
  606
+		));
  607
+		$this->assertEqual('/resource', $result);
  608
+		$result = $route->match(array(
  609
+			'controller' => 'resource', 'action' => 'create', 'http:method' => 'POST'
  610
+			));
  611
+		$this->assertEqual('/resource', $result);
  612
+		$result = $route->match(array(
  613
+			'controller' => 'resource', 'action' => 'create', 'http:method' => 'PUT'
  614
+		));
  615
+		$this->assertEqual('/resource', $result);
  616
+	}
  617
+
  618
+	/**
  619
+	 * Test route matching for routes with request method (http:method) GET
  620
+	 */
  621
+	public function testMatchWithRequestMethodGet() {
  622
+		$parameters = array('controller' => 'resource', 'action' => 'create');
  623
+
  624
+		$route = new Route(array(
  625
+			'template' => '/resource',
  626
+			'params' => $parameters + array('http:method' => 'GET')
  627
+		));
  628
+
  629
+		$result = $route->match(array('controller' => 'resource', 'action' => 'create', 'http:method' => 'GET'));
  630
+		$this->assertEqual('/resource', $result);
  631
+
  632
+		$result = $route->match(array('controller' => 'resource', 'action' => 'create'));
  633
+		$this->assertEqual('/resource', $result);
  634
+
  635
+		$result = $route->match(array('controller' => 'resource', 'action' => 'create', 'http:method' => 'POST'));
  636
+		$this->assertEqual(false, $result);
  637
+		$result = $route->match(array('controller' => 'resource', 'action' => 'create', 'http:method' => 'PUT'));
  638
+		$this->assertEqual(false, $result);
  639
+	}
  640
+
  641
+	/**
547 642
 	 * Tests that routes with optional trailing elements have unnecessary slashes trimmed.
548 643
 	 */
549 644
 	public function testTrimmingEmptyPathElements() {
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.