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