From 0cab0ad9cea3545d3180ae10676762a7c2b6ec9d Mon Sep 17 00:00:00 2001 From: Srdjan Date: Wed, 27 Jan 2016 16:57:18 +1300 Subject: [PATCH] Removed Koha::Cache->get_instance() There should be no cache singleton, full stop. If Koha is to move away from .pl scripts that is. As an interim measure Koha::Cache->get_instance() is replaced with C4::Context->cache, in the vein of C4::Context->memcached. In that respect it will continue to work in the singleton-ish way if context is used as a singleton, but supports cache-per-context. https://bugs.koha-community.org/show_bug.cgi?id=15562 --- C4/Biblio.pm | 4 +- C4/Calendar.pm | 15 +++-- C4/Context.pm | 111 +++++++++++++++++++++------------ C4/External/OverDrive.pm | 6 +- C4/Koha.pm | 3 +- C4/Utils/DataTables/ColumnsSettings.pm | 3 +- Koha/Cache.pm | 17 ----- Koha/Calendar.pm | 5 +- Koha/Template/Plugin/Cache.pm | 4 +- admin/biblio_framework.pl | 3 +- admin/koha2marclinks.pl | 2 +- admin/marc_subfields_structure.pl | 2 +- admin/marctagstructure.pl | 5 +- opac/svc/report | 5 +- svc/report | 4 +- t/Cache.t | 4 +- t/Calendar.t | 4 +- t/Context.t | 2 +- t/Koha_Template_Plugin_Cache.t | 4 +- t/db_dependent/Context.t | 10 +-- tools/newHolidays.pl | 4 +- 21 files changed, 108 insertions(+), 109 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 10faa83..6adcb1a 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -37,8 +37,8 @@ use C4::ClassSource; use C4::Charset; use C4::Linker; use C4::OAI::Sets; +use C4::Context; -use Koha::Cache; use Koha::Authority::Types; use vars qw($VERSION @ISA @EXPORT); @@ -1096,7 +1096,7 @@ sub GetMarcStructure { $frameworkcode = "" unless $frameworkcode; $forlibrarian = $forlibrarian ? 1 : 0; - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; my $cache_key = "MarcStructure-$forlibrarian-$frameworkcode"; my $cached = $cache->get_from_cache($cache_key); return $cached if $cached; diff --git a/C4/Calendar.pm b/C4/Calendar.pm index 99b0c3a..7e9e505 100644 --- a/C4/Calendar.pm +++ b/C4/Calendar.pm @@ -23,7 +23,6 @@ use Carp; use Date::Calc qw( Date_to_Days Today); use C4::Context; -use Koha::Cache; use constant ISO_DATE_FORMAT => "%04d-%02d-%02d"; @@ -276,7 +275,7 @@ sub insert_single_holiday { # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; return $self; @@ -320,7 +319,7 @@ sub insert_exception_holiday { $self->{'exception_holidays'}->{"$options{year}/$options{month}/$options{day}"}{description} = $options{description}; # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; return $self; @@ -420,7 +419,7 @@ UPDATE special_holidays SET title = ?, description = ? $self->{'single_holidays'}->{"$options{year}/$options{month}/$options{day}"}{description} = $options{description}; # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; return $self; @@ -462,7 +461,7 @@ UPDATE special_holidays SET title = ?, description = ? $self->{'exception_holidays'}->{"$options{year}/$options{month}/$options{day}"}{description} = $options{description}; # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; return $self; @@ -542,7 +541,7 @@ sub delete_holiday { } # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; return $self; @@ -572,7 +571,7 @@ sub delete_holiday_range { $sth->execute($self->{branchcode}, $options{day}, $options{month}, $options{year}); # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; } @@ -625,7 +624,7 @@ sub delete_exception_holiday_range { $sth->execute($self->{branchcode}, $options{day}, $options{month}, $options{year}); # changed the 'single_holidays' table, lets force/reset its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; } diff --git a/C4/Context.pm b/C4/Context.pm index ec45d91..d99e606 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -18,7 +18,7 @@ package C4::Context; use strict; use warnings; -use vars qw($VERSION $AUTOLOAD $context @context_stack $servers $memcached $ismemcached); +use vars qw($VERSION $AUTOLOAD $context @context_stack $memcached_servers); BEGIN { if ($ENV{'HTTP_USER_AGENT'}) { require CGI::Carp; @@ -79,20 +79,9 @@ BEGIN { } # else there is no browser to send fatals to! # Check if there are memcached servers set - $servers = $ENV{'MEMCACHED_SERVERS'}; - if ($servers) { - # Load required libraries and create the memcached object - require Cache::Memcached; - $memcached = Cache::Memcached->new({ - servers => [ $servers ], - debug => 0, - compress_threshold => 10_000, - expire_time => 600, - namespace => $ENV{'MEMCACHED_NAMESPACE'} || 'koha' - }); - # Verify memcached available (set a variable and test the output) - $ismemcached = $memcached->set('ismemcached','1'); - } + $memcached_servers = $ENV{'MEMCACHED_SERVERS'}; + # Load required libraries and create the memcached object + require Cache::Memcached if $memcached_servers; $VERSION = '3.07.00.049'; } @@ -109,6 +98,7 @@ use Carp; use C4::Boolean; use C4::Debug; use Koha; +use Koha::Cache; use Koha::Config::SysPrefs; =head1 NAME @@ -209,29 +199,61 @@ sub current { return $context; } -=head2 ismemcached +sub _new_memcached { + my $namespace = shift or die "No memcached namespace"; + + return unless $memcached_servers; + return Cache::Memcached->new({ + servers => [ $memcached_servers ], + debug => 0, + compress_threshold => 10_000, + expire_time => 600, + namespace => $namespace + }); +} +# Verify memcached available (test the output) +sub _ping_memcached { + my $memcached = shift or croak "No memcached"; + + return $memcached->set('ismemcached','1'); +} + +=head2 cache -Returns the value of the $ismemcached variable (0/1) +Returns the cache object or undef =cut -sub ismemcached { - return $ismemcached; +sub cache { + my $self = shift; + + $self = $context unless ref ($self); + return unless $self; + + return $self->{cache} ||= Koha::Cache->new(); } =head2 memcached -If $ismemcached is true, returns the $memcache variable. -Returns undef otherwise +Returns the memcached object or undef + +=head2 ismemcached =cut sub memcached { - if ($ismemcached) { - return $memcached; - } else { - return; - } + my $self = shift; + + $self = $context unless ref ($self); + return unless $self; + + my $memcached = $self->{memcached} or return; + return _ping_memcached($memcached) ? $memcached : undef; +} + +sub ismemcached { + my $self = shift; + return $self->memcached; } sub db_driver { @@ -273,10 +295,13 @@ sub import { # default context already exists? return if $context; - if ($ismemcached) { + return if $config_file && $config_file eq ":no_config"; + + my $memcached = _new_memcached($ENV{'MEMCACHED_NAMESPACE'} || 'koha'); + if ($memcached) { # retrieve from memcached - if (my $self = $memcached->get('kohaconf')) { - $context = $self; + if ($context = $memcached->get('kohaconf')) { + $context->{memcached} = $memcached; return; } } @@ -303,15 +328,10 @@ sub import { } # no ? so load it! - return if $config_file && $config_file eq ":no_config"; - my $new_ctx = __PACKAGE__->new($config_file); - return unless $new_ctx; - - # if successfully loaded, use it by default - $context = $new_ctx; - - if ($ismemcached) { - $memcached->set('kohaconf',$new_ctx); + $context = $pkg->new($config_file) or return; + if ( $memcached && _ping_memcached($memcached) ) { + $context->{memcached} = $memcached; + $memcached->set('kohaconf',$context); } } @@ -354,6 +374,8 @@ that, use C<&set_context>. sub new { my $class = shift; my $conf_fname = shift or croak "No conf"; + my $memcached_namespace = shift; + my $cache = shift; my $self = XMLin( $conf_fname, @@ -365,6 +387,15 @@ sub new { unless ref($self) && $self->{"config"}; $self->{"config_file"} = "$conf_fname"; + if ($memcached_namespace) { + my $memcached = $context->{memcached}; + $self->{memcached} = $memcached && $memcached->{namespace} eq $memcached_namespace + ? $memcached + : _new_memcached($memcached_namespace); + # Koha::Cache is far too complex to try to make any savings + $cache ||= Koha::Cache->new( {namespace => $memcached_namespace} ); + } + $self->{cache} = $cache; $self->{"Zconn"} = undef; # Zebra Connections $self->{"marcfromkohafield"} = undef; # the hash with relations between koha table fields and MARC field/subfield @@ -425,8 +456,8 @@ sub set_context $schema = $self->{schema} ||= Koha::Database->new_schema($self); } - # Save the old context, if any, on the stack - push @context_stack, $context if defined($context); + # Save the old context on the stack + push @context_stack, $context; # Set the new context $context = $new_context; diff --git a/C4/External/OverDrive.pm b/C4/External/OverDrive.pm index 369377a..5cd39ab 100644 --- a/C4/External/OverDrive.pm +++ b/C4/External/OverDrive.pm @@ -22,7 +22,7 @@ use warnings; use Koha; use JSON; -use Koha::Cache; +use C4::Context; use HTTP::Request; use HTTP::Request::Common; use LWP::Authen::Basic; @@ -98,9 +98,7 @@ sub GetOverDriveToken { return unless ( $key && $secret ) ; - my $cache; - - eval { $cache = Koha::Cache->get_instance() }; + my $cache = C4::Context->cache; my $token; $cache and $token = $cache->get_from_cache( "overdrive_token" ) and return $token; diff --git a/C4/Koha.pm b/C4/Koha.pm index 7171f16..15ece1e 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -25,7 +25,6 @@ use strict; use C4::Context; use C4::Branch; # Can be removed? -use Koha::Cache; use Koha::DateUtils qw(dt_from_string); use Koha::Libraries; use DateTime::Format::MySQL; @@ -1167,7 +1166,7 @@ sub GetAuthorisedValues { my $selected_key = defined($selected) ? $selected : ''; my $cache_key = "AuthorisedValues-$category-$selected_key-$opac-$branch_limit"; - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; my $result = $cache->get_from_cache($cache_key); return $result if $result; diff --git a/C4/Utils/DataTables/ColumnsSettings.pm b/C4/Utils/DataTables/ColumnsSettings.pm index eb0ca54..de65df9 100644 --- a/C4/Utils/DataTables/ColumnsSettings.pm +++ b/C4/Utils/DataTables/ColumnsSettings.pm @@ -5,11 +5,10 @@ use List::Util qw( first ); use YAML; use C4::Context; use Koha::Database; -use Koha::Cache; sub get_yaml { my $yml_path = C4::Context->config('intranetdir') . '/admin/columns_settings.yml'; - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; my $yaml = $cache->get_from_cache('ColumnsSettingsYaml'); unless ($yaml) { diff --git a/Koha/Cache.pm b/Koha/Cache.pm index 9856e80..25dfa59 100644 --- a/Koha/Cache.pm +++ b/Koha/Cache.pm @@ -46,23 +46,6 @@ use base qw(Class::Accessor); __PACKAGE__->mk_ro_accessors( qw( cache memcached_cache fastmmap_cache memory_cache )); -=head2 get_instance - - my $cache = Koha::Cache->get_instance(); - -This gets a shared instance of the cache, set up in a very default way. This is -the recommended way to fetch a cache object. If possible, it'll be -persistent across multiple instances. - -=cut - -our $singleton_cache; -sub get_instance { - my ($class) = @_; - $singleton_cache = $class->new() unless $singleton_cache; - return $singleton_cache; -} - =head2 new Create a new Koha::Cache object. This is required for all cache-related functionality. diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index a0b642b..2e4cf67 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -7,7 +7,6 @@ use DateTime; use DateTime::Set; use DateTime::Duration; use C4::Context; -use Koha::Cache; use Carp; sub new { @@ -57,7 +56,7 @@ sub _init { # lists breaks persistance engines. As of 2013-12-10, the RM # is allowing this with the expectation that prior to release of # 3.16, bug 8089 will be fixed and we can switch the caching over -# to Koha::Cache. +# to external cache our $exception_holidays; @@ -92,7 +91,7 @@ sub exception_holidays { sub single_holidays { my ( $self, $date ) = @_; my $branchcode = $self->{branchcode}; - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; my $single_holidays = $cache->get_from_cache('single_holidays'); # $single_holidays looks like: diff --git a/Koha/Template/Plugin/Cache.pm b/Koha/Template/Plugin/Cache.pm index 3b8f96c..5990626 100644 --- a/Koha/Template/Plugin/Cache.pm +++ b/Koha/Template/Plugin/Cache.pm @@ -22,7 +22,6 @@ use warnings; use vars qw( $VERSION ); use base qw( Template::Plugin ); use Template::Plugin; -use C4::Context; $VERSION = '0.01'; #------------------------------------------------------------------------ @@ -36,8 +35,7 @@ sub new { $cache = delete $params->{cache}; } else { - require Koha::Cache; - $cache = Koha::Cache->get_instance(); + $cache = $context->cache; } my $self = bless { CACHE => $cache, diff --git a/admin/biblio_framework.pl b/admin/biblio_framework.pl index 79a0db1..79f7060 100755 --- a/admin/biblio_framework.pl +++ b/admin/biblio_framework.pl @@ -26,12 +26,11 @@ use C4::Output; use Koha::Biblios; use Koha::BiblioFramework; use Koha::BiblioFrameworks; -use Koha::Cache; my $input = new CGI; my $frameworkcode = $input->param('frameworkcode') || q||; my $op = $input->param('op') || q|list|; -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; my @messages; my ( $template, $borrowernumber, $cookie ) = get_template_and_user( diff --git a/admin/koha2marclinks.pl b/admin/koha2marclinks.pl index 9ccca37..68f1e55 100755 --- a/admin/koha2marclinks.pl +++ b/admin/koha2marclinks.pl @@ -59,7 +59,7 @@ else { } my $dbh = C4::Context->dbh; -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; ################## ADD_FORM ################################## # called by default. Used to create form to add or modify a record diff --git a/admin/marc_subfields_structure.pl b/admin/marc_subfields_structure.pl index e6d2231..04ce003 100755 --- a/admin/marc_subfields_structure.pl +++ b/admin/marc_subfields_structure.pl @@ -78,7 +78,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( debug => 1, } ); -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; my $op = $input->param('op'); $tagfield =~ s/\,//g; diff --git a/admin/marctagstructure.pl b/admin/marctagstructure.pl index 7728687..26406f4 100755 --- a/admin/marctagstructure.pl +++ b/admin/marctagstructure.pl @@ -25,9 +25,6 @@ use C4::Auth; use C4::Koha; use C4::Context; use C4::Output; -use C4::Context; - -use Koha::Cache; # retrieve parameters my $input = new CGI; @@ -46,7 +43,7 @@ my $pagesize = 20; my $script_name = "/cgi-bin/koha/admin/marctagstructure.pl"; my $dbh = C4::Context->dbh; -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; # open template my ($template, $loggedinuser, $cookie) diff --git a/opac/svc/report b/opac/svc/report index bfc84e5..98d2aeb 100755 --- a/opac/svc/report +++ b/opac/svc/report @@ -23,12 +23,11 @@ use Modern::Perl; +use C4::Context; use C4::Reports::Guided; use JSON; use CGI qw ( -utf8 ); -use Koha::Cache; - my $query = CGI->new(); my $report_id = $query->param('id'); my $report_name = $query->param('name'); @@ -41,7 +40,7 @@ die "Sorry this report is not public\n" unless $report_rec->{public}; my @sql_params = $query->param('sql_params'); -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; my $cache_active = $cache->is_cache_active; my ($cache_key, $json_text); if ($cache_active) { diff --git a/svc/report b/svc/report index da1b9b3..d380090 100755 --- a/svc/report +++ b/svc/report @@ -25,7 +25,7 @@ use C4::Reports::Guided; use JSON; use CGI qw ( -utf8 ); -use Koha::Cache; +use C4::Context; my $query = CGI->new(); @@ -48,7 +48,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; my $cache_active = $cache->is_cache_active; my ($cache_key, $json_text); if ($cache_active) { diff --git a/t/Cache.t b/t/Cache.t index 4c50a2b..77c9a92 100644 --- a/t/Cache.t +++ b/t/Cache.t @@ -31,7 +31,7 @@ SKIP: { # Set a special namespace for testing, to avoid breaking # if test is run with a different user than Apache's. $ENV{ MEMCACHED_NAMESPACE } = 'unit_tests'; - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; skip "Cache not enabled", 28 unless ( $cache->is_cache_active() && defined $cache ); @@ -167,7 +167,7 @@ SKIP: { END { SKIP: { $ENV{ MEMCACHED_NAMESPACE } = 'unit_tests'; - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; skip "Cache not enabled", 1 unless ( $cache->is_cache_active() ); is( $destructorcount, 1, 'Destructor run exactly once' ); diff --git a/t/Calendar.t b/t/Calendar.t index 37f11f4..720d5c0 100755 --- a/t/Calendar.t +++ b/t/Calendar.t @@ -22,7 +22,7 @@ use Test::MockModule; use DateTime; use DateTime::Duration; -use Koha::Cache; +use C4::Context; use Koha::DateUtils; use Module::Load::Conditional qw/check_install/; @@ -88,7 +88,7 @@ fixtures_ok [ ], ], "add fixtures"; -my $cache = Koha::Cache->get_instance(); +my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; # 'MPL' branch is arbitrary, is not used at all but is needed for initialization diff --git a/t/Context.t b/t/Context.t index c3855cd..4fa0e83 100755 --- a/t/Context.t +++ b/t/Context.t @@ -2,7 +2,7 @@ use Modern::Perl; use DBI; -use Test::More tests => 24; +use Test::More tests => 29; use Test::MockModule; BEGIN { diff --git a/t/Koha_Template_Plugin_Cache.t b/t/Koha_Template_Plugin_Cache.t index da20f61..15ee048 100644 --- a/t/Koha_Template_Plugin_Cache.t +++ b/t/Koha_Template_Plugin_Cache.t @@ -1,6 +1,8 @@ use Modern::Perl; use Test::More tests => 2; +use C4::Context; + use_ok('Koha::Template::Plugin::Cache'); -ok(my $cache = Koha::Template::Plugin::Cache->new()); +ok(my $cache = Koha::Template::Plugin::Cache->new(C4::Context->current)); diff --git a/t/db_dependent/Context.t b/t/db_dependent/Context.t index cecb10e..4d50390 100755 --- a/t/db_dependent/Context.t +++ b/t/db_dependent/Context.t @@ -12,14 +12,8 @@ use Koha::Database; BEGIN { $debug = $ENV{DEBUG} || 0; - - # Note: The overall number of tests may vary by configuration. - # First we need to check your environmental variables - for (qw(KOHA_CONF PERL5LIB)) { - ok( $ret = $ENV{$_}, "ENV{$_} = $ret" ); - } - use_ok('C4::Context'); } +use_ok('C4::Context'); ok($dbh = C4::Context->dbh(), 'Getting dbh from C4::Context'); @@ -98,6 +92,8 @@ ok($trace_read, 'Retrieved syspref from database'); $trace_read = q{}; C4::Context->enable_syspref_cache(); +C4::Context->preference("SillyPreference"); +$trace_read = q{}; is(C4::Context->preference("SillyPreference"), 'thing3', "Retrieved syspref (value='thing3') successfully from cache"); is( $trace_read, q{}, 'Did not retrieve syspref from database'); $trace_read = q{}; diff --git a/tools/newHolidays.pl b/tools/newHolidays.pl index eda4c1b..38f290c 100755 --- a/tools/newHolidays.pl +++ b/tools/newHolidays.pl @@ -10,7 +10,7 @@ use CGI qw ( -utf8 ); use C4::Auth; use C4::Output; -use Koha::Cache; +use C4::Context; use C4::Calendar; use DateTime; @@ -129,6 +129,6 @@ sub add_holiday { } } # we updated the single_holidays table, so wipe its cache - my $cache = Koha::Cache->get_instance(); + my $cache = C4::Context->cache; $cache->clear_from_cache( 'single_holidays') ; } -- 1.9.1