From 68216c541bfd9b5baf5e3fe97c0a99d193dfccce Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 Signed-off-by: Nick Clemens --- 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 3a364ac8d0..a4f64c36fc 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -89,12 +89,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 3b697ec536..5844c537af 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -571,7 +571,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 @@ -707,10 +707,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 e2bdb0425f..f275530d97 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 ec39715a54..437ef58bd1 100644 --- a/Koha/Virtualshelf.pm +++ b/Koha/Virtualshelf.pm @@ -112,14 +112,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.39.5