Merge flash in session and finalize before sending response patches

diff --combined Changes
index 9b62b98,8f2f749,9b62b98..d23ead0
--- 1/Changes
--- 2/Changes
--- 3/Changes
+++ b/Changes
@@@@ -1,7 -1,10 -1,7 +1,14 @@@@
   Revision history for Perl extension Catalyst::Plugin::Session
   
---0.20    XXX
+++0.19_01 2009-01-09
+++        - Use shipit to package the dist
           - Switch to Module::install
+ +        - Flash data is now stored inside the session (key "__flash") to avoid
+ +          duplicate entry errors caused by simultaneous select/insert/delete of
 -           flash rows when using DBI as a Store.
+++          flash rows when using DBI as a Store. (Sergio Salvi)
+++        - Fix session finalization order that caused HTTP responses to be sent
+++          before the session is actually finalized and stored in its Store.
+++          (Sergio Salvi)
   
   0.19    2007-10-08
   

diff --combined Makefile.PL
index a8dfe6f,a8dfe6f,a8dfe6f..073ce16
--- 1/Makefile.PL
--- 2/Makefile.PL
--- 3/Makefile.PL
+++ b/Makefile.PL
@@@@ -16,6 -16,6 -16,6 +16,7 @@@@ requires 'Digest'
   requires 'File::Spec';
   requires 'File::Temp';
   requires 'Object::Signature';
+++requires 'MRO::Compat';
   
   # an indirect dep. needs a certain version.
   requires 'Tie::RefHash' => '1.34';

diff --combined lib/Catalyst/Plugin/Session.pm
index 1e9dd99,8b66e78,ee8ea37..e9c2a13
--- 1/lib/Catalyst/Plugin/Session.pm
--- 2/lib/Catalyst/Plugin/Session.pm
--- 3/lib/Catalyst/Plugin/Session.pm
+++ b/lib/Catalyst/Plugin/Session.pm
@@@@ -13,7 -13,7 -13,7 +13,7 @@@@ use overload            ()
   use Object::Signature   ();
   use Carp;
   
---our $VERSION = '0.20';
+++our $VERSION = '0.19_01';
   
   my @session_data_accessors; # used in delete_session
   BEGIN {
@@@@ -98,13 -98,13 -98,15 +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 {
@@@@ -168,12 -168,15 -170,12 +170,15 @@@@ sub _save_flash 
           
           my $sid = $c->sessionid;
   
+ +        my $session_data = $c->_session;
           if (%$flash_data) {
- -            $c->store_session_data( "flash:$sid", $flash_data );
+ +            $session_data->{__flash} = $flash_data;
           }
           else {
- -            $c->delete_session_data("flash:$sid");
+ +            delete $session_data->{__flash};
           }
+ +        $c->_session($session_data);
+ +        $c->_save_session;
       }
   }
   
@@@@ -238,8 -241,11 -240,8 +243,11 @@@@ sub _load_flash 
       $c->_tried_loading_flash_data(1);
   
       if ( my $sid = $c->sessionid ) {
- -        if ( my $flash_data = $c->_flash
- -            || $c->_flash( $c->get_session_data("flash:$sid") ) )
+ +
+ +        my $session_data = $c->session;
+ +        $c->_flash($session_data->{__flash});
+ +
+ +        if ( my $flash_data = $c->_flash )
           {
               $c->_flash_key_hashes({ map { $_ => Object::Signature::signature( \$flash_data->{$_} ) } keys %$flash_data });
               
@@@@ -687,6 -693,8 -689,6 +695,8 @@@@ changed, call C<keep_flash> and pass i
   This method is used to invalidate a session. It takes an optional parameter
   which will be saved in C<session_delete_reason> if provided.
   
+ +NOTE: This method will B<also> delete your flash data.
+ +
   =item session_delete_reason
   
   This accessor contains a string with the reason a session was deleted. Possible
@@@@ -756,10 -764,10 -758,10 +766,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
   
@@@@ -1002,10 -1010,10 -1004,10 +1012,14 @@@@ Andy Grundma
   
   Christian Hansen
   
---Yuval Kogman, C<nothingmuch@woobling.org> (current maintainer)
+++Yuval Kogman, C<nothingmuch@woobling.org>
   
   Sebastian Riedel
   
+++Tomas Doran (t0m) C<bobtfish@bobtfish.net> (current maintainer)
+++
+++Sergio Salvi
+++
   And countless other contributers from #catalyst. Thanks guys!
   
   =head1 COPYRIGHT & LICENSE

diff --combined t/03_flash.t
index 0ed2242,4000422,5873d98..f58222c
--- 1/t/03_flash.t
--- 2/t/03_flash.t
--- 3/t/03_flash.t
+++ b/t/03_flash.t
@@@@ -3,9 -3,10 -3,9 +3,10 @@@@
   use strict;
   use warnings;
   
- -use Test::More tests => 8;
+ +use Test::More tests => 12;
   use Test::MockObject::Extends;
   use Test::Exception;
+ +use Test::Deep;
   
   my $m;
   BEGIN { use_ok( $m = "Catalyst::Plugin::Session" ) }
@@@@ -19,47 -20,56 -19,47 +20,56 @@@@ $c->mock
           return $key =~ /expire/ ? time() + 1000 : $flash;
       },
   );
+ +$c->mock("debug" => sub { 0 });
   $c->mock("store_session_data" => sub { $flash = $_[2] });
   $c->mock("delete_session_data" => sub { $flash = {} });
   $c->set_always( _sessionid => "deadbeef" );
   $c->set_always( config     => { session => { expires => 1000 } } );
   $c->set_always( stash      => {} );
   
+ +is_deeply( $c->session, {}, "nothing in session" );
+ +
   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" );
   
+ +cmp_deeply( $c->session, { __updated => re('^\d+$'), __flash => $c->flash }, "session has __flash with flash data" );
+ +
   $c->flash(bar => "gorch");
   
   is_deeply( $c->flash, { foo => "moose", bar => "gorch" }, "two keys in flash" );
   
-  $c->finalize;
+ +cmp_deeply( $c->session, { __updated => re('^\d+$'), __flash => $c->flash }, "session still has __flash with flash data" );
+ +
 - $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" );
   
+ +cmp_deeply( $c->session, { __updated => re('^\d+$'), }, "session has empty __flash after clear_flash + finalize" );
+ +
   $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" );