Bugzilla – Attachment 150016 Details for
Bug 32894
Objects cache methods' result without invalidation
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32894: Remove wrong caching from Koha:: methods - simple
Bug-32894-Remove-wrong-caching-from-Koha-methods--.patch (text/plain), 10.64 KB, created by
Martin Renvoize (ashimema)
on 2023-04-21 10:28:39 UTC
(
hide
)
Description:
Bug 32894: Remove wrong caching from Koha:: methods - simple
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-04-21 10:28:39 UTC
Size:
10.64 KB
patch
obsolete
>From dcd16f94a5ba713487183a4eee9ee293af7072c3 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 28 Feb 2023 16:11:38 +0100 >Subject: [PATCH] Bug 32894: Remove wrong caching from Koha:: methods - simple > >In some of our Koha:: objects we have methods that cache their result and return it in subsequent calls. However there is no invalidation of the cache if the object is modified. > >For instance, in Koha/ArticleRequest.pm > >sub biblio { > my ($self) = @_; > > $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); > > return $self->{_biblio}; >} > >This pattern exists in several places. > >It can lead to confusion and incorrect results, such as: > >use Koha::ArticleRequests; >my $ar = Koha::ArticleRequest->new({ > borrowernumber => 42, > biblionumber => 42, >})->store; >say $ar->biblio->biblionumber; # Display 42, correct >$ar->set({ biblionumber => 24 })->store; >say $ar->biblio->biblionumber; # Display 42, wrong >$ar->discard_changes; >say $ar->biblio->biblionumber; # Display 42, wrong >$ar->delete; > >We should remove those caching and rely on DBIC/DBMS caching mechanism instead. > >This patch is adjusting the trivial occurrences > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > Koha/Account/Line.pm | 9 ++++----- > Koha/ArticleRequest.pm | 27 ++++++++++++-------------- > Koha/Biblio.pm | 8 +++----- > Koha/CirculationRule.pm | 21 +++++++++----------- > Koha/Hold.pm | 37 ++++++++++++++---------------------- > Koha/Library/Group.pm | 16 ++++++---------- > Koha/Object/Limit/Library.pm | 6 +----- > Koha/Virtualshelf.pm | 4 ++-- > Koha/Virtualshelfshare.pm | 2 +- > 9 files changed, 52 insertions(+), 78 deletions(-) > >diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm >index 6ab6120a1a..822d38b74d 100644 >--- a/Koha/Account/Line.pm >+++ b/Koha/Account/Line.pm >@@ -90,12 +90,11 @@ Return the checkout linked to this account line if exists > =cut > > sub checkout { >- my ( $self ) = @_; >- return unless $self->issue_id ; >+ my ($self) = @_; >+ return unless $self->issue_id; > >- $self->{_checkout} ||= Koha::Checkouts->find( $self->issue_id ); >- $self->{_checkout} ||= Koha::Old::Checkouts->find( $self->issue_id ); >- return $self->{_checkout}; >+ return Koha::Checkouts->find( $self->issue_id ) >+ || Koha::Old::Checkouts->find( $self->issue_id ); > } > > =head3 library >diff --git a/Koha/ArticleRequest.pm b/Koha/ArticleRequest.pm >index 83c88bee8c..29a2471d51 100644 >--- a/Koha/ArticleRequest.pm >+++ b/Koha/ArticleRequest.pm >@@ -178,9 +178,9 @@ Returns the Koha::Biblio object for this article request > sub biblio { > my ($self) = @_; > >- $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); >- >- return $self->{_biblio}; >+ my $rs = $self->result->biblionumber; >+ return unless $rs; >+ return Koha::Biblio->_new_from_dbic($rs); > } > > =head3 debit >@@ -208,10 +208,9 @@ Returns the Koha::Item object for this article request > > sub item { > my ($self) = @_; >- >- $self->{_item} ||= Koha::Items->find( $self->itemnumber() ); >- >- return $self->{_item}; >+ my $rs = $self->_result->itemnumber; >+ return unless $rs; >+ return Koha::Item->_new_from_dbic($rs); > } > > =head3 borrower >@@ -222,10 +221,9 @@ Returns the Koha::Patron object for this article request > > sub borrower { > my ($self) = @_; >- >- $self->{_borrower} ||= Koha::Patrons->find( $self->borrowernumber() ); >- >- return $self->{_borrower}; >+ my $rs = $self->_result->borrowernumber; >+ return unless $rs; >+ return Koha::Patron->_new_from_dbic($rs); > } > > =head3 branch >@@ -236,10 +234,9 @@ Returns the Koha::Library object for this article request > > sub branch { > my ($self) = @_; >- >- $self->{_branch} ||= Koha::Libraries->find( $self->branchcode() ); >- >- return $self->{_branch}; >+ my $rs = $self->_result->branchcode; >+ return unless $rs; >+ return Koha::Library->_new_from_dbic($rs); > } > > =head3 store >diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm >index a94665ddc8..0b0aae8c73 100644 >--- a/Koha/Biblio.pm >+++ b/Koha/Biblio.pm >@@ -535,7 +535,7 @@ sub current_holds { > > =head3 biblioitem > >-my $field = $self->biblioitem()->itemtype >+my $field = $self->biblioitem > > Returns the related Koha::Biblioitem object for this Biblio object > >@@ -671,10 +671,8 @@ Returns the related Koha::Subscriptions object for this Biblio object > > sub subscriptions { > my ($self) = @_; >- >- $self->{_subscriptions} ||= Koha::Subscriptions->search( { biblionumber => $self->biblionumber } ); >- >- return $self->{_subscriptions}; >+ my $rs = $self->_result->subscriptions; >+ return Koha::Subscriptions->_new_from_dbic($rs); > } > > =head3 has_items_waiting_or_intransit >diff --git a/Koha/CirculationRule.pm b/Koha/CirculationRule.pm >index 59638a5aa2..29edbe2c42 100644 >--- a/Koha/CirculationRule.pm >+++ b/Koha/CirculationRule.pm >@@ -41,10 +41,9 @@ Koha::CirculationRule - Koha CirculationRule object class > > sub library { > my ($self) = @_; >- >- $self->{_library} ||= Koha::Libraries->find( $self->branchcode ); >- >- return $self->{_library}; >+ my $rs = $self->_result->branchcode; >+ return unless $rs; >+ return Koha::Library->_new_from_dbic($rs); > } > > =head3 patron_category >@@ -53,10 +52,9 @@ sub library { > > sub patron_category { > my ($self) = @_; >- >- $self->{_patron_category} ||= Koha::Patron::Categories->find( $self->categorycode ); >- >- return $self->{_patron_category}; >+ my $rs = $self->_result->categorycode; >+ return unless $rs; >+ return Koha::Patron::Category->_new_from_dbic($rs); > } > > =head3 item_type >@@ -65,10 +63,9 @@ sub patron_category { > > sub item_type { > my ($self) = @_; >- >- $self->{_item_type} ||= Koha::ItemTypes->find( $self->itemtype ); >- >- return $self->{item_type}; >+ my $rs = $self->_result->itemtype; >+ return unless $rs; >+ return Koha::ItemTypes->_new_from_dbic($rs); > } > > =head3 clone >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index a5a9d03003..b1f49a43c5 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -516,10 +516,8 @@ Returns the related Koha::Biblio object for this hold > > sub biblio { > my ($self) = @_; >- >- $self->{_biblio} ||= Koha::Biblios->find( $self->biblionumber() ); >- >- return $self->{_biblio}; >+ my $rs = $self->_result->biblionumber; >+ return Koha::Biblio->_new_from_dbic($rs); > } > > =head3 patron >@@ -530,9 +528,8 @@ Returns the related Koha::Patron object for this hold > > sub patron { > my ($self) = @_; >- >- my $patron_rs = $self->_result->patron; >- return Koha::Patron->_new_from_dbic($patron_rs); >+ my $rs = $self->_result->patron; >+ return Koha::Patron->_new_from_dbic($rs); > } > > =head3 item >@@ -543,10 +540,9 @@ Returns the related Koha::Item object for this Hold > > sub item { > my ($self) = @_; >- >- $self->{_item} ||= Koha::Items->find( $self->itemnumber() ); >- >- return $self->{_item}; >+ my $rs = $self->_result->itemnumber; >+ return unless $rs; >+ return Koha::Item->_new_from_dbic($rs); > } > > =head3 item_group >@@ -557,10 +553,9 @@ Returns the related Koha::Biblio::ItemGroup object for this Hold > > sub item_group { > my ($self) = @_; >- >- my $item_group_rs = $self->_result->item_group; >- return unless $item_group_rs; >- return Koha::Biblio::ItemGroup->_new_from_dbic($item_group_rs); >+ my $rs = $self->_result->item_group; >+ return unless $rs; >+ return Koha::Biblio::ItemGroup->_new_from_dbic($rs); > } > > =head3 branch >@@ -571,10 +566,8 @@ Returns the related Koha::Library object for this Hold > > sub branch { > my ($self) = @_; >- >- $self->{_branch} ||= Koha::Libraries->find( $self->branchcode() ); >- >- return $self->{_branch}; >+ my $rs = $self->_result->branchcode; >+ return Koha::Library->_new_from_dbic($rs); > } > > =head3 desk >@@ -599,10 +592,8 @@ Returns the related Koha::Patron object for this Hold > # FIXME Should be renamed with ->patron > sub borrower { > my ($self) = @_; >- >- $self->{_borrower} ||= Koha::Patrons->find( $self->borrowernumber() ); >- >- return $self->{_borrower}; >+ my $rs = $self->_result->borrowernumber; >+ return Koha::Patron->_new_from_dbic($rs); > } > > =head3 is_suspended >diff --git a/Koha/Library/Group.pm b/Koha/Library/Group.pm >index 77a3d2196b..8c3e84f9e8 100644 >--- a/Koha/Library/Group.pm >+++ b/Koha/Library/Group.pm >@@ -42,10 +42,9 @@ Koha::Library::Group - Koha Library::Group object class > > sub parent { > my ($self) = @_; >- >- $self->{_parent} ||= Koha::Library::Groups->find( $self->parent_id ); >- >- return $self->{_parent}; >+ my $rs = $self->_result->parent; >+ return unless $rs; >+ return Koha::Library::Group->_new_from_dbic($rs); > } > > =head3 my @children = $self->children() >@@ -86,12 +85,9 @@ Returns the library for this group if one exists > > sub library { > my ($self) = @_; >- >- return unless $self->branchcode; >- >- $self->{_library} ||= Koha::Libraries->find( $self->branchcode ); >- >- return $self->{_library}; >+ my $rs = $self->_result->branchcode; >+ return unless $rs; >+ return Koha::Library->_new_from_dbic($rs); > } > > =head3 libraries >diff --git a/Koha/Object/Limit/Library.pm b/Koha/Object/Limit/Library.pm >index 557baf9038..cb2f29aa28 100644 >--- a/Koha/Object/Limit/Library.pm >+++ b/Koha/Object/Limit/Library.pm >@@ -178,11 +178,7 @@ Returns the internal resultset for the branch limitation table or creates it if > > sub _library_limit_rs { > my ($self) = @_; >- >- $self->{_library_limit_rs} ||= Koha::Database->new->schema->resultset( >- $self->_library_limits->{class} ); >- >- return $self->{_library_limit_rs}; >+ return Koha::Database->new->schema->resultset( $self->_library_limits->{class} ); > } > > 1; >diff --git a/Koha/Virtualshelf.pm b/Koha/Virtualshelf.pm >index 605916ef45..c5de149b35 100644 >--- a/Koha/Virtualshelf.pm >+++ b/Koha/Virtualshelf.pm >@@ -110,14 +110,14 @@ sub is_shelfname_valid { > > sub get_shares { > my ( $self ) = @_; >- my $rs = $self->{_result}->virtualshelfshares; >+ my $rs = $self->_result->virtualshelfshares; > my $shares = Koha::Virtualshelfshares->_new_from_dbic( $rs ); > return $shares; > } > > sub get_contents { > my ( $self ) = @_; >- my $rs = $self->{_result}->virtualshelfcontents; >+ my $rs = $self->_result->virtualshelfcontents; > my $contents = Koha::Virtualshelfcontents->_new_from_dbic( $rs ); > return $contents; > } >diff --git a/Koha/Virtualshelfshare.pm b/Koha/Virtualshelfshare.pm >index 8a9252efe2..96ac665844 100644 >--- a/Koha/Virtualshelfshare.pm >+++ b/Koha/Virtualshelfshare.pm >@@ -89,7 +89,7 @@ sub has_expired { > > sub sharee { > my $self = shift; >- return Koha::Patron->_new_from_dbic( $self->{_result}->borrowernumber ); >+ return Koha::Patron->_new_from_dbic( $self->_result->borrowernumber ); > } > > =head3 _type >-- >2.40.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32894
:
147499
|
147500
|
147502
|
147503
|
150016
|
150017
|
150018
|
150019
|
150020
|
150021
|
150827
|
150828
|
150847
|
150848
|
150849
|
150850
|
150851
|
150852
|
150853
|
150854
|
150855
|
152086
|
152087
|
152088
|
152089
|
152090
|
152091
|
152092
|
152093
|
152094