improved router to allow placeholders anywhere in a pattern to become…

… optional
mojolicious · Jan 21, 2014 · f87ceab · f87ceab
1 parent e8fd08c
commit f87ceab
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 5 deletions.
diff --git a/Changes b/Changes
@@ -10,6 +10,8 @@
   - Removed deprecated app_url method from Mojo::UserAgent.
   - Removed deprecated detect_proxy method from Mojo::UserAgent.
   - Removed deprecated need_proxy method from Mojo::UserAgent.
+  - Improved router to allow placeholders anywhere in a pattern to become
+    optional.
   - Improved request_ok method in Test::Mojo to handle WebSocket handshakes.
   - Improved Mojo::IOLoop::Server to use address and port for descriptor
     inheritance.

diff --git a/lib/Mojolicious/Guides/Routing.pod b/lib/Mojolicious/Guides/Routing.pod
@@ -349,9 +349,8 @@ already exist.
   $r->route('/:mymessage')
     ->to(controller => 'foo', action => 'bar', mymessage => 'hi');
 
-One more interesting effect, if a placeholder is at the end of a route and
-there is already a stash value of the same name present, it automatically
-becomes optional.
+One more interesting effect, a placeholder automatically becomes optional if
+there is already a stash value of the same name present.
 
   # / -> {controller => 'foo', action => 'bar', mymessage => 'hi'}
   $r->route('/:mymessage')

diff --git a/lib/Mojolicious/Routes/Pattern.pm b/lib/Mojolicious/Routes/Pattern.pm
@@ -142,8 +142,7 @@ sub _compile {
       $compiled = _compile_req($constraint) if $constraint;
 
       # Optional placeholder
-      $optional = 0 unless exists $defaults->{$name};
-      $compiled .= '?' if $optional;
+      exists $defaults->{$name} ? ($compiled .= '?') : ($optional = 0);
     }
 
     $block = "$compiled$block";

diff --git a/t/mojolicious/lite_app.t b/t/mojolicious/lite_app.t
@@ -60,6 +60,9 @@ get '/alternatives/:char' => [char => [qw(☃ ♥)]] => sub {
   $self->render(text => $self->url_for);
 };
 
+get '/an(:optional)placeholder' =>
+  {optional => 'none', inline => '<%= $optional %>-<%= url_for =%>'};
+
 get '/alterformat' => [format => ['json']] => {format => 'json'} => sub {
   my $self = shift;
   $self->render(text => $self->stash('format'));
@@ -519,6 +522,16 @@ $t->get_ok('/alternatives')->status_is(404)
 $t->get_ok('/alternatives/test')->status_is(404)
   ->header_is(Server => 'Mojolicious (Perl)')->content_is("Oops!\n");
 
+# Optional placeholder in the middle
+$t->get_ok('/anoptionalplaceholder')->status_is(200)
+  ->header_is(Server => 'Mojolicious (Perl)')
+  ->content_is('optional-/anoptionalplaceholder');
+
+# Optional placeholder in the middle without value
+$t->get_ok('/anplaceholder')->status_is(200)
+  ->header_is(Server => 'Mojolicious (Perl)')
+  ->content_is('none-/annoneplaceholder');
+
 # No format
 $t->get_ok('/alterformat')->status_is(200)
   ->header_is(Server => 'Mojolicious (Perl)')->content_is('json');

diff --git a/t/mojolicious/pattern.t b/t/mojolicious/pattern.t
@@ -16,6 +16,18 @@ is_deeply $pattern->match('/test/foo/'),
 ok !$pattern->match('/test/'), 'no result';
 is $pattern->render({controller => 'foo'}), '/test/foo', 'right result';
 
+# Optional placeholder in the middle
+$pattern = Mojolicious::Routes::Pattern->new('/test(name)123');
+$pattern->defaults({name => 'foo'});
+is_deeply $pattern->match('/test123', 1), {name => 'foo'}, 'right structure';
+is_deeply $pattern->match('/testbar123', 1), {name => 'bar'},
+  'right structure';
+is $pattern->render, '/testfoo123', 'right result';
+is $pattern->render({name => 'bar'}), '/testbar123', 'right result';
+$pattern->defaults({name => ''});
+is_deeply $pattern->match('/test123', 1), {name => ''}, 'right structure';
+is $pattern->render, '/test123', 'right result';
+
 # Root
 $pattern = Mojolicious::Routes::Pattern->new('/');
 $pattern->defaults({action => 'index'});