Attachment 105019 Details for Bug 25527 – Bug 25527: Add logger to Koha::ExternalContent

[patch] Bug 25527: Add logger to Koha::ExternalContent

Bug-25527-Add-logger-to-KohaExternalContent.patch (text/plain), 6.30 KB, created by Victor Grousset/tuxayo on 2020-05-18 13:57:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Victor Grousset/tuxayo

Created: 2020-05-18 13:57:01 UTC

Size: 6.30 KB

patch

obsolete

>From cc067d21858abc9a26e363a63c8beae40c40885d Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Mon, 18 May 2020 09:26:09 -0300
>Subject: [PATCH] Bug 25527: Add logger to Koha::ExternalContent
>
>This patch makes Koha::Logger initialization happen in the ->new method
>for the Koha::ExternalContent-derived classes. In the case of RecordedBooks,
>it doesn't look like it is used at all.
>
>In the case of OverDrive, it will now use the Koha::ExternalContent
>exported logger accessor.
>
>I added tests for this addition to Koha::ExternalContent to the
>OverDrive tests.
>
>I also removed references to Test::DBIx::Class as it is not used at all.
>
>To test:
>1. Apply this patch
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/Koha_ExternalContent_OverDrive.t
>=> SUCCESS: Tests pass!
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>---
> Koha/ExternalContent.pm                       |  6 +-
> Koha/ExternalContent/OverDrive.pm             |  4 +-
> Koha/ExternalContent/RecordedBooks.pm         |  3 -
> .../Koha_ExternalContent_OverDrive.t          | 61 +++++++++----------
> 4 files changed, 36 insertions(+), 38 deletions(-)
>
>diff --git a/Koha/ExternalContent.pm b/Koha/ExternalContent.pm
>index 7b8876bf3b..578e035cf5 100644
>--- a/Koha/ExternalContent.pm
>+++ b/Koha/ExternalContent.pm
>@@ -22,10 +22,11 @@ use Carp;
> use base qw(Class::Accessor);
> 
> use Koha;
>+use Koha::Logger;
> use Koha::Patrons;
> use C4::Auth;
> 
>-__PACKAGE__->mk_accessors(qw(client koha_session_id koha_patron));
>+__PACKAGE__->mk_accessors(qw(client koha_session_id koha_patron logger));
> 
> =head1 NAME
> 
>@@ -54,6 +55,9 @@ sub agent_string {
> sub new {
>     my $class     = shift;
>     my $params    = shift || {};
>+
>+    $params->{logger} = Koha::Logger->get();
>+
>     return bless $params, $class;
> }
> 
>diff --git a/Koha/ExternalContent/OverDrive.pm b/Koha/ExternalContent/OverDrive.pm
>index 0541b552cb..ad36c18ce8 100644
>--- a/Koha/ExternalContent/OverDrive.pm
>+++ b/Koha/ExternalContent/OverDrive.pm
>@@ -23,9 +23,6 @@ use Carp;
> use base qw(Koha::ExternalContent);
> use WebService::ILS::OverDrive::Patron;
> use C4::Context;
>-use Koha::Logger;
>-
>-use constant logger => Koha::Logger->get();
> 
> =head1 NAME
> 
>@@ -71,6 +68,7 @@ sub new {
>             user_agent_params => { agent => $class->agent_string }
>         ) );
>     }
>+
>     return $self;
> }
> 
>diff --git a/Koha/ExternalContent/RecordedBooks.pm b/Koha/ExternalContent/RecordedBooks.pm
>index 455087995b..2e8c33bffd 100644
>--- a/Koha/ExternalContent/RecordedBooks.pm
>+++ b/Koha/ExternalContent/RecordedBooks.pm
>@@ -24,9 +24,6 @@ use base qw(Koha::ExternalContent);
> use WebService::ILS::RecordedBooks::PartnerPatron;
> use WebService::ILS::RecordedBooks::Partner;
> use C4::Context;
>-use Koha::Logger;
>-
>-use constant logger => Koha::Logger->get();
> 
> __PACKAGE__->mk_accessors(qw(domain is_identified));
> 
>diff --git a/t/db_dependent/Koha_ExternalContent_OverDrive.t b/t/db_dependent/Koha_ExternalContent_OverDrive.t
>index c09ea2952d..d8c5c97289 100755
>--- a/t/db_dependent/Koha_ExternalContent_OverDrive.t
>+++ b/t/db_dependent/Koha_ExternalContent_OverDrive.t
>@@ -2,52 +2,51 @@
> 
> use Modern::Perl;
> 
>-use t::lib::Mocks;
> use Test::More;
> use Test::MockModule;
>+use t::lib::Mocks;
> 
> use Module::Load::Conditional qw( can_load check_install );
> 
> BEGIN {
>-    if ( check_install( module => 'Test::DBIx::Class' ) ) {
>-        plan tests => 5;
>+    if ( check_install( module => 'WebService::ILS::OverDrive::Patron' ) ) {
>+        plan tests => 6;
>     } else {
>-        plan skip_all => "Need Test::DBIx::Class"
>+        plan skip_all => "Need WebService::ILS::OverDrive::Patron"
>     }
> }
> 
>-use Test::DBIx::Class;
>+use_ok('Koha::ExternalContent::OverDrive');
> 
>-my $db = Test::MockModule->new('Koha::Database');
>-$db->mock( _new_schema => sub { return Schema(); } );
>+t::lib::Mocks::mock_preference('SessionStorage','tmp');
> 
>-SKIP: {
>-    skip "cannot find WebService::ILS::OverDrive::Patron", 5
>-      unless can_load( modules => { 'WebService::ILS::OverDrive::Patron' => undef } );
>+t::lib::Mocks::mock_preference('OverDriveClientKey', 'DUMMY');
>+t::lib::Mocks::mock_preference('OverDriveClientSecret', 'DUMMY');
>+t::lib::Mocks::mock_preference('OverDriveLibraryID', 'DUMMY');
> 
>-    use_ok('Koha::ExternalContent::OverDrive');
>+my $client = Koha::ExternalContent::OverDrive->new({koha_session_id => 'DUMMY'});
> 
>-    t::lib::Mocks::mock_preference('SessionStorage','tmp');
>+my $user_agent_string = $client->user_agent->agent();
>+ok ($user_agent_string =~ m/^Koha/, 'User Agent string is set')
>+    or diag("User Agent string: $user_agent_string");
> 
>-    t::lib::Mocks::mock_preference('OverDriveClientKey', 'DUMMY');
>-    t::lib::Mocks::mock_preference('OverDriveClientSecret', 'DUMMY');
>-    t::lib::Mocks::mock_preference('OverDriveLibraryID', 'DUMMY');
>+my $base_url = "http://mykoha.org";
>+ok ($client->auth_url($base_url), 'auth_url()');
>+local $@;
>+eval { $client->auth_by_code("blah", $base_url) };
>+ok($@, "auth_by_code() dies with bogus credentials");
>+SKIP: {
>+    skip "No exception", 1 unless $@;
>+    my $error_message = $client->error_message($@);
>+    ok($error_message =~ m/Authorization Failed/i, "error_message()")
>+        or diag("Original:\n$@\nTurned into:\n$error_message");
>+}
> 
>-    my $client = Koha::ExternalContent::OverDrive->new({koha_session_id => 'DUMMY'});
>+subtest 'logger() tests' => sub {
> 
>-    my $user_agent_string = $client->user_agent->agent();
>-    ok ($user_agent_string =~ m/^Koha/, 'User Agent string is set')
>-      or diag("User Agent string: $user_agent_string");
>+    plan tests => 2;
> 
>-    my $base_url = "http://mykoha.org";
>-    ok ($client->auth_url($base_url), 'auth_url()');
>-    local $@;
>-    eval { $client->auth_by_code("blah", $base_url) };
>-    ok($@, "auth_by_code() dies with bogus credentials");
>-    SKIP: {
>-        skip "No exception", 1 unless $@;
>-        my $error_message = $client->error_message($@);
>-        ok($error_message =~ m/Authorization Failed/i, "error_message()")
>-          or diag("Original:\n$@\nTurned into:\n$error_message");
>-    }
>-}
>+    my $external_content = Koha::ExternalContent::OverDrive->new({koha_session_id => 'DUMMY'});
>+    ok( $external_content->can('logger'), 'A Koha::ExternalContent object has a logger accessor' );
>+    is( ref($external_content->logger), 'Koha::Logger', 'The accessor return the right object type' );
>+};
>-- 
>2.26.2

Actions: View | Diff | Splinter Review

Attachments on bug 25527: 105013 | 105014 | 105017 | 105018 | 105019 | 105020 | 105021