From: Sergio Salvi <sergio@salvi.ca>
Date: Thu, 11 Dec 2008 16:42:39 +0000 (+0000)
Subject: finalize session before sending HTTP headers
X-Git-Tag: v0.19_01~2^3
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=92eaec324c5557133b0d06bb4de14ac8cae49d7b;hp=52feb18b99d74903d104da47b1ae9501f94d71b3;p=catagits%2FCatalyst-Plugin-Session.git

finalize session before sending HTTP headers
---

diff --git a/lib/Catalyst/Plugin/Session.pm b/lib/Catalyst/Plugin/Session.pm
index 1e9dd99..ee8ea37 100644
--- a/lib/Catalyst/Plugin/Session.pm
+++ b/lib/Catalyst/Plugin/Session.pm
@@ -98,13 +98,15 @@ sub finalize_headers {
     return $c->NEXT::finalize_headers(@_);
 }
 
-sub finalize {
+sub finalize_body {
     my $c = shift;
-    my $ret = $c->NEXT::finalize(@_);
 
-    # then finish the rest
+    # We have to finalize our session *before* $c->engine->finalize_xxx is called,
+    # because we do not want to send the HTTP response before the session is stored/committed to
+    # the session database (or whatever Session::Store you use).
     $c->finalize_session;
-    return $ret;
+
+    return $c->NEXT::finalize_body(@_);
 }
 
 sub finalize_session {
@@ -756,10 +758,10 @@ prepare time.
 This method is extended and will extend the expiry time before sending
 the response.
 
-=item finalize
+=item finalize_body
 
-This method is extended and will call finalize_session after the other
-finalizes run.  Here we persist the session data if a session exists.
+This method is extended and will call finalize_session before the other
+finalize_body methods run.  Here we persist the session data if a session exists.
 
 =item initialize_session_data
 
diff --git a/t/03_flash.t b/t/03_flash.t
index 0ed2242..5873d98 100644
--- a/t/03_flash.t
+++ b/t/03_flash.t
@@ -29,7 +29,7 @@ is_deeply( $c->flash, {}, "nothing in flash" );
 
 $c->flash->{foo} = "moose";
 
-$c->finalize;
+$c->finalize_body;
 
 is_deeply( $c->flash, { foo => "moose" }, "one key in flash" );
 
@@ -37,21 +37,21 @@ $c->flash(bar => "gorch");
 
 is_deeply( $c->flash, { foo => "moose", bar => "gorch" }, "two keys in flash" );
 
-$c->finalize;
+$c->finalize_body;
 
 is_deeply( $c->flash, { bar => "gorch" }, "one key in flash" );
 
-$c->finalize;
+$c->finalize_body;
 
 $c->flash->{test} = 'clear_flash';
 
-$c->finalize;
+$c->finalize_body;
 
 $c->clear_flash();
 
 is_deeply( $c->flash, {}, "nothing in flash after clear_flash" );
 
-$c->finalize;
+$c->finalize_body;
 
 is_deeply( $c->flash, {}, "nothing in flash after finalize after clear_flash" );
 
@@ -59,7 +59,7 @@ $c->flash->{bar} = "gorch";
 
 $c->config->{session}{flash_to_stash} = 1;
 
-$c->finalize;
+$c->finalize_body;
 $c->prepare_action;
 
 is_deeply( $c->stash, { bar => "gorch" }, "flash copied to stash" );
diff --git a/t/06_finalize.t b/t/06_finalize.t
deleted file mode 100644
index b1aac67..0000000
--- a/t/06_finalize.t
+++ /dev/null
@@ -1,41 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-
-use Test::More;
-
-BEGIN {
-    if ( eval { require Catalyst::Plugin::Session::State::Cookie } ) {
-        plan tests => 3;
-    } else {
-        plan skip_all => "Catalyst::Plugin::Session::State::Cookie required";
-    }
-}
-
-my $finalized = 0;
-
-{
-  package TestPlugin;
-  BEGIN { $INC{"TestPlugin.pm"} = 1 } # nasty hack for 5.8.6
-
-  sub finalize_session { $finalized = 1 }
-
-  sub finalize { die "already finalized_session()" if $finalized }
-
-  # Structure inheritance so TestPlugin->finalize() is called *after* 
-  # Catalyst::Plugin::Session->finalize()
-  package TestApp;
-
-  use Catalyst qw/
-    Session Session::Store::Dummy Session::State::Cookie +TestPlugin 
-  /;
-  __PACKAGE__->setup;
-}
-
-BEGIN { use_ok('Catalyst::Plugin::Session') }
-
-my $c = TestApp->new;
-eval { $c->finalize };
-ok(!$@, "finalize_session() called after all other finalize() methods");
-ok($finalized, "finalize_session() called");