Bug 25342

Summary: Scripts not running under plack can cause duplication of ES records
Product: Koha Reporter: Nick Clemens <nick>
Component: Searching - ElasticsearchAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact:
Severity: major    
Priority: P5 - low CC: 1joynelson, andrewfh, aspencatteam, ephetteplace, glasklas, jonathan.druart, lucas, martin.renvoize, severine.queune
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23986
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24823
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06, 19.05.11
Attachments: Bug 25342: Force ES id as string
Bug 25342: Force ES id as string
Bug 25342: Unit test
Bug 25342: Force ES id as string
Bug 25342: Unit test

Description Nick Clemens 2020-05-01 14:48:43 UTC
We received reports of duplicated records after batch modifications.

It seems that when passing the ID to elasticsearch it is parsed as a string under Plack, but is parsed as a number under CGI
Comment 1 Nick Clemens 2020-05-01 14:55:09 UTC
Created attachment 104108 [details] [review]
Bug 25342: Force ES id as string

To test:
1 - Be using ES
2 - Find a unique record by searchign for title or seomthing
3 - Edit the record
4 - Repeat search and confirm you are returned to the details page
5 - Go to Tools->MARC modification templates
6 - Add a template and define an action
7 - Go to Tools->Batch record modification
8 - Select Enter a list of record numbers
9 - Enter the biblionumber of the record you searched for
10 - Use the marc modification template you setup
11 - Complete the modification
12 - Repeat your search
13 - note you get two results, both pointing to same biblionumber
14 - Check the ES index, note that you have two entries, one with the biblionumber, and the other with #.0, e.g.:
    curl -XGET 'es:9200/koha_kohadev_biblios/data/14.0?pretty'
    curl -XGET 'es:9200/koha_kohadev_biblios/data/14?pretty'
15 - Apply patch
16 - Delete and rebuild ES index
    perl misc/search_tools/rebuild_elasticsearch.pl -d -b
17 - Repeat 2-12
18 - No duplication this time
Comment 2 David Nind 2020-05-02 06:59:10 UTC
Created attachment 104175 [details] [review]
Bug 25342: Force ES id as string

To test:
1 - Be using ES
2 - Find a unique record by searchign for title or seomthing
3 - Edit the record
4 - Repeat search and confirm you are returned to the details page
5 - Go to Tools->MARC modification templates
6 - Add a template and define an action
7 - Go to Tools->Batch record modification
8 - Select Enter a list of record numbers
9 - Enter the biblionumber of the record you searched for
10 - Use the marc modification template you setup
11 - Complete the modification
12 - Repeat your search
13 - note you get two results, both pointing to same biblionumber
14 - Check the ES index, note that you have two entries, one with the biblionumber, and the other with #.0, e.g.:
    curl -XGET 'es:9200/koha_kohadev_biblios/data/14.0?pretty'
    curl -XGET 'es:9200/koha_kohadev_biblios/data/14?pretty'
15 - Apply patch
16 - Delete and rebuild ES index
    perl misc/search_tools/rebuild_elasticsearch.pl -d -b
17 - Repeat 2-12
18 - No duplication this time

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Jonathan Druart 2020-05-04 09:10:57 UTC
Nick, this sounds familiar to me, did not we already get that kind of error with ES (int that needed to be stringified).

Still no chance to get tests?
Comment 4 David Gustafsson 2020-05-04 11:52:18 UTC Comment hidden (obsolete)
Comment 5 David Gustafsson 2020-05-04 11:53:01 UTC Comment hidden (obsolete)
Comment 6 David Gustafsson 2020-05-04 12:02:28 UTC Comment hidden (obsolete)
Comment 7 Nick Clemens 2020-05-07 13:08:32 UTC
Created attachment 104509 [details] [review]
Bug 25342: Unit test
Comment 8 Nick Clemens 2020-05-07 13:09:33 UTC
(In reply to Jonathan Druart from comment #3)
> Nick, this sounds familiar to me, did not we already get that kind of error
> with ES (int that needed to be stringified).
> 
> Still no chance to get tests?

The test is likely to pass before this patch as well, the issue is in the sending of the request somewhere, but this proves that at least stringifying it within Koha still gives what we expect
Comment 9 Jonathan Druart 2020-05-11 09:58:39 UTC
(In reply to Nick Clemens from comment #0)
> We received reports of duplicated records after batch modifications.
> 
> It seems that when passing the ID to elasticsearch it is parsed as a string
> under Plack, but is parsed as a number under CGI

Reading the bug title I undersetood command line script, but it will actually fix script called in CGI mode, right?
Comment 10 Nick Clemens 2020-05-11 10:49:09 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Nick Clemens from comment #0)
> > We received reports of duplicated records after batch modifications.
> > 
> > It seems that when passing the ID to elasticsearch it is parsed as a string
> > under Plack, but is parsed as a number under CGI
> 
> Reading the bug title I undersetood command line script, but it will
> actually fix script called in CGI mode, right?

Yes, scripts that are excluded from plack like batch modification of bibs and imports are affected and fixed by this
Comment 11 Jonathan Druart 2020-05-11 11:25:35 UTC
Created attachment 104685 [details] [review]
Bug 25342: Force ES id as string

To test:
1 - Be using ES
2 - Find a unique record by searchign for title or seomthing
3 - Edit the record
4 - Repeat search and confirm you are returned to the details page
5 - Go to Tools->MARC modification templates
6 - Add a template and define an action
7 - Go to Tools->Batch record modification
8 - Select Enter a list of record numbers
9 - Enter the biblionumber of the record you searched for
10 - Use the marc modification template you setup
11 - Complete the modification
12 - Repeat your search
13 - note you get two results, both pointing to same biblionumber
14 - Check the ES index, note that you have two entries, one with the biblionumber, and the other with #.0, e.g.:
    curl -XGET 'es:9200/koha_kohadev_biblios/data/14.0?pretty'
    curl -XGET 'es:9200/koha_kohadev_biblios/data/14?pretty'
15 - Apply patch
16 - Delete and rebuild ES index
    perl misc/search_tools/rebuild_elasticsearch.pl -d -b
17 - Repeat 2-12
18 - No duplication this time

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2020-05-11 11:25:39 UTC
Created attachment 104686 [details] [review]
Bug 25342: Unit test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Martin Renvoize 2020-05-12 10:25:26 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 14 Joy Nelson 2020-05-13 16:16:49 UTC
Pushed to 19.11.x for 19.11.06
Comment 15 Lucas Gass 2020-05-15 22:31:17 UTC
backported to 19.05.x for 19.05.11