From dcd16f94a5ba713487183a4eee9ee293af7072c3 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 --- 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