Summary: | Koha::AuthorisedValues->get_description_by_koha_field not caching results for non-existent values | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Architecture, internals, and plumbing | Assignee: | David Cook <dcook> |
Status: | Pushed to oldstable --- | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | david |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31216 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33989 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35579 |
||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.02,22.11.08
|
|
Circulation function: | |||
Attachments: |
Bug 34051: Cache empty hashref for non-existent authorised values
Bug 34051: Cache empty hashref for non-existent authorised values Bug 34051: Cache empty hashref for non-existent authorised values |
Description
David Cook
2023-06-19 00:58:51 UTC
Created attachment 152457 [details] [review] Bug 34051: Cache empty hashref for non-existent authorised values When fetching authorised value descriptions, we need to handle scenarios where a value doesn't exist in the cache and doesn't exist in the database. In this situation, we return an empty hashref, and this patch makes us cache this empty hashref. This is important because otherwise the function Koha::AuthorisedValues->get_description_by_koha_field will do a database call every time it encounters the same value that doesn't exist in the authorised values table. I'm not sure the best way to test this one... "prove t/db_dependent/AuthorisedValues.t" succeeds. Koha::AuthorisedValues->get_description_by_koha_field is used across Koha, but the scenario of having non-existent AV values doesn't seem to pop up a lot. I first noticed it in inventory.pl in bug 31216. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to create a SQL report from SQL /cgi-bin/koha/reports/guided_reports.pl?phase=Create%20report%20from%20SQL 2. Save a report with the following SQL: SELECT barcode FROM items where barcode <> ''; 3. Run the report 4. Download as CSV 5. Edit the CSV and remove the "barcode" heading 6. Go to edit item for barcode 3999900000001 /cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=1&itemnumber=1 7. Add "Damaged status" of "Damaged" 8. Click "Save changes" 9. koha-mysql kohadev 10. update items set notforloan = 9 where barcode = 3999900000001; 11. Go to inventory tool http://localhost:8081/cgi-bin/koha/tools/inventory.pl 12. Upload barcode file via "Choose file" 13. Uncheck "Compare barcodes list to results" 14. Open F12 tools 15. Click "Submit" 16. Click "OK" on confirm box 17. Note that the inventory job takes around 30 seconds to run instead of 50 seconds 17b. The exact figures may vary. If you compare with before patch inventory runs, you'll be saving roughly 20 seconds. 18. Note that "Damaged" appears in the "Damaged" column, which demonstrates that the authorized value lookup was completed for the item in the result list 19. Note that "9" appears in the "Not for loan" column, and "0" appears in the "Lost" and "Withdrawn" columns, which demonstrates that the authorized value lookup wasn't able to find descriptions for these codes. The test plan shows that the AV lookups work as expected and the inventory completes faster than before. Created attachment 152464 [details] [review] Bug 34051: Cache empty hashref for non-existent authorised values When fetching authorised value descriptions, we need to handle scenarios where a value doesn't exist in the cache and doesn't exist in the database. In this situation, we return an empty hashref, and this patch makes us cache this empty hashref. This is important because otherwise the function Koha::AuthorisedValues->get_description_by_koha_field will do a database call every time it encounters the same value that doesn't exist in the authorised values table. Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. Running through the test plan (before and after patch applied) - from Network tab for web developer tools: from about 20s (before) to about 10s. Created attachment 152474 [details] [review] Bug 34051: Cache empty hashref for non-existent authorised values When fetching authorised value descriptions, we need to handle scenarios where a value doesn't exist in the cache and doesn't exist in the database. In this situation, we return an empty hashref, and this patch makes us cache this empty hashref. This is important because otherwise the function Koha::AuthorisedValues->get_description_by_koha_field will do a database call every time it encounters the same value that doesn't exist in the authorised values table. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Nice find.. Passing QA Pushed to master for 23.11. Nice work everyone, thanks! Thanks for all the hard work! Pushed to 23.05.x for the next release Nice work everyone! Pushed to 22.11.x for next release |