[Catalyst-commits] r9045 - in trunk/Catalyst-Plugin-Session: . lib/Catalyst/Plugin t

t0m at dev.catalyst.perl.org t0m at dev.catalyst.perl.org
Fri Jan 9 01:55:55 GMT 2009


Author: t0m
Date: 2009-01-09 01:55:55 +0000 (Fri, 09 Jan 2009)
New Revision: 9045

Removed:
   trunk/Catalyst-Plugin-Session/t/06_finalize.t
Modified:
   trunk/Catalyst-Plugin-Session/Changes
   trunk/Catalyst-Plugin-Session/Makefile.PL
   trunk/Catalyst-Plugin-Session/lib/Catalyst/Plugin/Session.pm
   trunk/Catalyst-Plugin-Session/t/03_flash.t
Log:
Merge flash in session and finalize before sending response patches

Modified: trunk/Catalyst-Plugin-Session/Changes
===================================================================
--- trunk/Catalyst-Plugin-Session/Changes	2009-01-09 01:55:01 UTC (rev 9044)
+++ trunk/Catalyst-Plugin-Session/Changes	2009-01-09 01:55:55 UTC (rev 9045)
@@ -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. (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
 

Modified: trunk/Catalyst-Plugin-Session/Makefile.PL
===================================================================
--- trunk/Catalyst-Plugin-Session/Makefile.PL	2009-01-09 01:55:01 UTC (rev 9044)
+++ trunk/Catalyst-Plugin-Session/Makefile.PL	2009-01-09 01:55:55 UTC (rev 9045)
@@ -16,6 +16,7 @@
 requires 'File::Spec';
 requires 'File::Temp';
 requires 'Object::Signature';
+requires 'MRO::Compat';
 
 # an indirect dep. needs a certain version.
 requires 'Tie::RefHash' => '1.34';

Modified: trunk/Catalyst-Plugin-Session/lib/Catalyst/Plugin/Session.pm
===================================================================
--- trunk/Catalyst-Plugin-Session/lib/Catalyst/Plugin/Session.pm	2009-01-09 01:55:01 UTC (rev 9044)
+++ trunk/Catalyst-Plugin-Session/lib/Catalyst/Plugin/Session.pm	2009-01-09 01:55:55 UTC (rev 9045)
@@ -13,7 +13,7 @@
 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,15 @@
     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 +170,15 @@
         
         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 +243,11 @@
     $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 +695,8 @@
 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 +766,10 @@
 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 +1012,14 @@
 
 Christian Hansen
 
-Yuval Kogman, C<nothingmuch at woobling.org> (current maintainer)
+Yuval Kogman, C<nothingmuch at woobling.org>
 
 Sebastian Riedel
 
+Tomas Doran (t0m) C<bobtfish at bobtfish.net> (current maintainer)
+
+Sergio Salvi
+
 And countless other contributers from #catalyst. Thanks guys!
 
 =head1 COPYRIGHT & LICENSE

Modified: trunk/Catalyst-Plugin-Session/t/03_flash.t
===================================================================
--- trunk/Catalyst-Plugin-Session/t/03_flash.t	2009-01-09 01:55:01 UTC (rev 9044)
+++ trunk/Catalyst-Plugin-Session/t/03_flash.t	2009-01-09 01:55:55 UTC (rev 9045)
@@ -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 @@
         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_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" );

Deleted: trunk/Catalyst-Plugin-Session/t/06_finalize.t
===================================================================
--- trunk/Catalyst-Plugin-Session/t/06_finalize.t	2009-01-09 01:55:01 UTC (rev 9044)
+++ trunk/Catalyst-Plugin-Session/t/06_finalize.t	2009-01-09 01:55:55 UTC (rev 9045)
@@ -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");




More information about the Catalyst-commits mailing list