From 6f5e5eaee8a51f0ab29b2c959c41646ef23d2d5f Mon Sep 17 00:00:00 2001 From: Andrew Isherwood Date: Mon, 21 Jan 2019 14:20:03 +0000 Subject: [PATCH] Bug 20750: (follow-up) Remove Time::Piece dep The sorting of log entries was being achieved using Time::Piece::epoch to derive the epoch date from the log timestamps. Time::Piece is not used anywhere else, therefore it was desirable to find an alternative method that uses a library that is already in use in Koha. It would have been possible to use Koha::DateUtils::dt_from_string to do this, however, this function instantiates a DateTime object to do the conversion. DateTime objects are notoriously slow at instantiating, so for this use case where we're potentially converting a lot, it didn't seem ideal. Besides, dt_from_string is built to convert from a variety of formats, but since our timestamps are in a defined format, this seems overkill. In the end, we're using Time::Local::timelocal to obtain the epoch time. Time::Local is in use elsewhere and doesn't depend on DateTime. Also updated the unit test of get_request_logs, it was using the wrong date format anyway! --- Koha/Illrequest/Logger.pm | 24 ++++++++++++++++++++++-- t/db_dependent/Illrequest/Logger.t | 12 ++++++------ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/Koha/Illrequest/Logger.pm b/Koha/Illrequest/Logger.pm index 5643ecde80..c68d206bcc 100644 --- a/Koha/Illrequest/Logger.pm +++ b/Koha/Illrequest/Logger.pm @@ -19,7 +19,7 @@ package Koha::Illrequest::Logger; use Modern::Perl; use JSON qw( to_json from_json ); -use Time::Piece; +use Time::Local; use C4::Context; use C4::Templates; @@ -197,6 +197,26 @@ sub get_log_template { } } +=head3 get_epoch + + $epoch = Koha::Illrequest::Logger->get_epoch($timestamp); + +Given a timestamp string returned from GetLogs, get the epoch + +=cut + +sub get_epoch { + my $timestamp = shift; + + return if !$timestamp=~/\d{4}-\d{2}-\d{2}\s\d{2}:\d{2}:\d{2}/; + + my ($date, $time) = split(/\s/, $timestamp); + my ($y, $mon, $d) = split(/-/, $date); + my ($h, $min, $s) = split(/:/, $time); + + return timelocal($s, $min, $h, $d, $mon-1, $y); +} + =head3 get_request_logs $requestlogs = Koha::IllRequest::Logger->get_request_logs($request_id); @@ -230,7 +250,7 @@ sub get_request_logs { } my $format = '%Y-%m-%d %H:%M:%S'; - my @sorted = sort {Time::Piece->strptime($$b{'timestamp'}, $format)->epoch <=> Time::Piece->strptime($$a{'timestamp'}, $format)->epoch} @{$logs}; + my @sorted = sort { get_epoch($$b{'timestamp'}) <=> get_epoch($$a{'timestamp'}) } @{$logs}; return \@sorted; } diff --git a/t/db_dependent/Illrequest/Logger.t b/t/db_dependent/Illrequest/Logger.t index a88fc7c485..1e9ffcf5ad 100644 --- a/t/db_dependent/Illrequest/Logger.t +++ b/t/db_dependent/Illrequest/Logger.t @@ -32,17 +32,17 @@ my $logs = [ { info => '{"log_origin": "core"}', action => 'STATUS_CHANGE', - timestamp => 1538478742 + timestamp => '2018-10-02 11:12:22' }, { info => '{"log_origin": "core"}', action => 'STATUS_CHANGE', - timestamp => 1538478732 + timestamp => '2018-10-02 11:12:12' }, { info => '{"log_origin": "core"}', action => 'STATUS_CHANGE', - timestamp => 1538478752 + timestamp => '2018-10-02 11:12:32' } ]; # Mock the modules we use @@ -126,20 +126,20 @@ subtest 'Basics' => sub { { 'template' => 'mock', 'info' => { 'log_origin' => 'core' }, - 'timestamp' => 1538478752, + 'timestamp' => '2018-10-02 11:12:32', 'action' => 'STATUS_CHANGE' }, { 'template' => 'mock', 'action' => 'STATUS_CHANGE', - 'timestamp' => 1538478742, + 'timestamp' => '2018-10-02 11:12:22', 'info' => { 'log_origin' => 'core' } }, { 'template' => 'mock', 'action' => 'STATUS_CHANGE', 'info' => { 'log_origin' => 'core' }, - 'timestamp' => 1538478732 + 'timestamp' => '2018-10-02 11:12:12' } ]; my $me_mock = Test::MockModule->new('Koha::Illrequest::Logger'); -- 2.11.0