Skip Menu |

This queue is for tickets about the Catalyst-Plugin-Session CPAN distribution.

Report information
The Basics
Id: 58856
Status: resolved
Priority: 0/
Queue: Catalyst-Plugin-Session

People
Owner: bobtfish [...] bobtfish.net
Requestors: maz [...] mlx.net
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: dump_these sub causing session to be loaded
Date: Sun, 27 Jun 2010 02:20:21 +0800
To: bug-Catalyst-Plugin-Session [...] rt.cpan.org, mst [...] shadowcat.co.uk
From: John Maslanik <maz [...] mlx.net>
sub dump_these is causing the session to be loaded, even if the session is not already loaded. The docs state that dump_these will only dump the objects if session ID is defined. changing $c->sessionid to $c->_sessionid will cause dump_these to act as documented. This change also fixes issues with other modules such as Static::Simple. Before the change, files served by Static::Simple would load a session. After the change, static files do not cause sessions to be loaded. John Maslanik MLX Network Services
I'm not seeing this. Please see the attached patch which adds a test for this, which, if what you describe is true, should fail..
Subject: Session-tests-rt58856.patch
Index: t/live_app.t =================================================================== --- t/live_app.t (revision 13365) +++ t/live_app.t (working copy) @@ -16,8 +16,6 @@ } or plan skip_all => 'Test::WWW::Mechanize::Catalyst >= 0.51 is required for this test'; - - plan tests => 36; } use lib "t/lib"; @@ -83,6 +81,11 @@ $ua1->content_contains( "please login", "ua1 not logged in" ); $ua2->content_contains( "please login", "ua2 not logged in" ); +my $ua3 = Test::WWW::Mechanize::Catalyst->new; +$ua3->get_ok( "http://localhost/dump_these_loads_session"); +$ua3->content_contains('NOT'); + diag("Testing against Catalyst $Catalyst::VERSION"); diag("Testing Catalyst::Plugin::Session $Catalyst::Plugin::Session::VERSION"); +done_testing; Index: t/lib/SessionTestApp/Controller/Root.pm =================================================================== --- t/lib/SessionTestApp/Controller/Root.pm (revision 13365) +++ t/lib/SessionTestApp/Controller/Root.pm (working copy) @@ -88,4 +88,16 @@ } } +sub dump_these_loads_session : Global { + my ($self, $c) = @_; + + $c->dump_these(); + if ($c->_session) { + $c->res->write('LOADED') + } + else { + $c->res->write('NOT'); + } +} + 1; Index: Makefile.PL =================================================================== --- Makefile.PL (revision 13365) +++ Makefile.PL (working copy) @@ -26,7 +26,7 @@ requires 'Tie::RefHash' => '1.34'; # for Test::Store -requires 'Test::More'; +requires 'Test::More' => '0.88'; test_requires 'Test::Deep'; test_requires 'Test::Exception';
Subject: Re: [rt.cpan.org #58856] dump_these sub causing session to be loaded
Date: Mon, 28 Jun 2010 17:17:02 +0800
To: bug-Catalyst-Plugin-Session [...] rt.cpan.org
From: John Maslanik <maz [...] mlx.net>
Your test is not valid because there is no valid and active session when the test is ran. Performing login before performing the dump_these_loads_session test causes the test to fail. The following fails: my $ua3 = Test::WWW::Mechanize::Catalyst->new; $ua3->get_ok( "http://localhost/login", "log ua3 in" ); # Login to create the session 1st $ua3->get_ok( "http://localhost/dump_these_loads_session"); # Then this will fail $ua3->content_contains('NOT'); ------------ t/live_app.t ................ 17/? # Failed test 'Content contains "NOT"' # at t/live_app.t line 87. # searched: "LOADED" # can't find: "NOT" # LCSS: "O" # LCSS context: "LOADED" # Testing against Catalyst 5.80024 # Testing Catalyst::Plugin::Session 0.3 # Looks like you failed 1 test of 39. t/live_app.t ................ Dubious, test returned 1 (wstat 256, 0x100) Failed 1/39 subtests On 6/28/2010 5:56 AM, Tomas Doran via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=58856> > > I'm not seeing this. > > Please see the attached patch which adds a test for this, which, if what > you describe is true, should fail.. > >
Fixed in r13416, thanks for the bug report.