From 3e428da7788c9a2ecdcbf65207236d3153dc649d 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 --- 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 6ab6120a1a7..822d38b74d2 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 83c88bee8c3..29a2471d512 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 5bf369cd6a1..379e891d19c 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -499,7 +499,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 @@ -635,10 +635,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 59638a5aa2e..29edbe2c423 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 a5a9d03003f..b1f49a43c5b 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 77a3d2196b7..8c3e84f9e88 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 557baf90381..cb2f29aa283 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 605916ef45a..c5de149b35c 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 8a9252efe2d..96ac6658442 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.25.1