Bug 16505

Summary: rebuild_zebra.pl skips updates if -x is passed
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: dcook, gmcharlt, jonathan.druart, julian.maurice, kyle
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 16506    
Attachments: Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed
[SIGNED-OFF] Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed
Bug 16505: Make sure $as_xml will not be used later

Description Tomás Cohen Arazi (tcohen) 2016-05-12 13:20:16 UTC

    
Comment 1 Tomás Cohen Arazi (tcohen) 2016-05-13 15:10:26 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-05-13 20:08:16 UTC
Minor: use q{} instead of qq{} if you do not need to interpolate variables.
Comment 3 Tomás Cohen Arazi (tcohen) 2016-05-13 22:23:23 UTC
(In reply to Jonathan Druart from comment #2)
> Minor: use q{} instead of qq{} if you do not need to interpolate variables.

Heh, yes. That's a leftover from the first approach, that changed things a bit more, so I delayed it for an enhancement bug. Good catch
Comment 4 Tomás Cohen Arazi (tcohen) 2016-05-16 13:19:39 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2016-05-16 16:06:32 UTC Comment hidden (obsolete)
Comment 6 David Cook 2016-05-17 04:18:24 UTC
When did this issue appear? I think rebuild_zebra.pl is working fine in 3.20.3?
Comment 7 Jonathan Druart 2016-05-21 08:43:04 UTC
Created attachment 51674 [details] [review]
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed

Using rebuild_zerba.pl with the -x option switch, produces an incorrect output in
terms of what our XSLTs expect for indexing. This patch introduces the right namespace information
on the exported records so indexing succeeds.

To test:
- On current master, have some records on your db
- Run:
  $ sudo koha-shell kohadev
  $ cd kohaclone
  $ misc/migration_tools/rebuild_zebra.pl -r -b -k -x
=> you will get a message like this:

NOTHING cleaned : the export /tmp/NL5ufjUfpp has been kept.

- Run
  $ less /tmp/NL5ufjUfpp/biblio/exported_records
=> FAIL: The first line looks like this

<?xml version="1.0" encoding="UTF-8"?><collection><record

- Now run:
  $ xsltproc \
     /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl \
     /tmp/NL5ufjUfpp/biblio/exported_records
=> FAIL: No output
- Apply the patch
- Run:
  $ misc/migration_tools/rebuild_zebra.pl -r -b -k -x
- Take a look at the result file:
  $ less /tmp/asdiouqwiue/biblio/exported_records
=> SUCCESS: The start of the file looks like this:
<?xml version="1.0" encoding="UTF-8"?><collection xmlns="http://www.loc.gov/MARC21/slim">

- Run:
  $ xsltproc \
     /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl \
     /tmp/asdiouqwiue/biblio/exported_records
=> SUCCESS: There is actually indexing data :-D
- Sign off :-D

Edit: I changed qq{} for q{} as suggested by Jonathan.

Sponsored-by: American Numismatic Society

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works as described following test plan
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2016-05-21 09:24:21 UTC
Created attachment 51679 [details] [review]
Bug 16505: Make sure $as_xml will not be used later
Comment 9 Kyle M Hall (khall) 2016-05-23 17:14:40 UTC
Pushed to master for 16.05, thanks Tomas!

NOTE: Only the first patch was pushed. The second warning patch no longer applies. An updated patch should probably be pushed.
Comment 10 Kyle M Hall (khall) 2016-05-23 17:30:12 UTC
Second patch has now been pushed!
Comment 11 Julian Maurice 2016-05-25 08:54:59 UTC
This is weird, bug 16506 depends on the first patch of this bug, but the 2nd patch of this bug depends on bug 16506. They have been pushed to master in this order: 16505 (1st) -> 16506 (all) -> 16505 (2nd).
I think the 2nd patch here belongs to bug 16506.
Comment 12 Julian Maurice 2016-05-25 08:55:59 UTC
First patch pushed to 3.22.x, will be in 3.22.7
Comment 13 Tomás Cohen Arazi (tcohen) 2016-05-25 13:41:19 UTC
(In reply to Julian Maurice from comment #11)
> This is weird, bug 16506 depends on the first patch of this bug, but the 2nd
> patch of this bug depends on bug 16506. They have been pushed to master in
> this order: 16505 (1st) -> 16506 (all) -> 16505 (2nd).
> I think the 2nd patch here belongs to bug 16506.

I take the blame for the circular bug dependency :-D
And you are right about Jonathan's followup.