Sort bin rules in system preference SIP2SortBinMapping are not being used by the SIP server. This functionality has broken between the 23.11 and 24.05 versions. We are adding the mappings directly to the SIP2 server configuration XML file as a workaround in 24.05 for now since sort bin mappings still work using the sip config file.
I can confirm that the system preference SIP2SortBinMapping is currently not working in 24.05. We had built our entire sorting system around this feature in 23.11 and it worked fine, but after the 24.05 update the SIP messages that Koha sends do not include the CL field at all. As we make regular updates to sorting rules (e.g. with seasonal materials) it's not a sustainable way to do this manually on the server side on all self-service stations, one by one. This is an important feature to us, and we hope that the bug could be found and fixed soon.
Marking as CONFIRMED on behalf of Janne
I just tried to confirm the bug on my ktd setup (v24.05.05) to dig into it but in my case it worked: SIP2SortBinMapping: CPL:itype:eq:BK:5 kohadev-koha@kohadevbox:misc((v24.05.05))$ perl sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL -m checkout --item 39999000000498 --patron 42 Attempting socket connection to 127.0.0.1:6001...connected! Use of uninitialized value $hold_mode in string eq at sip_cli_emulator.pl line 123. SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'checkout' SEND: 11YN20250104 19020520250104 190205AOCPL|AA42|AB39999000000498|ACterm1|BIN| READ: 121NNY20250104 190205AOCPL|AA42|AB39999000000498|AJCGI programming with Perl /|AH20250109 235900| kohadev-koha@kohadevbox:misc((v24.05.05))$ perl sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL -m checkin --item 39999000000498 --patron 42 Attempting socket connection to 127.0.0.1:6001...connected! Use of uninitialized value $hold_mode in string eq at sip_cli_emulator.pl line 123. SEND: 9300CNterm1|COterm1|CPCPL| READ: 941 Trying 'checkin' SEND: 09N20250104 19020620250104 190206APCPL|AOCPL|AB39999000000498|ACterm1|BIN| READ: 101YNN20250104 190207AOCPL|AB39999000000498|AQCPL|AJCGI programming with Perl /|CL5|AA42|CRREF| --- In the checkin response the mentioned field CL is set to 5. Also tried on main (24.12.) and it worked. Can you give a little more info on your sort bin rules?
Hmm, that's strange. We don't have access to an emulator to check if would differ from a self-service station in our environment, but we have a similar result from two different libraries using stations from different manufacturers. Our rules are extensive, but we have also tried to leave only one row/rule to the Sip2SortBinMapping syspref without any impact to the behaviour. We also use comments for structuring the rules (there are around 400 rules in total for our library consortium) but they don't seem to be cause. Here are a few examples: # Varastoaineisto LAPK:location:eq:AIV:03 LAPK:location:eq:MUV:03 LAPK:location:eq:LAV:03 LAPK:location:eq:NUV:03 # Omat lehdet (jää palautusalueelle) LAPK:itype:eq:14VRKLEHTI:homebranch:eq:LAPK:00 LAPK:itype:eq:14VRKLEHLA:homebranch:eq:LAPK:00 We've tried to remove the leading zero to no effect.
(In reply to Janne Seppänen from comment #4) > Hmm, that's strange. We don't have access to an emulator to check if would > differ from a self-service station in our environment, but we have a similar > result from two different libraries using stations from different > manufacturers. > > Our rules are extensive, but we have also tried to leave only one row/rule > to the Sip2SortBinMapping syspref without any impact to the behaviour. We > also use comments for structuring the rules (there are around 400 rules in > total for our library consortium) but they don't seem to be cause. > > Here are a few examples: > > # Varastoaineisto > LAPK:location:eq:AIV:03 > LAPK:location:eq:MUV:03 > LAPK:location:eq:LAV:03 > LAPK:location:eq:NUV:03 > > # Omat lehdet (jää palautusalueelle) > LAPK:itype:eq:14VRKLEHTI:homebranch:eq:LAPK:00 > LAPK:itype:eq:14VRKLEHLA:homebranch:eq:LAPK:00 > > We've tried to remove the leading zero to no effect. You can find the emulator directly on your koha server: /usr/share/koha/bin/sip_cli_emulator.pl
I am not sure if this needs to be done, but have you tried restarting the SIP server after changing the values?
Our 09 checkin messages are in a form like this: 09N20250116 16144920250116 161449AP|AOORPK|ABOM0L154262|AC|AY5AZEFAB Maybe it has something to do with the message structure that breaks stuff. Could this be translated to the sip emulator for verifying the issue exists with this message structure?
In your checkin the AP is empty: currentLocation AP= institutionId AO=ORPK Maybe you can check via emulator when setting -l ORPK so that the whole request looks like: perl sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su USER -sp PASS -l ORPK -m checkin --item OM0L154262 if the response includes a CL field.
It seems parsing the mappings break when there are more than one line of them defined in SIP2SortBinMapping syspref. This gets logged in case of more than one line of mappings: [2025/01/20 14:25:02] [3705123] [WARN] TESTSIP1@10.0.2.202: Malformed preference line found: 'LAPK:location:eq:MUV:02 LAPK:location:eq:AIV:03' at /home/koha/Koha/C4/SIP/ILS/Transaction/Checkin.pm line 282, <STDIN> line 2. If there is only one line, the line's mapping rule works and the CL field gets set accordingly. I tried to hunt down a commit that's breaking the parsing of the syspref but I only found bug 34737 that touches the sub _get_sort_bin and reverting the commits did not resolve the issue with multiple lines of sort mappings.
So it seems the issue is that splitting the lines does not happen unless the syspref is written in windows with a carriage return + newline \r\n? We should also support UNIX LF.
"Windows typically uses CRLF (\r\n), macOS uses CR (\r), and Linux/Unix systems use LF (\n)". We need to support macOS \r as well.
Sounds like this is the problem, then: # Get the mapping and split on newlines my $raw_map = C4::Context->preference('SIP2SortBinMapping'); return unless $raw_map; @lines = split /\r\n/, $raw_map; https://git.koha-community.org/Koha-community/Koha/src/commit/bb2e88a1e3ebbed3d291cdf7450933aca0ff88ec/C4/SIP/ILS/Transaction/Checkin.pm#L264
And it looks like "split /\R/" should do the trick? POC: #!/usr/bin/perl use Modern::Perl; foreach my $string ( "a\r\nb", "a\rb", "a\nb" ) { say "=========="; say $string; say "----------"; my ( $a, $b ) = split /\R/, $string; say "$a + $b"; } https://perldoc.perl.org/perlrebackslash#Misc
Created attachment 176853 [details] [review] Bug 38375: Splitting SIP2SortBinMapping syspref does not support all operating systems' line endings
Maybe someone can help with a test plan if this needs one. I'll just post the patch here.
It would have been nice to have the relevant code in a subroutine, so we could have tests for it. Not sure if we can test it with different line endings, without testing in three different OSes?
Could we generate the rules like in the existing test, but with different line endings and rerun the same tests again? existing test (Transactions.t line 921): my $rules = <<"RULES"; $branch:homebranch:ne:\$holdingbranch:X\r $branch:effective_itemtype:eq:CD:0\r $branch:itemcallnumber:<:340:1\r $branch:itemcallnumber:<:370:2\r $branch:itemcallnumber:<:600:3\r $branch2:homebranch:ne:\$holdingbranch:X\r $branch2:effective_itemtype:eq:CD:4\r $branch2:itemcallnumber:>:600:5\r RULES new test for unix: $branch:homebranch:ne:\$holdingbranch:X\n $branch:effective_itemtype:eq:CD:0\n $branch:itemcallnumber:<:340:1\n $branch:itemcallnumber:<:370:2\n $branch:itemcallnumber:<:600:3\n $branch2:homebranch:ne:\$holdingbranch:X\n $branch2:effective_itemtype:eq:CD:4\n $branch2:itemcallnumber:>:600:5\n RULES
Oops I was watching the wrong Koha version. The test rules are found at /Koha/t/db_dependent/SIP/Transaction.t line 1108 forwards in main branch.
Yikes, this is nasty.. my apologies for not thinking about line-ending differences sooner.
Created attachment 178317 [details] [review] Bug 38375: Splitting SIP2SortBinMapping syspref does not support all operating systems' line endings Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 178318 [details] [review] Bug 38375: Use perl 5.10+ \R
Created attachment 178319 [details] [review] Bug 38375: Unit tests
Pushed for 25.05! Well done everyone, thank you!
Nice work everyone! Pushed to 24.11.x for 24.11.03
Needed for 24.05.x ?