Stabilize routing

cpanfile

@@ -4,7 +4,7 @@ requires 'Scalar::Util';
 requires 'Plack'                             => '1.0029';
 requires 'Plack::Middleware::Static';
 requires 'Plack::Middleware::ReverseProxy';
-requires 'Router::Boom'                      => '1.01';
+requires 'Router::Boom'                      => '1.03';
 requires 'Cwd';
 requires 'File::Basename';
 requires 'Text::Xslate'                      => '3.2.4';

lib/Kossy.pm

@@ -13,7 +13,7 @@ use JSON qw//;
 use Scalar::Util qw//;
 use Try::Tiny;
 use Encode;
-use Router::Boom;
+use Router::Boom::Method;
 use Class::Accessor::Lite (
     new => 0,
     rw => [qw/root_dir/]
@@ -74,9 +74,6 @@ sub psgi {
 sub build_app {
     my $self = shift;
 
-    #router
-    my $router = Router::Boom->new;
-    $router->add($_ => $self->_router->{$_} ) for keys %{$self->_router};
     my $security_header_local = $SECURITY_HEADER;
     my %match_cache;
 
@@ -110,17 +107,17 @@ sub build_app {
                     return @{$match_cache{$cache_key}};
                 }
                 my $path_info = Encode::decode_utf8( $env->{PATH_INFO},  Encode::FB_CROAK | Encode::LEAVE_SRC );
-                my @match = $router->match($path_info);
-                if ( !@match ) {
-                    $c->halt(404);
+                my ($dest, $captured, $method_not_allowed) = $self->_router->match($method, $path_info);
+                if ($method_not_allowed) {
+                    $c->halt(405);
                 }
 
-                if ( !exists $match[0]->{$method}) {
-                    $c->halt(405);
+                if (!$dest) {
+                    $c->halt(404);
                 }
-                $match_cache{$cache_key} = [$match[0]->{$method},$match[1]]
-                    if ! scalar keys %{$match[1]};
-                return ($match[0]->{$method},$match[1]);
+
+                $match_cache{$cache_key} = [$dest, $captured];
+                return ($dest, $captured);
             } catch {
                 if ( ref $_ && ref $_ eq 'Kossy::Exception' ) {
                     die $_; #rethrow
@@ -223,7 +220,7 @@ sub _router {
     my $klass = shift;
     my $class = ref $klass ? ref $klass : $klass;
     if ( !$_ROUTER->{$class} ) {
-        $_ROUTER->{$class} = {};
+        $_ROUTER->{$class} = Router::Boom::Method->new;
     }
     $_ROUTER->{$class};
 }
@@ -236,12 +233,13 @@ sub _connect {
         $code = $filter;
         $filter = [];
     }
-    for my $method ( @$methods ) {
-        $class->_router->{$pattern}->{$method} = {
-            __action__ => $code,
-            __filter__ => $filter
-        };
+    unless ( (ref $code ||'') eq 'CODE') {
+        Carp::croak "code must be CODE reference";
     }
+    $class->_router->add($methods, $pattern, {
+        __action__ => $code,
+        __filter__ => $filter
+    });
 }
 
 sub get {
@@ -421,7 +419,13 @@ per-request object, herds request and response
 
 =item args:HashRef
 
-Router::Simple->match result
+path parameters captured by Router::Boom
+
+    get '/user/:id' => sub {
+        my ($self, $c) = @_;
+        my $id = $c->args->{id};
+        ...
+    };
 
 =item halt(status_code, message)
 

t/000_kossy/routing.t

@@ -0,0 +1,51 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Plack::Test;
+use HTTP::Request::Common qw(GET POST PUT DELETE);
+
+{
+    package SampleApp;
+    use Kossy;
+
+    get '/'                              => sub { $_[1]->halt_text(200, 'GET /') };
+    get '/user/me'                       => sub { $_[1]->halt_text(200, 'GET /user/me') };
+    get '/user/:name'                    => sub { $_[1]->halt_text(200, 'GET /user/:name') };
+    get '/user/:name/likes'              => sub { $_[1]->halt_text(200, 'GET /user/:name/likes') };
+    post '/user/:name/likes'             => sub { $_[1]->halt_text(200, 'POST /user/:name/likes') };
+    router 'DELETE', '/user/:name/likes' => sub { $_[1]->halt_text(200, 'DELETE /user/:name/likes') };
+
+    get '/entry/:id'           => sub { $_[1]->halt_text(200, 'GET /entry/:id') };
+    router 'PUT', '/entry/:id' => sub { $_[1]->halt_text(200, 'PUT /entry/:id') };
+    get '/entry/foo'           => sub { $_[1]->halt_text(200, 'GET /entry/foo') };
+
+    get '/org/{org_id:[0-9]+}' => sub { $_[1]->halt_text(200, 'GET /org/{org_id}') };
+    get '/org/foo'             => sub { $_[1]->halt_text(200, 'GET /org/foo') };
+}
+
+my $app = SampleApp->psgi;
+
+test_psgi $app, sub {
+    my $cb = shift;
+
+    is $cb->(GET "/")->content,                  'GET /';
+    is $cb->(GET "/user/me")->content,           'GET /user/me';
+    is $cb->(GET "/user/foo")->content,          'GET /user/:name';
+    is $cb->(GET "/user/foo/likes")->content,    'GET /user/:name/likes';
+    is $cb->(POST "/user/foo/likes")->content,   'POST /user/:name/likes';
+    is $cb->(DELETE "/user/foo/likes")->content, 'DELETE /user/:name/likes';
+
+    is $cb->(GET "/entry/bar")->content, 'GET /entry/:id';
+    is $cb->(PUT "/entry/bar")->content, 'PUT /entry/:id';
+    is $cb->(GET "/entry/foo")->content, 'GET /entry/:id', 'Not match /entry/foo';
+
+    is $cb->(GET "/org/123")->content, 'GET /org/{org_id}';
+    is $cb->(GET "/org/foo")->content, 'GET /org/foo';
+    is $cb->(GET "/org/bar")->code, 404, 'Not match since `bar` is not number';
+
+    is $cb->(GET "/not_found")->code, 404, 'Not found path';
+    is $cb->(POST "/")->code, 405, 'Method not allowed';
+};
+
+done_testing;

t/100_bugs/003_random_routes.t

@@ -0,0 +1,30 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Kossy;
+use Plack::Test;
+use HTTP::Request::Common;
+
+get '/api/me' => sub {
+    my ($self, $c) = @_;
+    $c->halt_text(200, '/api/me');
+};
+
+get '/api/:name' => sub {
+    my ($self, $c) = @_;
+    $c->halt_text(200, '/api/:name');
+};
+
+my $app = __PACKAGE__->psgi;
+
+test_psgi $app, sub {
+    my $cb = shift;
+
+    # Matching routing was unstable due to random Hash keys.
+    # This change gives priority to the previously defined routing
+    my $res = $cb->(GET "/api/me");
+    is $res->content, '/api/me';
+};
+
+done_testing;