From c9b6fccd63e51e7948c75298ccbfa6f1d0540df0 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Tue, 17 Oct 2023 18:02:50 +0200 Subject: [PATCH] Bug 32476: Add support for caching methods with arguments --- Koha/Object.pm | 59 ++++++++++++++++++++++++++--------- Koha/Patron.pm | 12 +++---- t/db_dependent/Koha/Patrons.t | 10 +++--- 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/Koha/Object.pm b/Koha/Object.pm index 7452d615e0f..4086caf3169 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -25,6 +25,8 @@ use Mojo::JSON; use Scalar::Util qw( blessed looks_like_number ); use Try::Tiny qw( catch try ); use List::MoreUtils qw( any ); +use Data::Dumper; +use Digest::MD5 qw( md5_hex ); use Koha::Database; use Koha::Exceptions::Object; @@ -96,10 +98,33 @@ my $instance_cache_key = Koha::Object->_instance_cache_key($key); sub _instance_cache_key { my ($self, $key) = @_; - warn "_instance_cache_key must not be called on object that has not been stored" unless $self->id; + croak "_instance_cache_key must not be called on object that has not been stored" unless $self->id; return $self->_type . ':' . $self->id . ':' . $key; } +=head3 Koha::Object->_method_cache_key(); + +my $method_cache_key = Koha::Object->_method_cache_key($key, @args); + +=cut + +sub _method_cache_key { + my ($self, $method, @args) = @_; + if (@args) { + if(ref $args[0] eq 'HASH') { + if (!%{$args[0]} && @args == 1) { + return $method; + } + } + else { + croak "First argument to _instance_cache_key must be hashref"; + } + local $Data::Dumper::Sortkeys = 1; + return $method . md5_hex(Dumper(\@args)); + } + return $method; +} + =head3 Koha::Object->_instance_cache_get(); my $value = Koha::Object->_instance_cache_get($cache_key); @@ -123,7 +148,6 @@ Koha::Object->_instance_cache_set($cache_key, $value); sub _instance_cache_set { my ($self, $key, $value) = @_; return unless $self->id; - $self->{_cache}->{$key} = $value; return Koha::Cache::Memory::Lite->get_instance->set_in_cache( $self->_instance_cache_key($key), $value @@ -133,7 +157,6 @@ sub _instance_cache_set { =head3 Koha::Object->_instance_cache_clear($cache_key); Koha::Object->_instance_cache_clear($cache_key); -Koha::Object->_instance_cache_clear(); =cut @@ -145,34 +168,40 @@ sub _instance_cache_clear { ) } -=head3 Koha::Object->_accessor_cache($method_name); +=head3 Koha::Object->_method_cache($method_name); -my $value = Koha::Object->_accessor_cache($method_name); +my $value = Koha::Object->_method_cache($method_name, @args); =cut -sub _accessor_cache { - my ($self, $method_name) = @_; +sub _method_cache { + my ($self, $method_name, @args) = @_; - my $value = $self->_instance_cache_get($method_name); + my $cache_key = $self->_method_cache_key($method_name, @args); + my $value = $self->_instance_cache_get($cache_key); return $value if defined $value; - $value = $self->$method_name(); - $self->_instance_cache_set($method_name, $value); + if (@args) { + delete $args[0]->{cache}; + } + $value = $self->$method_name(@args); + + $self->_instance_cache_set($cache_key, $value); return $value; } -=head3 Koha::Object->accessor_cache_clear($method_name); +=head3 Koha::Object->_method_cache_clear($method_name, @args); -Koha::Object->accessor_cache_clear($method_name); +Koha::Object->_method_cache_clear($method_name, @args); =cut -sub accessor_cache_clear { - my ($self, $method_name) = @_; - $self->_instance_cache_clear($method_name); +sub _method_cache_clear { + my ($self, $method_name, @args) = @_; + my $cache_key = $self->_method_cache_key($method_name, @args); + $self->_instance_cache_clear($cache_key); } =head3 Koha::Object->_new_from_dbic(); diff --git a/Koha/Patron.pm b/Koha/Patron.pm index ffa80c247e2..dfdbceddf67 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -845,7 +845,7 @@ Override Patron::dateexpiry to invalidate Patron::is_expired cache. sub dateexpiry { my $self = shift @_; if (@_) { - $self->_instance_cache_clear('is_expired'); + $self->_method_cache_clear('is_expired'); } return $self->SUPER::dateexpiry(@_); } @@ -860,7 +860,7 @@ sub set { my $self = shift @_; my ($properties) = @_; if (exists $properties->{dateexpiry}) { - $self->_instance_cache_clear('is_expired'); + $self->_method_cache_clear('is_expired'); } return $self->SUPER::set(@_); } @@ -877,9 +877,9 @@ Returns 1 if the patron is expired or 0; sub is_expired { my ($self, $params) = @_; $params //= {}; - return $self->_accessor_cache('is_expired') if $params->{cache}; + return $self->_method_cache('is_expired') if $params->{cache}; - $self->_instance_cache_clear('is_expired'); + $self->_method_cache_clear('is_expired'); return $self->dateexpiry && $self->dateexpiry !~ '^9999' && dt_from_string( $self->dateexpiry ) < dt_from_string->truncate( to => 'day' ) ? 1 : 0; @@ -1137,9 +1137,9 @@ sub has_overdues { my ($self, $params) = @_; $params //= {}; - return $self->_accessor_cache('has_overdues') if $params->{cache}; + return $self->_method_cache('has_overdues', $params) if $params->{cache}; - $self->_instance_cache_clear('has_overdues'); + $self->_method_cache_clear('has_overdues', $params); my $date = dt_from_string(); my $dtf = Koha::Database->new->schema->storage->datetime_parser; return $self->_result->issues->search({ date_due => { '<' => $dtf->format_datetime($date) } })->count; diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 10264a3f901..de4414dc78a 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -275,10 +275,10 @@ subtest 'has_overdues' => sub { is( $retrieved_patron->has_overdues, 0, ); # Cache is set - is( $retrieved_patron->has_overdues({ cache => 1}), 0, ); + is( $retrieved_patron->has_overdues({ cache => 1 }), 0); is( $retrieved_patron->_instance_cache_get('has_overdues'), 0, 'has_overdues cache has been set'); # Cache is used - is( $retrieved_patron->has_overdues({ cache => 1}), 0, 'has_overdues cache is used'); + is( $retrieved_patron->has_overdues({ cache => 1 }), 0, 'has_overdues cache is used'); $issue->delete(); my $yesterday = DateTime->today(time_zone => C4::Context->tz())->add( days => -1 ); @@ -339,9 +339,9 @@ subtest 'is_expired' => sub { # Set cache $patron->is_expired({ cache => 1 }); - # Clear cache using public method accessor_cache_clear - $patron->accessor_cache_clear('is_expired'); - is( $patron->_instance_cache_get('is_expired'), undef , 'Cache has been invalidated after calling accessor_cache_clear' ); + # Clear cache using _method_cache_clear + $patron->_method_cache_clear('is_expired'); + is( $patron->_instance_cache_get('is_expired'), undef , 'Cache has been invalidated after calling _method_cache_clear' ); $patron->delete; }; -- 2.43.0