In Koha/REST/V1/Biblios.pm (methods get and get_public), MARC::Record::as_xml_record is called without a $format parameter, which means it relies on the package variable $MARC::File::XML::RecordFormat to be correctly set, which may or may not be the case depending on what the current starman/mojo worker has done before. We should pass the format explicitly to avoid problems. This is only a problem for UNIMARC, because $MARC::File::XML::RecordFormat defaults to 'USMARC'.
Created attachment 144207 [details] [review] Bug 32336: Fix encoding of MARCXML output for REST API (UNIMARC) Test plan: 1. Be sure to test on a UNIMARC instance 2. Enable system preferences RESTPublicAPI and RESTPublicAnonymousRequests 3. Create a biblio with some diacritics 4. Restart koha and do not interact with OPAC or the staff interface (this may change $MARC::File::XML::RecordFormat) 5. Query the public API to retrieve the created biblio in MARCXML. For instance with cURL: curl -H 'Accept: application/marcxml+xml' \ http://koha.local/api/v1/public/biblios/<biblionumber> You should see encoding issues. 6. Apply the patch and restart Koha. 7. Repeat step 5. You should see no encoding issues. 8. You should also test /api/v1/biblios/<biblionumber> (this one requires authentication, so cURL might not be the most practical tool; use your favorite tool for this)
Please add tests
Created attachment 146846 [details] [review] Bug 32336: Add unit test for UNIMARC encoding problems in REST API
Created attachment 146847 [details] [review] Bug 32336: Fix encoding of MARCXML output for REST API (UNIMARC) Test plan: 1. Be sure to test on a UNIMARC instance 2. Enable system preferences RESTPublicAPI and RESTPublicAnonymousRequests 3. Create a biblio with some diacritics 4. Restart koha and do not interact with OPAC or the staff interface (this may change $MARC::File::XML::RecordFormat) 5. Query the public API to retrieve the created biblio in MARCXML. For instance with cURL: curl -H 'Accept: application/marcxml+xml' \ http://koha.local/api/v1/public/biblios/<biblionumber> You should see encoding issues. 6. Apply the patch and restart Koha. 7. Repeat step 5. You should see no encoding issues. 8. You should also test /api/v1/biblios/<biblionumber> (this one requires authentication, so cURL might not be the most practical tool; use your favorite tool for this)
(In reply to Julian Maurice from comment #4) > 8. You should also test /api/v1/biblios/<biblionumber> (this one > requires authentication, so cURL might not be the most practical > tool; use your favorite tool for this) FYI you can use -u koha:koha ;) curl -v -s -u koha:koha --request GET 'http://localhost:8081/api/v1/biblios/1' --header 'Accept: application/marcxml+xml'
Created attachment 148614 [details] [review] Bug 32336: Add unit test for UNIMARC encoding problems in REST API Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 148615 [details] [review] Bug 32336: Fix encoding of MARCXML output for REST API (UNIMARC) Test plan: 1. Be sure to test on a UNIMARC instance 2. Enable system preferences RESTPublicAPI and RESTPublicAnonymousRequests 3. Create a biblio with some diacritics 4. Restart koha and do not interact with OPAC or the staff interface (this may change $MARC::File::XML::RecordFormat) 5. Query the public API to retrieve the created biblio in MARCXML. For instance with cURL: curl -H 'Accept: application/marcxml+xml' \ http://koha.local/api/v1/public/biblios/<biblionumber> You should see encoding issues. 6. Apply the patch and restart Koha. 7. Repeat step 5. You should see no encoding issues. 8. You should also test /api/v1/biblios/<biblionumber> (this one requires authentication, so cURL might not be the most practical tool; use your favorite tool for this) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I think we need some more here: we should really be using my $metadata = $biblio->metadata; my $schema = $biblio->schema // C4::Context->preference("marcflavour"); Should we also file a new bug for list()?
Created attachment 148628 [details] [review] Bug 32336: (QA follow-up) Use $metadata->schema When we worked on Koha::Biblio::Metadata, we agreed biblio_metadata.schema would be the sourceof truth when it comes to the record schema. This patch acknowledges that, while retaining the fallback to the syspref. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Tests pass, behavior is unchanged. 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
If you agree with my follow-up please set it back to PQA, otherwise let's discuss it properly. Best regards.
Patch makes sense. Back to Passed QA Thanks for the follow-up Tomas
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to stable for 22.11.x