Bug 36891

Summary: Restore returning 404 from svc/bib when the bib number doesn't exist
Product: Koha Reporter: Phil Ringnalda <phil>
Component: CatalogingAssignee: Phil Ringnalda <phil>
Status: Pushed to oldoldstable --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, dcook, fridolin.somers, lauren_denny, lucas, m.de.rooy, wainuiwitikapark
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes requests made for records that don't exist using the /svc/bib/<biblionumber> HTTP API. A 404 error (Not Found) is now returned if a record doesn't exist, instead of a 505 error (HTTP Version Not Supported).
Version(s) released in:
24.11.00,24.05.02,23.11.07,23.05.15
Circulation function:
Bug Depends on:    
Bug Blocks: 37212    
Attachments: Bug 36891: Naive attempt to fix svc/bib to return 404 when bib number not found
Bug 36891: Restore returning 404 from svc/bib when the bib number doesn't exist
Bug 36891: Restore returning 404 from svc/bib when the bib number doesn't exist
Bug 36891: Restore returning 404 from svc/bib when the bib number doesn't exist

Description Phil Ringnalda 2024-05-16 23:23:49 UTC
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.
Comment 1 Phil Ringnalda 2024-05-17 00:22:52 UTC
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.
Comment 2 Phil Ringnalda 2024-05-17 00:23:59 UTC
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.
Comment 3 Phil Ringnalda 2024-05-17 15:59:57 UTC
D'oh, I told myself in comment 0 to move that if (defined $record), not add a second if (defined $biblio).
Comment 4 Phil Ringnalda 2024-05-17 16:08:15 UTC
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
Comment 5 David Nind 2024-05-19 06:34:07 UTC
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>
Comment 6 Marcel de Rooy 2024-05-24 10:02:32 UTC
Having the impression that I got the 404 also without your patch. But I cant understand that either? Is Plack fooling me?
Comment 7 Phil Ringnalda 2024-05-28 18:26:53 UTC
(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?
Comment 8 Nick Clemens (kidclamp) 2024-06-25 21:24:38 UTC
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>
Comment 9 Katrin Fischer 2024-06-27 09:52:38 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Lucas Gass (lukeg) 2024-07-18 20:17:56 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 11 Fridolin Somers 2024-07-19 09:31:47 UTC
Pushed to 23.11.x for 23.11.07
Comment 12 David Nind 2024-07-29 19:00:27 UTC
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).
Comment 13 Wainui Witika-Park 2024-09-17 02:11:53 UTC
Applied to 23.05.x for 23.05.15