Add some logging to MFHDParser.pm, and prevent returning raw parsing errors to the...

In concert with c13383, this will give cataloguers the ability to fix errors in MFHD records
without requiring a system administrator to manipulate SQL directly. Sweet relief.

git-svn-id: svn://svn.open-ils.org/ILS/trunk@13384 dcc99617-32d9-48b4-a31d-7c20da2025e4

diff --git a/Open-ILS/src/perlmods/OpenILS/Utils/MFHDParser.pm b/Open-ILS/src/perlmods/OpenILS/Utils/MFHDParser.pm
index 5e7e5ec..33725d6 100644 (file)
--- a/Open-ILS/src/perlmods/OpenILS/Utils/MFHDParser.pm
+++ b/Open-ILS/src/perlmods/OpenILS/Utils/MFHDParser.pm
@@ -54,6 +54,9 @@ Returns a Perl hash containing fields of interest from the MFHD record
 sub mfhd_to_hash {
        my ($self, $mfhd_xml) = @_;
 
+       my $marc;
+       my $mfhd;
+
        my $location = '';
        my $holdings = [];
        my $supplements = [];
@@ -65,81 +68,109 @@ sub mfhd_to_hash {
        my $missing = []; # Laurentian extension to MFHD standard
        my $incomplete = []; # Laurentian extension to MFHD standard
 
-       my $marc = MARC::Record->new_from_xml($mfhd_xml);
-       my $mfhd = MFHD->new($marc);
+       try {
+               $marc = MARC::Record->new_from_xml($mfhd_xml);
+       } otherwise {
+               $logger->error("Failed to convert MFHD XML to MARC: " . shift());
+               $logger->error("Failed MFHD XML: $mfhd_xml");
+       };
 
-       foreach my $field ($marc->field('852')) {
-               foreach my $subfield_ref ($field->subfields) {
-                       my ($subfield, $data) = @$subfield_ref;
-                       $location .= $data . " -- ";
-               }
+       if (!$marc) {
+               return undef;
        }
+
+       try {
+               $mfhd = MFHD->new($marc);
+       } otherwise {
+               $logger->error("Failed to parse MFHD: " . shift());
+               $logger->error("Failed MFHD XML: $mfhd_xml");
+       };
+
+       if (!$mfhd) {
+               return undef;
+       }
+
+       try {
+               foreach my $field ($marc->field('852')) {
+                       foreach my $subfield_ref ($field->subfields) {
+                               my ($subfield, $data) = @$subfield_ref;
+                               $location .= $data . " -- ";
+                       }
+               }
+       } otherwise {
+               $logger->error("MFHD location parsing error: " . shift());
+       };
+
        $location =~ s/ -- $//;
 
-       foreach my $field ($marc->field('866')) {
-               my $textual_holdings = $self->format_textual_holdings($field);
-               if ($textual_holdings) {
-                       push @$holdings, $textual_holdings;
+       try {
+               foreach my $field ($marc->field('866')) {
+                       my $textual_holdings = $self->format_textual_holdings($field);
+                       if ($textual_holdings) {
+                               push @$holdings, $textual_holdings;
+                       }
                }
-       }
-       foreach my $field ($marc->field('867')) {
-               my $textual_holdings = $self->format_textual_holdings($field);
-               if ($textual_holdings) {
-                       push @$supplements, $textual_holdings;
+               foreach my $field ($marc->field('867')) {
+                       my $textual_holdings = $self->format_textual_holdings($field);
+                       if ($textual_holdings) {
+                               push @$supplements, $textual_holdings;
+                       }
                }
-       }
-       foreach my $field ($marc->field('868')) {
-               my $textual_holdings = $self->format_textual_holdings($field);
-               if ($textual_holdings) {
-                       push @$indexes, $textual_holdings;
+               foreach my $field ($marc->field('868')) {
+                       my $textual_holdings = $self->format_textual_holdings($field);
+                       if ($textual_holdings) {
+                               push @$indexes, $textual_holdings;
+                       }
                }
-       }
 
-       foreach my $cap_id ($mfhd->captions('853')) {
-               my @curr_holdings = $mfhd->holdings('863', $cap_id);
-               next unless scalar @curr_holdings;
-               foreach (@curr_holdings) {
-                       push @$current_holdings, $_->format();
+               foreach my $cap_id ($mfhd->captions('853')) {
+                       my @curr_holdings = $mfhd->holdings('863', $cap_id);
+                       next unless scalar @curr_holdings;
+                       foreach (@curr_holdings) {
+                               push @$current_holdings, $_->format();
+                       }
                }
-       }
 
-       foreach my $cap_id ($mfhd->captions('854')) {
-               my @curr_supplements = $mfhd->holdings('864', $cap_id);
-               next unless scalar @curr_supplements;
-               foreach (@curr_supplements) {
-                       push @$current_supplements, $_->format();
+               foreach my $cap_id ($mfhd->captions('854')) {
+                       my @curr_supplements = $mfhd->holdings('864', $cap_id);
+                       next unless scalar @curr_supplements;
+                       foreach (@curr_supplements) {
+                               push @$current_supplements, $_->format();
+                       }
                }
-       }
 
-       foreach my $cap_id ($mfhd->captions('855')) {
-               my @curr_indexes = $mfhd->holdings('865', $cap_id);
-               next unless scalar @curr_indexes;
-               foreach (@curr_indexes) {
-                       push @$current_indexes, $_->format();
+               foreach my $cap_id ($mfhd->captions('855')) {
+                       my @curr_indexes = $mfhd->holdings('865', $cap_id);
+                       next unless scalar @curr_indexes;
+                       foreach (@curr_indexes) {
+                               push @$current_indexes, $_->format();
+                       }
                }
-       }
 
-       # Laurentian extensions
-       foreach my $field ($marc->field('530')) {
-               my $online_stmt = $self->format_textual_holdings($field);
-               if ($online_stmt) {
-                       push @$online, $online_stmt;
+               # Laurentian extensions
+               foreach my $field ($marc->field('530')) {
+                       my $online_stmt = $self->format_textual_holdings($field);
+                       if ($online_stmt) {
+                               push @$online, $online_stmt;
+                       }
                }
-       }
 
-       foreach my $field ($marc->field('590')) {
-               my $missing_stmt = $self->format_textual_holdings($field);
-               if ($missing_stmt) {
-                       push @$missing, $missing_stmt;
+               foreach my $field ($marc->field('590')) {
+                       my $missing_stmt = $self->format_textual_holdings($field);
+                       if ($missing_stmt) {
+                               push @$missing, $missing_stmt;
+                       }
                }
-       }
 
-       foreach my $field ($marc->field('591')) {
-               my $incomplete_stmt = $self->format_textual_holdings($field);
-               if ($incomplete_stmt) {
-                       push @$incomplete, $incomplete_stmt;
+               foreach my $field ($marc->field('591')) {
+                       my $incomplete_stmt = $self->format_textual_holdings($field);
+                       if ($incomplete_stmt) {
+                               push @$incomplete, $incomplete_stmt;
+                       }
                }
-       }
+       } otherwise {
+               $logger->error("MFHD statement parsing error: " . shift());
+       };
 
        return { location => $location, holdings => $holdings, current_holdings => $current_holdings,
                        supplements => $supplements, current_supplements => $current_supplements,
@@ -194,6 +225,11 @@ sub generate_svr {
 
        $record->id($id);
        $record->owning_lib($owning_lib);
+
+       if (!$holdings) {
+               return $record;
+       }
+
        $record->location($holdings->{location});
        $record->holdings($holdings->{holdings});
        $record->current_holdings($holdings->{current_holdings});
@@ -209,3 +245,5 @@ sub generate_svr {
 }
 
 1;
+
+# vim: ts=4:sw=4:noet