Bug 15209 - C4::Koha routines expecting a MARC::Record object should check it is defined
Summary: C4::Koha routines expecting a MARC::Record object should check it is defined
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic data support (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-18 14:52 UTC by Colin Campbell
Modified: 2017-06-14 22:10 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (7.02 KB, patch)
2015-11-18 15:13 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 15209 Validate passed MARC::Record objs in C4::Koha (7.09 KB, patch)
2015-12-28 15:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15209: Add tests (1.50 KB, patch)
2015-12-28 15:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15209: Check the parameter at the beginning of the subroutines (5.54 KB, patch)
2015-12-28 15:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15209 Validate passed MARC::Record objs in C4::Koha (7.15 KB, patch)
2016-01-08 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15209: Add tests (1.55 KB, patch)
2016-01-08 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15209: Check the parameter at the beginning of the subroutines (5.59 KB, patch)
2016-01-08 17:34 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2015-11-18 14:52:00 UTC
This came to light when trying to get plack opac running. A number of routines in C4::Koha return various identifiers from the marc record but do not check it is valid before calling methods on it. The resulting errors cause the plack instance to abort. It is good practice to handle these errors with minimum inconvenience to the user
Comment 1 Colin Campbell 2015-11-18 15:13:59 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-12-28 15:49:37 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-12-28 15:49:41 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-12-28 15:49:44 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2016-01-08 17:34:09 UTC
Created attachment 46447 [details] [review]
Bug 15209 Validate passed MARC::Record objs in C4::Koha

Ensure that a passed MARC::Record is defined before calling
its methods. Otherwise you are open to occurences of the
error 'Can't call method "field" on an undefined value'
In a CGI environment you can live with such sloppiness but
in a persistent environment the error can cause the instance
to abort.
Made all routines passed a MARC::Record validate it before calling
its methods. Changed the parameter name from the meaningless
record to marcrecord to indicate its content. Added an explicit return
for all cases where no valid data returned. Cleaned up some logic for
clarity. I think we can assume that GetNormalizedOCLCNumber meant to
look at all 035s till it found an OCLC number not just the first.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2016-01-08 17:34:17 UTC
Created attachment 46448 [details] [review]
Bug 15209: Add tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2016-01-08 17:34:19 UTC
Created attachment 46449 [details] [review]
Bug 15209: Check the parameter at the beginning of the subroutines

It seems better to check if parameters exist at the beginning of a
subroutine.
It makes the code easier to read and there is 1 indentation level less.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Brendan Gallagher 2016-01-27 02:09:56 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks
Comment 9 Julian Maurice 2016-01-29 10:37:37 UTC
Patches pushed to 3.22.x, will be in 3.22.3
Comment 10 Frédéric Demians 2016-02-12 06:36:47 UTC
This patch has been pushed to 3.20.x, will be in 3.20.9.