From 68216c541bfd9b5baf5e3fe97c0a99d193dfccce 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>
Signed-off-by: Nick Clemens <nick@bywatersolutions.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 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