View | Details | Raw Unified | Return to bug 35104
Collapse All | Expand All

(-)a/C4/Biblio.pm (-1 / +12 lines)
Lines 298-303 sub AddBiblio { Link Here
298
                    {
298
                    {
299
                        skip_record_index => 1,
299
                        skip_record_index => 1,
300
                        record_source_id  => $options->{record_source_id},
300
                        record_source_id  => $options->{record_source_id},
301
                        ( ref $options->{warnings} eq 'ARRAY' ? ( warnings => $options->{warnings} ) : () ),
301
                    }
302
                    }
302
                );
303
                );
303
304
Lines 394-400 sub ModBiblio { Link Here
394
              ( exists $options->{record_source_id} )
395
              ( exists $options->{record_source_id} )
395
            ? ( record_source_id => $options->{record_source_id} )
396
            ? ( record_source_id => $options->{record_source_id} )
396
            : ()
397
            : ()
397
        )
398
        ),
399
        (
400
              ( ref $options->{warnings} eq 'ARRAY' )
401
            ? ( warnings => $options->{warnings} )
402
            : ()
403
        ),
398
    };
404
    };
399
405
400
    if ( !$record ) {
406
    if ( !$record ) {
Lines 2998-3003 sub ModBiblioMarc { Link Here
2998
        }
3004
        }
2999
    )->store;
3005
    )->store;
3000
3006
3007
    if ( $m_rs->stripped_nonxml && ref $options->{warnings} eq 'ARRAY' ) {
3008
        push @{ $options->{warnings} },
3009
            { type => 'nonxml_stripped', message => $m_rs->stripped_nonxml };
3010
    }
3011
3001
    unless ($skip_record_index) {
3012
    unless ($skip_record_index) {
3002
        my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
3013
        my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
3003
        $indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" );
3014
        $indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" );
(-)a/Koha/Biblio/Metadata.pm (-8 / +52 lines)
Lines 26-31 use C4::Items qw( GetMarcItem ); Link Here
26
26
27
use Koha::Database;
27
use Koha::Database;
28
use Koha::Exceptions::Metadata;
28
use Koha::Exceptions::Metadata;
29
use Koha::Logger;
29
use Koha::RecordSources;
30
use Koha::RecordSources;
30
31
31
use base qw(Koha::Object);
32
use base qw(Koha::Object);
Lines 45-50 Koha::Metadata - Koha Metadata Object class Link Here
45
Metadata specific store method to catch errant characters prior
46
Metadata specific store method to catch errant characters prior
46
to committing to the database.
47
to committing to the database.
47
48
49
If the MARCXML cannot be parsed but can be recovered by stripping non-XML
50
characters (C<StripNonXmlChars>), the stripped version is saved and
51
C<stripped_nonxml> is set on the object so callers can notify the user.
52
53
If the MARCXML cannot be recovered at all, a
54
I<Koha::Exceptions::Metadata::Invalid> exception is thrown.
55
48
=cut
56
=cut
49
57
50
sub store {
58
sub store {
Lines 62-81 sub store { Link Here
62
        my $marcxml_error = $@;
70
        my $marcxml_error = $@;
63
        chomp $marcxml_error;
71
        chomp $marcxml_error;
64
        unless ($marcxml) {
72
        unless ($marcxml) {
65
            warn $marcxml_error;
73
66
            Koha::Exceptions::Metadata::Invalid->throw(
74
            # Attempt recovery by stripping non-XML characters
67
                id             => $self->id,
75
            my $stripped_metadata = StripNonXmlChars( $self->metadata );
68
                biblionumber   => $self->biblionumber,
76
            my $stripped_marcxml  = eval { MARC::Record::new_from_xml( $stripped_metadata, 'UTF-8', $self->schema ); };
69
                format         => $self->format,
77
70
                schema         => $self->schema,
78
            if ($stripped_marcxml) {
71
                decoding_error => $marcxml_error,
79
72
            );
80
                # Stripping fixed it – save the clean version and record what happened
81
                Koha::Logger->get->warn(
82
                    sprintf(
83
                        "Non-XML characters stripped from bibliographic record (biblionumber=%s): %s",
84
                        $self->biblionumber // 'N/A', $marcxml_error
85
                    )
86
                );
87
                $self->metadata($stripped_metadata);
88
                $self->{_stripped_nonxml} = $marcxml_error;
89
            } else {
90
91
                # Truly unrecoverable
92
                Koha::Logger->get->warn($marcxml_error);
93
                Koha::Exceptions::Metadata::Invalid->throw(
94
                    id             => $self->id,
95
                    biblionumber   => $self->biblionumber,
96
                    format         => $self->format,
97
                    schema         => $self->schema,
98
                    decoding_error => $marcxml_error,
99
                );
100
            }
73
        }
101
        }
74
    }
102
    }
75
103
76
    return $self->SUPER::store;
104
    return $self->SUPER::store;
77
}
105
}
78
106
107
=head3 stripped_nonxml
108
109
    if ( my $error = $metadata->stripped_nonxml ) {
110
        # warn user that non-XML characters were removed on last store()
111
    }
112
113
Returns the original decode error string if non-XML characters were stripped
114
during the most recent call to C<store()>, or C<undef> otherwise.
115
116
=cut
117
118
sub stripped_nonxml {
119
    my $self = shift;
120
    return $self->{_stripped_nonxml};
121
}
122
79
=head3 record
123
=head3 record
80
124
81
my $record = $metadata->record;
125
my $record = $metadata->record;
(-)a/cataloguing/addbiblio.pl (-30 / +63 lines)
Lines 703-710 if ( $op eq "cud-addbiblio" ) { Link Here
703
    # it is not a duplicate (determined either by Koha itself or by user checking it's not a duplicate)
703
    # it is not a duplicate (determined either by Koha itself or by user checking it's not a duplicate)
704
    if ( !$duplicatebiblionumber or $confirm_not_duplicate ) {
704
    if ( !$duplicatebiblionumber or $confirm_not_duplicate ) {
705
        my $oldbibitemnum;
705
        my $oldbibitemnum;
706
        my @save_warnings;
706
        my $encode_error = try {
707
        my $encode_error = try {
707
            if ( $is_a_modif ) {
708
            if ($is_a_modif) {
708
                ModBiblio(
709
                ModBiblio(
709
                    $record,
710
                    $record,
710
                    $biblionumber,
711
                    $biblionumber,
Lines 714-724 if ( $op eq "cud-addbiblio" ) { Link Here
714
                            source       => $z3950 ? 'z3950' : 'intranet',
715
                            source       => $z3950 ? 'z3950' : 'intranet',
715
                            categorycode => $logged_in_patron->categorycode,
716
                            categorycode => $logged_in_patron->categorycode,
716
                            userid       => $logged_in_patron->userid,
717
                            userid       => $logged_in_patron->userid,
717
                        }
718
                        },
719
                        warnings => \@save_warnings,
718
                    }
720
                    }
719
                );
721
                );
720
            } else {
722
            } else {
721
                ( $biblionumber, $oldbibitemnum ) = AddBiblio( $record, $frameworkcode );
723
                ( $biblionumber, $oldbibitemnum ) =
724
                    AddBiblio( $record, $frameworkcode, { warnings => \@save_warnings } );
722
            }
725
            }
723
            return 0;
726
            return 0;
724
        } catch {
727
        } catch {
Lines 730-770 if ( $op eq "cud-addbiblio" ) { Link Here
730
            }
733
            }
731
            return 1;
734
            return 1;
732
        };
735
        };
733
        if (!$encode_error && ($redirect eq "items" || ($mode ne "popup" && !$is_a_modif && $redirect ne "view" && $redirect ne "just_save"))){
736
734
            if ($frameworkcode eq 'FA'){
737
        my ($nonxml_stripped) = grep { $_->{type} eq 'nonxml_stripped' } @save_warnings;
735
            print $input->redirect(
738
        if ( $nonxml_stripped && !$encode_error ) {
736
                '/cgi-bin/koha/cataloguing/additem.pl?'
739
            $record = Koha::Biblios->find($biblionumber)->metadata->record;
737
                .'biblionumber='.$biblionumber
740
            build_tabs( $template, $record, $dbh, $encoding, $input );
738
                .'&frameworkcode='.$frameworkcode
741
            $template->param(
739
                .'&circborrowernumber='.$fa_circborrowernumber
742
                biblionumber    => $biblionumber,
740
                .'&branch='.$fa_branch
743
                popup           => $mode,
741
                .'&barcode='.uri_escape_utf8($fa_barcode)
744
                frameworkcode   => $frameworkcode,
742
                .'&stickyduedate='.$fa_stickyduedate
745
                itemtype        => $frameworkcode,
743
                .'&duedatespec='.$fa_duedatespec
746
                borrowernumber  => $loggedinuser,
747
                tab             => scalar $input->param('tab'),
748
                NONXML_STRIPPED => $nonxml_stripped->{message},
744
            );
749
            );
750
            output_html_with_http_headers $input, $cookie, $template->output;
745
            exit;
751
            exit;
746
            }
752
        }
747
            else {
753
        if (
748
            print $input->redirect(
754
            !$encode_error
755
            && ( $redirect eq "items"
756
                || ( $mode ne "popup" && !$is_a_modif && $redirect ne "view" && $redirect ne "just_save" ) )
757
            )
758
        {
759
            if ( $frameworkcode eq 'FA' ) {
760
                print $input->redirect( '/cgi-bin/koha/cataloguing/additem.pl?'
761
                        . 'biblionumber='
762
                        . $biblionumber
763
                        . '&frameworkcode='
764
                        . $frameworkcode
765
                        . '&circborrowernumber='
766
                        . $fa_circborrowernumber
767
                        . '&branch='
768
                        . $fa_branch
769
                        . '&barcode='
770
                        . uri_escape_utf8($fa_barcode)
771
                        . '&stickyduedate='
772
                        . $fa_stickyduedate
773
                        . '&duedatespec='
774
                        . $fa_duedatespec );
775
                exit;
776
            } else {
777
                print $input->redirect(
749
                    "/cgi-bin/koha/cataloguing/additem.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode&searchid=$searchid"
778
                    "/cgi-bin/koha/cataloguing/additem.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode&searchid=$searchid"
750
            );
779
                );
751
            exit;
780
                exit;
752
            }
781
            }
753
        } elsif(!$encode_error && (($is_a_modif || $redirect eq "view") && $redirect ne "just_save")){
782
        } elsif ( !$encode_error && ( ( $is_a_modif || $redirect eq "view" ) && $redirect ne "just_save" ) ) {
754
            my $defaultview = C4::Context->preference('IntranetBiblioDefaultView');
783
            my $defaultview = C4::Context->preference('IntranetBiblioDefaultView');
755
            my $views = { C4::Search::enabled_staff_search_views };
784
            my $views       = {C4::Search::enabled_staff_search_views};
756
            if ($defaultview eq 'isbd' && $views->{can_view_ISBD}) {
785
            if ( $defaultview eq 'isbd' && $views->{can_view_ISBD} ) {
757
                print $input->redirect("/cgi-bin/koha/catalogue/ISBDdetail.pl?biblionumber=$biblionumber&searchid=$searchid");
786
                print $input->redirect(
758
            } elsif  ($defaultview eq 'marc' && $views->{can_view_MARC}) {
787
                    "/cgi-bin/koha/catalogue/ISBDdetail.pl?biblionumber=$biblionumber&searchid=$searchid");
759
                print $input->redirect("/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode&searchid=$searchid");
788
            } elsif ( $defaultview eq 'marc' && $views->{can_view_MARC} ) {
760
            } elsif  ($defaultview eq 'labeled_marc' && $views->{can_view_labeledMARC}) {
789
                print $input->redirect(
761
                print $input->redirect("/cgi-bin/koha/catalogue/labeledMARCdetail.pl?biblionumber=$biblionumber&searchid=$searchid");
790
                    "/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode&searchid=$searchid"
791
                );
792
            } elsif ( $defaultview eq 'labeled_marc' && $views->{can_view_labeledMARC} ) {
793
                print $input->redirect(
794
                    "/cgi-bin/koha/catalogue/labeledMARCdetail.pl?biblionumber=$biblionumber&searchid=$searchid");
762
            } else {
795
            } else {
763
                print $input->redirect("/cgi-bin/koha/catalogue/detail.pl?biblionumber=$biblionumber&searchid=$searchid");
796
                print $input->redirect(
797
                    "/cgi-bin/koha/catalogue/detail.pl?biblionumber=$biblionumber&searchid=$searchid");
764
            }
798
            }
765
            exit;
799
            exit;
766
        }
800
        } elsif ( !$encode_error && ( $redirect eq "just_save" ) ) {
767
        elsif ( !$encode_error && ( $redirect eq "just_save" ) ) {
768
            my $tab = $input->param('current_tab');
801
            my $tab = $input->param('current_tab');
769
            print $input->redirect(
802
            print $input->redirect(
770
                "/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=$biblionumber&framework=$frameworkcode&tab=$tab&searchid=$searchid"
803
                "/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=$biblionumber&framework=$frameworkcode&tab=$tab&searchid=$searchid"
(-)a/cataloguing/merge.pl (-2 / +6 lines)
Lines 92-99 if ( $op eq 'cud-merge' ) { Link Here
92
92
93
    # Modifying the reference record
93
    # Modifying the reference record
94
    my $invalid_metadata;
94
    my $invalid_metadata;
95
    my @merge_warnings;
95
    my $modded = try {
96
    my $modded = try {
96
        ModBiblio($record, $ref_biblionumber, $frameworkcode);
97
        ModBiblio( $record, $ref_biblionumber, $frameworkcode, { warnings => \@merge_warnings } );
97
        return 1;
98
        return 1;
98
    } catch {
99
    } catch {
99
        my $exception = $_;
100
        my $exception = $_;
Lines 105-110 if ( $op eq 'cud-merge' ) { Link Here
105
        return 0;
106
        return 0;
106
    };
107
    };
107
108
109
    my ($nonxml_stripped) = grep { $_->{type} eq 'nonxml_stripped' } @merge_warnings;
110
108
    my $report_header = {};
111
    my $report_header = {};
109
    if ($modded) {
112
    if ($modded) {
110
        foreach my $biblionumber ( $ref_biblionumber, @biblionumbers ) {
113
        foreach my $biblionumber ( $ref_biblionumber, @biblionumbers ) {
Lines 159-165 if ( $op eq 'cud-merge' ) { Link Here
159
        result           => 1,
162
        result           => 1,
160
        report_records   => \@report_records,
163
        report_records   => \@report_records,
161
        report_header    => $report_header,
164
        report_header    => $report_header,
162
        ref_biblionumber => scalar $input->param('ref_biblionumber')
165
        ref_biblionumber => scalar $input->param('ref_biblionumber'),
166
        ( $nonxml_stripped ? ( NONXML_STRIPPED => $nonxml_stripped->{message} ) : () ),
163
    );
167
    );
164
168
165
    #-------------------------
169
    #-------------------------
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt (+12 lines)
Lines 805-810 Link Here
805
            <pre class="problem">[% INVALID_METADATA | html %]</pre>
805
            <pre class="problem">[% INVALID_METADATA | html %]</pre>
806
        </div>
806
        </div>
807
    [% END %]
807
    [% END %]
808
    [% IF ( NONXML_STRIPPED ) %]
809
        <div class="alert alert-warning">
810
            <h1>Record saved with modifications</h1>
811
            <p
812
                ><strong
813
                    >Non-XML characters were automatically stripped from this record before saving. The record has been saved successfully, but please review it to ensure the data is correct. If data appears to be missing or incorrect, edit
814
                    and re-save the record.</strong
815
                ></p
816
            >
817
            <pre class="problem">[% NONXML_STRIPPED | html %]</pre>
818
        </div>
819
    [% END %]
808
820
809
    [% IF marc21_fixlen %]
821
    [% IF marc21_fixlen %]
810
        <div class="alert alert-warning"><h1>WARNING</h1><p>The following fixed-length control field(s) have an invalid length: [% marc21_fixlen.failed.join(',') | html %]. Please fix.</p></div>
822
        <div class="alert alert-warning"><h1>WARNING</h1><p>The following fixed-length control field(s) have an invalid length: [% marc21_fixlen.failed.join(',') | html %]. Please fix.</p></div>
(-)a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt (-1 / +10 lines)
Lines 62-68 Link Here
62
                    [% IF error.code == 'CANNOT_MOVE' %]
62
                    [% IF error.code == 'CANNOT_MOVE' %]
63
                        The following items could not be moved from the old record to the new one: [% error.value | html %]
63
                        The following items could not be moved from the old record to the new one: [% error.value | html %]
64
                    [% ELSIF error.code == 'INVALID_METADATA' %]
64
                    [% ELSIF error.code == 'INVALID_METADATA' %]
65
                        <p>This record had encoding issues, non-xml characters have been stripped in order to allow editing. Please be cautious when saving that some data may have been removed from the record.</p>
65
                        <p>The merged record could not be saved because it contains characters that are not valid XML. The merge has been aborted.</p>
66
                        <pre class="problem">[% error.value | html %]</pre>
66
                        <pre class="problem">[% error.value | html %]</pre>
67
                    [% ELSE %]
67
                    [% ELSE %]
68
                        [% error | html %]
68
                        [% error | html %]
Lines 73-78 Link Here
73
            [% END %]
73
            [% END %]
74
        [% ELSE %]
74
        [% ELSE %]
75
            <div class="alert alert-info">The merge was successful. <a href="/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=[% ref_biblionumber | uri %]">View the merged record.</a></div>
75
            <div class="alert alert-info">The merge was successful. <a href="/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=[% ref_biblionumber | uri %]">View the merged record.</a></div>
76
            [% IF NONXML_STRIPPED %]
77
                <div class="alert alert-warning">
78
                    <strong
79
                        >Non-XML characters were automatically stripped from the merged record before saving. Please <a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=[% ref_biblionumber | uri %]">review the record</a> to ensure
80
                        the data is correct.</strong
81
                    >
82
                    <pre class="problem">[% NONXML_STRIPPED | html %]</pre>
83
                </div>
84
            [% END %]
76
            <div class="page-section">
85
            <div class="page-section">
77
                <h3>Report</h3>
86
                <h3>Report</h3>
78
                <table>
87
                <table>
(-)a/t/db_dependent/Koha/Biblio/Metadata.t (-21 / +26 lines)
Lines 18-24 Link Here
18
use Modern::Perl;
18
use Modern::Perl;
19
19
20
use Test::NoWarnings;
20
use Test::NoWarnings;
21
use Test::More tests => 6;
21
use Test::More tests => 7;
22
use Test::Exception;
22
use Test::Exception;
23
use Test::MockModule;
23
use Test::MockModule;
24
use Test::Warn;
24
use Test::Warn;
Lines 28-33 use t::lib::Mocks; Link Here
28
28
29
use C4::Biblio qw( AddBiblio );
29
use C4::Biblio qw( AddBiblio );
30
use Koha::Database;
30
use Koha::Database;
31
use MARC::Record;
32
use MARC::Field;
31
33
32
BEGIN {
34
BEGIN {
33
    use_ok('Koha::Biblio::Metadatas');
35
    use_ok('Koha::Biblio::Metadatas');
Lines 73-112 EOX Link Here
73
75
74
        lives_ok { $record->store }
76
        lives_ok { $record->store }
75
        'Valid MARCXML record stores successfully';
77
        'Valid MARCXML record stores successfully';
78
        is( $record->stripped_nonxml, undef, 'stripped_nonxml is undef for clean record' );
76
79
77
        $schema->storage->txn_rollback;
80
        $schema->storage->txn_rollback;
78
    };
81
    };
79
82
80
    subtest 'Invalid MARCXML handling' => sub {
83
    subtest 'MARCXML with strippable non-XML characters' => sub {
84
        plan tests => 3;
81
        $schema->storage->txn_begin;
85
        $schema->storage->txn_begin;
82
        my $invalid_marcxml = <<'EOX';
86
83
<?xml version="1.0" encoding="UTF-8"?>
87
        # Build MARCXML containing a non-XML control character (chr(31) = Unit Separator)
84
<record xmlns="http://www.loc.gov/MARC21/slim">
88
        my $bad_title   = 'Title with' . chr(31) . ' bad character';
85
  <leader>00925nam a22002411a 4500</leader>
89
        my $clean_title = 'Title with bad character';
86
  <controlfield tag="001">1234567</controlfield>
90
87
  <datafield tag="245" ind1="1" ind2="0">
91
        # Generate the MARCXML string with the embedded bad character
88
    <subfield code="a">This string will  break your record</subfield>
92
        my $marc_record = MARC::Record->new;
89
  </datafield>
93
        $marc_record->append_fields( MARC::Field->new( '245', '', '', 'a' => $bad_title ) );
90
  <!-- Invalid XML structure -->
94
        my $marcxml_with_bad_char = $marc_record->as_xml_record('MARC21');
91
</record>
92
EOX
93
95
94
        my $record = Koha::Biblio::Metadata->new(
96
        my $record = Koha::Biblio::Metadata->new(
95
            {
97
            {
96
                format       => 'marcxml',
98
                format       => 'marcxml',
97
                metadata     => $invalid_marcxml,
99
                metadata     => $marcxml_with_bad_char,
98
                biblionumber => $biblio->{biblionumber},
100
                biblionumber => $biblio->{biblionumber},
99
                schema       => 'MARC21',
101
                schema       => 'MARC21',
100
            }
102
            }
101
        );
103
        );
102
104
103
        throws_ok { $record->store }
105
        lives_ok { $record->store } 'MARCXML with strippable characters stores successfully';
104
        'Koha::Exceptions::Metadata::Invalid',
106
105
            'Invalid MARCXML throws expected exception';
107
        ok( $record->stripped_nonxml, 'stripped_nonxml is set after stripping' );
108
109
        my $stored = Koha::Biblio::Metadatas->find( { biblionumber => $biblio->{biblionumber}, format => 'marcxml' } );
110
        is(
111
            $stored->record->field('245')->subfield('a'),
112
            $clean_title,
113
            'Stored record has control character removed'
114
        );
106
115
107
        my $thrown = $@;
108
        ok( $thrown->decoding_error, 'Exception contains decoding error message' );
109
        is( $thrown->biblionumber, $biblio->{biblionumber}, 'Exception contains correct biblionumber' );
110
        $schema->storage->txn_rollback;
116
        $schema->storage->txn_rollback;
111
    };
117
    };
112
118
113
- 

Return to bug 35104