Attachment 84266 Details for Bug 20750 – Bug 20750: (follow-up) Remove Time::Piece dep

[patch] Bug 20750: (follow-up) Remove Time::Piece dep

Bug-20750-follow-up-Remove-TimePiece-dep.patch (text/plain), 4.25 KB, created by Andrew Isherwood on 2019-01-21 14:25:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2019-01-21 14:25:12 UTC

Size: 4.25 KB

patch

obsolete

>From 44588fda74230f2db7aeed0c32fd801991ac06e9 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 20750: 75304 | 75326 | 75329 | 75330 | 77757 | 77816 | 77817 | 77818 | 77931 | 79822 | 79823 | 79824 | 80860 | 82082 | 82083 | 82085 | 82096 | 82097 | 82098 | 82099 | 82500 | 82501 | 82502 | 84193 | 84194 | 84195 | 84203 | 84266 | 84497 | 84498 | 84821 | 84823 | 84824 | 84825 | 84826 | 84827 | 84973 | 84974 | 84975 | 84976 | 84977 | 84978 | 84979 | 85097 | 85098 | 85099 | 85101 | 85113 | 85238 | 85266 | 85697 | 85698 | 85699 | 85700 | 85701 | 85702 | 85703 | 85704 | 85705 | 85706 | 85707 | 85708 | 85709 | 85710 | 85756 | 85757 | 85765 | 85826 | 85827 | 85829 | 85830 | 85831 | 85832 | 85833 | 85834 | 85835 | 85836 | 85837 | 85838 | 85839 | 85840 | 85841 | 85842 | 85843 | 85844 | 85845 | 85846 | 85847 | 85848 | 86434 | 86435 | 86436 | 86534 | 86535 | 86536