Bug 23542

Summary: SRU import encoding issue
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: Z39.50 / SRU / OpenSearch ServersAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: major    
Priority: P5 - low CC: aleisha, didier.gautheron, lucas, m.de.rooy, paul.poulain, sandboxes, tomascohen, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18153
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03, 19.11.09, 19.05.15
Attachments: Bug 23542: fix SRU import ending issue
Bug 23542: fix SRU import ending issue
Bug 23542: Fix SRU import encoding
Bug 23542: Fix SRU import encoding
Bug 23542: Fix SRU import encoding
Bug 23542: Fix SRU import encoding
Bug 23542: Fix SRU import encoding

Description Fridolin Somers 2019-09-05 12:32:14 UTC
When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.
Comment 1 Fridolin Somers 2019-09-05 12:56:33 UTC
Created attachment 92606 [details] [review]
Bug 23542: fix SRU import ending issue

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.
Looks like this does not affect autorities import for SRU server.

In C4::Breeding::_handle_one_result(), in case of a Z39.50 server, when creating MARC::Record MarcToUTF8Record() is called which calls SetMarcUnicodeFlag(). This ensures that leader indicates UTF-8 encoding.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to SetMarcUnicodeFlag() in cas of a SRU server in C4::Breeding::_handle_one_result().

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check SRU connexion on a MARC21 database is still OK
Comment 2 Marcel de Rooy 2019-09-05 13:42:58 UTC
Not sure if this is the right solution.
Did you check if you pass new_from_xml the correct parameters in case of UNIMARC ?
Comment 3 Fridolin Somers 2019-09-18 12:55:06 UTC
(In reply to Marcel de Rooy from comment #2)
> Not sure if this is the right solution.
> Did you check if you pass new_from_xml the correct parameters in case of
> UNIMARC ?

This is used in some other places of code, its a trick
Comment 4 Biblibre Sandboxes 2019-10-03 12:55:11 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 5 Biblibre Sandboxes 2019-10-03 12:55:33 UTC
Created attachment 93588 [details] [review]
Bug 23542: fix SRU import ending issue

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.
Looks like this does not affect autorities import for SRU server.

In C4::Breeding::_handle_one_result(), in case of a Z39.50 server, when creating MARC::Record MarcToUTF8Record() is called which calls SetMarcUnicodeFlag(). This ensures that leader indicates UTF-8 encoding.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to SetMarcUnicodeFlag() in cas of a SRU server in C4::Breeding::_handle_one_result().

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check SRU connexion on a MARC21 database is still OK

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 6 Marcel de Rooy 2019-10-04 06:53:26 UTC
QA: Looking here
Comment 7 Marcel de Rooy 2019-10-04 08:26:23 UTC
QA Comment:
When testing with LOC SRU, I am having two records producing the cryptic error code '500' on the search title=vermeer with your patch. While without your patch, I do not get this error.
I found some ZOOM error codes in the range 10000 - 10018 or so; i can't place the returned 500.
The dialog alert contains:
    LIBRARY OF CONGRESS SRU record 19: 500
    LIBRARY OF CONGRESS SRU record 20: 500

Also please provide more feedback as to your statement: "This is used in some other places of code, its a trick."
What kind of trick is this? Please give the other code locations?

Changed status.
Comment 8 Fridolin Somers 2019-12-10 12:47:22 UTC
(In reply to Marcel de Rooy from comment #7)
> 
> Also please provide more feedback as to your statement: "This is used in
> some other places of code, its a trick."
> What kind of trick is this? Please give the other code locations?

Indeed I cant find it now.
And I think this issue also exists with SRU for authorities.
Needs work.
Comment 9 Didier Gautheron 2020-05-27 17:08:03 UTC
(In reply to Marcel de Rooy from comment #7)
> QA Comment:
> When testing with LOC SRU, I am having two records producing the cryptic
> error code '500' on the search title=vermeer with your patch. While without
> your patch, I do not get this error.
> I found some ZOOM error codes in the range 10000 - 10018 or so; i can't
> place the returned 500.
> The dialog alert contains:
>     LIBRARY OF CONGRESS SRU record 19: 500
>     LIBRARY OF CONGRESS SRU record 20: 500
> 
> Also please provide more feedback as to your statement: "This is used in
> some other places of code, its a trick."
> What kind of trick is this? Please give the other code locations?
> 
> Changed status.

I get LIBRARY OF CONGRESS SRU error without the patch, sometime.

I captured the network traffic and it's an http server error 500, way before Koha. Maybe an overloaded server?

As far I can tell there's no difference in requests to server in error and in no error cases.
Comment 10 Paul Poulain 2020-06-29 09:24:54 UTC
Suspecting the patch misses something. IIRC, Authorities does not handle unicode like biblios. Thus, we have UNIMARC and UNIMARCAUTH as available parameters. I suspect (without having checked) that the patch won't work for BNF unimarc authorities.
Comment 11 Fridolin Somers 2020-06-29 12:12:28 UTC
Ah I think I found the correct same code :

In Koha::MetadataRecord::Authority : 
get_from_breeding {
    my $record=eval {MARC::Record->new_from_xml(StripNonXmlChars($marcxml),'UTF-8',
        (C4::Context->preference("marcflavour") eq "UNIMARC"?"UNIMARCAUTH":C4::Context->preference("marcflavour")))};
    return if ($@);
    $record->encoding('UTF-8');

So it is more "$record->encoding('UTF-8')" that is missing. It sets 'a' in leader position 9.
This is called in SetMarcUnicodeFlag().

See also Bug 18153.
Comment 12 Fridolin Somers 2020-06-29 13:19:11 UTC
Created attachment 106397 [details] [review]
Bug 23542: Fix SRU import encoding

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.

Tests show that difference between Z39.50 server and SRU is that leader contains 'a' at postion 9.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to MARC::Record->encoding('UTF-8') in case of a SRU server in C4::Breeding.

Same use exists in Koha::MetadataRecord::Authority::get_from_breeding().

In case of import via Z3950, MarcToUTF8Record() is called,
 which calls SetMarcUnicodeFlag(),
 which calls MARC::Record->encoding('UTF-8')

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check also Authorities import via SRU
13) Check also SRU imports on a MARC21 database
Comment 13 Fridolin Somers 2020-06-29 13:26:19 UTC
Created attachment 106399 [details] [review]
Bug 23542: Fix SRU import encoding

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.

Tests show that difference between Z39.50 server and SRU is that leader contains 'a' at postion 9.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to MARC::Record->encoding('UTF-8') in case of a SRU server in C4::Breeding.

Same use exists in Koha::MetadataRecord::Authority::get_from_breeding().

In case of import via Z3950, MarcToUTF8Record() is called,
 which calls SetMarcUnicodeFlag(),
 which calls MARC::Record->encoding('UTF-8')

Patch also uses system preference marcflavor instead of SRU syntax to
build MARC::Record, like for z3950.

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check also Authorities import via SRU
13) Check also SRU imports on a MARC21 database
Comment 14 Fridolin Somers 2020-06-29 13:42:23 UTC
Created attachment 106400 [details] [review]
Bug 23542: Fix SRU import encoding

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.

Tests show that difference between Z39.50 server and SRU is that leader contains 'a' at postion 9.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to MARC::Record->encoding('UTF-8') in case of a SRU server in C4::Breeding.

Same use exists in Koha::MetadataRecord::Authority::get_from_breeding().

In case of import via Z3950, MarcToUTF8Record() is called,
 which calls SetMarcUnicodeFlag(),
 which calls MARC::Record->encoding('UTF-8')

Patch also uses system preference marcflavor instead of SRU syntax to
build MARC::Record, like for z3950.

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check also Authorities import via SRU
13) Check also SRU imports on a MARC21 database
Comment 15 Fridolin Somers 2020-06-29 13:51:07 UTC
(In reply to Fridolin SOMERS from comment #11)
> Ah I think I found the correct same code :
> 
> In Koha::MetadataRecord::Authority : 
> get_from_breeding {
>     my $record=eval
> {MARC::Record->new_from_xml(StripNonXmlChars($marcxml),'UTF-8',
>         (C4::Context->preference("marcflavour") eq
> "UNIMARC"?"UNIMARCAUTH":C4::Context->preference("marcflavour")))};
>     return if ($@);
>     $record->encoding('UTF-8');
> 
> So it is more "$record->encoding('UTF-8')" that is missing. It sets 'a' in
> leader position 9.
> This is called in SetMarcUnicodeFlag().
> 
> See also Bug 18153.

Here you see that MARC::Records->new_from_usmarc() calls :
  $marc->encoding() eq 'UTF-8'
https://metacpan.org/source/GMCHARLT/MARC-Record-2.0.7/lib/MARC/File/USMARC.pm#L172
Comment 16 Marcel de Rooy 2020-07-03 06:39:20 UTC
Looking here
Comment 17 Marcel de Rooy 2020-07-03 07:08:25 UTC
Without your patch with BNF:

200 1  _aStrate-Ã -gemmes
       _bTexte imprimé
       _fTerry Pratchett
       _g[trad. par Dominique Haas]
Comment 18 Marcel de Rooy 2020-07-03 07:51:41 UTC
The crux is adding the $marcrecord->encoding('UTF-8') !
You should not change the new_from_xml call. We should respect the syntax parameter here instead of the marc flavour of the Koha instance.
If you configured BNF correctly, it will say UNIMARC in the syntax field.

Amending the patch.
Comment 19 Marcel de Rooy 2020-07-03 08:01:30 UTC
Created attachment 106487 [details] [review]
Bug 23542: Fix SRU import encoding

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.

Tests show that difference between Z39.50 server and SRU is that leader contains 'a' at postion 9.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to MARC::Record->encoding('UTF-8') in case of a SRU server in C4::Breeding.

Same use exists in Koha::MetadataRecord::Authority::get_from_breeding().

In case of import via Z3950, MarcToUTF8Record() is called,
 which calls SetMarcUnicodeFlag(),
 which calls MARC::Record->encoding('UTF-8')

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check also Authorities import via SRU
13) Check also SRU imports on a MARC21 database

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: Removed change to new_from_xml call. We should respect syntax.
But the added MARC::Record encoding does the tric! Which is implicit
for Z3950 targets where MarcToUTF8Record does the same.
Comment 20 Marcel de Rooy 2020-07-03 08:03:59 UTC
(In reply to Marcel de Rooy from comment #18)
> The crux is adding the $marcrecord->encoding('UTF-8') !
> You should not change the new_from_xml call. We should respect the syntax
> parameter here instead of the marc flavour of the Koha instance.
> If you configured BNF correctly, it will say UNIMARC in the syntax field.
> 
> Amending the patch.

Also note that we have syntax USMARC for many MARC21 targets. This is the field value for the MARC21/USMARC option in the syntax combo.
Changing it would break the MARC21 search.
Comment 21 Fridolin Somers 2020-07-03 08:15:17 UTC
(In reply to Marcel de Rooy from comment #20)
> (In reply to Marcel de Rooy from comment #18)
> > The crux is adding the $marcrecord->encoding('UTF-8') !
> > You should not change the new_from_xml call. We should respect the syntax
> > parameter here instead of the marc flavour of the Koha instance.
> > If you configured BNF correctly, it will say UNIMARC in the syntax field.
> > 
> > Amending the patch.
> 
> Also note that we have syntax USMARC for many MARC21 targets. This is the
> field value for the MARC21/USMARC option in the syntax combo.
> Changing it would break the MARC21 search.

Ah oki.
Its just strange that MarcToUTF8Record uses C4::Context->preference('marcflavour').
Comment 22 Marcel de Rooy 2020-07-03 08:17:31 UTC
Note to QA:
This is a bit of a workaround to fix the SRU import.
See also bug 18153.
Leader 9 is actually undefined in UNIMARC.
Field 100 $a/26-29 Character Sets (Mandatory) should be 50
  50 = ISO 10646 Level 3 (Unicode, UTF-8)

But we depend heavily on MARC::Record.
Comment 23 Marcel de Rooy 2020-07-03 08:39:09 UTC
(In reply to Fridolin SOMERS from comment #21)
> Its just strange that MarcToUTF8Record uses
> C4::Context->preference('marcflavour').

Well, that is a Koha routine with other parameters. But I agree that the codebase contains inconsistencies in this area.
Comment 24 Fridolin Somers 2020-07-03 12:06:40 UTC
(In reply to Marcel de Rooy from comment #23)
> (In reply to Fridolin SOMERS from comment #21)
> > Its just strange that MarcToUTF8Record uses
> > C4::Context->preference('marcflavour').
> 
> Well, that is a Koha routine with other parameters. But I agree that the
> codebase contains inconsistencies in this area.

Thanks a lot, that would be for another bug report ;)
Comment 25 Fridolin Somers 2020-07-30 13:33:15 UTC
I rise to Major importance because a lot of libraries look forward to use this new SRU of BNF
Comment 26 Tomás Cohen Arazi 2020-08-11 13:21:45 UTC
Created attachment 108059 [details] [review]
Bug 23542: Fix SRU import encoding

When importing records from a SRU server, the diacritics have bad encoding.
I reproduce with BNF server so it may be a UNIMARC issue.

Tests show that difference between Z39.50 server and SRU is that leader contains 'a' at postion 9.
Looking at MARC::Record->encoding() shows that encoding depends on leader even for UNIMARC.
So this patch adds a call to MARC::Record->encoding('UTF-8') in case of a SRU server in C4::Breeding.

Same use exists in Koha::MetadataRecord::Authority::get_from_breeding().

In case of import via Z3950, MarcToUTF8Record() is called,
 which calls SetMarcUnicodeFlag(),
 which calls MARC::Record->encoding('UTF-8')

Test plan :
1) Use a UNIMARC database
2) Configure a connexion to a UNIMARC SRU, for example BNF,
   see https://doc.biblibre.com/koha/autour_de_koha/serveurs_z3950_sru#serveur_de_la_bnf
3) Go to cataloguing module
4) Click on 'New from Z39.50/SRU'
5) Choose only the SRU target
6) Search for ISBN 2266072889
7) Confirm you see good encoding : diacritic on 'a' of title 'Strate-a-gemmes'
8) Click on 'Marc preview'
9) Confirm you see good encoding
10) Click import
11) Confirm you see good encoding
12) Check also Authorities import via SRU
13) Check also SRU imports on a MARC21 database

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: Removed change to new_from_xml call. We should respect syntax.
But the added MARC::Record encoding does the tric! Which is implicit
for Z3950 targets where MarcToUTF8Record does the same.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 27 Jonathan Druart 2020-08-13 05:57:03 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 28 Lucas Gass 2020-08-17 14:39:55 UTC
backported to 20.05.x for 20.05.03
Comment 29 Aleisha Amohia 2020-08-18 01:50:55 UTC
backported to 19.11.x for 19.11.09
Comment 30 Victor Grousset/tuxayo 2020-09-20 14:51:37 UTC
Backported to 19.05.x branch for 19.05.15