Summary: | C4::Koha routines expecting a MARC::Record object should check it is defined | ||
---|---|---|---|
Product: | Koha | Reporter: | Colin Campbell <colin.campbell> |
Component: | MARC Bibliographic data support | Assignee: | Colin Campbell <colin.campbell> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | brendan, f.demians, jonathan.druart, julian.maurice |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Proposed patch
Bug 15209 Validate passed MARC::Record objs in C4::Koha Bug 15209: Add tests Bug 15209: Check the parameter at the beginning of the subroutines Bug 15209 Validate passed MARC::Record objs in C4::Koha Bug 15209: Add tests Bug 15209: Check the parameter at the beginning of the subroutines |
Description
Colin Campbell
2015-11-18 14:52:00 UTC
Created attachment 44963 [details] [review] Proposed patch routines now return undef rather than aborting if undefined record is passed to them Created attachment 46007 [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> Created attachment 46008 [details] [review] Bug 15209: Add tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 46009 [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> 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> 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> 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> Pushed to Master - Should be in the May 2016 release. Thanks Patches pushed to 3.22.x, will be in 3.22.3 This patch has been pushed to 3.20.x, will be in 3.20.9. |