Bugzilla – Attachment 40860 Details for
Bug 14494
Terribly slow checkout caused by DateTime->new in far future
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14494: Prevent slow checkout if the patron does not have an expiry date
Bug-14494-Prevent-slow-checkout-if-the-patron-does.patch (text/plain), 5.34 KB, created by
Jonathan Druart
on 2015-07-09 09:00:58 UTC
(
hide
)
Description:
Bug 14494: Prevent slow checkout if the patron does not have an expiry date
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2015-07-09 09:00:58 UTC
Size:
5.34 KB
patch
obsolete
>From fb70ce5af105c488040f778d65140e7383e62ad5 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@koha-community.org> >Date: Thu, 9 Jul 2015 09:52:28 +0100 >Subject: [PATCH] Bug 14494: Prevent slow checkout if the patron does not have > an expiry date > >If a patron has a expiry date set to 9999-12-31 (for organizations for >instance), the checkouts are very slow. > >It's caused by 2 different calls to DateTime in CanBookBeIssued: >1/ > DateTime->new( year => 9999, month => 12, day => 31, time_zone => C4::Context->tz ); >The time_zone should not be set (as it's done in Koha::DateUtils), set to UTC or floating tz. > >2/ > DateTime->compare($today, $expiry_dt) >The comparaison of 2 DT with 1 related to 9999 is very slow, as you can >imagine. > >For 1/ we need to call Koha::DateUtils::dt_from_string (actually, we >should never call DateTime directly). >For 2/ we just need to test if the date is != 9999, no need to compare >it in this case. > >Test plan: >Before this patch, confirm that the checkouts are slow if the patron has a >dateexpiry set to 9999-12-31. >update borrowers set dateexpiry="9999-12-31" where borrowernumber=42; > >After this patch, you should not see any regression when checking out >items to an expired patron and to a valid patron. >--- > C4/Circulation.pm | 21 ++++++--------------- > t/db_dependent/Circulation.t | 41 ++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 46 insertions(+), 16 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index de4b299..aac82bc 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -767,24 +767,15 @@ sub CanBookBeIssued { > if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') { > $issuingimpossible{EXPIRED} = 1; > } else { >- my ($y, $m, $d) = split /-/,$borrower->{'dateexpiry'}; >- if ($y && $m && $d) { # are we really writing oinvalid dates to borrs >- my $expiry_dt = DateTime->new( >- year => $y, >- month => $m, >- day => $d, >- time_zone => C4::Context->tz, >- ); >- $expiry_dt->truncate( to => 'day'); >- my $today = $now->clone()->truncate(to => 'day'); >- if (DateTime->compare($today, $expiry_dt) == 1) { >- $issuingimpossible{EXPIRED} = 1; >- } >- } else { >- carp("Invalid expity date in borr"); >+ my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'sql' ); >+ $expiry_dt->truncate( to => 'day'); >+ my $today = $now->clone()->truncate(to => 'day'); >+ >+ if ($expiry_dt->year < 9999 && DateTime->compare($today, $expiry_dt) == 1) { > $issuingimpossible{EXPIRED} = 1; > } > } >+ > # > # BORROWER STATUS > # >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 30a5fe7..bb8b3bf 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,6 +18,7 @@ > use Modern::Perl; > > use DateTime; >+use Time::HiRes qw( gettimeofday ); > use C4::Biblio; > use C4::Branch; > use C4::Items; >@@ -26,8 +27,9 @@ use C4::Reserves; > use C4::Overdues qw(UpdateFine); > use Koha::DateUtils; > use Koha::Database; >+use t::lib::TestBuilder; > >-use Test::More tests => 61; >+use Test::More tests => 65; > > BEGIN { > use_ok('C4::Circulation'); >@@ -594,6 +596,43 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > is ( $count, 0, "Calling UpdateFine on non-existant fine with an amount of 0 does not result in an empty fine" ); > } > >+{ >+ my $builder = t::lib::TestBuilder->new(); >+ my $item = $builder->build( { source => 'Item' } ); >+ my $patron = $builder->build( >+ { source => 'Borrower', >+ value => { dateexpiry => '9999-12-31' } >+ } >+ ); >+ $patron->{flags} = C4::Members::patronflags($borrower); >+ my $duration = gettimeofday(); >+ my ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( $patron, $item->{barcode} ); >+ $duration = gettimeofday() - $duration; >+ cmp_ok $duration, '<', 1, "CanBookBeIssued should not be take more than 1s if the patron is expired"; >+ is( not( exists $issuingimpossible->{EXPIRED} ), 1, 'The patron should not be considered as expired if dateexpiry is 9999-*' ); >+ >+ $item = $builder->build( { source => 'Item' } ); >+ $patron = $builder->build( >+ { source => 'Borrower', >+ value => { dateexpiry => '0000-00-00' } >+ } >+ ); >+ $patron->{flags} = C4::Members::patronflags($borrower); >+ ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( $patron, $item->{barcode} ); >+ is( $issuingimpossible->{EXPIRED}, 1, 'The patron should be considered as expired if dateexpiry is 0000-00-00' ); >+ >+ my $tomorrow = dt_from_string->add_duration( DateTime::Duration->new( days => 1 ) ); >+ $item = $builder->build( { source => 'Item' } ); >+ $patron = $builder->build( >+ { source => 'Borrower', >+ value => { dateexpiry => output_pref( { dt => $tomorrow, dateonly => 1, dateformat => 'sql' } ) }, >+ } >+ ); >+ $patron->{flags} = C4::Members::patronflags($borrower); >+ ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( $patron, $item->{barcode} ); >+ is( not( exists $issuingimpossible->{EXPIRED} ), 1, 'The patron should not be considered as expired if dateexpiry is tomorrow' ); >+} >+ > $dbh->rollback; > > 1; >-- >2.1.0
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 14494
:
40860
|
40873
|
40874
|
40879
|
40880
|
40881
|
40882
|
40883
|
40908
|
40909
|
40910
|
40911
|
40912
|
40932
|
40933
|
40934
|
40935