Attachment 64425 Details for Bug 18821 – Bug 18821 - GetPatronLastActivity is a performance killer

[patch] Bug 18821 - GetPatronLastActivity is a performance killer

Bug-18821---GetPatronLastActivity-is-a-performance.patch (text/plain), 3.74 KB, created by Kyle M Hall (khall) on 2017-06-19 13:52:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2017-06-19 13:52:24 UTC

Size: 3.74 KB

patch

obsolete

>From 070bf84853c386107927fcea5a70314bfa4ae870 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Mon, 19 Jun 2017 09:49:00 -0400
>Subject: [PATCH] Bug 18821 - GetPatronLastActivity is a performance killer
>
>Enabling GetPatronLastActivity can cause an extreme performance hit, especially on the staff side during checkin and checkout. It makes more sense to only update the last activity timestamp on the first access of a given session. This will greatly improve performance while retaining the same basic information.
>
>Test Plan:
>1) Apply this patch
>2) Start a new session ( a private browser window works well )
>3) Note the lastseen column in the borrowers table is updated
>4) Browse a few pages, not the lastseen column is not updated again
>5) Close the browser window and repeat steps 2-4
>6) prove t/db_dependent/Members.t
>---
> Koha/Patron.pm           | 11 ++++++++---
> t/db_dependent/Members.t | 20 ++++++++++++++++++--
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index 015cb41..3068ce5 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -400,9 +400,14 @@ sub has_overdues {
> 
> sub track_login {
>     my ( $self, $params ) = @_;
>-    return if
>-        !$params->{force} &&
>-        !C4::Context->preference('TrackLastPatronActivity');
>+
>+    warn "track_login()";
>+    return unless C4::Context->preference('TrackLastPatronActivity') || $params->{force};
>+    warn "Pref set OR forced";
>+    return if C4::Context->userenv->{'tracked_for_session'} && !$params->{force};
>+    warn "Untracked OR forced";
>+
>+    C4::Context->userenv->{'tracked_for_session'} = 1;
>     $self->lastseen( dt_from_string() )->store;
> }
> 
>diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
>index 47d0b20..e43a7b5 100755
>--- a/t/db_dependent/Members.t
>+++ b/t/db_dependent/Members.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 65;
>+use Test::More tests => 67;
> use Test::MockModule;
> use Data::Dumper qw/Dumper/;
> use C4::Context;
>@@ -356,12 +356,28 @@ $patstodel = GetBorrowersToExpunge( { last_seen => '2016-02-15' });
> is( scalar @$patstodel, 2, 'TrackLastPatronActivity - 2 patrons must be deleted' );
> $patstodel = GetBorrowersToExpunge( { last_seen => '2016-04-04' });
> is( scalar @$patstodel, 3, 'TrackLastPatronActivity - 3 patrons must be deleted' );
>+
>+
>+# Test method last_seen / TrackLastPatronActivity
> my $patron2 = $builder->build({ source => 'Borrower', value => { lastseen => undef } });
> t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '0' );
> Koha::Patrons->find( $patron2->{borrowernumber} )->track_login;
> is( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should not be changed' );
>+
> Koha::Patrons->find( $patron2->{borrowernumber} )->track_login({ force => 1 });
>-isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, undef, 'Lastseen should be changed now' );
>+my $last_seen = Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen;
>+isnt( $last_seen, undef, 'Lastseen should be changed now' );
>+
>+# Last seen shouldn't be updated a second time for this session
>+t::lib::Mocks::mock_preference( 'TrackLastPatronActivity', '1' );
>+Koha::Patrons->find( $patron2->{borrowernumber} )->track_login();
>+is( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, $last_seen, 'Lastseen should not be changed' );
>+
>+# If it's forced, it should still be updated
>+sleep(1); # We need to wait a tiny bit to make sure the timestamp will be different
>+Koha::Patrons->find( $patron2->{borrowernumber} )->track_login({ force => 1 });
>+isnt( Koha::Patrons->find( $patron2->{borrowernumber} )->lastseen, $last_seen, 'Lastseen should be changed if forced' );
>+
> 
> # Regression tests for BZ13502
> ## Remove all entries with userid='' (should be only 1 max)
>-- 
>2.10.2

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