Bugzilla – Attachment 46447 Details for
Bug 15209
C4::Koha routines expecting a MARC::Record object should check it is defined
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15209 Validate passed MARC::Record objs in C4::Koha
Bug-15209-Validate-passed-MARCRecord-objs-in-C4Koh.patch (text/plain), 7.15 KB, created by
Kyle M Hall (khall)
on 2016-01-08 17:34:09 UTC
(
hide
)
Description:
Bug 15209 Validate passed MARC::Record objs in C4::Koha
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2016-01-08 17:34:09 UTC
Size:
7.15 KB
patch
obsolete
>From 477e8d6a178cecf698c9cdb98ecc1d40b202a50a Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Wed, 18 Nov 2015 14:53:58 +0000 >Subject: [PATCH] Bug 15209 Validate passed MARC::Record objs in C4::Koha > >Ensure that a passed MARC::Record is defined before calling >its methods. Otherwise you are open to occurences of the >error 'Can't call method "field" on an undefined value' >In a CGI environment you can live with such sloppiness but >in a persistent environment the error can cause the instance >to abort. >Made all routines passed a MARC::Record validate it before calling >its methods. Changed the parameter name from the meaningless >record to marcrecord to indicate its content. Added an explicit return >for all cases where no valid data returned. Cleaned up some logic for >clarity. I think we can assume that GetNormalizedOCLCNumber meant to >look at all 035s till it found an OCLC number not just the first. > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Koha.pm | 132 ++++++++++++++++++++++++++++++------------------------------- > 1 file changed, 64 insertions(+), 68 deletions(-) > >diff --git a/C4/Koha.pm b/C4/Koha.pm >index 12ac19c..ef71d93 100644 >--- a/C4/Koha.pm >+++ b/C4/Koha.pm >@@ -1465,7 +1465,7 @@ MARC field, replacing any blanks with '#'. > sub display_marc_indicators { > my $field = shift; > my $indicators = ''; >- if ($field->tag() >= 10) { >+ if ($field && $field->tag() >= 10) { > $indicators = $field->indicator(1) . $field->indicator(2); > $indicators =~ s/ /#/g; > } >@@ -1473,111 +1473,107 @@ sub display_marc_indicators { > } > > sub GetNormalizedUPC { >- my ($record,$marcflavour) = @_; >- my (@fields,$upc); >- >- if ($marcflavour eq 'UNIMARC') { >- @fields = $record->field('072'); >- foreach my $field (@fields) { >- my $upc = _normalize_match_point($field->subfield('a')); >- if ($upc ne '') { >- return $upc; >+ my ($marcrecord,$marcflavour) = @_; >+ >+ if ($marcrecord) { >+ if ($marcflavour eq 'UNIMARC') { >+ my @fields = $marcrecord->field('072'); >+ foreach my $field (@fields) { >+ my $upc = _normalize_match_point($field->subfield('a')); >+ if ($upc) { >+ return $upc; >+ } > } >- } > >- } >- else { # assume marc21 if not unimarc >- @fields = $record->field('024'); >- foreach my $field (@fields) { >- my $indicator = $field->indicator(1); >- my $upc = _normalize_match_point($field->subfield('a')); >- if ($indicator == 1 and $upc ne '') { >- return $upc; >+ } >+ else { # assume marc21 if not unimarc >+ my @fields = $marcrecord->field('024'); >+ foreach my $field (@fields) { >+ my $indicator = $field->indicator(1); >+ my $upc = _normalize_match_point($field->subfield('a')); >+ if ($upc && $indicator == 1 ) { >+ return $upc; >+ } > } > } > } >+ return; > } > > # Normalizes and returns the first valid ISBN found in the record > # ISBN13 are converted into ISBN10. This is required to get some book cover images. > sub GetNormalizedISBN { >- my ($isbn,$record,$marcflavour) = @_; >- my @fields; >+ my ($isbn,$marcrecord,$marcflavour) = @_; > if ($isbn) { > # Koha attempts to store multiple ISBNs in biblioitems.isbn, separated by " | " > # anything after " | " should be removed, along with the delimiter > ($isbn) = split(/\|/, $isbn ); > return _isbn_cleanup($isbn); > } >- return unless $record; >- >- if ($marcflavour eq 'UNIMARC') { >- @fields = $record->field('010'); >- foreach my $field (@fields) { >- my $isbn = $field->subfield('a'); >- if ($isbn) { >- return _isbn_cleanup($isbn); >- } else { >- return; >+ if ($marcrecord) { >+ >+ if ($marcflavour eq 'UNIMARC') { >+ my @fields = $marcrecord->field('010'); >+ foreach my $field (@fields) { >+ my $isbn = $field->subfield('a'); >+ if ($isbn) { >+ return _isbn_cleanup($isbn); >+ } > } > } >- } >- else { # assume marc21 if not unimarc >- @fields = $record->field('020'); >- foreach my $field (@fields) { >- $isbn = $field->subfield('a'); >- if ($isbn) { >- return _isbn_cleanup($isbn); >- } else { >- return; >+ else { # assume marc21 if not unimarc >+ my @fields = $marcrecord->field('020'); >+ foreach my $field (@fields) { >+ $isbn = $field->subfield('a'); >+ if ($isbn) { >+ return _isbn_cleanup($isbn); >+ } > } > } > } >+ return; > } > > sub GetNormalizedEAN { >- my ($record,$marcflavour) = @_; >- my (@fields,$ean); >- >- if ($marcflavour eq 'UNIMARC') { >- @fields = $record->field('073'); >- foreach my $field (@fields) { >- $ean = _normalize_match_point($field->subfield('a')); >- if ($ean ne '') { >- return $ean; >+ my ($marcrecord,$marcflavour) = @_; >+ >+ if ($marcrecord) { >+ if ($marcflavour eq 'UNIMARC') { >+ my @fields = $marcrecord->field('073'); >+ foreach my $field (@fields) { >+ my $ean = _normalize_match_point($field->subfield('a')); >+ if ( $ean ) { >+ return $ean; >+ } > } > } >- } >- else { # assume marc21 if not unimarc >- @fields = $record->field('024'); >- foreach my $field (@fields) { >- my $indicator = $field->indicator(1); >- $ean = _normalize_match_point($field->subfield('a')); >- if ($indicator == 3 and $ean ne '') { >- return $ean; >+ else { # assume marc21 if not unimarc >+ my @fields = $marcrecord->field('024'); >+ foreach my $field (@fields) { >+ my $indicator = $field->indicator(1); >+ my $ean = _normalize_match_point($field->subfield('a')); >+ if ( $ean && $indicator == 3 ) { >+ return $ean; >+ } > } > } > } >+ return; > } >-sub GetNormalizedOCLCNumber { >- my ($record,$marcflavour) = @_; >- my (@fields,$oclc); > >- if ($marcflavour eq 'UNIMARC') { >- # TODO: add UNIMARC fields >- } >- else { # assume marc21 if not unimarc >- @fields = $record->field('035'); >+sub GetNormalizedOCLCNumber { >+ my ($marcrecord,$marcflavour) = @_; >+ if ($marcrecord && $marcflavour ne 'UNIMARC' ) { >+ my @fields = $marcrecord->field('035'); > foreach my $field (@fields) { >- $oclc = $field->subfield('a'); >+ my $oclc = $field->subfield('a'); > if ($oclc =~ /OCoLC/) { > $oclc =~ s/\(OCoLC\)//; > return $oclc; >- } else { >- return; > } > } > } >+ return; > } > > sub GetAuthvalueDropbox { >-- >2.1.4
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 15209
:
44963
|
46007
|
46008
|
46009
| 46447 |
46448
|
46449