Created attachment 179240 [details] [review] Bug 39322: Don't fetch borrower attributes if none kept for pseudonymization This patch simply checks the count of pseudonymized borrower attributes before looping through them To test: 1 - Define several borrower attributes, none kept for pseudonymization 2 - Add some values for those attributes to some patrons 3 - Generate some statistics for those patrons (checkin, checkout, renew, etc) 4 - Enable pseudonymization 5 - On command line: export DBIC_TRACE=1 6 - Run the pseudonymization script with a date in the future to ensure all transactions are captured: perl misc/maintenance/pseudonymize_statistics.pl -v -c --before 2026-01-01 7 - Note the borrower attributes are fetched 8 - Apply patch 9 - repeat 6 10 - Note the attributes are only counted, not fetched 11 - Make one of the attributes kept 12 - repeat 6 13 - Note the attributes are counted and fetched
I had a go at testing, but I'm not sure about a couple of items in the test plan: 1. Step 1 - .."none kept for pseudonymization": How do you do this? Or is this only shown for the attribute once pseudonymization is enabled (the "Keep for pseudonymization" field)? 2. Step 7: How do I tell this? The output from running the command is listed at the end. For statistics, I checked out a couple of items and checked them back in. Is this sufficient? Testing notes (using KTD): 1. Generate a password in KTD from the shell: htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/' 2. Edit /etc/koha/sites/kohadev/koha-conf.xml 3. Add an entry before the </config>: <bcrypt_settings>whateverstep1generated</bcrypt_settings> 4. Restart everything: restart_all Output from step 6 ------------------ perl misc/maintenance/pseudonymize_statistics.pl -v -c --before 2026-01-01 Searching for statistics before 2026-01-01 SELECT COUNT( * ) FROM `statistics` `me` WHERE ( `datetime` <= ? ): '2026-01-01' 4 statistics found SELECT COUNT( * ) FROM `pseudonymized_transactions` `me` WHERE ( `datetime` <= ? ): '2026-01-01' SELECT COUNT( * ) FROM `statistics` `me` WHERE ( `datetime` <= ? ): '2026-01-01' SELECT `me`.`datetime`, `me`.`branch`, `me`.`value`, `me`.`type`, `me`.`other`, `me`.`itemnumber`, `me`.`itemtype`, `me`.`location`, `me`.`borrowernumber`, `me`.`ccode`, `me`.`categorycode`, `me`.`interface` FROM `statistics` `me` WHERE ( `datetime` <= ? ): '2026-01-01' INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name":null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI","firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null,"register_name":null,"id":null}', '{"statistic":{"itemtype":"BK","interface":"intranet","type":"issue","borrowernumber":5,"value":0,"location":"GEN","branch":"CPL","itemnumber":578,"datetime":"2025-03-16 20:58:50","categorycode":"PT","ccode":"REF","other":""}}', NULL, '2025-03-16T21:05:29', 'default', '1', NULL, 'new', 'pseudonymize_statistic' INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name":null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI","firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null,"register_name":null,"id":null}', '{"statistic":{"type":"return","itemtype":"BK","interface":"intranet","value":0,"location":"GEN","branch":"CPL","borrowernumber":5,"datetime":"2025-03-16 20:58:55","categorycode":"PT","itemnumber":578,"ccode":"REF","other":""}}', NULL, '2025-03-16T21:05:29', 'default', '1', NULL, 'new', 'pseudonymize_statistic' INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name":null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI","firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null,"register_name":null,"id":null}', '{"statistic":{"ccode":"REF","other":"","categorycode":"PT","datetime":"2025-03-16 20:59:07","itemnumber":296,"value":0,"location":"GEN","branch":"CPL","borrowernumber":23,"type":"issue","itemtype":"BK","interface":"intranet"}}', NULL, '2025-03-16T21:05:29', 'default', '1', NULL, 'new', 'pseudonymize_statistic' INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name":null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI","firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null,"register_name":null,"id":null}', '{"statistic":{"categorycode":"PT","datetime":"2025-03-16 20:59:15","itemnumber":296,"ccode":"REF","other":"","type":"return","interface":"intranet","itemtype":"BK","branch":"CPL","value":0,"location":"GEN","borrowernumber":23}}', NULL, '2025-03-16T21:05:29', 'default', '1', NULL, 'new', 'pseudonymize_statistic' SELECT COUNT( * ) FROM `statistics` `me` WHERE ( `datetime` <= ? ): '2026-01-01' 4 statistics pseudonymized
(In reply to David Nind from comment #2) > I had a go at testing, but I'm not sure about a couple of items in the test > plan: > > 1. Step 1 - .."none kept for pseudonymization": How do you do this? Or is > this only shown for the attribute once pseudonymization is enabled (the > "Keep for pseudonymization" field)? Correct, it only shows when pseudonymization is available > 2. Step 7: How do I tell this? The output from running the command is listed > at the end. > > For statistics, I checked out a couple of items and checked them back in. Is > this sufficient? > > > Testing notes (using KTD): > 1. Generate a password in KTD from the shell: htpasswd -bnBC 10 "" password > | tr -d ':\n' | sed 's/$2y/$2a/' > 2. Edit /etc/koha/sites/kohadev/koha-conf.xml > 3. Add an entry before the </config>: > <bcrypt_settings>whateverstep1generated</bcrypt_settings> > 4. Restart everything: restart_all > > Output from step 6 > ------------------ > > perl misc/maintenance/pseudonymize_statistics.pl -v -c --before 2026-01-01 > Searching for statistics before 2026-01-01 > SELECT COUNT( * ) FROM `statistics` `me` WHERE ( `datetime` <= ? ): > '2026-01-01' > 4 statistics found > SELECT COUNT( * ) FROM `pseudonymized_transactions` `me` WHERE ( `datetime` > <= ? ): '2026-01-01' > SELECT COUNT( * ) FROM `statistics` `me` WHERE ( `datetime` <= ? ): > '2026-01-01' > SELECT `me`.`datetime`, `me`.`branch`, `me`.`value`, `me`.`type`, > `me`.`other`, `me`.`itemnumber`, `me`.`itemtype`, `me`.`location`, > `me`.`borrowernumber`, `me`.`ccode`, `me`.`categorycode`, `me`.`interface` > FROM `statistics` `me` WHERE ( `datetime` <= ? ): '2026-01-01' > INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, > `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) > VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, > '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name": > null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI", > "firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null, > "register_name":null,"id":null}', > '{"statistic":{"itemtype":"BK","interface":"intranet","type":"issue", > "borrowernumber":5,"value":0,"location":"GEN","branch":"CPL","itemnumber": > 578,"datetime":"2025-03-16 > 20:58:50","categorycode":"PT","ccode":"REF","other":""}}', NULL, > '2025-03-16T21:05:29', 'default', '1', NULL, 'new', 'pseudonymize_statistic' > INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, > `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) > VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, > '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name": > null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI", > "firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null, > "register_name":null,"id":null}', > '{"statistic":{"type":"return","itemtype":"BK","interface":"intranet", > "value":0,"location":"GEN","branch":"CPL","borrowernumber":5,"datetime": > "2025-03-16 > 20:58:55","categorycode":"PT","itemnumber":578,"ccode":"REF","other":""}}', > NULL, '2025-03-16T21:05:29', 'default', '1', NULL, 'new', > 'pseudonymize_statistic' > INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, > `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) > VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, > '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name": > null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI", > "firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null, > "register_name":null,"id":null}', > '{"statistic":{"ccode":"REF","other":"","categorycode":"PT","datetime":"2025- > 03-16 > 20:59:07","itemnumber":296,"value":0,"location":"GEN","branch":"CPL", > "borrowernumber":23,"type":"issue","itemtype":"BK","interface":"intranet"}}', > NULL, '2025-03-16T21:05:29', 'default', '1', NULL, 'new', > 'pseudonymize_statistic' > INSERT INTO `background_jobs` ( `borrowernumber`, `context`, `data`, > `ended_on`, `enqueued_on`, `queue`, `size`, `started_on`, `status`, `type`) > VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ? ): NULL, > '{"cardnumber":null,"interface":"commandline","shibboleth":null,"desk_name": > null,"flags":null,"number":null,"desk_id":null,"branch":null,"surname":"CLI", > "firstname":"CLI","branchname":null,"register_id":null,"emailaddress":null, > "register_name":null,"id":null}', > '{"statistic":{"categorycode":"PT","datetime":"2025-03-16 > 20:59:15","itemnumber":296,"ccode":"REF","other":"","type":"return", > "interface":"intranet","itemtype":"BK","branch":"CPL","value":0,"location": > "GEN","borrowernumber":23}}', NULL, '2025-03-16T21:05:29', 'default', '1', > NULL, 'new', 'pseudonymize_statistic' > SELECT COUNT( * ) FROM `statistics` `me` WHERE ( `datetime` <= ? ): > '2026-01-01' > 4 statistics pseudonymized
(In reply to David Nind from comment #2) > I had a go at testing, but I'm not sure about a couple of items in the test > plan: > > 1. Step 1 - .."none kept for pseudonymization": How do you do this? Or is > this only shown for the attribute once pseudonymization is enabled (the > "Keep for pseudonymization" field)? Correct, the option only shows once Pseudonymization is enabled > 2. Step 7: How do I tell this? The output from running the command is listed > at the end. Ahm mthis depends on bug 39301, added to bug, You will see a line like: SELECT `me`.`id`, `me`.`borrowernumber`, `me`.`code`, `me`.`attribute` FROM `borrower_attributes` `me` WHERE ( `me`.`borrowernumber` = ? ) ORDER BY `me`.`code`, `attribute`: '5' > For statistics, I checked out a couple of items and checked them back in. Is > this sufficient? Yes, it should be
Created attachment 180109 [details] [review] Bug 39322: Don't fetch borrower attributes if none kept for pseudonymization This patch simply checks the count of pseudonymized borrower attributes before looping through them To test: 1 - Define several borrower attributes, none kept for pseudonymization 2 - Add some values for those attributes to some patrons 3 - Generate some statistics for those patrons (checkin, checkout, renew, etc) 4 - Enable pseudonymization 5 - On command line: export DBIC_TRACE=1 6 - Run the pseudonymization script with a date in the future to ensure all transactions are captured: perl misc/maintenance/pseudonymize_statistics.pl -v -c --before 2026-01-01 7 - Note the borrower attributes are fetched 8 - Apply patch 9 - repeat 6 10 - Note the attributes are only counted, not fetched 11 - Make one of the attributes kept 12 - repeat 6 13 - Note the attributes are counted and fetched Signed-off-by: David Nind <david@davidnind.com>
(In reply to Nick Clemens (kidclamp) from comment #4) > (In reply to David Nind from comment #2) > > I had a go at testing, but I'm not sure about a couple of items in the test > > plan: > > > > 1. Step 1 - .."none kept for pseudonymization": How do you do this? Or is > > this only shown for the attribute once pseudonymization is enabled (the > > "Keep for pseudonymization" field)? > > Correct, the option only shows once Pseudonymization is enabled > > > 2. Step 7: How do I tell this? The output from running the command is listed > > at the end. > > Ahm mthis depends on bug 39301, added to bug, You will see a line like: > SELECT `me`.`id`, `me`.`borrowernumber`, `me`.`code`, `me`.`attribute` FROM > `borrower_attributes` `me` WHERE ( `me`.`borrowernumber` = ? ) ORDER BY > `me`.`code`, `attribute`: '5' > > > > For statistics, I checked out a couple of items and checked them back in. Is > > this sufficient? > > Yes, it should be Thanks Nick! Updated testing notes (using KTD): 1. Generate a password in KTD from the shell: htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/' 2. Edit /etc/koha/sites/kohadev/koha-conf.xml 3. Add an entry before the </config>: <bcrypt_settings>whateverstep1generated</bcrypt_settings> 4. Restart everything: restart_all 5. Before step 6, apply bug 39301
Created attachment 181858 [details] [review] Bug 39322: Don't fetch borrower attributes if none kept for pseudonymization This patch simply checks the count of pseudonymized borrower attributes before looping through them To test: 1 - Define several borrower attributes, none kept for pseudonymization 2 - Add some values for those attributes to some patrons 3 - Generate some statistics for those patrons (checkin, checkout, renew, etc) 4 - Enable pseudonymization 5 - On command line: export DBIC_TRACE=1 6 - Run the pseudonymization script with a date in the future to ensure all transactions are captured: perl misc/maintenance/pseudonymize_statistics.pl -v -c --before 2026-01-01 7 - Note the borrower attributes are fetched 8 - Apply patch 9 - repeat 6 10 - Note the attributes are only counted, not fetched 11 - Make one of the attributes kept 12 - repeat 6 13 - Note the attributes are counted and fetched Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 181859 [details] [review] Bug 39322: (follow-up) Further improvements This patch updates the query logic of create_from_statistic for borrower attributes. We replace the count of attributes types with a query to fetch all attributes type codes where keep_for_pseudonymization is set. We then check for content in that array before using it to perform a filtered search on the patrons extended_attributes to only return attributes set for pseudonymization. This should result in the same skip of extended_attributes fetch should no attribute types be set to pseudonymization, but it should also reduce the 1 call to attribute types per attribute the patron has to just a single filtered call for patron attributes attributes followed by the store call for each pseudonymizated attribute. Test plan: As before, except there should be even fewer sql calls to the database than before for the case where there are attributes.
I'm just getting Nick to confirm my follow-up, but I'm basically happy here to PQA. Lets change the status as soon as Nick says he's happy or not with the follow-up.
Created attachment 181863 [details] [review] Bug 39322: (follow-up) Further improvements This patch updates the query logic of create_from_statistic for borrower attributes. We replace the count of attributes types with a query to fetch all attributes type codes where keep_for_pseudonymization is set. We then check for content in that array before using it to perform a filtered search on the patrons extended_attributes to only return attributes set for pseudonymization. This should result in the same skip of extended_attributes fetch should no attribute types be set to pseudonymization, but it should also reduce the 1 call to attribute types per attribute the patron has to just a single filtered call for patron attributes attributes followed by the store call for each pseudonymizated attribute. Test plan: As before, except there should be even fewer sql calls to the database than before for the case where there are attributes. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Can you please rebase? We are moving a bit fast right now... Bug 39322 - When pseudonymizing don't fetch patron attributes if none are kept 181863 - Bug 39322: (follow-up) Further improvements Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 39322: (follow-up) Further improvements error: sha1 information is lacking or useless (Koha/PseudonymizedTransaction.pm). error: could not build fake ancestor Patch failed at 0001 Bug 39322: (follow-up) Further improvements hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-39322-follow-up-Further-improvements-lfu_vkmh.patch
Are we missing the first patch maybe?
(In reply to Katrin Fischer from comment #12) > Are we missing the first patch maybe? Looks like it was that, unobsoleted, applies now
Pushed for 25.05! Well done everyone, thank you!