Merge what I've done over the weekend
Tomas Doran [Sun, 7 Aug 2011 18:42:06 +0000 (19:42 +0100)]
Changes
Makefile.PL
TODO
lib/Catalyst.pm
lib/Catalyst/Test.pm
t/aggregate/unit_load_catalyst_test.t

diff --git a/Changes b/Changes
index b2aea32..35028a9 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,36 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+ Fixed extensions:
+
+  - A number of modules have been updated to pass their tests or not
+    produce deprecation warnings with the latest version of Catalyst.
+
+    These are:
+
+    Test::WWW::Mechanize::Catalyst - has been updated to not produce
+        deprecation warnings.
+
+    Catalyst::ActionRole::ACL - has been updated to fix failing tests
+        (although older versions still function perfectly with this
+        version of Catalyst).
+
+    Catalyst::Plugin::Session::Store::DBIC - has been updated to fix
+        failing tests (although older versions still function perfectly
+        with this version of Catalyst).
+
+ Backward compatibility fixes:
+
+  - Fix calling MyApp->engine_class to set the engine class manually.
+
+  - Re-add a $res->headers->{status} field to Catalyst::Test responses.
+    This _should_ be accessed with $c->res->code instead, but is here
+    for backward compatibility.
+
+ Documentation:
+
+  - Documentation which was in the now removed Catalyst::Engine::* classes
+    has been moved to Catalyst::Manual::Deployment
+
 5.89003 2011-07-28 20:11:50 (TRIAL release)
 
  Backward compatibility fixes:
index 63a99cd..a2d9499 100644 (file)
@@ -137,27 +137,32 @@ my %conflicts = (
     'Catalyst::Plugin::Unicode::Encoding' => '0.2',
     'Catalyst::Plugin::Authentication' => '0.10010', # _config accessor in ::Credential::Password
     'Catalyst::Authentication::Credential::HTTP' => '1.009',
-    'Catalyst::Plugin::Session::Store::File' => '0.16',
-    'Catalyst::Plugin::Session' => '0.21',
-    'Catalyst::Plugin::Session::State::Cookie' => '0.10',
+    'Catalyst::Plugin::Session::Store::File'     => '0.16',
+    'Catalyst::Plugin::Session'                  => '0.21',
+    'Catalyst::Plugin::Session::State::Cookie'   => '0.10',
     'Catalyst::Plugin::Session::Store::FastMmap' => '0.09',
-    'Catalyst::Controller::AllowDisable' => '0.03',
-    'Reaction' => '0.001999',
-    'Catalyst::Plugin::Upload::Image::Magick' => '0.03',
-    'Catalyst::Plugin::ConfigLoader'   => '0.22', # Older versions work but
+    'Catalyst::Controller::AllowDisable'         => '0.03',
+    'Reaction'                                   => '0.001999',
+    'Catalyst::Plugin::Upload::Image::Magick'    => '0.03',
+    'Catalyst::Plugin::ConfigLoader'             => '0.22', # Older versions work but
                                                   # throw Data::Visitor warns
-    'Catalyst::Devel'                  => '1.19',
-    'Catalyst::Plugin::SmartURI'       => '0.032',
-    'CatalystX::CRUD'                  => '0.37',
-    'Catalyst::Action::RenderView'     => '0.07',
-    'Catalyst::Plugin::DebugCookie'    => '0.999002',
-    'Catalyst::Plugin::Authentication' => '0.100091',
-    'CatalystX::Imports'               => '0.03',
-    'Catalyst::Plugin::HashedCookies'  => '1.03',
-    'Catalyst::Action::REST'           => '0.67',
-    'CatalystX::CRUD'                  => '0.42',
-    'CatalystX::CRUD::Model::RDBO'     => '0.20',
-    'Catalyst::View::Mason'            => '0.17',
+    'Catalyst::Devel'                            => '1.19',
+    'Catalyst::Plugin::SmartURI'                 => '0.032',
+    'CatalystX::CRUD'                            => '0.37',
+    'Catalyst::Action::RenderView'               => '0.07',
+    'Catalyst::Plugin::DebugCookie'              => '0.999002',
+    'Catalyst::Plugin::Authentication'           => '0.100091',
+    'CatalystX::Imports'                         => '0.03',
+    'Catalyst::Plugin::HashedCookies'            => '1.03',
+    'Catalyst::Action::REST'                     => '0.67',
+    'CatalystX::CRUD'                            => '0.42',
+    'CatalystX::CRUD::Model::RDBO'               => '0.20',
+    'Catalyst::View::Mason'                      => '0.17',
+#    Note these are not actually needed - they fail tests against the
+#    new version, but still work fine..
+#    'Catalyst::ActionRole::ACL'                  => '0.05',
+#    'Catalyst::Plugin::Session::Store::DBIC'     => '0.11',
+      'Test::WWW::Mechanize::Catalyst'            => '0.53', # Dep warnings unless upgraded.
 );
 check_conflicts(%conflicts);
 
diff --git a/TODO b/TODO
index fe3d68f..b905301 100644 (file)
--- a/TODO
+++ b/TODO
@@ -24,48 +24,59 @@ subclass of Catalyst::Log, no ::Plugin:: needed.
 See also: Catalyst::Plugin::Log::Dispatch and
 http://github.com/willert/catalyst-plugin-log4perl-simple/tree
 
-# REFACTORING
+## Capture arguments that the plack engine component was run with somewhere,
+    to more easily support custom args from scripts (e.g. Gitalist's
+    --git_dir)
 
-##  PSGI
+## throw away the restarter and allow using the restarters Plack provides
 
-###  Blockers
+## remove per-request state from the engine instance
 
-  * Fix nginx middlewares so that they are generic, or can somehow
-    be used by people with their own .psgi files
-  * Fix a sane / nicer way to do custom engines.
-  * I've noticed a small difference with Catalyst::Test. The latest stable
-    version include two headers, 'host' and 'https'. They are missing from
-    this version.
+## be smarter about how we use PSGI - not every response needs to be delayed
+    and streaming
 
-### Things to discuss
+#  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
 
   * Catalyst::Engine::HTTP::Prefork no longer works since it requires
     Catalyst::Engine::CGI which no longer is in the cataplack distribution.
     Investigation shows moving CE:CGI to CE:HTTP:Prefork allows tests to pass.
 
-### PSGI Release Punchlist
+#  PSGI
 
-  * Release a version of Task::Catalyst without CE:PSGI and CE:HTTP:Prefork
-    since those are deprecated.
+##  To do at release time
 
-#### Script survey
+  - Release psgi branch of Catalyst-Devel
+  - Release new Task::Catalyst
+  - Release 5.9 branch of Catalyst-Manual
+  - Release Catalyst::Engine::HTTP::Prefork with deprecation notice
+    (and maybe compat?)
 
-###  Nice to have
+##  Blockers
 
-  * Capture arguments that the plack engine component was run with somewhere,
-    to more easily support custom args from scripts (e.g. Gitalist's
-    --git_dir)
-  * throw away the restarter and allow using the restarters Plack provides
-  * remove per-request state from the engine instance
-  * be smarter about how we use PSGI - not every response needs to be delayed
-    and streaming
+  * Better docs for stopping using Engine::HTTP::Prefork and
+    starting using Starman, maybe?
 
-##  The horrible hack for plugin setup - replacing it:
+  * Test::WWW::Mechanize::Catalyst new release
 
- * Have a look at the Devel::REPL BEFORE_PLUGIN stuff
-   I wonder if what we need is that combined with plugins-as-roles
+  * Test nginx middleware to determine if it is needed with:
+
+    root app - with use_request_uri_for_path
+    root app - without use_request_uri_for_path
+    non-root app - with use_request_uri_for_path
+    non-root app - without use_request_uri_for_path
+
+    If it isn't needed, remove. If it is needed, split it out into it's own
+    file and document why it's needed.
+
+  * I've noticed a small difference with Catalyst::Test. The latest stable
+    version include two headers, 'host' and 'https'. They are missing from
+    this version - Pedro Melo on list
+    ^^ Cannot replicate this? Mailed back to ask for tests..
 
-## App / ctx split:
+# App / ctx split:
 
   NOTE - these are notes that t0m thought up after doing back compat for
          catalyst_component_class, may be inaccurate, wrong or missing things
index 5626071..03f34f8 100644 (file)
@@ -2603,26 +2603,28 @@ Sets up engine.
 =cut
 
 sub engine_class {
-    my $class = shift;
-    confess("Setting ->engine_class manually is no longer supported. XXX FIXME") if scalar @_;
+    my ($class, $requested_engine) = @_;
+
+    if (!$class->engine_loader || $requested_engine) {
+        $class->engine_loader(
+            Catalyst::EngineLoader->new({
+                application_name => $class,
+                (defined $requested_engine
+                     ? (requested_engine => $requested_engine) : ()),
+            }),
+        );
+    }
     $class->engine_loader->catalyst_engine_class;
 }
 
 sub setup_engine {
     my ($class, $requested_engine) = @_;
 
-    $class->engine_loader(
-        Catalyst::EngineLoader->new({
-            application_name => $class,
-            (defined $requested_engine
-                 ? (requested_engine => $requested_engine) : ()),
-        }),
-    );
+    my $engine = $class->engine_class($requested_engine);
 
     # Don't really setup_engine -- see _setup_psgi_app for explanation.
     return if $class->loading_psgi_file;
 
-    my $engine = $class->engine_class;
     Class::MOP::load_class($engine);
 
     if ($ENV{MOD_PERL}) {
@@ -3059,8 +3061,46 @@ to be shown in hit debug tables in the test server.
 =item *
 
 C<use_request_uri_for_path> - Controlls if the C<REQUEST_URI> or C<PATH_INFO> environment
-variable should be used for determining the request path. See L<Catalyst::Engine::CGI/PATH DECODING>
-for more information.
+variable should be used for determining the request path. 
+
+Most web server environments pass the requested path to the application using environment variables,
+from which Catalyst has to reconstruct the request base (i.e. the top level path to / in the application,
+exposed as C<< $c->request->base >>) and the request path below that base.
+
+There are two methods of doing this, both of which have advantages and disadvantages. Which method is used
+is determined by the C<< $c->config(use_request_uri_for_path) >> setting (which can either be true or false).
+
+=over
+
+=item use_request_uri_for_path => 0
+
+This is the default (and the) traditional method that Catalyst has used for determining the path information.
+The path is synthesised from a combination of the C<PATH_INFO> and C<SCRIPT_NAME> environment variables.
+The allows the application to behave correctly when C<mod_rewrite> is being used to redirect requests
+into the application, as these variables are adjusted by mod_rewrite to take account for the redirect.
+
+However this method has the major disadvantage that it is impossible to correctly decode some elements
+of the path, as RFC 3875 says: "C<< Unlike a URI path, the PATH_INFO is not URL-encoded, and cannot
+contain path-segment parameters. >>" This means PATH_INFO is B<always> decoded, and therefore Catalyst
+can't distinguish / vs %2F in paths (in addition to other encoded values).
+
+=item use_request_uri_for_path => 1
+
+This method uses the C<REQUEST_URI> and C<SCRIPT_NAME> environment variables. As C<REQUEST_URI> is never
+decoded, this means that applications using this mode can correctly handle URIs including the %2F character
+(i.e. with C<AllowEncodedSlashes> set to C<On> in Apache).
+
+Given that this method of path resolution is provably more correct, it is recommended that you use
+this unless you have a specific need to deploy your application in a non-standard environment, and you are
+aware of the implications of not being able to handle encoded URI paths correctly.
+
+However it also means that in a number of cases when the app isn't installed directly at a path, but instead
+is having paths rewritten into it (e.g. as a .cgi/fcgi in a public_html directory, with mod_rewrite in a
+.htaccess file, or when SSI is used to rewrite pages into the app, or when sub-paths of the app are exposed
+at other URIs than that which the app is 'normally' based at with C<mod_rewrite>), the resolution of
+C<< $c->request->base >> will be incorrect.
+
+=back
 
 =item *
 
index ef3def5..5c0cbe7 100644 (file)
@@ -311,6 +311,10 @@ sub _local_request {
             for my $f ( $h->header_field_names ) {
                 $resp->init_header( $f, [ $h->header($f) ] );
             }
+            # Another horrible hack to make the response headers have a
+            # 'status' field. This is for back-compat, but you should
+            # call $resp->code instead!
+            $resp->init_header('status', [ $resp->code ]);
         },
     }, @_);
 }
index 399b190..68dfbdf 100644 (file)
@@ -153,4 +153,8 @@ lives_ok {
     request(GET('/dummy'), []);
 } 'array additional param to request method ignored';
 
+my $res = request(GET('/'));
+is $res->code, 200, 'Response code 200';
+is $res->headers->{status}, 200, 'Back compat "status" header present';
+
 done_testing;