From: John Napiorkowski Date: Thu, 19 Mar 2015 13:56:50 +0000 (-0500) Subject: basic chaining in place X-Git-Tag: 5.90089_002~39 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=a82c96cf2bf688f97140bad7fd3979a531416a22 basic chaining in place --- diff --git a/lib/Catalyst/Action.pm b/lib/Catalyst/Action.pm index 37582e5..fd444da 100644 --- a/lib/Catalyst/Action.pm +++ b/lib/Catalyst/Action.pm @@ -107,12 +107,77 @@ has args_constraints => ( return \@args; } +has captures_constraints => ( + is=>'ro', + init_arg=>undef, + traits=>['Array'], + isa=>'ArrayRef', + required=>1, + lazy=>1, + builder=>'_build_captures_constraints', + handles => { + has_captures_constraints => 'count', + number_of_captures_constraints => 'count', + }); + + sub _build_captures_constraints { + my $self = shift; + my @arg_protos = @{$self->attributes->{CaptureArgs}||[]}; + + return [] unless scalar(@arg_protos); + # If there is only one arg and it looks like a number + # we assume its 'classic' and the number is the number of + # constraints. + my @args = (); + if( + scalar(@arg_protos) == 1 && + looks_like_number($arg_protos[0]) + ) { + return \@args; + } else { + @args = + map { $self->resolve_type_constraint($_) || die "$_ is not a constraint!" } + @arg_protos; + } + + return \@args; + } + sub resolve_type_constraint { my ($self, $name) = @_; my $tc = eval "package ${\$self->class}; $name" || undef; return $tc || Moose::Util::TypeConstraints::find_or_parse_type_constraint($name); } +has number_of_captures => ( + is=>'ro', + init_arg=>undef, + isa=>'Int', + required=>1, + lazy=>1, + builder=>'_build_number_of_captures'); + + sub _build_number_of_captures { + my $self = shift; + if( ! exists $self->attributes->{CaptureArgs} ) { + # If there are no defined capture args, thats considered 0. + return 0; + } elsif(!defined($self->attributes->{CaptureArgs}[0])) { + # If you fail to give a defined value, that's also 0 + return 0; + } elsif( + scalar(@{$self->attributes->{CaptureArgs}}) == 1 && + looks_like_number($self->attributes->{CaptureArgs}[0]) + ) { + # 'Old school' numbered captures + return $self->attributes->{CaptureArgs}[0]; + } else { + # New hotness named arg constraints + return $self->number_of_captures_constraints; + } + } + + use overload ( # Stringify to reverse for debug output etc. @@ -164,6 +229,10 @@ sub match { # return 1; #} } else { + # Because of the way chaining works, we can expect args that are totally not + # what you'd expect length wise. When they don't match length, thats a fail + return 0 unless scalar( @{ $c->req->args } ) == $self->normalized_arg_number; + for my $i(0..$#{ $c->req->args }) { $self->args_constraints->[$i]->check($c->req->args->[$i]) || return 0; } @@ -175,20 +244,38 @@ sub match { } } -sub match_captures { 1 } +sub match_captures { + my ($self, $c, $captures) = @_; + my @captures = @{$captures||[]}; + + return 1 unless scalar(@captures); # If none, just say its ok + + if($self->has_captures_constraints) { + if( + $self->number_of_captures_constraints == 1 && + ( + $self->captures_constraints->[0]->is_a_type_of('Ref') || + $self->captures_constraints->[0]->is_a_type_of('ClassName') + ) + ) { + return 1 if $self->captures_constraints->[0]->check($c->req->args); + } else { + for my $i(0..$#captures) { + $self->captures_constraints->[$i]->check($captures[$i]) || return 0; + } + return 1; + } + } else { + return 1; + } + return 1; +} sub compare { my ($a1, $a2) = @_; return $a1->normalized_arg_number <=> $a2->normalized_arg_number; } -sub number_of_captures { - my ( $self ) = @_; - - return 0 unless exists $self->attributes->{CaptureArgs}; - return $self->attributes->{CaptureArgs}[0] || 0; -} - sub scheme { return exists $_[0]->attributes->{Scheme} ? $_[0]->attributes->{Scheme}[0] : undef; } @@ -196,7 +283,7 @@ sub scheme { sub list_extra_info { my $self = shift; return { - Args => $self->attributes->{Args}[0], + Args => $self->normalized_arg_number, CaptureArgs => $self->number_of_captures, } } diff --git a/lib/Catalyst/DispatchType/Chained.pm b/lib/Catalyst/DispatchType/Chained.pm index e29e5b5..47335a8 100644 --- a/lib/Catalyst/DispatchType/Chained.pm +++ b/lib/Catalyst/DispatchType/Chained.pm @@ -236,7 +236,7 @@ sub recurse_match { my @try_actions = @{$children->{$try_part}}; TRY_ACTION: foreach my $action (@try_actions) { if (my $capture_attr = $action->attributes->{CaptureArgs}) { - my $capture_count = $capture_attr->[0] || 0; + my $capture_count = $action->number_of_captures|| 0; # Short-circuit if not enough remaining parts next TRY_ACTION unless @parts >= $capture_count; @@ -248,7 +248,7 @@ sub recurse_match { push(@captures, splice(@parts, 0, $capture_count)); # check if the action may fit, depending on a given test by the app - if ($action->can('match_captures')) { next TRY_ACTION unless $action->match_captures($c, \@captures) } + next TRY_ACTION unless $action->match_captures($c, \@captures); # try the remaining parts against children of this action my ($actions, $captures, $action_parts, $n_pathparts) = $self->recurse_match( @@ -309,32 +309,6 @@ Calls register_path for every Path attribute for the given $action. =cut -sub _check_args_attr { - my ( $self, $action, $name ) = @_; - - return unless exists $action->attributes->{$name}; - - if (@{$action->attributes->{$name}} > 1) { - Catalyst::Exception->throw( - "Multiple $name attributes not supported registering " . $action->reverse() - ); - } - my $args = $action->attributes->{$name}->[0]; - if (defined($args) and not ( - Scalar::Util::looks_like_number($args) and - int($args) == $args and $args >= 0 - )) { - require Data::Dumper; - local $Data::Dumper::Terse = 1; - local $Data::Dumper::Indent = 0; - $args = Data::Dumper::Dumper($args); - Catalyst::Exception->throw( - "Invalid $name($args) for action " . $action->reverse() . - " (use '$name' or '$name()')" - ); - } -} - sub register { my ( $self, $c, $action ) = @_; @@ -382,10 +356,6 @@ sub register { $self->_actions->{'/'.$action->reverse} = $action; - foreach my $name (qw(Args CaptureArgs)) { - $self->_check_args_attr($action, $name); - } - if (exists $action->attributes->{Args} and exists $action->attributes->{CaptureArgs}) { Catalyst::Exception->throw( "Combining Args and CaptureArgs attributes not supported registering " . diff --git a/t/arg_constraints.t b/t/arg_constraints.t index 57a90fe..b753e21 100644 --- a/t/arg_constraints.t +++ b/t/arg_constraints.t @@ -93,10 +93,24 @@ BEGIN { my ($self, $c, $int) = @_; $c->res->body('match'); } + sub any_priority :Path('priority_test') Args(1) { $_[1]->res->body('any_priority') } sub int_priority :Path('priority_test') Args(Int) { $_[1]->res->body('int_priority') } + sub chain_base :Chained(/) CaptureArgs(1) { } + + sub any_priority_chain :Chained(chain_base) PathPart('') Args(1) { $_[1]->res->body('any_priority_chain') } + + sub int_priority_chain :Chained(chain_base) PathPart('') Args(Int) { $_[1]->res->body('int_priority_chain') } + + sub link_int :Chained(chain_base) PathPart('') CaptureArgs(Int) { } + + sub any_priority_link :Chained(link_int) PathPart('') Args(1) { $_[1]->res->body('any_priority_link') } + + sub int_priority_link :Chained(link_int) PathPart('') Args(Int) { $_[1]->res->body('int_priority_link') } + + sub default :Default { my ($self, $c, $int) = @_; $c->res->body('default'); @@ -195,4 +209,24 @@ SKIP: { is $res->content, 'name: mary, age: 36'; } +{ + my $res = request '/chain_base/capture/arg'; + is $res->content, 'any_priority_chain'; +} + +{ + my $res = request '/chain_base/cap1/100/arg'; + is $res->content, 'any_priority_link'; +} + +{ + my $res = request '/chain_base/cap1/101/102'; + is $res->content, 'int_priority_link'; +} + +{ + my $res = request '/chain_base/capture/100'; + is $res->content, 'int_priority_chain', 'got expected'; +} + done_testing; diff --git a/t/dead_load_bad_args.t b/t/dead_load_bad_args.t index 98b566c..58c2f11 100644 --- a/t/dead_load_bad_args.t +++ b/t/dead_load_bad_args.t @@ -4,6 +4,13 @@ use lib 't/lib'; use Test::More; +# This test needs to be rewritten (and the code it was using as well) since +# when we added the arg and capturearg type constraint support, we now allow +# non integer values. however we could probably support some additional sanity +# testing on the values, so this is a nice TODO for someone -jnap + +plan skip_all => 'Removing this test because constraint arg types allow this'; + use Catalyst::Test 'TestApp'; for my $fail (