From cc067d21858abc9a26e363a63c8beae40c40885d Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Signed-off-by: Victor Grousset/tuxayo --- 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