From fded2d236fd786e727ad9623eecbd4a2127dc940 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 3a364ac8d0c..a4f64c36fc6 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 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 3b697ec5362..5844c537af6 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 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 e2bdb0425f5..f275530d974 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 ec39715a547..437ef58bd13 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 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