Bug 18821: (QA follow-up) Last tweaks for performance
[1] passing unsafe has no use since it is a scalar, removed it to unconfuse [2] remove caching when pref is disabled [3] caching userid removes the need for calling Patron->find each time [4] subsequent changes in unit test [5] cosmetic renames to move from session to daily basis (changed dev angle) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> First call going thru Koha::Patron takes about 0.0150 sec. Subsequent calls only use caching and take about 0.0006 sec. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
parent
1e19bb3284
commit
2bae585fcd
26
C4/Auth.pm
26
C4/Auth.pm
|
|
@ -56,7 +56,7 @@ BEGIN {
|
|||
@ISA = qw(Exporter);
|
||||
@EXPORT = qw(&checkauth &get_template_and_user &haspermission &get_user_subpermissions);
|
||||
@EXPORT_OK = qw(&check_api_auth &get_session &check_cookie_auth &checkpw &checkpw_internal &checkpw_hash
|
||||
&get_all_subpermissions &get_user_subpermissions track_login_for_session
|
||||
&get_all_subpermissions &get_user_subpermissions track_login_daily
|
||||
);
|
||||
%EXPORT_TAGS = ( EditPermissions => [qw(get_all_subpermissions get_user_subpermissions)] );
|
||||
$ldap = C4::Context->config('useldapserver') || 0;
|
||||
|
|
@ -1202,7 +1202,7 @@ sub checkauth {
|
|||
);
|
||||
}
|
||||
|
||||
track_login_for_session( $userid );
|
||||
track_login_daily( $userid );
|
||||
|
||||
return ( $userid, $cookie, $sessionID, $flags );
|
||||
}
|
||||
|
|
@ -2077,30 +2077,26 @@ sub getborrowernumber {
|
|||
return 0;
|
||||
}
|
||||
|
||||
=head2 track_login_for_session
|
||||
=head2 track_login_daily
|
||||
|
||||
track_login_for_session( $userid );
|
||||
track_login_daily( $userid );
|
||||
|
||||
C<$userid> the userid of the member
|
||||
|
||||
Wraps the call to $patron->track_login, the method used to update borrowers.lastseen.
|
||||
Wraps the call to $patron->track_login, the method used to update borrowers.lastseen. We only call track_login once a day.
|
||||
|
||||
=cut
|
||||
|
||||
sub track_login_for_session {
|
||||
sub track_login_daily {
|
||||
my $userid = shift;
|
||||
return unless $userid;
|
||||
|
||||
my $patron = Koha::Patrons->find( { userid => $userid } );
|
||||
return unless $patron;
|
||||
return if !$userid || !C4::Context->preference('TrackLastPatronActivity');
|
||||
|
||||
my $cache = Koha::Caches->get_instance();
|
||||
my $cache_key = "seen-for-session-" . $patron->id;
|
||||
my $cached = $cache->get_from_cache( $cache_key, { unsafe => 1 } );
|
||||
|
||||
my $cache_key = "track_login_" . $userid;
|
||||
my $cached = $cache->get_from_cache($cache_key);
|
||||
my $today = dt_from_string()->ymd;
|
||||
return if $cached && $cached eq $today;
|
||||
|
||||
my $patron = Koha::Patrons->find({ userid => $userid });
|
||||
return unless $patron;
|
||||
$patron->track_login;
|
||||
$cache->set_in_cache( $cache_key, $today );
|
||||
}
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ subtest 'checkauth() tests' => sub {
|
|||
|
||||
};
|
||||
|
||||
subtest 'track_login_for_session() tests' => sub {
|
||||
subtest 'track_login_daily tests' => sub {
|
||||
|
||||
plan tests => 5;
|
||||
|
||||
|
|
@ -78,34 +78,34 @@ subtest 'track_login_for_session() tests' => sub {
|
|||
$patron->store();
|
||||
|
||||
my $cache = Koha::Caches->get_instance();
|
||||
my $cache_key = "seen-for-session-" . $patron->id;
|
||||
my $cache_key = "track_login_" . $patron->userid;
|
||||
$cache->clear_from_cache($cache_key);
|
||||
|
||||
t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' );
|
||||
|
||||
is( $patron->lastseen, undef, 'Patron should have not last seen when newly created' );
|
||||
|
||||
C4::Auth::track_login_for_session( $userid );
|
||||
C4::Auth::track_login_daily( $userid );
|
||||
$patron->_result()->discard_changes();
|
||||
isnt( $patron->lastseen, undef, 'Patron should have last seen set when TrackLastPatronActivity = 1' );
|
||||
|
||||
sleep(1); # We need to wait a tiny bit to make sure the timestamp will be different
|
||||
my $last_seen = $patron->lastseen;
|
||||
C4::Auth::track_login_for_session( $userid );
|
||||
C4::Auth::track_login_daily( $userid );
|
||||
$patron->_result()->discard_changes();
|
||||
is( $patron->lastseen, $last_seen, 'Patron last seen should be unchanged if passed the same session' );
|
||||
is( $patron->lastseen, $last_seen, 'Patron last seen should still be unchanged' );
|
||||
|
||||
$cache->clear_from_cache($cache_key);
|
||||
C4::Auth::track_login_for_session( $userid );
|
||||
C4::Auth::track_login_daily( $userid );
|
||||
$patron->_result()->discard_changes();
|
||||
isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed if given a new session' );
|
||||
isnt( $patron->lastseen, $last_seen, 'Patron last seen should be changed if we cleared the cache' );
|
||||
|
||||
t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' );
|
||||
sleep(1);
|
||||
$last_seen = $patron->lastseen;
|
||||
C4::Auth::track_login_for_session( $userid );
|
||||
$patron->lastseen( undef )->store;
|
||||
$cache->clear_from_cache($cache_key);
|
||||
C4::Auth::track_login_daily( $userid );
|
||||
$patron->_result()->discard_changes();
|
||||
is( $patron->lastseen, $last_seen, 'Patron should have last seen unchanged when TrackLastPatronActivity = 0' );
|
||||
is( $patron->lastseen, undef, 'Patron should still have last seen unchanged when TrackLastPatronActivity = 0' );
|
||||
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue