Bug 13705

Summary: Move indexation subs in C4::ZebraIndex
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Architecture, internals, and plumbingAssignee: Alex Arnaud <alex.arnaud>
Status: Patch doesn't apply --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: alex.arnaud, bgkriegel, chris, jonathan.druart, katrin.fischer, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 13706    
Attachments: Bug 13705: Move indexation subs in C4::ZebraIndex
Bug 13705: Prevent a warning in _fix_unimarc_100
Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm
Bug 13705 - Fix QA comments
[SIGNED_OFF] Bug 13705: Move indexation subs in C4::ZebraIndex
[SIGNED_OFF] Bug 13705: Prevent a warning in _fix_unimarc_100
[SIGNED-OFF] Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm
[SIGNED-OFF] Bug 13705 - Fix QA comments

Description Julian Maurice 2015-02-13 08:32:41 UTC
Move indexation-related subs from rebuild_zebra.pl to new module C4::ZebraIndex and create two new subs IndexRecord and DeleteRecordIndex
    
This should allow easier indexation from scripts other than rebuild_zebra.pl
Comment 1 Julian Maurice 2015-02-13 08:33:43 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2015-02-13 08:33:46 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2015-02-13 08:35:16 UTC
To test, run rebuild_zebra.pl trying the different options. The behaviour should remain identical.
Comment 4 Bernardo Gonzalez Kriegel 2015-05-02 01:38:51 UTC
We still add things on C4?
Just a question

1) License need to be updated to GPL v3
2) koha-qa complains

 FAIL   C4/ZebraIndex.pm
   OK     pod
   OK     forbidden patterns
   OK     critic
   FAIL   valid
                "my" variable $string masks earlier declaration in same scope
Comment 5 Alex Arnaud 2015-06-10 12:44:55 UTC Comment hidden (obsolete)
Comment 6 Alex Arnaud 2015-06-10 12:44:59 UTC Comment hidden (obsolete)
Comment 7 Alex Arnaud 2015-06-10 12:48:05 UTC
The 2 last patch are follow up fixing licence statement and a qa warning.
Comment 8 Bernardo Gonzalez Kriegel 2016-04-04 19:04:11 UTC
Hi, new errors 

 FAIL   C4/ZebraIndex.pm
   OK     critic
   OK     forbidden patterns
   OK     pod
   OK     spelling
                0
   FAIL   valid
                Name "Cache::RemovalStrategy::FIELDS" used only once: possible typo
                Name "Cache::RemovalStrategy::LRU::FIELDS" used only once: possible typo
                Name "Tie::Hash::FIELDS" used only once: possible typo
Comment 9 Katrin Fischer 2016-04-04 20:39:36 UTC
I think you can continue - this should be fixed by another patch that passed QA recently.
Comment 10 Bernardo Gonzalez Kriegel 2016-04-05 01:44:16 UTC
Created attachment 49888 [details] [review]
[SIGNED_OFF] Bug 13705: Move indexation subs in C4::ZebraIndex

Move indexation-related subs from rebuild_zebra.pl to new module
C4::ZebraIndex and create two new subs IndexRecord and DeleteRecordIndex

This should allow easier indexation from scripts other than
rebuild_zebra.pl

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 11 Bernardo Gonzalez Kriegel 2016-04-05 01:44:24 UTC
Created attachment 49889 [details] [review]
[SIGNED_OFF] Bug 13705: Prevent a warning in _fix_unimarc_100

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 12 Bernardo Gonzalez Kriegel 2016-04-05 01:44:31 UTC
Created attachment 49890 [details] [review]
[SIGNED-OFF] Bug 13705 - Update copyright and GPL Licence statement in C4/ZebraIndex.pm

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 13 Bernardo Gonzalez Kriegel 2016-04-05 01:44:40 UTC
Created attachment 49891 [details] [review]
[SIGNED-OFF] Bug 13705 - Fix QA comments

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Tested using options -v -a -b -x -r -w -z --length --offset -nosanitize
No problems found, same results as before.
Output slightly different:

Old)
====================
exporting authority
====================
301....................................................................................................
Records exported: 389
====================
REINDEXING zebra
====================

New)
====================
REINDEXING zebra
====================
....................................................................................................100
....................................................................................................200
....................................................................................................300
.........................................................................................
389 records exported to /tmp/PBlyusIn1Z/exported_records
====================
REINDEXING zebra
===================

No errors save for those on comment #8
Comment 14 Jonathan Druart 2016-04-14 06:57:31 UTC
I would be great to see some tests to cover this new module (even if it's not new code).
Comment 15 Katrin Fischer 2016-04-14 09:38:57 UTC
Just a general question: why C4 and not Koha?
Comment 16 Julian Maurice 2016-04-14 10:10:13 UTC
(In reply to Katrin Fischer from comment #15)
> Just a general question: why C4 and not Koha?
The first version of this patch was written in 2012 (for bug 7419), that's why :)
Should we move the code to Koha::ZebraIndex ?
Comment 17 Julian Maurice 2016-04-14 10:18:56 UTC
(In reply to Jonathan Druart from comment #14)
> I would be great to see some tests to cover this new module (even if it's
> not new code).
How can we test code that should modify the Zebra index, without really modifying it ? Should we create another Zebra instance for tests purpose ?
Comment 18 Jonathan Druart 2016-04-15 07:17:51 UTC
(In reply to Julian Maurice from comment #17)
> (In reply to Jonathan Druart from comment #14)
> > I would be great to see some tests to cover this new module (even if it's
> > not new code).
> How can we test code that should modify the Zebra index, without really
> modifying it ? Should we create another Zebra instance for tests purpose ?

We already do that in t/db_dependent/www/search_utf8.t and we do not care about not modifying the Zebra index.