Bugzilla – Attachment 75729 Details for
Bug 18821
TrackLastPatronActivity is a performance killer
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18821: (QA follow-up) Last tweaks for performance
Bug-18821-QA-follow-up-Last-tweaks-for-performance.patch (text/plain), 5.49 KB, created by
Marcel de Rooy
on 2018-06-01 09:15:00 UTC
(
hide
)
Description:
Bug 18821: (QA follow-up) Last tweaks for performance
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2018-06-01 09:15:00 UTC
Size:
5.49 KB
patch
obsolete
>From 93ee9a655433fb92d273ae9c0d5beaa0b91b47f2 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 1 Jun 2018 11:00:57 +0200 >Subject: [PATCH] Bug 18821: (QA follow-up) Last tweaks for performance >Content-Type: text/plain; charset=utf-8 > >[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. >--- > C4/Auth.pm | 26 +++++++++++--------------- > t/db_dependent/Auth.t | 22 +++++++++++----------- > 2 files changed, 22 insertions(+), 26 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index ae2e7f6..60c8c87 100644 >--- a/C4/Auth.pm >+++ b/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 ); > } >diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t >index cfff2f7..5b7ce18 100644 >--- a/t/db_dependent/Auth.t >+++ b/t/db_dependent/Auth.t >@@ -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' ); > > }; > >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18821
:
64425
|
64426
|
65687
|
73175
|
73706
|
73863
|
73891
|
73892
|
73893
|
75139
|
75288
|
75289
|
75427
|
75632
|
75633
|
75634
|
75635
|
75727
|
75728
|
75729
|
75733