From 959b72f1cf58564d4156202b51bf831ede469c00 Mon Sep 17 00:00:00 2001 From: Sawyer X Date: Wed, 14 Dec 2016 16:06:35 +0100 Subject: [PATCH] Introduce new DSL keyword: uri_for_route: This new DSL provides a uri_for()-style resolution, but uses named routes for this. get 'view_product' => '/view/:product/:id' => sub {...}; get 'scary' => '/*/:foo/**' => sub {...}; # somewhere else in your App my $uri = uri_for_route( 'view_product' => { 'product' => 'phone', 'id' => 'K2V3', }); # $uri = /view/phone/K2V3 $uri = uri_for_route( 'view_product', { 'foo' => 'bar', 'splat' => [ 'baz', ['quux'] ], }, { 'id' => 4 }, ); # /baz/bar/quux?id=4 * This works on any non-HEAD method (GET, POST, PATCH, PUT, DELETE, and if you create your own) * Splat and Megasplat are supported. Mixing it with named params is also supported. * Query parameters are supported * HTML escaping is supported * Lots of testing. * Documentation updated. This is not yet supported in the template itself. --- lib/Dancer2/Core/App.pm | 75 +++++++++- lib/Dancer2/Core/DSL.pm | 8 ++ lib/Dancer2/Core/Route.pm | 6 +- lib/Dancer2/Manual.pod | 7 + lib/Dancer2/Manual/Keywords.pod | 136 ++++++++++++++++++ t/dsl/uri_for.t | 48 +++++++ t/dsl/uri_for_route.t | 246 ++++++++++++++++++++++++++++++++ t/named_routes.t | 21 ++- t/uri_for.t | 30 ---- 9 files changed, 530 insertions(+), 47 deletions(-) create mode 100644 t/dsl/uri_for.t create mode 100644 t/dsl/uri_for_route.t delete mode 100644 t/uri_for.t diff --git a/lib/Dancer2/Core/App.pm b/lib/Dancer2/Core/App.pm index b18527fa3..dfd65bc08 100644 --- a/lib/Dancer2/Core/App.pm +++ b/lib/Dancer2/Core/App.pm @@ -4,13 +4,14 @@ package Dancer2::Core::App; use Moo; use Carp qw; use Scalar::Util 'blessed'; +use List::Util (); use Module::Runtime 'is_module_name'; use Safe::Isa; use Sub::Quote; use File::Spec; use Module::Runtime qw< require_module use_module >; use List::Util (); -use Ref::Util qw< is_ref is_globref is_scalarref >; +use Ref::Util qw< is_ref is_arrayref is_globref is_scalarref is_regexpref >; use Plack::App::File; use Plack::Middleware::FixMissingBodyInRedirect; @@ -608,6 +609,12 @@ has routes => ( }, ); +has 'route_names' => ( + 'is' => 'rw', + 'isa' => HashRef, + 'default' => sub { {} }, +); + # add_hook will add the hook to the first "hook candidate" it finds that support # it. If none, then it will try to add the hook to the current application. around add_hook => sub { @@ -1244,9 +1251,16 @@ sub add_route { ); my $method = $route->method; - push @{ $self->routes->{$method} }, $route; + if ( $method ne 'head' && $route->has_name() ) { + my $name = $route->name; + $self->route_names->{$name} + and die "Route with this name ($name) already exists"; + + $self->route_names->{$name} = $route; + } + return $route; } @@ -1694,6 +1708,63 @@ sub response_not_found { return $response; } +sub uri_for_route { + my ( $self, $route_name, $route_params, $query_params, $dont_escape ) = @_; + my $route = $self->route_names->{$route_name} + or die "Cannot find route named '$route_name'"; + + my $string = $route->spec_route; + is_regexpref($string) + and die "uri_for_route() does not support regexp route paths"; + + # Convert splat only to the general purpose structure + if ( is_arrayref($route_params) ) { + $route_params = { 'splat' => $route_params }; + } + + # The regexes are taken and altered from: + # Dancer2::Core::Route::_build_regexp_from_string. + + # Replace :foo with arg (route parameters) + # Not a fan of all this regex play to handle typed parameter -- SX + my @params = $string =~ m{:([^/.\?]+)}xmsg; + + foreach my $param (@params) { + $param =~ s{^([^\[]+).*}{$1}xms; + my $value = $route_params->{$param} + or die "Route $route_name uses the parameter '${param}', which was not provided"; + + $string =~ s!\Q:$param\E(\[[^\]]+\])?!$value!xmsg; + } + + # TODO: Can we cut this down by replacing on the spot? + # I think that will be tricky because we first need all **, then * + + $string =~ s!\Q**\E!(?#megasplat)!g; + $string =~ s!\*!(?#splat)!g; + + # TODO: Can we cut this down? + my @token_or_splat = + $string =~ /\(\?#((?:mega)?splat)\)/g; + + my $splat_params = $route_params->{'splat'}; + if ($splat_params && @token_or_splat) { + $#{$splat_params} == $#token_or_splat + or die 'Mismatch in amount of splat args and splat elements'; + + for ( my $i = 0; $i < @{$splat_params}; $i++ ) { + if ( is_arrayref($splat_params->[$i]) ){ + my $megasplat = join '/', @{ $splat_params->[$i] }; + $string =~ s{\Q(?#megasplat)\E}{$megasplat}; + } else { + $string =~ s{\Q(?#splat)\E}{$splat_params->[$i]}; + } + } + } + + return $self->request->uri_for( $string, $query_params ); +} + 1; __END__ diff --git a/lib/Dancer2/Core/DSL.pm b/lib/Dancer2/Core/DSL.pm index 13d6d437e..5a036f25b 100644 --- a/lib/Dancer2/Core/DSL.pm +++ b/lib/Dancer2/Core/DSL.pm @@ -122,6 +122,7 @@ sub dsl_keywords { true => { is_global => 1 }, upload => { is_global => 0 }, uri_for => { is_global => 0 }, + uri_for_route => { is_global => 0 }, var => { is_global => 0 }, vars => { is_global => 0 }, warning => { is_global => 1 }, @@ -244,13 +245,17 @@ sub _normalize_route { # Options are optional, try to deduce their presence from arg length. if ( @_ == 4 ) { # @_ = ( NAME, REGEXP, OPTIONS, CODE ) + # get 'foo', '/foo', { 'user_agent' => '...' }, sub {...} @args{qw} = @_; } elsif ( @_ == 2 ) { # @_ = ( REGEXP, CODE ) + # get '/foo', sub {...} @args{qw} = ( $_[0], {}, $_[1] ); } elsif ( @_ == 3 ) { # @_ = ( REGEXP, OPTIONS, CODE ) + # get '/foo', { 'user_agent' => '...', sub {...} # @_ = ( NAME, REGEXP, CODE ) + # get 'foo', '/foo',sub {...} if (ref $_[1] eq 'HASH') { @args{qw} = @_; } else { @@ -425,6 +430,9 @@ sub captures { $Dancer2::Core::Route::REQUEST->captures } sub uri_for { shift; $Dancer2::Core::Route::REQUEST->uri_for(@_); } +# Should this really be in App or should it go in the request? +sub uri_for_route { shift->app->uri_for_route(@_); } + sub splat { $Dancer2::Core::Route::REQUEST->splat } sub params { shift; $Dancer2::Core::Route::REQUEST->params(@_); } diff --git a/lib/Dancer2/Core/Route.pm b/lib/Dancer2/Core/Route.pm index 8d21daf76..caec9dbb8 100644 --- a/lib/Dancer2/Core/Route.pm +++ b/lib/Dancer2/Core/Route.pm @@ -15,9 +15,9 @@ our ( $REQUEST, $RESPONSE, $RESPONDER, $WRITER, $ERROR_HANDLER ); my $count = 0; has name => ( - is => 'ro', - isa => Str, - default => sub { $count++ }, + is => 'ro', + isa => Str, + predicate => 'has_name', ); has method => ( diff --git a/lib/Dancer2/Manual.pod b/lib/Dancer2/Manual.pod index d0244e24a..bbf9217cf 100644 --- a/lib/Dancer2/Manual.pod +++ b/lib/Dancer2/Manual.pod @@ -213,6 +213,13 @@ coderef to execute, which returns the response. The above route specifies that, for GET requests to C, the code block provided should be executed. + +You can also provide routes with a name: + + get 'hi_to' => '/hello/:name' => sub {...}; + +See C on how this can be used. + =head3 Retrieving request parameters The L, diff --git a/lib/Dancer2/Manual/Keywords.pod b/lib/Dancer2/Manual/Keywords.pod index bd1c9a920..ebdf02623 100644 --- a/lib/Dancer2/Manual/Keywords.pod +++ b/lib/Dancer2/Manual/Keywords.pod @@ -200,6 +200,12 @@ Defines a route for HTTP B requests to the given URL: del '/resource' => sub { ... }; +You can also provide the route with a name: + + del 'rec' => '/resource' => sub { ... }; + +See C on how this can be used. + =head2 delayed Stream a response asynchronously. For more information, please see @@ -349,6 +355,14 @@ Defines a route for HTTP B requests to the given path: Note that a route to match B requests is automatically created as well. +You can also provide the route with a name: + + get 'index' => '/' => sub { + return "Hello world"; + } + +See C on how this can be used. + =head2 halt Sets a response object with the content given. @@ -516,6 +530,12 @@ Defines a route for HTTP B requests to the given URL: intended to work as a "partial-PUT", transferring just the changes; please see L for further details.) +You can also provide the route with a name: + + patch 'rec' => '/resource' => sub { ... }; + +See C on how this can be used. + =head2 path Concatenates multiple paths together, without worrying about the underlying @@ -534,6 +554,14 @@ Defines a route for HTTP B requests to the given URL: return "Hello world"; } +You can also provide the route with a name: + + post 'index' => '/' => sub { + return "Hello world"; + } + +See C on how this can be used. + =head2 prefix Defines a prefix for each route handler, like this: @@ -625,6 +653,12 @@ Defines a route for HTTP B requests to the given URL: put '/resource' => sub { ... }; +You can also provide the route with a name: + + put 'rec' => '/resource' => sub { ... }; + +See C on how this can be used. + =head2 query_parameters Returns a L object from the request parameters. @@ -1146,6 +1180,108 @@ URL encoding via a third parameter: uri_for('/path', { foo => 'qux%3Dquo' }, 1); # would return http://localhost:5000/path?foo=qux%3Dquo +=head2 uri_for_route + +An enhanced version of C that utilizes their names. + + get 'view_entry' => '/entry/view/:id' => sub {...}; + +Now that the route has a name we can use C to +create a URI for it: + + my $path = uri_for_route( + 'view_entry', + { 'id' => 3 }, + { 'foo' => 'bar' }, + ); + + # (assuming it's run on a local server in HTTP port 5000) + # $path = 'http://localhost:5000/entry/view/3?foo=bar' + +This works for every HTTP method, except C (which is +effectively a C). + +There are multiple arguments options: + +=over 4 + +=item * Route parameters + +The first argument controls the route parameters: + + get 'test' => '/:foo/:bar' => sub {1}; + # ... + $path = uri_for_route( 'test', { 'foo' => 'hello', 'bar' => 'world' } ); + # $path = http://localhost:5000/hello/world + +=item * Splat route parameters + +If you provide an arrayref instead of hashref, it will assume on +these being splat and megasplat args: + + get 'test' => '/*/*/**' => sub {1}; + # ... + $path = uri_for_route( + 'test', + [ 'hello', 'world', [ 'myhello', 'myworld' ], + ); + # $path = http://localhost:5000/hello/world/myhello/myworld + +=item * Mixed route parameters + +If you have a route that includes both, the plat and megasplat +arguments need to be under the C key: + + patch 'test' => '/*/:id/*/:foo/*' => sub {1}; + # ... + $path = uri_for_route( + 'test', + { + 'id' => 4, + 'foo ' => 'bar', + 'splat' => [ 'hello', 'world' ], + } + ); + # $path = http://localhost:5000/hello/4/world/bar + +=item * Query parameters + +If you want to create a path the query parameters, use the +second argument: + + get 'index' => '/:foo' => sub {1}; + get 'update_form' => '/update' => sub {1}; + + # ... + + $path = uri_for_route( + 'index', + { 'foo' => 'bar' }, + { 'id' => 1 }, + ); + # $path = http://localhost:5000/bar?id=1 + + $path = uri_for_route( 'update_form', {}, { 'id' => 2 } ); + # $path = http://localhost:5000/update?id=2 + +(Technically, only C requests should include query parameters, but +C does not enforce this.) + +=item * Escaping + +The final parameter determines whether the URI will be URI-escaped: + + get 'show_entry' => '/view/:str_id' => sub {1}; + # ... + $path = uri_for_route( 'show_entry' => { 'str_id' => '!£%^@' }, {}, 1 ); + # $path = http://localhost/view/!@%C3%82%C2%A3$% + +This is useful when your ID is not HTML-safe and might include HTML +tags and Javascript code or include characters that interfere with the +URI request string (like a forward slash). + +=back + =head2 var Provides an accessor for variables shared between hooks and route diff --git a/t/dsl/uri_for.t b/t/dsl/uri_for.t new file mode 100644 index 000000000..7712ce5c3 --- /dev/null +++ b/t/dsl/uri_for.t @@ -0,0 +1,48 @@ +use strict; +use warnings; +use Test::More 'tests' => 2; +use Plack::Test; +use Plack::Builder; +use HTTP::Request::Common; + +{ + package App; + use Dancer2; + get '/' => sub { return uri_for('/foo'); }; +} + +{ + package MountedApp; + use Dancer2; + get '/' => sub { return uri_for('/bar'); }; +} + +my $prefix = 'http://localhost'; + +subtest 'Non-mounted app' => sub { + my $app = Plack::Test->create( App->to_app ); + my $res; + + $res = $app->request( GET "$prefix/" ); + ok( $res->is_success, 'Successful request' ); + is( $res->content, "$prefix/foo", 'Correct regular path' ); +}; + +subtest 'Mounted app' => sub { + my $app = Plack::Test->create( + builder { + mount '/mount' => MountedApp->to_app; + mount '/' => App->to_app; + } + ); + + my $res; + + $res = $app->request( GET "$prefix/" ); + ok( $res->is_success, 'Successful request' ); + is( $res->content, "$prefix/foo", 'Correct mounted regular path' ); + + $res = $app->request( GET "$prefix/mount" ); + ok( $res->is_success, 'Successful request' ); + is($res->content, "$prefix/mount/bar", 'Correct mounted regular path'); +}; diff --git a/t/dsl/uri_for_route.t b/t/dsl/uri_for_route.t new file mode 100644 index 000000000..c327b884b --- /dev/null +++ b/t/dsl/uri_for_route.t @@ -0,0 +1,246 @@ +use strict; +use warnings; +use Test::More 'tests' => 3; +use Plack::Test; +use Plack::Builder; +use HTTP::Request::Common; +use JSON::MaybeXS; + +{ + package App; + use Dancer2; + our $tested; + + # Static with route params + # Static with code + # Static with options and code + get 'view_entry_static1' => '/view1/:id' => sub {1}; + get 'view_entry_static2' => '/view2/:id' => { 'user_agent' => 'UA/1.0' }, sub {1}; + + # static with typed route param + get 'view_user' => '/:prefix/user/:username[Str]' => sub {1}; + + # splat / megasplat + get 'view_entry_splat' => '/viewsplat/*/*/**' => sub {1}; + + # Mixed with splat/megasplat + # Different method + patch 'view_entry_mixed' => '/view_mixed/*/**/:id' => sub {1}; + + # Regexp - fails + get 'view_entry_regexp1' => qr{/rview1/[0-9]+} => sub {1}; + + post '/uri_for_route' => sub { + my $params = JSON::MaybeXS::decode_json( request->content ); + return uri_for_route( + $params->{'route_name'}, + $params->{'route_params'}, + $params->{'query_params'} // {}, + !!$params->{'dont_escape'}, + ); + }; + + get '/fail_uri_for_route' => sub { + my $failed = 0; + eval { + uri_for_route('vvv'); + 1; + } or do { + ::like( + $@, + qr/\QCannot find route named 'vvv'\E/xms, + 'Cannot retrieve nonexistent route', + ); + + $failed++; + }; + + return $failed; + }; + + get '/fail_uri_for_route_splat_args' => sub { + my $failed = 0; + eval { + uri_for_route( + 'view_entry_splat', + ['foo'], + ); + + 1; + } or do { + ::like( + $@, + qr/\QMismatch in amount of splat args and splat elements\E/xms, + 'Cannot handle mismatched splat args and elements', + ); + + $failed++; + }; + + return $failed; + }; + + get '/fail_uri_for_route_leftovers' => sub { + my $failed = 0; + eval { + uri_for_route('view_entry_static1'); + 1; + } or do { + my $msg = 'Route view_entry_static1 uses the parameter \'id\', ' + . 'which was not provided'; + + ::like( + $@, + qr/\Q$msg\E/xms, + 'Cannot handle leftover route parameters', + ); + + $failed++; + }; + + return $failed; + }; + + # Error defining two routes with the same name, regardless of method + eval { + get 'view_entry_splat' => '/' => sub {1}; + 1; + } or do { + ::like( + $@, + qr/\QRoute with this name (view_entry_splat) already exists\E/xms, + 'Cannot register two routes with same name', + ); + + $tested = 1; + }; +} + +sub test_app { + my ( $app, $mount_path ) = @_; + + my $prefix = 'http://localhost'; + $mount_path + and $prefix .= $mount_path; + + my ( $path, $res ); + + # Test static paths + foreach my $idx ( 1 .. 2 ) { + $res = $app->request( + POST( + "$prefix/uri_for_route", + 'Content' => JSON::MaybeXS::encode_json({ + 'route_name' => "view_entry_static$idx", + 'route_params' => { 'id' => $idx }, + 'query_params' => { 'foo' => $idx }, + }), + ) + ); + + $path = "$prefix/view$idx/$idx?foo=$idx"; + ok( $res->is_success, 'Successful request' ); + is( $res->content, $path, "Correct path: $path" ); + } + + # Test splat + megasplat + $res = $app->request( + POST( + "$prefix/uri_for_route", + 'Content' => JSON::MaybeXS::encode_json({ + 'route_name' => 'view_entry_splat', + 'route_params' => [ 'foo', 'bar', [ 'baz', 'quux' ] ], + 'query_params' => { 'id' => 'di' }, + }), + ) + ); + + $path = "$prefix/viewsplat/foo/bar/baz/quux?id=di"; + ok( $res->is_success, 'Successful request' ); + is( $res->content, $path, "Correct path: $path" ); + + # Test mixed + $res = $app->request( + POST( + "$prefix/uri_for_route", + 'Content' => JSON::MaybeXS::encode_json( + { 'route_name' => 'view_entry_mixed', + 'route_params' => { + 'id' => 'di', + 'splat' => ['foo', ['bar', 'baz']] + }, + 'query_params' => {'foo' => 'bar'}, + } + ), + ) + ); + + $path = "$prefix/view_mixed/foo/bar/baz/di?foo=bar"; + ok( $res->is_success, 'Successful request' ); + is( $res->content, $path, "Correct path: $path" ); + + # Test escaping + $res = $app->request( + POST( + "$prefix/uri_for_route", + 'Content' => JSON::MaybeXS::encode_json({ + 'route_name' => 'view_entry_static1', + 'route_params' => { 'id' => '!@£$%' }, + }), + ) + ); + + $path = "$prefix/view1/!@%C3%82%C2%A3\$%"; + ok( $res->is_success, 'Successful request' ); + is( $res->content, $path, "Correct path: $path" ); + + # Test nonexistent route name + $res = $app->request( GET "$prefix/fail_uri_for_route" ); + ok( $res->is_success, 'Successful request' ); + is( $res->content, '1', 'Successfully tested nonexistent failure mode' ); + + # Test splat + megasplat (incorrect amount) + $res = $app->request( GET "$prefix/fail_uri_for_route_splat_args" ); + ok( $res->is_success, 'Successful request' ); + is( $res->content, '1', 'Successfully tested mismatch splat args/elements failure mode' ); + + # Test mixed with not all filled (named args left) + $res = $app->request( GET "$prefix/fail_uri_for_route_leftovers" ); + ok( $res->is_success, 'Successful request' ); + is( $res->content, '1', 'Successfully tested leftover args failure mode' ); + + # Static with typed route parameters + $res = $app->request( + POST( + "$prefix/uri_for_route", + 'Content' => JSON::MaybeXS::encode_json({ + 'route_name' => 'view_user', + 'route_params' => { 'prefix' => 'foo', 'username' => 'sawyer' }, + 'query_params' => { 'foo' => 1 }, + }), + ) + ); + + $path = "$prefix/foo/user/sawyer?foo=1"; + ok( $res->is_success, 'Successful request' ); + is( $res->content, $path, "Correct path for typed route param: $path" ); +} + +subtest 'Non-mounted app' => sub { + my $app = Plack::Test->create( App->to_app ); + test_app($app); + ok( $App::tested, 'Check for duplicate route names done successfully' ); +}; + +subtest 'Mounted app' => sub { + my $app = Plack::Test->create( + builder { + mount '/mount' => App->to_app; + mount '/' => sub { + return { Plack::Response->new(200, [], ['OK'] ) } + }, + } + ); + + test_app( $app, '/mount' ); +}; diff --git a/t/named_routes.t b/t/named_routes.t index 4cd86b153..4e933ad19 100644 --- a/t/named_routes.t +++ b/t/named_routes.t @@ -24,7 +24,7 @@ use HTTP::Request::Common; }; # Name, Regexp, Options, Code - get 'base_regex', qr{^/r$} => { 'user_agent' => 'XX' }, sub { + get 'base_regex', qr{^/r$}, {}, sub { 'Base Regex'; }; @@ -35,7 +35,7 @@ use HTTP::Request::Common; my $test = Plack::Test->create( MyApp->to_app ); -subtest 'Named route' => sub { +subtest 'Named static route' => sub { plan 'tests' => 2; my $response = $test->request( GET '/view' ); @@ -51,7 +51,7 @@ subtest 'Named regex route' => sub { is( $response->content, 'View Regex', 'Regex route with name' ); }; -subtest 'Named route with options' => sub { +subtest 'Named static route with options' => sub { plan 'tests' => 2; my $response = $test->request( GET '/', 'User-Agent' => 'XX' ); @@ -74,19 +74,16 @@ subtest 'Route objects' => sub { my @apps = @{ Dancer2::runner->apps }; is( scalar @apps, 1, 'Only one app exists' ); - my @routes = @{ $apps[0]->routes->{'get'} }; - is( scalar @routes, 7, 'Five routes registered' ); + my %routes = %{ $apps[0]->route_names() }; + is( scalar keys %routes, 4, 'Four named routes registered' ); is_deeply( - [ map $_->name, @routes ], + [ sort keys %routes ], [ - 'view_static', - 'view_regex', - 'base_static', 'base_regex', - 0, # /ignore1 (0) + HEAD (Plack::Middleware::Head) (1) - 2, # /ignore2 (2) + HEAD (Plack::Middleware::Head) (3) - 4, # /ignore3 (4) + HEAD (Plack::Middleware::Head) (5) + 'base_static', + 'view_regex', + 'view_static', ], 'All the right route names', ); diff --git a/t/uri_for.t b/t/uri_for.t deleted file mode 100644 index 7eff34789..000000000 --- a/t/uri_for.t +++ /dev/null @@ -1,30 +0,0 @@ -use strict; -use warnings; -use Test::More import => ['!pass']; -use Plack::Test; -use HTTP::Request::Common; -use Ref::Util qw; - -{ - package App; - use Dancer2; - get '/foo' => sub { - return uri_for('/foo'); - }; -} - -my $app = App->to_app; -ok( is_coderef($app), 'Got app' ); - -test_psgi $app, sub { - my $cb = shift; - - is( $cb->( GET '/foo' )->code, 200, '/foo code okay' ); - is( - $cb->( GET '/foo' )->content, - 'http://localhost/foo', - 'uri_for works as expected', - ); -}; - -done_testing;