fixed route pattern bug

mojolicious · Nov 1, 2011 · 087ed7d · 087ed7d
1 parent a1422fe
commit 087ed7d
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 42 deletions.
diff --git a/Changes b/Changes
@@ -5,6 +5,7 @@ This file documents the revision history for Perl extension Mojolicious.
           latest editor's draft.
         - Improved Mojo::ByteStream to generate most Mojo::Util based methods
           automatically.
+        - Fixed route pattern bug.
 
 2.20    2011-11-01 00:00:00
         - Changed semantics of almost all functions in Mojo::Util.

diff --git a/lib/Mojolicious/Routes/Match.pm b/lib/Mojolicious/Routes/Match.pm
@@ -34,7 +34,7 @@ sub match {
   my $dictionary = $self->{dictionary} ||= $r->dictionary;
   my $path       = $self->{path};
   my $pattern    = $r->pattern;
-  my $captures   = $pattern->shape_match(\$path);
+  my $captures   = $pattern->shape_match(\$path, $r->is_endpoint);
   return unless $captures;
   $self->{path} = $path;
   $captures = {%{$self->captures}, %$captures};

diff --git a/lib/Mojolicious/Routes/Pattern.pm b/lib/Mojolicious/Routes/Pattern.pm
@@ -2,6 +2,8 @@ package Mojolicious::Routes::Pattern;
 use Mojo::Base -base;
 
 has defaults => sub { {} };
+has format   => sub {qr#\.([^/\)]+)$#};
+has [qw/pattern regex/];
 has quote_end      => ')';
 has quote_start    => '(';
 has relaxed_start  => '.';
@@ -10,7 +12,6 @@ has symbol_start   => ':';
 has symbols        => sub { [] };
 has tree           => sub { [] };
 has wildcard_start => '*';
-has [qw/format pattern regex/];
 
 # "This is the worst kind of discrimination. The kind against me!"
 sub new {
@@ -39,10 +40,8 @@ sub parse {
   $self->reqs($reqs);
 
   # Format in pattern
-  if ($pattern =~ s|\.([^/\)]+)$||) {
-    $reqs->{format}           = quotemeta $1;
-    $self->defaults->{format} = $1;
-    $self->{strict}           = 1;
+  if ($pattern =~ $self->format) {
+    $reqs->{format} = quotemeta($self->{strict} = $1);
   }
 
   # Tokenize
@@ -91,37 +90,33 @@ sub render {
 }
 
 sub shape_match {
-  my ($self, $pathref) = @_;
+  my ($self, $pathref, $detect) = @_;
 
   # Compile on demand
   my $regex;
   $regex = $self->_compile unless $regex = $self->regex;
 
   # Match
-  if (my @captures = $$pathref =~ $regex) {
-    $$pathref =~ s/$regex//;
-
-    # Merge captures
-    my $result = {%{$self->defaults}};
-    for my $symbol (@{$self->symbols}) {
-      last unless @captures;
-
-      # Merge
-      my $capture = shift @captures;
-      $result->{$symbol} = $capture if defined $capture;
-    }
-
-    # Format
-    my $format = $self->format;
-    if ($format && $$pathref =~ s/$format//) { $result->{format} ||= $1 }
-    elsif ($self->reqs->{format}) {
-      return if !$result->{format} || $self->{strict};
-    }
-
-    return $result;
+  return unless my @captures = $$pathref =~ $regex;
+  $$pathref =~ s/($regex)//;
+
+  # Merge captures
+  my $result = {%{$self->defaults}};
+  for my $symbol (@{$self->symbols}) {
+    last unless @captures;
+    my $capture = shift @captures;
+    $result->{$symbol} = $capture if defined $capture;
   }
 
-  return;
+  # Format
+  $result->{format} ||= $self->{strict} if $detect && exists $self->{strict};
+  my $req = $self->reqs->{format};
+  return $result if defined $req && !$req;
+  my $format = $self->format;
+  if ($detect && $$pathref =~ s|^/?$format||) { $result->{format} ||= $1 }
+  elsif ($req) { return if !$result->{format} }
+
+  return $result;
 }
 
 sub _compile {
@@ -131,8 +126,8 @@ sub _compile {
   my $reqs = $self->reqs;
   if (!exists $reqs->{format} || $reqs->{format}) {
     my $format =
-      defined $reqs->{format} ? _compile_req($reqs->{format}) : '([^\/]+)';
-    $self->format(qr#^/?\.$format$#);
+      defined $reqs->{format} ? _compile_req($reqs->{format}) : '([^/\)]+)';
+    $self->format(qr#\.$format$#);
   }
 
   # Compile tree to regular expression
@@ -438,6 +433,7 @@ Render pattern into a path with parameters.
 =head2 C<shape_match>
 
   my $result = $pattern->shape_match(\$path);
+  my $result = $pattern->shape_match(\$path, $detect);
 
 Match pattern against a path and remove matching parts.
 

diff --git a/t/mojolicious/pattern.t b/t/mojolicious/pattern.t
@@ -1,7 +1,7 @@
 #!/usr/bin/env perl
 use Mojo::Base -strict;
 
-use Test::More tests => 53;
+use Test::More tests => 50;
 
 use Mojo::ByteStream 'b';
 
@@ -59,15 +59,6 @@ is $pattern->render({controller => 'zzz', action => 'lala'}), '/zzztest/lala',
 $result = $pattern->match('/test/lala');
 is $result, undef, 'no result';
 
-# Format
-$pattern = Mojolicious::Routes::Pattern->new('/(controller)test/(action)');
-is $pattern->defaults->{format}, undef, 'no value';
-$pattern =
-  Mojolicious::Routes::Pattern->new('/(:controller)test/:action.html');
-is $pattern->defaults->{format}, 'html', 'right value';
-$pattern = Mojolicious::Routes::Pattern->new('/index.cgi');
-is $pattern->defaults->{format}, 'cgi', 'right value';
-
 # Relaxed
 $pattern = Mojolicious::Routes::Pattern->new('/test/(.controller)/:action');
 $result  = $pattern->match('/test/foo.bar/baz');

diff --git a/t/mojolicious/routes.t b/t/mojolicious/routes.t
@@ -1,7 +1,7 @@
 #!/usr/bin/env perl
 use Mojo::Base -strict;
 
-use Test::More tests => 340;
+use Test::More tests => 351;
 
 # "They're not very heavy, but you don't hear me not complaining."
 use_ok 'Mojolicious::Routes';
@@ -187,6 +187,11 @@ $r->route('/regex/alternatives/:alternatives',
   alternatives => qr/foo|bar|baz/)
   ->to(controller => 'regex', action => 'alternatives');
 
+# /versioned/2.0/test
+# /versioned/2.0/test.xml
+$r->route('/versioned')->route('/2.0')->to(controller => 'foo')
+  ->route('/test')->to(action => 'bar');
+
 # Make sure stash stays clean
 my $m = Mojolicious::Routes::Match->new(get => '/clean')->match($r);
 is $m->stack->[0]->{clean},     1,     'right value';
@@ -698,3 +703,21 @@ $m =
   Mojolicious::Routes::Match->new(get => '/regex/alternatives/yada')
   ->match($r);
 is $m->stack->[0], undef, 'no value';
+
+# Route with version
+$m = Mojolicious::Routes::Match->new(get => '/versioned/2.0/test')->match($r);
+is $m->stack->[0]->{controller}, 'foo', 'right value';
+is $m->stack->[0]->{action},     'bar', 'right value';
+is $m->stack->[0]->{format},     undef, 'no value';
+is $m->stack->[1], undef, 'no value';
+is $m->path_for, '/versioned/2.0/test', 'right path';
+$m =
+  Mojolicious::Routes::Match->new(get => '/versioned/2.0/test.xml')
+  ->match($r);
+is $m->stack->[0]->{controller}, 'foo', 'right value';
+is $m->stack->[0]->{action},     'bar', 'right value';
+is $m->stack->[0]->{format},     'xml', 'right value';
+is $m->stack->[1], undef, 'no value';
+is $m->path_for, '/versioned/2.0/test', 'right path';
+$m = Mojolicious::Routes::Match->new(get => '/versioned/3.0/test')->match($r);
+is $m->stack->[0], undef, 'no value';