Description
Kyle M Hall (khall)
2016-08-09 13:07:17 UTC
Created attachment 54202 [details] [review] Bug 17088 - Bad MARC XML can halt export_records.pl If export_records.pl encounters a record that fails to load correctly as a MARC::Record the export process will fail and only the records encountered up to that point will be exported. Test Plan: 1) Pick a record in your database, and replace the biblioitems.marcxml field with some random characters 2) Export your records using the command: misc/export_records.pl --format xml --record-type bibs --filename /tmp/biblios.xml 3) Note you get an error and the script ends prematurely 4) Apply this patch 5) Repeat step 2 6) Note the full record set is exported ( except for your bad record ) 7) Note an error is still generated for your bad record ( useful for tracking down bad records ). Testing this now. Created attachment 55609 [details] [review] Bug 17088 - Bad MARC XML can halt export_records.pl If export_records.pl encounters a record that fails to load correctly as a MARC::Record the export process will fail and only the records encountered up to that point will be exported. Test Plan: 1) Pick a record in your database, and replace the biblioitems.marcxml field with some random characters 2) Export your records using the command: misc/export_records.pl --format xml --record-type bibs --filename /tmp/biblios.xml 3) Note you get an error and the script ends prematurely 4) Apply this patch 5) Repeat step 2 6) Note the full record set is exported ( except for your bad record ) 7) Note an error is still generated for your bad record ( useful for tracking down bad records ). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 55610 [details] [review] Bug 17088: [Follow-up] Adjust POD in Logger.pm Not important enough to go on its own. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 55611 [details] [review] Bug 17088: [Follow-up] Use Logger for failed exports Fixes a TODO for logging unsupported record_type in _get_record_for_export. Also logs a warning when the record_type parameter is not supplied at all in sub export. Replaces a warn by a log message about an invalid record for format iso2709. Also adds a log message about an invalid record for format xml. Adds a trivial unit test for passing no record_type to export. Test plan: See also first patch. Run t/db_dependent/Exporter/Record.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Also tested the log messages for iso2709 and xml by manipulating the record_type parameter with: $params->{record_type}='xx'; before calling _get_record_for_export in Record.pm. Kyle: Added follow-up for logging. If you prefer to let them go on their own, feel free to move them. They are quite simple however. Created attachment 56032 [details] [review] Bug 17088 - Bad MARC XML can halt export_records.pl If export_records.pl encounters a record that fails to load correctly as a MARC::Record the export process will fail and only the records encountered up to that point will be exported. Test Plan: 1) Pick a record in your database, and replace the biblioitems.marcxml field with some random characters 2) Export your records using the command: misc/export_records.pl --format xml --record-type bibs --filename /tmp/biblios.xml 3) Note you get an error and the script ends prematurely 4) Apply this patch 5) Repeat step 2 6) Note the full record set is exported ( except for your bad record ) 7) Note an error is still generated for your bad record ( useful for tracking down bad records ). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 56033 [details] [review] Bug 17088: [Follow-up] Adjust POD in Logger.pm Not important enough to go on its own. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 56034 [details] [review] Bug 17088: [Follow-up] Use Logger for failed exports Fixes a TODO for logging unsupported record_type in _get_record_for_export. Also logs a warning when the record_type parameter is not supplied at all in sub export. Replaces a warn by a log message about an invalid record for format iso2709. Also adds a log message about an invalid record for format xml. Adds a trivial unit test for passing no record_type to export. Test plan: See also first patch. Run t/db_dependent/Exporter/Record.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Also tested the log messages for iso2709 and xml by manipulating the record_type parameter with: $params->{record_type}='xx'; before calling _get_record_for_export in Record.pm. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 56035 [details] [review] Bug 17088: Add tests to cover the changes Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to Master - Should be in the November Release 16.11 - Thanks! Pushed in 16.05. Will be in 16.05.05. Pushed to 3.22.x, will be in 3.22.12 |