Merge branch pass_component_names:
Tomas Doran [Sat, 1 Aug 2009 00:39:39 +0000 (00:39 +0000)]
svn merge -r  10899:10927  http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/branches/pass_component_names

Changes
Makefile.PL
TODO
lib/Catalyst.pm
lib/Catalyst/Action.pm
lib/Catalyst/Component.pm
lib/Catalyst/Controller.pm
lib/Catalyst/Engine/FastCGI.pm
lib/Catalyst/Request.pm
t/aggregate/live_component_controller_attributes.t [moved from t/aggregate/live_component_controller_atttributes.t with 100% similarity]
t/unit_core_component_loading.t

diff --git a/Changes b/Changes
index 70a4777..b0d8a94 100644 (file)
--- a/Changes
+++ b/Changes
@@ -3,8 +3,12 @@
   Bug fixes:
        - Fix replace_constructor warning to actually work if you make your
          application class immutable without that option.
-       - Fix POD to refer to ->config(key => $val), rather than
-         ->config->{key} = $val, as the latter form is deprecated.
+       - Depend on Module::Pluggable 3.9 to prevent a bug wherein components
+         in inner packages might not be registered. This especially affected
+         tests.
+       - Catalyst::Engine::FastCGI - relax the check for versions of Microsoft
+         IIS. Provides compatibility with Windows 2008 R2 as well as 
+         (hopefully) future versions.
 
   Refactoring / cleanups:
        - Deleted the Restarter engine and its Watcher code. Use the
   New features:
        - private_path method for Catalyst::Action + docs + tests (groditi)
 
+  Documentation:
+       - Fix POD to refer to ->config(key => $val), rather than
+         ->config->{key} = $val, as the latter form is deprecated.
+       - Clearer docs for the 'uri_for' method.
+
 5.80007 2009-06-30 23:54:34
 
   Bug fixes:
index de59ee8..6e8fcc7 100644 (file)
@@ -23,7 +23,7 @@ requires 'HTTP::Request';
 requires 'HTTP::Response';
 requires 'HTTP::Request::AsCGI' => '0.8';
 requires 'LWP::UserAgent';
-requires 'Module::Pluggable' => '3.01';
+requires 'Module::Pluggable' => '3.9';
 requires 'Path::Class' => '0.09';
 requires 'Scalar::Util';
 requires 'Sub::Exporter';
diff --git a/TODO b/TODO
index 141b67b..2dd12f7 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,49 +1,63 @@
-Known Bugs:
+# Known Bugs:
 
    - Bug ->go or ->visit causes actions which have Args or CaptureArgs called
      twice when called via ->go or ->visit.
 
      Test app: http://github.com/bobtfish/catalyst-app-bug-go_chain/tree/master
 
-Compatibility warnings to add:
-
-   - $self->config should warn as config should only ever be called as a
-     class method.
-
-Proposed functionality / feature additions:
-
-    - Log setup needs to be less lame, so Catalyst::Plugin::Log::* can die
-      in a fire. Having $c->log_class would be a good start. kane volunteered
-      to do some of this.
-
-      Simple example: Catalyst::Plugin::Log::Colorful should just be a
-      subclass of Catalyst::Log, no ::Plugin:: needed.
-
-      See also: Catalyst::Plugin::Log::Dispatch and
-      http://github.com/willert/catalyst-plugin-log4perl-simple/tree
-
-  Fixes to component interfaces to deal with anon classes:
-
-22:16 <@mst> t0m: explain what breaks
-22:16 <@mst> I'm not 100% sure we have the same problem in mind
-22:17 <@t0m> well, various shit in Catalyst utils to resolve app from MyApp::Controller::Foo breaks
-22:17 <@mst> Caelum: http://scsys.co.uk:8001/31240
-22:17 <@mst> t0m: the trick would be to not need to do that
-22:17 <@mst> t0m: $self->_application should be fine
-22:18 <@t0m> $action->class can't be fed back into $c->component() 
-22:18 <@t0m> which breaks ::REST
-22:18 <@mst> right
-22:18 <@mst> we need to
-22:19 <@mst> (a) add an ->_component_name or similar method to controllers
-22:19 <@mst> (b) pass action objects either that or the controller
-22:20 <@t0m> do you have a controller calling $action->dispatch? Course you must, the action lives in a controller..
-22:21 <@mst> and that doesn't matter anyway
-22:21 <@mst> dispatch has $c
-22:21 <@mst> it needs to do
-22:21 <@mst> $c->component($self->_component_name)
-22:21 <@mst> and that _component_name needs to come from the controller at register_actions time
-22:22 <@t0m> confound: Oi, please to be releasing my Action::REST fixes, I'm about to do more..
-22:22 <@t0m> mst: right, gotcha
-22:22 <@mst> note that needs to happen in core
-22:22 <@mst> then actions can use that instead of $self->class
-22:22 <@mst> and shit will work
+   - Bricas' Exception blog post
+
+     http://bricas.vox.com/library/post/catalyst-exceptionclass.html
+
+     Broken by recent exception refactoring
+
+# Compatibility warnings to add:
+
+  - $self->config should warn as config should only ever be called as a
+    class method.
+
+# Proposed functionality / feature additions:
+
+## Log setup needs to be less lame
+
+So Catalyst::Plugin::Log::* can die
+in a fire. Having $c->log_class would be a good start. kane volunteered
+to do some of this.
+
+Simple example: Catalyst::Plugin::Log::Colorful should just be a
+subclass of Catalyst::Log, no ::Plugin:: needed.
+
+See also: Catalyst::Plugin::Log::Dispatch and
+http://github.com/willert/catalyst-plugin-log4perl-simple/tree
+
+# REFACTORING
+
+##  The horrible hack for plugin setup - replacing it:
+
+ * Have a look at the Devel::REPL BEFORE_PLUGIN stuff
+   I wonder if what we need is that combined with plugins-as-roles
+
+## App / ctx split:
+
+  NOTE - these are notes that t0m thought up after doing back compat for
+         _component_class, may be inaccurate, wrong or missing things
+         bug mst (at least) to correct before trying more than the first 2
+         steps. Please knock yourself out on the first two however :)
+
+  - Eliminate actions in MyApp from the main test suite
+  - Uncomment warning in C::C::register_action_methods, add tests it works
+    by mocking out the logging..
+  - Remove MyApp @ISA controller (ask metaclass if it has attributes, and if
+                                  so you need back compat :/)
+  - Make Catalyst::Context, move the per request stuff in there, handles from
+    main app class to delegate
+  - Make an instance of the app class which is a global variable
+  - Make new instance of the context class, not the app class per-request
+  - Remove the components as class data, move to instance data on the app
+    class (you probably have to do this for _all_ the class data, good luck!)
+  - Make it possible for users to spin up different instances of the app class
+    (with different config etc each)
+  - Profit! (Things like changing the complete app config per vhost, i.e.
+    writing a config loader / app class role which dispatches per vhost to
+    differently configured apps is piss easy)
+
index b238376..b601962 100644 (file)
@@ -335,9 +335,11 @@ call to forward.
     $c->forward(qw/MyApp::Model::DBIC::Foo do_stuff/);
     $c->forward('MyApp::View::TT');
 
-Note that forward implies an C<<eval { }>> around the call (actually
-C<execute> does), thus de-fatalizing all 'dies' within the called
-action. If you want C<die> to propagate you need to do something like:
+Note that L<< forward|/"$c->forward( $action [, \@arguments ] )" >> implies
+an C<< eval { } >> around the call (actually
+L<< execute|/"$c->execute( $class, $coderef )" >> does), thus de-fatalizing
+all 'dies' within the called action. If you want C<die> to propagate you
+need to do something like:
 
     $c->forward('foo');
     die $c->error if $c->error;
@@ -357,8 +359,8 @@ sub forward { my $c = shift; no warnings 'recursion'; $c->dispatcher->forward( $
 
 =head2 $c->detach()
 
-The same as C<forward>, but doesn't return to the previous action when
-processing is finished.
+The same as L<< forward|/"$c->forward( $action [, \@arguments ] )" >>, but
+doesn't return to the previous action when processing is finished.
 
 When called with no arguments it escapes the processing chain entirely.
 
@@ -370,23 +372,27 @@ sub detach { my $c = shift; $c->dispatcher->detach( $c, @_ ) }
 
 =head2 $c->visit( $class, $method, [, \@captures, \@arguments ] )
 
-Almost the same as C<forward>, but does a full dispatch, instead of just
-calling the new C<$action> / C<$class-E<gt>$method>. This means that C<begin>,
-C<auto> and the method you go to are called, just like a new request.
+Almost the same as L<< forward|/"$c->forward( $action [, \@arguments ] )" >>,
+but does a full dispatch, instead of just calling the new C<$action> /
+C<< $class->$method >>. This means that C<begin>, C<auto> and the method
+you go to are called, just like a new request.
 
 In addition both C<< $c->action >> and C<< $c->namespace >> are localized.
-This means, for example, that $c->action methods such as C<name>, C<class> and
-C<reverse> return information for the visited action when they are invoked
-within the visited action.  This is different from the behavior of C<forward>
-which continues to use the $c->action object from the caller action even when
+This means, for example, that C<< $c->action >> methods such as
+L<name|Catalyst::Action/name>, L<class|Catalyst::Action/class> and
+L<reverse|Catalyst::Action/reverse> return information for the visited action
+when they are invoked within the visited action.  This is different from the
+behavior of L<< forward|/"$c->forward( $action [, \@arguments ] )" >>, which
+continues to use the $c->action object from the caller action even when
 invoked from the callee.
 
-C<$c-E<gt>stash> is kept unchanged.
+C<< $c->stash >> is kept unchanged.
 
-In effect, C<visit> allows you to "wrap" another action, just as it
-would have been called by dispatching from a URL, while the analogous
-C<go> allows you to transfer control to another action as if it had
-been reached directly from a URL.
+In effect, L<< visit|/"$c->visit( $action [, \@captures, \@arguments ] )" >>
+allows you to "wrap" another action, just as it would have been called by
+dispatching from a URL, while the analogous
+L<< go|/"$c->go( $action [, \@captures, \@arguments ] )" >> allows you to
+transfer control to another action as if it had been reached directly from a URL.
 
 =cut
 
@@ -396,12 +402,12 @@ sub visit { my $c = shift; $c->dispatcher->visit( $c, @_ ) }
 
 =head2 $c->go( $class, $method, [, \@captures, \@arguments ] )
 
-Almost the same as C<detach>, but does a full dispatch like C<visit>,
+Almost the same as L<< detach|/"$c->detach( $action [, \@arguments ] )" >>, but does a full dispatch like L</visit>,
 instead of just calling the new C<$action> /
-C<$class-E<gt>$method>. This means that C<begin>, C<auto> and the
+C<< $class->$method >>. This means that C<begin>, C<auto> and the
 method you visit are called, just like a new request.
 
-C<$c-E<gt>stash> is kept unchanged.
+C<< $c->stash >> is kept unchanged.
 
 =cut
 
@@ -818,8 +824,8 @@ Returns or takes a hashref containing the application's configuration.
 
     __PACKAGE__->config( { db => 'dsn:SQLite:foo.db' } );
 
-You can also use a C<YAML>, C<XML> or C<Config::General> config file
-like myapp.conf in your applications home directory. See
+You can also use a C<YAML>, C<XML> or L<Config::General> config file
+like C<myapp.conf> in your applications home directory. See
 L<Catalyst::Plugin::ConfigLoader>.
 
 =head3 Cascading configuration
@@ -918,7 +924,7 @@ Returns the engine instance. See L<Catalyst::Engine>.
 Merges C<@path> with C<< $c->config->{home} >> and returns a
 L<Path::Class::Dir> object. Note you can usually use this object as
 a filename, but sometimes you will have to explicitly stringify it
-yourself by calling the C<<->stringify>> method.
+yourself by calling the C<< ->stringify >> method.
 
 For example:
 
@@ -1158,30 +1164,42 @@ sub setup_finalize {
     $class->setup_finished(1);
 }
 
-=head2 $c->uri_for( $action, \@captures?, @args?, \%query_values? )
-
 =head2 $c->uri_for( $path, @args?, \%query_values? )
 
-=over
-
-=item $action
-
-A Catalyst::Action object representing the Catalyst action you want to
-create a URI for. To get one for an action in the current controller,
-use C<< $c->action('someactionname') >>. To get one from different
-controller, fetch the controller using C<< $c->controller() >>, then
-call C<action_for> on it.
-
-You can maintain the arguments captured by an action (e.g.: Regex, Chained)
-using C<< $c->req->captures >>.
+=head2 $c->uri_for( $action, \@captures?, @args?, \%query_values? )
 
-  # For the current action
-  $c->uri_for($c->action, $c->req->captures);
+Constructs an absolute L<URI> object based on the application root, the
+provided path, and the additional arguments and query parameters provided.
+When used as a string, provides a textual URI.
+
+If the first argument is a string, it is taken as a public URI path relative
+to C<< $c->namespace >> (if it doesn't begin with a forward slash) or
+relative to the application root (if it does). It is then merged with 
+C<< $c->request->base >>; any C<@args> are appended as additional path
+components; and any C<%query_values> are appended as C<?foo=bar> parameters.
+
+If the first argument is a L<Catalyst::Action> it represents an action which
+will have its path resolved using C<< $c->dispatcher->uri_for_action >>. The
+optional C<\@captures> argument (an arrayref) allows passing the captured
+variables that are needed to fill in the paths of Chained and Regex actions;
+once the path is resolved, C<uri_for> continues as though a path was
+provided, appending any arguments or parameters and creating an absolute
+URI.
+
+The captures for the current request can be found in 
+C<< $c->request->captures >>, and actions can be resolved using
+C<< Catalyst::Controller->action_for($name) >>. If you have a private action
+path, use C<< $c->uri_for_action >> instead.
+
+  # Equivalent to $c->req->uri
+  $c->uri_for($c->action, $c->req->captures, 
+      @{ $c->req->args }, $c->req->params);
 
   # For the Foo action in the Bar controller
-  $c->uri_for($c->controller('Bar')->action_for('Foo'), $c->req->captures);
+  $c->uri_for($c->controller('Bar')->action_for('Foo'));
 
-=back
+  # Path to a static resource
+  $c->uri_for('/static/images/logo.png');
 
 =cut
 
@@ -1606,25 +1624,6 @@ sub _stats_finish_execute {
     $c->stats->profile( end => $info );
 }
 
-=head2 $c->_localize_fields( sub { }, \%keys );
-
-=cut
-
-#Why does this exist? This is no longer safe and WILL NOT WORK.
-# it doesnt seem to be used anywhere. can we remove it?
-sub _localize_fields {
-    my ( $c, $localized, $code ) = ( @_ );
-
-    my $request = delete $localized->{request} || {};
-    my $response = delete $localized->{response} || {};
-
-    local @{ $c }{ keys %$localized } = values %$localized;
-    local @{ $c->request }{ keys %$request } = values %$request;
-    local @{ $c->response }{ keys %$response } = values %$response;
-
-    $code->();
-}
-
 =head2 $c->finalize
 
 Finalizes the request.
@@ -2134,7 +2133,7 @@ reference. Items in the array beginning with C<::> will have the
 application class name prepended to them.
 
 All components found will also have any
-L<Devel::InnerPackage|inner packages> loaded and set up as components.
+L<inner packages|Devel::InnerPackage> loaded and set up as components.
 Note, that modules which are B<not> an I<inner package> of the main
 file namespace loaded will not be instantiated as components.
 
@@ -2216,11 +2215,10 @@ sub setup_component {
 
     my $suffix = Catalyst::Utils::class2classsuffix( $component );
     my $config = $class->config->{ $suffix } || {};
-    # Stash _component_name in the config here, so that custom COMPONENT
-    # methods also pass it. local to avoid pointlessly shitting in config
-    # for the debug screen, as $component is already the key name.
-    local $config->{_component_name} = $component;
-
+    $config->{_component_name} = $component; # Put this in args here, rather
+                                             # than in COMPONENT as there
+                                             # are lots of custom COMPONENT
+                                             # methods..
     my $instance = eval { $component->COMPONENT( $class, $config ); };
 
     if ( my $error = $@ ) {
@@ -2687,7 +2685,7 @@ Wiki:
 
 =head2 L<Catalyst::Manual> - The Catalyst Manual
 
-=head2 L<Catalyst::Component>, L<Catalyst::Base> - Base classes for components
+=head2 L<Catalyst::Component>, L<Catalyst::Controller> - Base classes for components
 
 =head2 L<Catalyst::Engine> - Core engine
 
@@ -2745,10 +2743,14 @@ Gary Ashton Jones
 
 Geoff Richards
 
+hobbs: Andrew Rodland <andrew@cleverdomain.org>
+
 ilmari: Dagfinn Ilmari MannsÃ¥ker <ilmari@ilmari.org>
 
 jcamacho: Juan Camacho
 
+jester: Jesse Sheidlower
+
 jhannah: Jay Hannah <jay@jays.net>
 
 Jody Belka
@@ -2757,6 +2759,8 @@ Johan Lindstrom
 
 jon: Jon Schutz <jjschutz@cpan.org>
 
+konobi: Scott McWhirter <konobi@cpan.org>
+
 marcus: Marcus Ramberg <mramberg@cpan.org>
 
 miyagawa: Tatsuhiko Miyagawa <miyagawa@bulknews.net>
@@ -2787,8 +2791,6 @@ random: Roland Lammel <lammel@cpan.org>
 
 sky: Arthur Bergman
 
-the_jester: Jesse Sheidlower
-
 t0m: Tomas Doran <bobtfish@bobtfish.net>
 
 Ulf Edvinsson
index fdb237c..af65452 100644 (file)
@@ -113,7 +113,7 @@ Returns a code reference to this action.
 
 =head2 dispatch( $c )
 
-Dispatch this action against a context
+Dispatch this action against a context.
 
 =head2 execute( $controller, $c, @args )
 
@@ -145,11 +145,11 @@ C<private_path> of an action is always suitable for passing to C<forward>.
 
 =head2 name
 
-returns the sub name of this action.
+Returns the sub name of this action.
 
 =head2 meta
 
-Provided by Moose
+Provided by Moose.
 
 =head1 AUTHORS
 
index ee1f99a..c065664 100644 (file)
@@ -60,14 +60,7 @@ component loader with config() support and a process() method placeholder.
 __PACKAGE__->mk_classdata('_plugins');
 __PACKAGE__->mk_classdata('_config');
 
-has _component_name => ( is => 'ro' ); # Cannot be required => 1 as context
-                                       # class @ISA component - HATE
-# Make accessor callable as a class method, as we need to call setup_actions
-# on the application class, which we don't have an instance of, ewwwww
-around _component_name => sub {
-    my ($orig, $self) = (shift, shift);
-    blessed($self) ? $self->$orig(@_) : $self;
-};
+has _component_name => ( is => 'ro' );
 
 sub BUILDARGS {
     my $class = shift;
@@ -179,6 +172,15 @@ something like this:
       return $class->new($app, $args);
   }
 
+=head2 _component_name
+
+The name of the component within an application. This is used to
+pass the component's name to actions generated (becoming
+C<< $action->class >>). This is needed so that the L</COMPONENT> method can
+return an instance of a different class (e.g. a L<Class::MOP> anonymous class),
+(as finding the component name by C<< ref($self) >> will not work correctly in
+such cases).
+
 =head2 $c->config
 
 =head2 $c->config($hashref)
index c8db2e7..37e36e1 100644 (file)
@@ -156,7 +156,7 @@ around action_namespace => sub {
         }
     }
 
-    my $namespace = Catalyst::Utils::class2prefix($self->_component_name, $case_s) || '';
+    my $namespace = Catalyst::Utils::class2prefix(ref($self) ? $self->_component_name : $self, $case_s) || '';
     $self->$orig($namespace) if ref($self);
     return $namespace;
 };
@@ -207,15 +207,10 @@ sub register_actions {
 
 sub register_action_methods {
     my ( $self, $c, @methods ) = @_;
-    my $class = $self->_component_name;
+    my $class = blessed($self) ? $self->_component_name : $self;
     #this is still not correct for some reason.
     my $namespace = $self->action_namespace($c);
 
-    # Uncomment as soon as you fix the tests :)
-    #if (!blessed($self) && $self eq $c && scalar(@methods)) {
-    #    $c->log->warn("Action methods found defined in your application class, $self. This is deprecated, please move them into a Root controller.");
-    #}
-
     foreach my $method (@methods) {
         my $name = $method->name;
         my $attributes = $method->attributes;
index 125a762..280fee1 100644 (file)
@@ -235,7 +235,7 @@ sub _fix_env
         $env->{PATH_INFO} ||= delete $env->{SCRIPT_NAME};
     }
     # Fix the environment variables PATH_INFO and SCRIPT_NAME when running under IIS
-    elsif ( $env->{SERVER_SOFTWARE} =~ /IIS\/[67].0/ ) {
+    elsif ( $env->{SERVER_SOFTWARE} =~ /IIS\/[6-9]\.[0-9]/ ) {
         my @script_name = split(m!/!, $env->{PATH_INFO});
         my @path_translated = split(m!/|\\\\?!, $env->{PATH_TRANSLATED});
         my @path_info;
index 643a298..4a40516 100644 (file)
@@ -223,7 +223,7 @@ Arguments get automatically URI-unescaped for you.
 
 =head2 $req->args
 
-Shortcut for arguments.
+Shortcut for L</arguments>.
 
 =head2 $req->base
 
index c8098c6..42f1eac 100644 (file)
@@ -1,7 +1,8 @@
 # 2 initial tests, and 6 per component in the loop below
 # (do not forget to update the number of components in test 3 as well)
 # 5 extra tests for the loading options
-use Test::More tests => 2 + 6 * 24 + 5;
+# One test for components in inner packages
+use Test::More tests => 2 + 6 * 24 + 5 + 1;
 
 use strict;
 use warnings;
@@ -199,4 +200,17 @@ eval "package $appclass; use Catalyst; __PACKAGE__->setup";
 
 is($@, '', "Didn't load component twice");
 
+$appclass = "InnerComponent";
+
+{
+  package InnerComponent::Controller::Test;
+  use base 'Catalyst::Controller';
+}
+
+$INC{'InnerComponent/Controller/Test.pm'} = 1;
+
+eval "package $appclass; use Catalyst; __PACKAGE__->setup";
+
+isa_ok($appclass->controller('Test'), 'Catalyst::Controller');
+
 rmtree($libdir);