Added ability to avoid DB hits when restoring from session
Jay Kuri [Fri, 15 Feb 2008 07:52:39 +0000 (07:52 +0000)]
Changes
Makefile.PL
lib/Catalyst/Authentication/Store/DBIx/Class.pm
lib/Catalyst/Authentication/Store/DBIx/Class/User.pm
t/04-authsessions.t
t/07-authsessions-cached.t [new file with mode: 0644]
t/lib/TestApp.pm

diff --git a/Changes b/Changes
index cf7cc1d..82df0ba 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,12 @@
 Revision history for Catalyst-Plugin-Authentication-Store-DBIx-Class
 
+0.104   2008-02-15
+        Added ability to avoid DB hits when restoring from session
+
+0.103   2008-02-07
+        Added missing DBIx::Class dependancy in Makefile.PL so 
+        that the damn test bots stop emailing me. 
+
 0.102   2008-01-23
         Catalyst::Authentication::Store::DBIx::Class::User
           - Explicitly call auto_create() against resultset()
index ef7657c..eb60866 100644 (file)
@@ -4,13 +4,37 @@ if( -e 'MANIFEST.SKIP' ) {
     system( 'pod2text lib/Catalyst/Authentication/Store/DBIx/Class.pm > README' );
 }
 
+## I'd love to use can_use - but I can't seem to test for success. :-/
+eval { require Catalyst::Plugin::Authentication::Store::DBIx::Class or die 'footy'; };
+
+if (!$@) {   #} can_use("Catalyst::Plugin::Authentication::Store::DBIx::Class") ) {
+    print STDERR <<EOM;
+*******************************************
+***  WARNING:  DEPRECATED MODULE FOUND  ***
+*******************************************
+
+You have the Catalyst::Plugin::Authentication::Store::DBIx::Class installed.
+The module you are installing supercedes it and it's presence has been known
+to cause conflicts.   We STRONGLY recommend you remove the old module before
+proceeding.  
+
+You have 5 seconds to abort this install to remove the old module.
+EOM
+    sleep 5;
+    print STDERR "Ok. Proceeding anyway...\n\nYou are entering a dimension not only of sight and sound, but of mind...\n\n";
+}
+
+
 name 'Catalyst-Authentication-Store-DBIx-Class';
 all_from 'lib/Catalyst/Authentication/Store/DBIx/Class.pm';
 
 perl_version '5.8.1';
 
-requires 'Catalyst::Runtime';
-requires 'Catalyst::Plugin::Authentication';
+requires (  'Catalyst::Runtime'                 => 0,
+            'Catalyst::Plugin::Authentication'  => '0.10005',
+            'DBIx::Class'                       => 0,
+         );
+
 
 test_requires 'Test::More';
 
index e7379d0..8c20100 100644 (file)
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 use base qw/Class::Accessor::Fast/;
 
-our $VERSION= "0.102";
+our $VERSION= "0.104";
 
 
 BEGIN {
@@ -43,10 +43,9 @@ sub new {
 sub from_session {
     my ( $self, $c, $frozenuser ) = @_;
 
-    return $frozenuser if ref $frozenuser;
-    
+#    return $frozenuser if ref $frozenuser;
+
     my $user = $self->config->{'store_user_class'}->new($self->{'config'}, $c);
-    
     return $user->from_session($frozenuser, $c);
 }
 
@@ -113,7 +112,7 @@ This documentation refers to version 0.10.
                                 },
                                 store => {
                                     class => 'DBIx::Class',
-                                   user_class => 'MyApp::Users',
+                                   user_class => 'MyApp::User',
                                    id_field => 'user_id',
                                    role_relation => 'roles',
                                    role_field => 'rolename',                   
@@ -165,11 +164,12 @@ The DBIx::Class storage module has several configuration options
                                 },
                                 store => {
                                     class => 'DBIx::Class',
-                                   user_class => 'MyApp::Users',
+                                   user_class => 'MyApp::User',
                                    id_field => 'user_id',
                                    role_relation => 'roles',
                                    role_field => 'rolename',
-                                   ignore_fields_in_find => [ 'remote_name' ]          
+                                   ignore_fields_in_find => [ 'remote_name' ],
+                                   use_userdata_from_session => 1,          
                                }
                                }
                        }
@@ -223,6 +223,23 @@ user. This makes it possible to avoid problems when a credential requires an
 authinfo element whose name overlaps with a column name in your users table.
 If this doesn't make sense to you, you probably don't need it.
 
+=item use_userdata_from_session
+
+Under normal circumstances, on each request the user's data is re-retrieved 
+from the database using the primary key for the user table.  When this flag 
+is set in the configuration, it causes the DBIx::Class store to avoid this
+database hit on session restore.  Instead, the user object's column data 
+is retrieved from the session and used as-is.  
+
+B<NOTE>: Since the user object's column
+data is only stored in the session during the initial authentication of 
+the user, turning this on can potentially lead to a situation where the data
+in $c->user is different from what is stored the database.  You can force
+a reload of the data from the database at any time by calling $c->user->get_object(1);
+Note that this will update $c->user for the remainder of this request.  
+It will NOT update the session.  If you need to update the session
+you should call $c->update_user_in_session() as well.  
+
 =item store_user_class
 
 This allows you to override the authentication user class that the 
@@ -321,7 +338,7 @@ the user.  An example will probably make more sense:
             password => $password,
             'dbix_class' => 
                 {
-                    searchargs = [ { -or => [ username => $username,
+                    searchargs => [ { -or => [ username => $username,
                                               email => $email,
                                               clientid => $clientid ] 
                                    },
@@ -349,7 +366,7 @@ within your login action and use it for retrieving the user. A simple example:
        
     if ($c->authenticate({ 
                            password => $password,
-                           'dbix_class' => {  resultset = $rs }
+                           'dbix_class' => {  resultset => $rs }
                          })) {
        # do successful authentication actions here.
     } 
index e77b717..8810514 100644 (file)
@@ -2,6 +2,7 @@ package Catalyst::Authentication::Store::DBIx::Class::User;
 
 use strict;
 use warnings;
+use Data::Dumper;
 use base qw/Catalyst::Authentication::User/;
 use base qw/Class::Accessor::Fast/;
 
@@ -21,7 +22,7 @@ sub new {
     
     bless $self, $class;
     
-
+    ## Note to self- add handling of multiple-column primary keys.
     if (!exists($self->config->{'id_field'})) {
         my @pks = $self->{'resultset'}->result_source->primary_columns;
         if ($#pks == 0) {
@@ -135,15 +136,23 @@ sub roles {
 sub for_session {
     my $self = shift;
     
-    return $self->get($self->config->{'id_field'});
+    #return $self->get($self->config->{'id_field'});
+    my %userdata = $self->_user->get_columns();
+    return \%userdata;
 }
 
 sub from_session {
     my ($self, $frozenuser, $c) = @_;
     
-    # this could be a lot better.  But for now it just assumes $frozenuser is an id and uses find_user
-    # XXX: hits the database on every request?  Not good...
-    return $self->load( { $self->config->{'id_field'} => $frozenuser }, $c);
+    ## if use_userdata_from_session is defined in the config, we fill in the user data from the session.
+    if (exists($self->config->{'use_userdata_from_session'}) && $self->config->{'use_userdata_from_session'} != 0) {
+        my $obj = $self->resultset->new_result({ %$frozenuser });
+        $obj->in_storage(1);
+        $self->_user($obj);
+        return $self;
+    } else {
+        return $self->load( { $self->config->{'id_field'} => $frozenuser->{$self->config->{'id_field'}} }, $c);
+    }
 }
 
 sub get {
@@ -157,15 +166,19 @@ sub get {
 }
 
 sub get_object {
-    my $self = shift;
+    my ($self, $force) = @_;
     
+    if ($force) {
+        $self->_user->discard_changes;
+    }
+
     return $self->_user;
 }
 
 sub obj {
-    my $self = shift;
+    my ($self, $force) = @_;
     
-    return $self->get_object;
+    return $self->get_object($force);
 }
 
 sub auto_create {
index 432dd01..21a0766 100644 (file)
@@ -26,6 +26,11 @@ BEGIN {
         or plan skip_all =>
         "Catalyst::Plugin::Session >= 0.02 is required for this test";
 
+    eval { require Catalyst::Plugin::Session::State::Cookie; }
+        or plan skip_all =>
+        "Catalyst::Plugin::Session::State::Cookie is required for this test";
+
+
     plan tests => 8;
 
     $ENV{TESTAPP_DB_FILE} = "$FindBin::Bin/auth.db" unless exists($ENV{TESTAPP_DB_FILE});
@@ -44,6 +49,7 @@ BEGIN {
                     store => {
                         'class' => 'DBIx::Class',
                         'user_class' => 'TestApp::User',
+                        'use_userdata_from_session' => 0,
                     },
                 },
             },
diff --git a/t/07-authsessions-cached.t b/t/07-authsessions-cached.t
new file mode 100644 (file)
index 0000000..b498926
--- /dev/null
@@ -0,0 +1,98 @@
+#!perl
+
+use strict;
+use warnings;
+use DBI;
+use File::Path;
+use FindBin;
+use Test::More;
+use lib "$FindBin::Bin/lib";
+
+BEGIN {
+    eval { require Test::WWW::Mechanize::Catalyst }
+      or plan skip_all =>
+      "Test::WWW::Mechanize::Catalyst is required for this test";
+
+    eval { require DBD::SQLite }
+        or plan skip_all =>
+        "DBD::SQLite is required for this test";
+
+    eval { require DBIx::Class }
+        or plan skip_all =>
+        "DBIx::Class is required for this test";
+
+    eval { require Catalyst::Plugin::Session; 
+           die unless $Catalyst::Plugin::Session::VERSION >= 0.02 }
+        or plan skip_all =>
+        "Catalyst::Plugin::Session >= 0.02 is required for this test";
+
+    eval { require Catalyst::Plugin::Session::State::Cookie; }
+        or plan skip_all =>
+        "Catalyst::Plugin::Session::State::Cookie is required for this test";
+
+
+    plan tests => 8;
+
+    $ENV{TESTAPP_DB_FILE} = "$FindBin::Bin/auth.db" unless exists($ENV{TESTAPP_DB_FILE});
+
+    $ENV{TESTAPP_CONFIG} = {
+        name => 'TestApp',
+        authentication => {
+            default_realm => "users",
+            realms => {
+                users => {
+                    credential => {
+                        'class' => "Password",
+                        'password_field' => 'password',
+                        'password_type' => 'clear'
+                    },
+                    store => {
+                        'class' => 'DBIx::Class',
+                        'user_class' => 'TestApp::User',
+                        'use_userdata_from_session' => 1,
+                    },
+                },
+            },
+        },
+    };
+
+    $ENV{TESTAPP_PLUGINS} = [
+        qw/Authentication
+           Session
+           Session::Store::Dummy
+           Session::State::Cookie
+           /
+    ];
+}
+
+use SetupDB;
+
+use Test::WWW::Mechanize::Catalyst 'TestApp';
+my $m = Test::WWW::Mechanize::Catalyst->new;
+
+# log a user in
+{
+    $m->get_ok( 'http://localhost/user_login?username=joeuser&password=hackme', undef, 'request ok' );
+    $m->content_is( 'joeuser logged in', 'user logged in ok' );
+}
+
+# verify the user is still logged in
+{
+    $m->get_ok( 'http://localhost/get_session_user', undef, 'request ok' );
+    $m->content_is( 'joeuser', 'user still logged in' );
+}
+
+# log the user out
+{
+    $m->get_ok( 'http://localhost/user_logout', undef, 'request ok' );
+    $m->content_is( 'logged out', 'user logged out ok' );
+}
+
+# verify there is no session
+{
+    $m->get_ok( 'http://localhost/get_session_user', undef, 'request ok' );
+    $m->content_is( '', "user's session deleted" );
+}
+
+# clean up
+unlink $ENV{TESTAPP_DB_FILE};
index eac5136..265dfe8 100644 (file)
@@ -116,9 +116,9 @@ sub user_logout : Global {
 \r
 sub get_session_user : Global {\r
     my ( $self, $c ) = @_;\r
-\r
\r
     if ( $c->user_exists ) {\r
-        $c->res->body( $c->user->get('username') );\r
+        $c->res->body($c->user->get('username')); # . " " . Dumper($c->user->get_columns()) );\r
     }\r
 }\r
 \r