Bug 37349 - Use cache for authority types when linking bibliographic records to authorities
Summary: Use cache for authority types when linking bibliographic records to authorities
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Nick Clemens (kidclamp)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-07-12 15:56 UTC by Nick Clemens (kidclamp)
Modified: 2025-03-31 09:50 UTC (History)
5 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:
24.11.00
Circulation function:


Attachments
Bug 37349: Use cache for authority types and remove extra fetch (2.66 KB, patch)
2024-07-12 15:58 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37349: Use cache for authority types and remove extra fetch (2.67 KB, patch)
2024-08-07 14:32 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37349: Use cache for authority types and remove extra fetch (2.72 KB, patch)
2024-08-09 16:59 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 37349: Use cache for authority types and remove extra fetch (2.82 KB, patch)
2024-08-16 08:24 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 37349: Unit tests (4.33 KB, patch)
2024-11-22 18:03 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37349: (follow-up) Cache the authority type, not just the key (1.28 KB, patch)
2024-11-22 18:03 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-07-12 15:56:40 UTC
LinkBibHeadingstoAuthorities fetches the authority type for the heading for each heading, sometimes more than once, we can improve this
Comment 1 Nick Clemens (kidclamp) 2024-07-12 15:58:52 UTC
Created attachment 168922 [details] [review]
Bug 37349: Use cache for authority types and remove extra fetch

This patch caches the authority types when fetched during linking to avoid grabbing the same type more than once.
Additionally it removes a second call to fetch the same type in some scenarios

To test:
1 - Apply patch
2 - Enable linking during cataloging/updating records
3 - Edit a record and confirm it is linked ocrrectly
4 - Run the authority linking cron and confirm it works as expected
Comment 2 Nick Clemens (kidclamp) 2024-08-07 14:32:20 UTC
Created attachment 170142 [details] [review]
Bug 37349: Use cache for authority types and remove extra fetch

This patch caches the authority types when fetched during linking to avoid grabbing the same type more than once.
Additionally it removes a second call to fetch the same type in some scenarios

To test:
1 - Apply patch
2 - Enable linking during cataloging/updating records
3 - Edit a record and confirm it is linked ocrrectly
4 - Run the authority linking cron and confirm it works as expected
Comment 3 Phil Ringnalda 2024-08-07 15:22:27 UTC
Even though I've never had the need to modify an authority type's auth_tag_to_report, shouldn't doing so invalidate the cache? Or at least have a warning in admin/authtypes.pl?op=add_form that restarting Koha is required for it to take effect?
Comment 4 Nick Clemens (kidclamp) 2024-08-07 16:04:04 UTC
The code here is using the L1 cache, this is cleared for every request

i.e. if you change a type it will be picked up in the next code call - this just ensures it is cached during a single process run
Comment 5 Phil Ringnalda 2024-08-09 16:59:21 UTC
Created attachment 170211 [details] [review]
Bug 37349: Use cache for authority types and remove extra fetch

This patch caches the authority types when fetched during linking to avoid grabbing the same type more than once.
Additionally it removes a second call to fetch the same type in some scenarios

To test:
1 - Apply patch
2 - Enable linking during cataloging/updating records
3 - Edit a record and confirm it is linked ocrrectly
4 - Run the authority linking cron and confirm it works as expected

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Comment 6 Marcel de Rooy 2024-08-16 08:24:09 UTC
Created attachment 170436 [details] [review]
Bug 37349: Use cache for authority types and remove extra fetch

This patch caches the authority types when fetched during linking to avoid grabbing the same type more than once.
Additionally it removes a second call to fetch the same type in some scenarios

To test:
1 - Apply patch
2 - Enable linking during cataloging/updating records
3 - Edit a record and confirm it is linked ocrrectly
4 - Run the authority linking cron and confirm it works as expected

Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2024-08-16 08:24:41 UTC
(In reply to Nick Clemens (kidclamp) from comment #0)
> LinkBibHeadingstoAuthorities fetches the authority type for the heading for
> each heading, sometimes more than once, we can improve this

Moving to enh
Comment 8 Katrin Fischer 2024-08-27 09:36:30 UTC
Should we have a unit test for this change to C4/Biblio.pm?

t/db_dependent/Biblio.t passes.
Comment 9 Katrin Fischer 2024-08-27 10:15:12 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Katrin Fischer 2024-11-14 15:32:16 UTC
Still waiting for reply on unit test (comment#8)
Comment 11 Katrin Fischer 2024-11-21 14:25:28 UTC
(In reply to Katrin Fischer from comment #10)
> Still waiting for reply on unit test (comment#8)

Giving up on the test for now.
Comment 12 Katrin Fischer 2024-11-22 10:23:02 UTC
Skipping for release notes as it would be quite technical:

This patch caches the authority types when fetched during linking to avoid grabbing the same type more than once.
Additionally it removes a second call to fetch the same type in some scenarios
Comment 13 Nick Clemens (kidclamp) 2024-11-22 18:03:22 UTC
Created attachment 174940 [details] [review]
Bug 37349: Unit tests

This patch adds tests to verify the cahcing of authority type in LinkBibHeadingsToAuthority
and adjusts other tests to clear the cache when changing marc flavour
Comment 14 Nick Clemens (kidclamp) 2024-11-22 18:03:24 UTC
Created attachment 174941 [details] [review]
Bug 37349: (follow-up) Cache the authority type, not just the key

Test were failing because I neglected to store the value in the cache, I was only
adding the cache key
Comment 15 Katrin Fischer 2024-11-25 11:10:57 UTC
Picked last 2 patches for main.
Comment 16 Wainui Witika-Park 2024-12-16 02:46:58 UTC
(In reply to Katrin Fischer from comment #15)
> Picked last 2 patches for main.

Should I do the same for 24.05?
Comment 17 Katrin Fischer 2024-12-16 13:49:32 UTC
(In reply to Wainui Witika-Park from comment #16)
> (In reply to Katrin Fischer from comment #15)
> > Picked last 2 patches for main.
> 
> Should I do the same for 24.05?

Hi Wainui, I think this patch set is not in 24.05.
Comment 18 Wainui Witika-Park 2024-12-16 23:12:13 UTC
(In reply to Katrin Fischer from comment #17)
> (In reply to Wainui Witika-Park from comment #16)
> > (In reply to Katrin Fischer from comment #15)
> > > Picked last 2 patches for main.
> > 
> > Should I do the same for 24.05?
> 
> Hi Wainui, I think this patch set is not in 24.05.

Ok thanks Katrin!

Not backporting to 24.05 unless requested