Bug 27784 - Unknown authority types break elasticsearch authorities indexing
Summary: Unknown authority types break elasticsearch authorities indexing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-24 20:42 UTC by Nick Clemens
Modified: 2021-12-13 21:11 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This patch fixes Elasticsearch indexing failures caused by 'SUBDIV' type authority records in Koha. It skips the step of parsing the authorities into the linking form if the type contains '_SUBD'. Notes: - Koha currently doesn't have support for 'SUBDIV' type authority records. - They can be added to the authority types in the staff interface, however, values are hard coded in various modules and Koha has no concept of how to link a subfield heading to a record, as we only deal in whole fields.
Version(s) released in:
21.05.00,20.11.04,20.05.10,19.11.16


Attachments
Bug 27784: Unit tests (2.62 KB, patch)
2021-02-26 14:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27784: Don't parse subdivision authorities as headings (2.54 KB, patch)
2021-02-26 14:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Sample record (457 bytes, application/x-wine-extension-mrc)
2021-02-26 20:32 UTC, Nick Clemens
Details
Bug 27784: Unit tests (2.67 KB, patch)
2021-03-01 16:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 27784: Don't parse subdivision authorities as headings (2.59 KB, patch)
2021-03-01 16:14 UTC, David Nind
Details | Diff | Splinter Review
Bug 27784: Unit tests (2.72 KB, patch)
2021-03-03 10:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27784: Don't parse subdivision authorities as headings (2.65 KB, patch)
2021-03-03 10:41 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-02-24 20:42:07 UTC
We are seeing more libraries ending up with 'SUBDIV' type authority records in their systems. Koha has no support for these.

They can be added to the authority types in the staff interface, however, values are hardcoded in various modules and Koha has no concept of how to link a subfield heading to a record, we only deal in whole fields

The larger problem is something to think about, but more directly we should not allow indexing to die when an unknown type is encountered

To recreate:
1 - Import a subdivision authority record via Z39 or use the one attached to this bug
2 - perl misc/search_tools/rebuild_elasticsearch.pl -v -d
3 - Authority indexing dies:
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
Use of uninitialized value $tag in regexp compilation at /usr/share/perl5/MARC/Record.pm line 206.
Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 207.
Can't call method "tag" on an undefined value at /kohadevbox/koha/C4/Heading.pm line 71.
Comment 1 Nick Clemens 2021-02-26 14:32:37 UTC
Created attachment 117378 [details] [review]
Bug 27784: Unit tests
Comment 2 Nick Clemens 2021-02-26 14:32:39 UTC
Created attachment 117379 [details] [review]
Bug 27784: Don't parse subdivision authorities as headings

Subdivision authorities are not used for linking, however, they are recognized by
C4::AuthoritiesMarc

While these records are not used for linking, they could provide reference and
should be allowed to exist in the catalog without breaking ES indexing

THis patch simply skips the step of parsing the authorities into the linking form
if the type contains '_SUBD'

To test:
1 - Import a subdivision authority record via Z39 or use the one attached to this bug
2 - perl misc/search_tools/rebuild_elasticsearch.pl -v -d
3 - Authority indexing dies:
   Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
   Use of uninitialized value $tag in regexp compilation at /usr/share/perl5/MARC/Record.pm line 206.
   Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 207.
   Can't call method "tag" on an undefined value at /kohadevbox/koha/C4/Heading.pm line 71.
4 - Apply patches
5 - reindex
6 - Success!
Comment 3 Nick Clemens 2021-02-26 20:32:58 UTC
Created attachment 117401 [details]
Sample record
Comment 4 David Nind 2021-03-01 16:14:14 UTC
Created attachment 117459 [details] [review]
Bug 27784: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2021-03-01 16:14:18 UTC
Created attachment 117460 [details] [review]
Bug 27784: Don't parse subdivision authorities as headings

Subdivision authorities are not used for linking, however, they are recognized by
C4::AuthoritiesMarc

While these records are not used for linking, they could provide reference and
should be allowed to exist in the catalog without breaking ES indexing

THis patch simply skips the step of parsing the authorities into the linking form
if the type contains '_SUBD'

To test:
1 - Import a subdivision authority record via Z39 or use the one attached to this bug
2 - perl misc/search_tools/rebuild_elasticsearch.pl -v -d
3 - Authority indexing dies:
   Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
   Use of uninitialized value $tag in regexp compilation at /usr/share/perl5/MARC/Record.pm line 206.
   Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 207.
   Can't call method "tag" on an undefined value at /kohadevbox/koha/C4/Heading.pm line 71.
4 - Apply patches
5 - reindex
6 - Success!

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2021-03-01 16:16:33 UTC
Thanks Nick for providing a sample record!
Comment 7 Martin Renvoize 2021-03-03 10:41:31 UTC
Created attachment 117563 [details] [review]
Bug 27784: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-03-03 10:41:34 UTC
Created attachment 117564 [details] [review]
Bug 27784: Don't parse subdivision authorities as headings

Subdivision authorities are not used for linking, however, they are recognized by
C4::AuthoritiesMarc

While these records are not used for linking, they could provide reference and
should be allowed to exist in the catalog without breaking ES indexing

THis patch simply skips the step of parsing the authorities into the linking form
if the type contains '_SUBD'

To test:
1 - Import a subdivision authority record via Z39 or use the one attached to this bug
2 - perl misc/search_tools/rebuild_elasticsearch.pl -v -d
3 - Authority indexing dies:
   Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 202.
   Use of uninitialized value $tag in regexp compilation at /usr/share/perl5/MARC/Record.pm line 206.
   Use of uninitialized value $tag in hash element at /usr/share/perl5/MARC/Record.pm line 207.
   Can't call method "tag" on an undefined value at /kohadevbox/koha/C4/Heading.pm line 71.
4 - Apply patches
5 - reindex
6 - Success!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2021-03-03 10:42:39 UTC
A reasonable and pragmatic solution to the issue.

Whilst it would be nice to add support for subdivision linking, this patch is a pragmatic solution to prevent indexing explosions.  It works as expected, has a unit regression test and passes the QA script.

Passing QA
Comment 10 Jonathan Druart 2021-03-03 10:59:41 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 11 Fridolin Somers 2021-03-04 12:20:36 UTC
Pushed to 20.11.x for 20.11.04
Comment 12 Andrew Fuerste-Henry 2021-03-14 19:07:00 UTC
Pushed to 20.05.x for 20.05.10
Comment 13 Victor Grousset/tuxayo 2021-03-23 06:21:45 UTC
Backported: Pushed to 19.11.x branch for 19.11.16