[Bug 43028] New: Isolate one undecodable record from aborting a batch import or revert
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 Bug ID: 43028 Summary: Isolate one undecodable record from aborting a batch import or revert Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Tools Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org Depends on: 35104 Target Milestone: --- Bug 35104 makes Koha::Biblio::Metadata->store() throw Koha::Exceptions::Metadata::Invalid for MARCXML it cannot save. Batch import (C4::ImportBatch::BatchCommitRecords/_batchCommitItems), batch revert (BatchRevertRecords), the corresponding background jobs, the CLI tools built on them (misc/commit_file.pl, misc/migration_tools/bulkmarcimport.pl), and Koha::Import::Record::get_marc_record all needed updating so one bad record doesn't take the rest of the batch down with it. This patch: - Catches the exception per-record in BatchCommitRecords (which already commits per-record) and per-item in _batchCommitItems, flagging the record/item with an 'error' status and message and counting it via a new $num_errored return value, instead of letting it propagate. - Does the same for BatchRevertRecords, which runs as a single whole-batch transaction: both the marcxml_old decode step and the ModBiblio/ModAuthority call that follows it are isolated, so one record that can't be restored doesn't roll back every other record already reverted in the same batch. - Threads the new error counts through Koha::BackgroundJob::MARCImportCommitBatch/RevertBatch's job report, and counts errored records towards job progress so a batch containing them doesn't get stuck showing as still running. - Reports the new counts in misc/commit_file.pl's CLI summary and logs a warning line for records automatically repaired during bulkmarcimport.pl. - Surfaces each errored row's reason as a tooltip on its "Error" status badge in the manage-marc-import listing. - Makes Koha::Import::Record::get_marc_record reuse Koha::Biblio::Metadata::repair_marcxml (bug 35104) instead of its own weaker strip-and-retry, so staged import records get the same recovery (including empty-datafield handling) as the interactive cataloguing form. Depends on bug 35104. Test plan: 1. Apply this patch on top of bug 35104. 2. Stage a MARC file for import (Tools > Stage MARC records for import) containing at least one deliberately malformed record (e.g. a datafield with no subfields, or invalid control characters that cannot be repaired) alongside normal records. 3. From Manage staged records, commit the batch. Confirm: the bad record is flagged with an 'error' status and a tooltip explaining why; every other record in the batch imports successfully; the job report/CLI summary shows a "Number of records with errors" count. 4. Revert the same batch. Confirm one record whose marcxml_old cannot be restored does not prevent the rest of the batch from reverting, and the job report reflects the error count. 5. Run misc/migration_tools/bulkmarcimport.pl from the CLI against a file containing a record with a recoverable fault (e.g. a non-XML character). Confirm a WARNING line is logged for that record and the import otherwise completes normally. 6. prove t/db_dependent/ImportBatch.t t/db_dependent/Koha/BackgroundJob/MARCImportCommitBatch.t Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35104 [Bug 35104] We should warn when attempting to save MARC records that contain characters invalid in XML -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201705 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201705&action=edit Bug 43028: Isolate one undecodable record from aborting a batch import or revert BatchCommitRecords already ran each record in its own transaction; extend that to catch Koha::Exceptions::Metadata::Invalid (bug 35104) per-record instead of letting it propagate, flagging the record with an 'error' status and import_error message and counting it via the new $num_errored return value, rather than aborting records after it in the same batch. _batchCommitItems gets the same per-item treatment for undecodable item MARCXML. BatchRevertRecords runs as a single whole-batch transaction, so the same care is needed in its 'restore' branch too: both the marcxml_old decode step and the ModBiblio/ModAuthority call that follows it are isolated, so one record that can't be restored doesn't roll back every other record already reverted in the same batch. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201706 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201706&action=edit Bug 43028: Report errored records in the batch import/revert background job summary Thread BatchCommitRecords' new num_errored count and BatchRevertRecords' num_errors through the background job report, and count errored records towards job progress/completion - otherwise a batch containing errored records would never reach its expected progress total and the job would appear stuck at "running". Show the new count in the commit batch report template. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201707&action=edit Bug 43028: Reuse Koha::Biblio::Metadata's MARCXML repair in Import::Record get_marc_record() previously reimplemented its own bare strip-and-retry fallback, independent of Koha::Biblio::Metadata's recovery (bug 35104). That meant staged import records didn't benefit from the empty-datafield recovery added alongside the non-XML stripping, and the two implementations could silently drift apart. Use the shared repair_marcxml() instead. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201708&action=edit Bug 43028: Show and report undecodable records in remaining batch import tooling - misc/commit_file.pl: report the new num_errored count from BatchCommitRecords in its CLI summary. - misc/migration_tools/bulkmarcimport.pl: log a warning line (and to the logfile, if enabled) for every record automatically repaired during a bulk import, via the same warnings option used elsewhere - consistent with this script's existing warn-to-stderr convention. - tools/batch_records_ajax.pl / manage-marc-import.tt: surface each errored row's import_error as a tooltip on its "Error" status badge in the manage-marc-import listing, instead of just the bare status. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201705|0 |1 is obsolete| | Attachment #201706|0 |1 is obsolete| | Attachment #201707|0 |1 is obsolete| | Attachment #201708|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201731 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201731&action=edit Bug 43028: Isolate one undecodable record from aborting a batch import or revert BatchCommitRecords already ran each record in its own transaction; extend that to catch Koha::Exceptions::Metadata::Invalid (bug 35104) per-record instead of letting it propagate, flagging the record with an 'error' status and import_error message and counting it via the new $num_errored return value, rather than aborting records after it in the same batch. _batchCommitItems gets the same per-item treatment for undecodable item MARCXML. BatchRevertRecords runs as a single whole-batch transaction, so the same care is needed in its 'restore' branch too: both the marcxml_old decode step and the ModBiblio/ModAuthority call that follows it are isolated, so one record that can't be restored doesn't roll back every other record already reverted in the same batch. Signed-off-by: Baptiste Bayche <baptiste.bayche@inlibro.com> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Sponsored-by: OpenFifth -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201732&action=edit Bug 43028: Report errored records in the batch import/revert background job summary Thread BatchCommitRecords' new num_errored count and BatchRevertRecords' num_errors through the background job report, and count errored records towards job progress/completion - otherwise a batch containing errored records would never reach its expected progress total and the job would appear stuck at "running". Show the new count in the commit batch report template. Signed-off-by: Baptiste Bayche <baptiste.bayche@inlibro.com> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Sponsored-by: OpenFifth -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201733 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201733&action=edit Bug 43028: Reuse Koha::Biblio::Metadata's MARCXML repair in Import::Record get_marc_record() previously reimplemented its own bare strip-and-retry fallback, independent of Koha::Biblio::Metadata's recovery (bug 35104). That meant staged import records didn't benefit from the empty-datafield recovery added alongside the non-XML stripping, and the two implementations could silently drift apart. Use the shared repair_marcxml() instead. Signed-off-by: Baptiste Bayche <baptiste.bayche@inlibro.com> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Sponsored-by: OpenFifth -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201734 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201734&action=edit Bug 43028: Show and report undecodable records in remaining batch import tooling - misc/commit_file.pl: report the new num_errored count from BatchCommitRecords in its CLI summary. - misc/migration_tools/bulkmarcimport.pl: log a warning line (and to the logfile, if enabled) for every record automatically repaired during a bulk import, via the same warnings option used elsewhere - consistent with this script's existing warn-to-stderr convention. - tools/batch_records_ajax.pl / manage-marc-import.tt: surface each errored row's import_error as a tooltip on its "Error" status badge in the manage-marc-import listing, instead of just the bare status. Signed-off-by: Baptiste Bayche <baptiste.bayche@inlibro.com> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Sponsored-by: OpenFifth -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43028 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org