From 61a948e2b8de630e8a16d4d6fc592df7da4204a7 Mon Sep 17 00:00:00 2001 From: Jared Camins-Esakov Date: Mon, 6 Aug 2012 15:10:15 -0400 Subject: [PATCH] Bug 7417 follow-up: incorporate QA comments Content-Type: text/plain; charset="UTF-8" This patch incorporates the two QA comments: * Renames Koha::Authority to Koha::DataObject::Authority * Uses Modern::Perl instead of 'use strict; use warnings;' This patch also improves test coverage, and eliminates the need for a database to test the EmbedSeeFromHeadings RecordProcessor plugin. --- C4/AuthoritiesMarc.pm | 4 +- Koha/{ => DataObject}/Authority.pm | 17 ++-- Koha/Filter/MARC/EmbedSeeFromHeadings.pm | 9 +- Koha/Filter/MARC/Null.pm | 3 +- Koha/RecordProcessor.pm | 3 +- Koha/RecordProcessor/Base.pm | 3 +- t/RecordProcessor_EmbedSeeFromHeadings.t | 121 ++++++++++++++++++++ t/db_dependent/Koha_Authority.t | 15 +-- .../RecordProcessor_EmbedSeeFromHeadings.t | 66 ----------- 9 files changed, 145 insertions(+), 96 deletions(-) rename Koha/{ => DataObject}/Authority.pm (82%) create mode 100755 t/RecordProcessor_EmbedSeeFromHeadings.t delete mode 100755 t/db_dependent/RecordProcessor_EmbedSeeFromHeadings.t diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm index c533ad0..c2c3991 100644 --- a/C4/AuthoritiesMarc.pm +++ b/C4/AuthoritiesMarc.pm @@ -26,7 +26,7 @@ use C4::AuthoritiesMarc::MARC21; use C4::AuthoritiesMarc::UNIMARC; use C4::Charset; use C4::Log; -use Koha::Authority; +use Koha::DataObject::Authority; use vars qw($VERSION @ISA @EXPORT); @@ -849,7 +849,7 @@ Returns MARC::Record of the authority passed in parameter. sub GetAuthority { my ($authid)=@_; - my $authority = Koha::Authority->get_from_authid($authid); + my $authority = Koha::DataObject::Authority->get_from_authid($authid); return ($authority->record); } diff --git a/Koha/Authority.pm b/Koha/DataObject/Authority.pm similarity index 82% rename from Koha/Authority.pm rename to Koha/DataObject/Authority.pm index b64f8de..2287644 100644 --- a/Koha/Authority.pm +++ b/Koha/DataObject/Authority.pm @@ -1,4 +1,4 @@ -package Koha::Authority; +package Koha::DataObject::Authority; # Copyright 2012 C & P Bibliography Services # @@ -19,7 +19,7 @@ package Koha::Authority; =head1 NAME -Koha::Authority - class to encapsulate authority records in Koha +Koha::DataObject::Authority - class to encapsulate authority records in Koha =head1 SYNOPSIS @@ -31,8 +31,7 @@ Authority data. =cut -use strict; -use warnings; +use Modern::Perl; use C4::Context; use MARC::Record; use MARC::File::XML; @@ -44,9 +43,9 @@ __PACKAGE__->mk_accessors(qw( authid authtype record marcflavour )); =head2 new - my $auth = Koha::Authority->new($record); + my $auth = Koha::DataObject::Authority->new($record); -Create a new Koha::Authority object based on the provided record. +Create a new Koha::DataObject::Authority object based on the provided record. =cut sub new { @@ -61,9 +60,9 @@ sub new { =head2 get_from_authid - my $auth = Koha::Authority->get_from_authid($authid); + my $auth = Koha::DataObject::Authority->get_from_authid($authid); -Create the Koha::Authority object associated with the provided authid. +Create the Koha::DataObject::Authority object associated with the provided authid. =cut sub get_from_authid { @@ -77,7 +76,7 @@ sub get_from_authid { my ($authtypecode, $marcxml) = $sth->fetchrow; my $record=eval {MARC::Record->new_from_xml(StripNonXmlChars($marcxml),'UTF-8', (C4::Context->preference("marcflavour") eq "UNIMARC"?"UNIMARCAUTH":C4::Context->preference("marcflavour")))}; - return undef if ($@); + return if ($@); $record->encoding('UTF-8'); my $self = $class->SUPER::new( { authid => $authid, diff --git a/Koha/Filter/MARC/EmbedSeeFromHeadings.pm b/Koha/Filter/MARC/EmbedSeeFromHeadings.pm index ea7e38b..6457b4c 100644 --- a/Koha/Filter/MARC/EmbedSeeFromHeadings.pm +++ b/Koha/Filter/MARC/EmbedSeeFromHeadings.pm @@ -30,10 +30,9 @@ Filter to embed see from headings into MARC records. =cut -use strict; -use warnings; +use Modern::Perl; use Carp; -use Koha::Authority; +use Koha::DataObject::Authority; use base qw(Koha::RecordProcessor::Base); our $NAME = 'EmbedSeeFromHeadings'; @@ -54,7 +53,7 @@ sub filter { my $record = shift; my $newrecord; - return undef unless defined $record; + return unless defined $record; if (ref $record eq 'ARRAY') { my @recarray; @@ -78,7 +77,7 @@ sub _processrecord { next unless $authid; - my $authority = Koha::Authority->get_from_authid($authid); + my $authority = Koha::DataObject::Authority->get_from_authid($authid); next unless $authority; my $auth_marc = $authority->record; my @seefrom = $auth_marc->field('4..'); diff --git a/Koha/Filter/MARC/Null.pm b/Koha/Filter/MARC/Null.pm index 578781c..a84384b 100644 --- a/Koha/Filter/MARC/Null.pm +++ b/Koha/Filter/MARC/Null.pm @@ -31,8 +31,7 @@ RecordProcessor. =cut -use strict; -use warnings; +use Modern::Perl; use Carp; use base qw(Koha::RecordProcessor::Base); diff --git a/Koha/RecordProcessor.pm b/Koha/RecordProcessor.pm index 3d4e1bf..96ade9a 100644 --- a/Koha/RecordProcessor.pm +++ b/Koha/RecordProcessor.pm @@ -57,8 +57,7 @@ clone it I to passing it off to the RecordProcessor. =cut -use strict; -use warnings; +use Modern::Perl; use Module::Load::Conditional qw(can_load); use Module::Pluggable::Object; diff --git a/Koha/RecordProcessor/Base.pm b/Koha/RecordProcessor/Base.pm index 00bc623..e051b74 100644 --- a/Koha/RecordProcessor/Base.pm +++ b/Koha/RecordProcessor/Base.pm @@ -58,8 +58,7 @@ clone it I to passing it off to the RecordProcessor. =cut -use strict; -use warnings; +use Modern::Perl; use base qw(Class::Accessor); diff --git a/t/RecordProcessor_EmbedSeeFromHeadings.t b/t/RecordProcessor_EmbedSeeFromHeadings.t new file mode 100755 index 0000000..ee3185a --- /dev/null +++ b/t/RecordProcessor_EmbedSeeFromHeadings.t @@ -0,0 +1,121 @@ +#!/usr/bin/perl + +# Copyright 2012 C & P Bibliography Services +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 2 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; +use DBI; +use File::Spec; +use MARC::Record; +use MARC::File::XML; + +use Test::More tests => 9; +use Test::MockModule; + +use C4::Context; + +BEGIN { + use_ok('Koha::RecordProcessor'); +} + +# Mock the call to create a database handler +my $module = new Test::MockModule('C4::Context'); +$module->mock('_new_dbh', sub { + my $dbh = DBI->connect( 'DBI:Mock:', '', '' ) + || die "Cannot create handle: $DBI::errstr\n"; + return $dbh +}); +$module->mock('preference', sub { + return 'MARC21' if (pop @_ eq 'marcflavour'); + return 0; +}); + + +# Mock data +my $auth = MARC::Record->new; +$auth->add_fields( + [ '001', '1234' ], + [ '150', ' ', ' ', a => 'Cooking' ], + [ '450', ' ', ' ', a => 'Cookery' ], +); + +my $mockauthority = [ + ['authtypecode','marcxml'], + ['TOPIC', $auth->as_xml_record() ], + ]; + +my $dbh = C4::Context->dbh(); # since we mocked _new_dbh we get a mock handle + +my $bib = MARC::Record->new; +$bib->add_fields( + [ '245', '0', '4', a => 'The Ifrane cookbook' ], + [ '650', ' ', ' ', a => 'Cooking', 9 => '1234' ] + ); + +my $resultbib = MARC::Record->new; +$resultbib->add_fields( + [ '245', '0', '4', a => 'The Ifrane cookbook' ], + [ '650', ' ', ' ', a => 'Cooking', 9 => '1234' ], + [ '650', 'z', ' ', a => 'Cookery' ] + ); + +my $processor = Koha::RecordProcessor->new( { filters => ( 'EmbedSeeFromHeadings' ) } ); +is(ref($processor), 'Koha::RecordProcessor', 'Created record processor'); +is(ref($processor->filters->[0]), 'Koha::Filter::MARC::EmbedSeeFromHeadings', 'Loaded EmbedSeeFromHeadings filter'); + +$dbh->{mock_add_resultset} = $mockauthority; + +my $result = $processor->process(undef); + +is($result, undef, "Don't flip out when asked to process nothing"); + +$result = $processor->process($bib); + +my $history = $dbh->{mock_all_history}; +is(scalar(@{$history}), 1, 'Correct number of statements executed') ; +$dbh->{mock_clear_history} = 1; + +is_deeply($result, $resultbib, 'Inserted see-from heading to record'); + +my @bibs; +$bib = MARC::Record->new; +$bib->add_fields( + [ '245', '0', '4', a => 'The Ifrane cookbook' ], + [ '650', ' ', ' ', a => 'Cooking', 9 => '1234' ] + ); +push @bibs, $bib; +$bib = MARC::Record->new; +$bib->add_fields( + [ '245', '0', '2', a => 'A story of great tragedy, and fish' ], + [ '650', ' ', ' ', a => 'Fish', 9 => '5432' ] + ); +push @bibs, $bib; +my @resultbibs; +push @resultbibs, $resultbib; +push @resultbibs, $bib; + + +$dbh->{mock_add_resultset} = $mockauthority; + +$result = $processor->process(\@bibs); + +is(scalar(@$result), 2, 'Processed two records'); + +$history = $dbh->{mock_all_history}; +is(scalar(@{$history}), 2, 'Correct number of statements executed'); + +is_deeply($result, \@resultbibs, 'Handled both records correctly'); diff --git a/t/db_dependent/Koha_Authority.t b/t/db_dependent/Koha_Authority.t index dc57774..c437a41 100755 --- a/t/db_dependent/Koha_Authority.t +++ b/t/db_dependent/Koha_Authority.t @@ -17,14 +17,13 @@ # with Koha; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -use strict; -use warnings; +use Modern::Perl; use C4::Context; use Test::More; BEGIN { - use_ok('Koha::Authority'); + use_ok('Koha::DataObject::Authority'); } my $record = MARC::Record->new; @@ -34,9 +33,9 @@ $record->add_fields( [ '150', ' ', ' ', a => 'Cooking' ], [ '450', ' ', ' ', a => 'Cookery' ], ); -my $authority = Koha::Authority->new($record); +my $authority = Koha::DataObject::Authority->new($record); -is(ref($authority), 'Koha::Authority', 'Created valid Koha::Authority object'); +is(ref($authority), 'Koha::DataObject::Authority', 'Created valid Koha::DataObject::Authority object'); is_deeply($authority->record, $record, 'Saved record'); @@ -51,15 +50,15 @@ SKIP: $authid = $row->{'authid'}; } skip 'No authorities', 3 unless $authid; - $authority = Koha::Authority->get_from_authid($authid); + $authority = Koha::DataObject::Authority->get_from_authid($authid); - is(ref($authority), 'Koha::Authority', 'Retrieved valid Koha::Authority object'); + is(ref($authority), 'Koha::DataObject::Authority', 'Retrieved valid Koha::DataObject::Authority object'); is($authority->authid, $authid, 'Object authid is correct'); is($authority->record->field('001')->data(), $authid, 'Retrieved correct record'); - $authority = Koha::Authority->get_from_authid('alphabetsoup'); + $authority = Koha::DataObject::Authority->get_from_authid('alphabetsoup'); is($authority, undef, 'No invalid record is retrieved'); } diff --git a/t/db_dependent/RecordProcessor_EmbedSeeFromHeadings.t b/t/db_dependent/RecordProcessor_EmbedSeeFromHeadings.t deleted file mode 100755 index 4742983..0000000 --- a/t/db_dependent/RecordProcessor_EmbedSeeFromHeadings.t +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/perl - -# Copyright 2012 C & P Bibliography Services -# -# This file is part of Koha. -# -# Koha is free software; you can redistribute it and/or modify it under the -# terms of the GNU General Public License as published by the Free Software -# Foundation; either version 2 of the License, or (at your option) any later -# version. -# -# Koha is distributed in the hope that it will be useful, but WITHOUT ANY -# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR -# A PARTICULAR PURPOSE. See the GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License along -# with Koha; if not, write to the Free Software Foundation, Inc., -# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - -use strict; -use warnings; -use File::Spec; -use MARC::Record; -use Koha::Authority; - -use Test::More; -use Test::MockModule; - -BEGIN { - use_ok('Koha::RecordProcessor'); -} - -my $module = new Test::MockModule('MARC::Record'); -$module->mock('new_from_xml', sub { - my $record = MARC::Record->new; - - $record->add_fields( - [ '001', '1234' ], - [ '150', ' ', ' ', a => 'Cooking' ], - [ '450', ' ', ' ', a => 'Cookery' ], - ); - - return $record; -}); - -my $bib = MARC::Record->new; -$bib->add_fields( - [ '245', '0', '4', a => 'The Ifrane cookbook' ], - [ '650', ' ', ' ', a => 'Cooking', 9 => '1234' ] - ); - -my $resultbib = MARC::Record->new; -$resultbib->add_fields( - [ '245', '0', '4', a => 'The Ifrane cookbook' ], - [ '650', ' ', ' ', a => 'Cooking', 9 => '1234' ], - [ '650', 'z', ' ', a => 'Cookery' ] - ); - -my $processor = Koha::RecordProcessor->new( { filters => ( 'EmbedSeeFromHeadings' ) } ); -is(ref($processor), 'Koha::RecordProcessor', 'Created record processor'); - -my $result = $processor->process($bib); - -is_deeply($result, $resultbib, 'Inserted see-from heading to record'); - -done_testing(); -- 1.7.2.5