Bugzilla – Attachment 22447 Details for
Bug 11032
Validate passed records in Biblio.pm
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[SIGNED OFF] Bug 11032: Check a valid MARC::Record passed to Biblio
SIGNED-OFF-Bug-11032-Check-a-valid-MARCRecord-pass.patch (text/plain), 12.34 KB, created by
Katrin Fischer
on 2013-10-27 09:21:14 UTC
(
hide
)
Description:
[SIGNED OFF] Bug 11032: Check a valid MARC::Record passed to Biblio
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2013-10-27 09:21:14 UTC
Size:
12.34 KB
patch
obsolete
>From 0fb2a5303952f915fa0a7ac39baf11a205905df8 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Thu, 10 Oct 2013 18:06:14 +0100 >Subject: [PATCH] [SIGNED OFF] Bug 11032: Check a valid MARC::Record passed to > Biblio > >Intermittently problems in the calling environment >cause a C4::Biblio to be called with and undefined >MARC::Record object. This results in the process >dieing and returning to the enduser a low level >message such as 'cannot call method x on an undefined >object' >For exported subroutines taking a MARC::Record object >check that object is defined otherwise return a logical >return value and log a stack trace to the error log. >A couple of cases were checking but dieing, this may have >unwelcome results in a persistent environment so croak has >been downgraded to carp > >Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> >Adds lots of checks for $record in various places, should >not affect behaviour. >Passed all tests and QA script, including new unit tests. >Tested adding and saving a new record. >Also tested detail and result pages without XSLT. >--- > C4/Biblio.pm | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > t/Biblio.t | 82 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 187 insertions(+), 5 deletions(-) > create mode 100755 t/Biblio.t > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index add0f74..71e9366 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -250,6 +250,10 @@ sub AddBiblio { > my $frameworkcode = shift; > my $options = @_ ? shift : undef; > my $defer_marc_save = 0; >+ if (!$record) { >+ carp('AddBiblio called with undefined record'); >+ return; >+ } > if ( defined $options and exists $options->{'defer_marc_save'} and $options->{'defer_marc_save'} ) { > $defer_marc_save = 1; > } >@@ -299,11 +303,16 @@ in the C<biblio> and C<biblioitems> tables, as well as > which fields are used to store embedded item, biblioitem, > and biblionumber data for indexing. > >+Returns 1 on success 0 on failure >+ > =cut > > sub ModBiblio { > my ( $record, $biblionumber, $frameworkcode ) = @_; >- croak "No record" unless $record; >+ if (!$record) { >+ carp 'No record passed to ModBiblio'; >+ return 0; >+ } > > if ( C4::Context->preference("CataloguingLog") ) { > my $newrecord = GetMarcBiblio($biblionumber); >@@ -473,11 +482,17 @@ sub DelBiblio { > > Automatically links headings in a bib record to authorities. > >+Returns the number of headings changed >+ > =cut > > sub BiblioAutoLink { > my $record = shift; > my $frameworkcode = shift; >+ if (!$record) { >+ carp('Undefined record passed to BiblioAutoLink'); >+ return 0; >+ } > my ( $num_headings_changed, %results ); > > my $linker_module = >@@ -485,7 +500,7 @@ sub BiblioAutoLink { > unless ( can_load( modules => { $linker_module => undef } ) ) { > $linker_module = 'C4::Linker::Default'; > unless ( can_load( modules => { $linker_module => undef } ) ) { >- return 0, 0; >+ return 0; > } > } > >@@ -522,6 +537,10 @@ sub LinkBibHeadingsToAuthorities { > my $frameworkcode = shift; > my $allowrelink = shift; > my %results; >+ if (!$bib) { >+ carp 'LinkBibHeadingsToAuthorities called on undefined bib record'; >+ return ( 0, {}); >+ } > require C4::Heading; > require C4::AuthoritiesMarc; > >@@ -671,6 +690,11 @@ Get MARC fields from a keyword defined in fieldmapping table. > > sub GetRecordValue { > my ( $field, $record, $frameworkcode ) = @_; >+ >+ if (!$record) { >+ carp 'GetRecordValue called with undefined record'; >+ return; >+ } > my $dbh = C4::Context->dbh; > > my $sth = $dbh->prepare('SELECT fieldcode, subfieldcode FROM fieldmapping WHERE frameworkcode = ? AND field = ?'); >@@ -1302,7 +1326,8 @@ sub GetCOinSBiblio { > > # get the coin format > if ( ! $record ) { >- return; >+ carp 'GetCOinSBiblio called with undefined record'; >+ return; > } > my $pos7 = substr $record->leader(), 7, 1; > my $pos6 = substr $record->leader(), 6, 1; >@@ -1443,10 +1468,20 @@ sub GetCOinSBiblio { > =head2 GetMarcPrice > > return the prices in accordance with the Marc format. >+ >+returns 0 if no price found >+returns undef if called without a marc record or with >+an unrecognized marc format >+ > =cut > > sub GetMarcPrice { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcPrice called on undefined record'; >+ return; >+ } >+ > my @listtags; > my $subfield; > >@@ -1518,10 +1553,19 @@ sub MungeMarcPrice { > return the quantity of a book. Used in acquisition only, when importing a file an iso2709 from a bookseller > Warning : this is not really in the marc standard. In Unimarc, Electre (the most widely used bookseller) use the 969$a > >+returns 0 if no quantity found >+returns undef if called without a marc record or with >+an unrecognized marc format >+ > =cut > > sub GetMarcQuantity { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcQuantity called on undefined record'; >+ return; >+ } >+ > my @listtags; > my $subfield; > >@@ -1608,6 +1652,10 @@ Get the control number / record Identifier from the MARC record and return it. > > sub GetMarcControlnumber { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcControlnumber called on undefined record'; >+ return; >+ } > my $controlnumber = ""; > # Control number or Record identifier are the same field in MARC21, UNIMARC and NORMARC > # Keep $marcflavour for possible later use >@@ -1631,6 +1679,10 @@ ISBNs stored in different fields depending on MARC flavour > > sub GetMarcISBN { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcISBN called on undefined record'; >+ return; >+ } > my $scope; > if ( $marcflavour eq "UNIMARC" ) { > $scope = '010'; >@@ -1672,6 +1724,10 @@ ISSNs are stored in different fields depending on MARC flavour > > sub GetMarcISSN { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcISSN called on undefined record'; >+ return; >+ } > my $scope; > if ( $marcflavour eq "UNIMARC" ) { > $scope = '011'; >@@ -1697,6 +1753,10 @@ The note are stored in different fields depending on MARC flavour > > sub GetMarcNotes { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcNotes called on undefined record'; >+ return; >+ } > my $scope; > if ( $marcflavour eq "UNIMARC" ) { > $scope = '3..'; >@@ -1741,6 +1801,10 @@ The subjects are stored in different fields depending on MARC flavour > > sub GetMarcSubjects { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcSubjects called on undefined record'; >+ return; >+ } > my ( $mintag, $maxtag, $fields_filter ); > if ( $marcflavour eq "UNIMARC" ) { > $mintag = "600"; >@@ -1826,6 +1890,10 @@ The authors are stored in different fields depending on MARC flavour > > sub GetMarcAuthors { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcAuthors called on undefined record'; >+ return; >+ } > my ( $mintag, $maxtag, $fields_filter ); > > # tagslib useful for UNIMARC author reponsabilities >@@ -1914,6 +1982,10 @@ Assumes web resources (not uncommon in MARC21 to omit resource type ind) > > sub GetMarcUrls { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcUrls called on undefined record'; >+ return; >+ } > > my @marcurls; > for my $field ( $record->field('856') ) { >@@ -1969,6 +2041,11 @@ The series are stored in different fields depending on MARC flavour > > sub GetMarcSeries { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcSeries called on undefined record'; >+ return; >+ } >+ > my ( $mintag, $maxtag, $fields_filter ); > if ( $marcflavour eq "UNIMARC" ) { > $mintag = "225"; >@@ -2038,6 +2115,11 @@ Get all host records (773s MARC21, 461 UNIMARC) from the MARC record and returns > > sub GetMarcHosts { > my ( $record, $marcflavour ) = @_; >+ if (!$record) { >+ carp 'GetMarcHosts called on undefined record'; >+ return; >+ } >+ > my ( $tag,$title_subf,$bibnumber_subf,$itemnumber_subf); > $marcflavour ||="MARC21"; > if ( $marcflavour eq "MARC21" || $marcflavour eq "NORMARC" ) { >@@ -2481,12 +2563,19 @@ our $inverted_field_map; > Extract data from a MARC bib record into a hashref representing > Koha biblio, biblioitems, and items fields. > >+If passed an undefined record will log the error and return an empty >+hash_ref >+ > =cut > > sub TransformMarcToKoha { > my ( $dbh, $record, $frameworkcode, $limit_table ) = @_; > >- my $result; >+ my $result = {}; >+ if (!defined $record) { >+ carp('TransformMarcToKoha called with undefined record'); >+ return $result; >+ } > $limit_table = $limit_table || 0; > $frameworkcode = '' unless defined $frameworkcode; > >@@ -2799,7 +2888,10 @@ if $itemnumbers is defined, only specified itemnumbers are embedded > > sub EmbedItemsInMarcBiblio { > my ($marc, $biblionumber, $itemnumbers) = @_; >- croak "No MARC record" unless $marc; >+ if ( !$marc ) { >+ carp 'EmbedItemsInMarcBiblio: No MARC record passed'; >+ return; >+ } > > $itemnumbers = [] unless defined $itemnumbers; > >@@ -3262,6 +3354,10 @@ sub ModBiblioMarc { > # pass the MARC::Record to this function, and it will create the records in > # the marc field > my ( $record, $biblionumber, $frameworkcode ) = @_; >+ if ( !$record ) { >+ carp 'ModBiblioMarc passed an undefined record'; >+ return; >+ } > > # Clone record as it gets modified > $record = $record->clone(); >@@ -3623,6 +3719,10 @@ Removes all nsb/nse chars from a record > > sub RemoveAllNsb { > my $record = shift; >+ if (!$record) { >+ carp 'RemoveAllNsb called with undefined record'; >+ return; >+ } > > SetUTF8Flag($record); > >diff --git a/t/Biblio.t b/t/Biblio.t >new file mode 100755 >index 0000000..9a1b8f4 >--- /dev/null >+++ b/t/Biblio.t >@@ -0,0 +1,82 @@ >+#!/usr/bin/perl >+# >+use strict; >+use warnings; >+ >+use Test::More tests => 22; >+ >+BEGIN { >+ use_ok('C4::Biblio'); >+} >+ >+# test returns if undef record passed >+# carp messages appear on stdout >+ >+my @arr = AddBiblio(undef, q{}); >+my $elements = @arr; >+ >+is($elements, 0, 'Add Biblio returns empty array for undef record'); >+ >+my $ret = ModBiblio(undef, 0, ''); >+ >+is( $ret, 0, 'ModBiblio returns zero if not passed rec'); >+ >+$ret = BiblioAutoLink(undef, q{}); >+ >+is( $ret, 0, 'BiblioAutoLink returns zero if not passed rec'); >+ >+$ret = GetRecordValue('100', undef, q{}); >+ok( !defined $ret, 'GetRecordValue returns undef if not passed rec'); >+ >+@arr = LinkBibHeadingsToAuthorities(q{}, q{}); >+is($arr[0], 0, 'LinkBibHeadingsToAuthorities correct error return'); >+ >+$ret = GetCOinSBiblio(); >+ok( !defined $ret, 'GetCOinSBiblio returns undef if not passed rec'); >+ >+$ret = GetMarcPrice(undef, 'MARC21'); >+ok( !defined $ret, 'GetMarcPrice returns undef if not passed rec'); >+ >+$ret = GetMarcQuantity(undef, 'MARC21'); >+ok( !defined $ret, 'GetMarcQuantity returns undef if not passed rec'); >+ >+$ret = GetMarcControlnumber(); >+ok( !defined $ret, 'GetMarcControlnumber returns undef if not passed rec'); >+ >+$ret = GetMarcISBN(); >+ok( !defined $ret, 'GetMarcISBN returns undef if not passed rec'); >+ >+$ret = GetMarcISSN(); >+ok( !defined $ret, 'GetMarcISSN returns undef if not passed rec'); >+ >+$ret = GetMarcNotes(); >+ok( !defined $ret, 'GetMarcNotes returns undef if not passed rec'); >+ >+$ret = GetMarcSubjects(); >+ok( !defined $ret, 'GetMarcSubjects returns undef if not passed rec'); >+ >+$ret = GetMarcAuthors(); >+ok( !defined $ret, 'GetMarcAuthors returns undef if not passed rec'); >+ >+$ret = GetMarcUrls(); >+ok( !defined $ret, 'GetMarcUrls returns undef if not passed rec'); >+ >+$ret = GetMarcSeries(); >+ok( !defined $ret, 'GetMarcSeries returns undef if not passed rec'); >+ >+$ret = GetMarcHosts(); >+ok( !defined $ret, 'GetMarcHosts returns undef if not passed rec'); >+ >+my $hash_ref = TransformMarcToKoha(undef, undef); >+ >+isa_ok( $hash_ref, 'HASH'); >+ >+$elements = keys %{$hash_ref}; >+ >+is($elements, 0, 'Empty hashref returned for undefined record in TransformMarcToKoha'); >+ >+$ret = ModBiblioMarc(); >+ok( !defined $ret, 'ModBiblioMarc returns undef if not passed rec'); >+ >+$ret = RemoveAllNsb(); >+ok( !defined $ret, 'RemoveAllNsb returns undef if not passed rec'); >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 11032
:
21958
|
22447
|
22832