Prior to bug 29697, svc/bib used GetMarcBiblio which returned undef if the requested bib number didn't exist, and returned a 404 if the record was undefined. That was switched to using Koha::Biblios->find, which returns undef if the bib number doesn't exist, and then calling undef->metadata->record, which doesn't go well. The if (defined $record) just needs to move up a line and be if (defined $biblio) on the return from the call that actually does return undef.
Created attachment 166858 [details] [review] Bug 36891: Naive attempt to fix svc/bib to return 404 when bib number not found But of course it can't be simple. After just adding if ( defined $biblio ) and reindenting and tidying what I indented, I'm told that I made two more untidy lines. And there's a bad smell around the "if ( defined $record )" since I don't see how it could be false, because $biblio->metadata->record doesn't return undef. But maybe I'm missing something.
The nice part is that it works, http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 returns a bib, http://127.0.0.1:8081/cgi-bin/koha/svc/bib/999999999 returns a 404.
D'oh, I told myself in comment 0 to move that if (defined $record), not add a second if (defined $biblio).
Created attachment 166914 [details] [review] Bug 36891: Restore returning 404 from svc/bib when the bib number doesn't exist Changing from GetMarcBiblio to Biblios->find plus metadata->record lost the way that svc/bib used to return 404 when the bib number wasn't found. This patch restores that by checking for undef after the Biblios->find step. Test plan: 1. Load e.g. http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 which returns an XML bib record 2. Load http://127.0.0.1:8081/cgi-bin/koha/svc/bib/99999999 and get a 500 error 2. Appply patch, restart_all 4. Reload http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 and get the bib again 5. Reload http://127.0.0.1:8081/cgi-bin/koha/svc/bib/99999999 and get a 404
Created attachment 166923 [details] [review] Bug 36891: Restore returning 404 from svc/bib when the bib number doesn't exist Changing from GetMarcBiblio to Biblios->find plus metadata->record lost the way that svc/bib used to return 404 when the bib number wasn't found. This patch restores that by checking for undef after the Biblios->find step. Test plan: 1. Load e.g. http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 which returns an XML bib record 2. Load http://127.0.0.1:8081/cgi-bin/koha/svc/bib/99999999 and get a 500 error 2. Appply patch, restart_all 4. Reload http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 and get the bib again 5. Reload http://127.0.0.1:8081/cgi-bin/koha/svc/bib/99999999 and get a 404 Signed-off-by: David Nind <david@davidnind.com>
Having the impression that I got the 404 also without your patch. But I cant understand that either? Is Plack fooling me?
(In reply to Marcel de Rooy from comment #6) > Having the impression that I got the 404 also without your patch. But I cant > understand that either? Is Plack fooling me? I hope it's Plack fooling you, since the code path to have that happen would be to have eval { $record = undef->metadata->record } not set $@. If you change https://git.koha-community.org/Koha-community/Koha/src/commit/b16e2059a94cf8f803c5687460e73ccf6c85a5f0/svc/bib#L84 to return something other than 404, like maybe 406 Not Acceptable, do you get that, or still 404?
Created attachment 168101 [details] [review] Bug 36891: Restore returning 404 from svc/bib when the bib number doesn't exist Changing from GetMarcBiblio to Biblios->find plus metadata->record lost the way that svc/bib used to return 404 when the bib number wasn't found. This patch restores that by checking for undef after the Biblios->find step. Test plan: 1. Load e.g. http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 which returns an XML bib record 2. Load http://127.0.0.1:8081/cgi-bin/koha/svc/bib/99999999 and get a 500 error 2. Appply patch, restart_all 4. Reload http://127.0.0.1:8081/cgi-bin/koha/svc/bib/289 and get the bib again 5. Reload http://127.0.0.1:8081/cgi-bin/koha/svc/bib/99999999 and get a 404 Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed for 24.11! Well done everyone, thank you!
Backported to 24.05.x for upcoming 24.05.02
Pushed to 23.11.x for 23.11.07
I'm not sure if I got the name of this (old?) API right in the release note (I used the information from https://wiki.koha-community.org/wiki/Koha_/svc/_HTTP_API).
Applied to 23.05.x for 23.05.15