[Koha-bugs] [Bug 32476] Add caching for relatively expensive patron methods

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32476

--- Comment #4 from David Gustafsson <glasklas at gmail.com> ---
I don't think it is possibly to reduce code duplication without sacrificing
readability and/or increasing code complexity. The only way I can think of at
the top of my head would be to have a general method "handle_cache_lookup" (or
some better name) that takes a cache_key and and callback for getting the
uncached value, something like this:

sub is_expired {
  my ($self, $params) = @_;

  my $get_is_expired = sub {
    my $is_expired = 
      $self->dateexpiry &&                                       
      $self->dateexpiry !~ '^9999' &&                            
      dt_from_string( $self->dateexpiry ) < dt_from_string->truncate( to =>
'day' );
    return $is_expired ? 1 : 0;
  }
  return $self->handle_cache_lookup($params->{cache}, 'Patron_is_expired' .
$self->borrowernumber, $get_is_expired);
}

sub handle_cache_lookup {
  my ($self, $use_cache, $cache_key, $get_uncached) = @_;

  my $cache = Koha::Cache::Memory::Lite->get_instance;
  if ($use_cache) {
    my $value = $cache->get_from_cache($cache_key);
    return $value if defined $value;
  }
  else {
     $cache->clear_from_cache($cache_key)
  }

  my $value = $get_uncached->();

  if ($use_cache) {
    $cache->set_in_cache($cache_key, $value);
  }
  return $value;
}

I personally sceptical it's worth the tradeoff.

I created a new version using Memory::Lite instead of class attributes for
caching. It does make the code a litte bit more verbose though even though I do
acknowledge the current way is inconsistent with how caching is performed in
the rest of the code base and a bit of a hack. I would prefer if Memory::Lite
had namespaces, right now there is only one bucket the entire cache if ->flush
is called somewhere else. Right now I think it's unlikely as the cache is
flushed in just a few places, but this is the primary reason why I opted
storing the cached values in the object itself.

-- 
You are receiving this mail because:
You are watching all bug changes.