Bug 40911 - SIP Patron->new() generates ORM warnings with undefined/empty patron_id
Summary: SIP Patron->new() generates ORM warnings with undefined/empty patron_id
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-09-30 14:32 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-09-30 15:16 UTC (History)
4 users (show)

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


Attachments
Bug 40911: Regression tests (7.63 KB, patch)
2025-09-30 15:16 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 40911: Fix SIP Patron->new() warnings with undefined/empty patron_id (3.62 KB, patch)
2025-09-30 15:16 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2025-09-30 14:32:22 UTC
Noticed in production, when C4::SIP::ILS::Patron->new is passed a $patron_id that is not defined or empty -or any processing makes it like that- (e.g. faulty plugins, malformed SIP requests) logs get flooded with DBIx::Class errors and Perl warnings when it should never reach the ORM with a query because there's nothing to search for.

Similar to bug 40910 for Item->new(), the Patron constructor should validate input early and return gracefully without generating warnings for undefined/empty patron  identifiers.

Expected behavior: Silent return for invalid input, proper debug logging for
troubleshooting
Actual behavior: Warning spam in SIP server logs, potential DBIx::Class constraint errors

This affects SIP server stability and makes log analysis difficult due to noise from invalid requests.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-09-30 15:16:03 UTC
Created attachment 187109 [details] [review]
Bug 40911: Regression tests
Comment 2 Tomás Cohen Arazi (tcohen) 2025-09-30 15:16:10 UTC
Created attachment 187110 [details] [review]
Bug 40911: Fix SIP Patron->new() warnings with undefined/empty patron_id

This patch prevents warnings when C4::SIP::ILS::Patron->new()
receives undefined, empty, or whitespace-only patron_id values
that could occur from faulty plugins or malformed SIP requests.

Changes:
- Add early validation for undefined/empty patron_id before processing
- Add validation for hashref keys to ensure they contain valid values
- Add better debug logging for empty hashref scenarios
- Prevent potential siplog formatting issues with hashref patron_id
- No functional changes - behavior preserved for valid patron identifiers

Test plan:
1. Apply the regression tests
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/SIP/Patron.t
=> FAIL: Tests fail! Warning validation expects no warnings but warnings occur
3. Apply this patch
4. Repeat step 2
=> SUCCESS: Tests pass! No warnings generated for invalid input
5. Verify no warnings in SIP logs for malformed requests
6. Sign off :-D