Bug 9496 - rebuild_zebra.pl should handle invalid XML records better
Summary: rebuild_zebra.pl should handle invalid XML records better
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Paul Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-26 00:43 UTC by Galen Charlton
Modified: 2014-05-26 21:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
bug 9496: improve error checking in rebuild_zebra.pl (3.91 KB, patch)
2013-01-26 00:50 UTC, Galen Charlton
Details | Diff | Splinter Review
Signed off patch bug 9496 (3.98 KB, patch)
2013-01-28 15:16 UTC, Larry Baerveldt
Details | Diff | Splinter Review
bug 9496: improve error checking in rebuild_zebra.pl (4.06 KB, patch)
2013-03-20 00:22 UTC, Mason James
Details | Diff | Splinter Review
[PASSED QA] bug 9496: improve error checking in rebuild_zebra.pl (4.08 KB, patch)
2013-03-20 13:22 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2013-01-26 00:43:46 UTC
If a bib or authority MARCXML record is not well-formed, rebuild_zebra.pl should catch it so that invalid XML is not included in the file it passes to zebraidx.  This is particularly important when DOM mode indexing is used.  This is because in DOM mode zebraidx is processing a single XML document (namely, the MARCXML import), and if it reaches a record that causes the XML file to be invalid, it will not handle any records past that point.  Worse, it doesn't complain quite loudly enough when that happens, so a user could find their Koha catalog to be partially indexed for no apparent reason.

In addition, when rebuild_zebra is run using the -nosanitize option, zebraidx in DOM mode will refuse to process the output because there are extraneous XML declarations.
Comment 1 Galen Charlton 2013-01-26 00:50:03 UTC Comment hidden (obsolete)
Comment 2 Larry Baerveldt 2013-01-28 15:15:31 UTC
Patch passed testing. Signing off.
--Larry Baerveldt
Comment 3 Larry Baerveldt 2013-01-28 15:16:20 UTC Comment hidden (obsolete)
Comment 4 Mason James 2013-03-20 00:22:12 UTC Comment hidden (obsolete)
Comment 5 Mason James 2013-03-20 00:25:28 UTC
> Signed-off-by: Mason James <mtj@kohaaloha.com>

passing QA, patch tested and works well

testing 1 commit(s) (applied to 2f3cf47 'Merge branch 'bug_8979' into 3.12-mas')
Comment 6 Paul Poulain 2013-03-20 13:22:33 UTC
Created attachment 16514 [details] [review]
[PASSED QA] bug 9496: improve error checking in rebuild_zebra.pl

When using rebuild_zebra to index all records, skip over
bibliographic or authority records that don't come out
as valid XML.  Also, strip extraneous XML declarations when
using --nosanitize.

Test plans
----------
Note that both plans assume that DOM indexing is turned on.

Test plan #1
============

[1] Run rebuild_zebra.pl with the -x -nosanitize options.  Without
    the patch, zebraidx should terminate early and complain
    about invalid XML.
[2] With the patch, the rebuild_zebra.pl should work without
    error.

Test plan #2
============
[1] Intentionally make a MARCXML record invalid, e.g, by running
    the following SQL:

    UPDATE bilbioitems SET marcxml = CONCATENATE(marcxml, 'junk')
    WHERE biblionumber = 123;

[2] Run rebuild_zebra.pl -b -x -r
[3] Without the patch, only part of the database will be indexed.
[4] With the patch, rebuild_zebra.pl will not export the bad
    record and will give an error message saying so, but will
    successfully index the rest of the records.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Larry Baerveldt <larry@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 7 Paul Poulain 2013-03-20 13:23:01 UTC
QA comment:
 * small patch, passes koha-qa.pl
 * i've checked it does not break GRS indexeing
 * looks good & sound logical
 * I don't have a DOM test database, so I can't test this patch working as advertised BUT : patch waiting for more than 2 months, and trusting author, so passed QA
Comment 8 Paul Poulain 2013-03-20 13:23:39 UTC
mason, seems we passed QA this patch almost at the same time !
Comment 9 Jared Camins-Esakov 2013-03-22 02:25:24 UTC
This patch has been pushed to master.
Comment 10 Chris Cormack 2013-03-29 07:20:15 UTC
Pushed to 3.10.x will be in 3.10.5