From: Gareth Kirwan <gbjk@thermeon.com>
Date: Tue, 28 Aug 2012 15:22:42 +0000 (+0100)
Subject: Change Catalsyt _parse_attrs so that when sub attr handlers:
X-Git-Tag: 5.90017~5
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=0b0aee670c39f8cb8f140eb62de9bfaf2c343a24;hp=d0cacee71a316290bc01f0e12681c16bdc1e84e2

Change Catalsyt _parse_attrs so that when sub attr handlers:
1) Can return multiple pairs of new attributes
2) Get their returned attributes passed through the correct attribute handler.
e.g sub _parse_Whatever_attr { return Chained => 'foo', PathPart => 'bar' }
Will now work because both new attributes are respected, and the Chained attribute is passed
to _parse_Chained_attr and fixedup correctly by that.
---

diff --git a/lib/Catalyst/Controller.pm b/lib/Catalyst/Controller.pm
index cea7234..445d4af 100644
--- a/lib/Catalyst/Controller.pm
+++ b/lib/Catalyst/Controller.pm
@@ -407,16 +407,30 @@ sub _parse_attrs {
 
     my %final_attributes;
 
-    foreach my $key (keys %raw_attributes) {
+    while (my ($key, $value) = each %raw_attributes){
+        my $new_attrs = $self->_parse_attr($c, $name, $key => $value );
+        push @{ $final_attributes{$_} }, @{ $new_attrs->{$_} } for keys %$new_attrs;
+    }
 
-        my $raw = $raw_attributes{$key};
+    return \%final_attributes;
+}
 
-        foreach my $value (ref($raw) eq 'ARRAY' ? @$raw : $raw) {
+sub _parse_attr {
+    my ($self, $c, $name, $key, $values) = @_;
 
-            my $meth = "_parse_${key}_attr";
-            if ( my $code = $self->can($meth) ) {
-                ( $key, $value ) = $self->$code( $c, $name, $value );
+    my %final_attributes;
+    foreach my $value (ref($values) eq 'ARRAY' ? @$values : $values) {
+        my $meth = "_parse_${key}_attr";
+        if ( my $code = $self->can($meth) ) {
+            my %new_attrs = $self->$code( $c, $name, $value );
+            while (my ($new_key, $value) = each %new_attrs){
+                my $new_attrs = $key eq $new_key ?
+                    { $new_key => [$value] } :
+                    $self->_parse_attr($c, $name, $new_key => $value );
+                push @{ $final_attributes{$_} }, @{ $new_attrs->{$_} } for keys %$new_attrs;
             }
+        }
+        else {
             push( @{ $final_attributes{$key} }, $value );
         }
     }
@@ -426,14 +440,16 @@ sub _parse_attrs {
 
 sub _parse_Global_attr {
     my ( $self, $c, $name, $value ) = @_;
-    return $self->_parse_Path_attr( $c, $name, "/$name" );
+    # _parse_attr will call _parse_Path_attr for us
+    return Path => "/$name";
 }
 
 sub _parse_Absolute_attr { shift->_parse_Global_attr(@_); }
 
 sub _parse_Local_attr {
     my ( $self, $c, $name, $value ) = @_;
-    return $self->_parse_Path_attr( $c, $name, $name );
+    # _parse_attr will call _parse_Path_attr for us
+    return Path => $name;
 }
 
 sub _parse_Relative_attr { shift->_parse_Local_attr(@_); }
@@ -519,7 +535,7 @@ sub _parse_MyAction_attr {
     my ( $self, $c, $name, $value ) = @_;
 
     my $appclass = Catalyst::Utils::class2appclass($self);
-    $value = "${appclass}::Action::${value}";
+    $value = "+${appclass}::Action::${value}";
 
     return ( 'ActionClass', $value );
 }
diff --git a/t/aggregate/live_component_controller_attributes.t b/t/aggregate/live_component_controller_attributes.t
index e8832d9..d991b79 100644
--- a/t/aggregate/live_component_controller_attributes.t
+++ b/t/aggregate/live_component_controller_attributes.t
@@ -3,17 +3,44 @@
 use strict;
 use warnings;
 
+use Data::Dumper;
+$Data::Dumper::Maxdepth=1;
 use FindBin;
 use lib "$FindBin::Bin/../lib";
 
-use Test::More tests => 4;
+use Test::More tests => 13;
 use Catalyst::Test 'TestApp';
 
+sub ok_actions {
+    my ($response, $actions, $msg) = @_;
+    my $expected = join ", ",
+        (map { "TestApp::Controller::Attributes->$_" } @$actions),
+        'TestApp::Controller::Root->end';
+    is( $response->header('x-catalyst-executed') => $expected,
+        $msg//'Executed correct acitons');
+    }
+
 ok( my $response = request('http://localhost/attributes/view'),
     'get /attributes/view' );
 ok( !$response->is_success, 'Response Unsuccessful' );
 
 ok( $response = request('http://localhost/attributes/foo'),
     "get /attributes/foo" );
+ok_actions($response => ['foo']);
+
+ok( $response = request('http://localhost/attributes/all_attrs'),
+    "get /attributes/all_attrs" );
+ok( $response->is_success, "Response OK" );
+ok_actions($response => [qw/fetch all_attrs_action/]);
 
+ok( $response = request('http://localhost/attributes/some_attrs'),
+    "get /attributes/some_attrs" );
 ok( $response->is_success, "Response OK" );
+ok_actions($response => [qw/fetch some_attrs_action/]);
+
+ok( $response = request('http://localhost/attributes/one_attr'),
+    "get /attributes/one_attr" );
+ok( $response->is_success, "Response OK" );
+ok_actions($response => [qw/fetch one_attr_action/]);
+
+
diff --git a/t/lib/TestApp/Controller/Attributes.pm b/t/lib/TestApp/Controller/Attributes.pm
index 6f8020b..6cb536c 100644
--- a/t/lib/TestApp/Controller/Attributes.pm
+++ b/t/lib/TestApp/Controller/Attributes.pm
@@ -4,27 +4,38 @@ use warnings;
 package My::AttributesBaseClass;
 use base qw( Catalyst::Controller );
 
-sub fetch : Chained('/') PathPrefix CaptureArgs(1) {
+sub fetch : Chained('/') PathPrefix CaptureArgs(0) { }
 
-}
+sub left_alone :Chained('fetch') PathPart Args(0) { }
 
-sub view : PathPart Chained('fetch') Args(0) {
+sub view : PathPart Chained('fetch') Args(0) { }
 
-}
+sub foo { } # no attributes
 
-sub foo {    # no attributes
+package TestApp::Controller::Attributes;
+use base qw(My::AttributesBaseClass);
 
+sub _parse_MakeMeVisible_attr {
+    my ($self, $c, $name, $value) = @_;
+    if (!$value){
+        return Chained => 'fetch', PathPart => 'all_attrs', Args => 0;
+    }
+    elsif ($value eq 'some'){
+        return Chained => 'fetch', Args => 0;
+    }
+    elsif ($value eq 'one'){
+        return PathPart => 'one_attr';
+    }
 }
 
-package TestApp::Controller::Attributes;
-use base qw(My::AttributesBaseClass);
+sub view { }    # override attributes to "hide" url
 
-sub view {    # override attributes to "hide" url
+sub foo : Local { }
 
-}
+sub all_attrs_action :MakeMeVisible { }
 
-sub foo : Local {
+sub some_attrs_action :MakeMeVisible('some') PathPart('some_attrs') { }
 
-}
+sub one_attr_action :MakeMeVisible('one') Chained('fetch') Args(0) { }
 
 1;