From 6cb6d4ec26a985d6fd02940aaca2a611da6ae81b Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 17 Jun 2019 08:07:03 -0300 Subject: [PATCH] Bug 21073: (QA follow-up) Simplify logic This patch simplifies the logic inside GetPlugins so: - It uses Koha::Plugins::Methods instead of plain SQL - It doesn't do more DB calls than needed, by filtering on method in the initial query to Koha::Plugins::Methods. It also relies on the (newly introduced) ->is_enabled method in Koha::Plugins::Base, for better readability. To test: - Run the tests and notice no behaviour changes are introduced. Signed-off-by: Tomas Cohen Arazi --- Koha/Plugins.pm | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm index be1d091631..9dc42c5155 100644 --- a/Koha/Plugins.pm +++ b/Koha/Plugins.pm @@ -70,33 +70,33 @@ If you pass multiple keys in the metadata hash, all keys must match. sub GetPlugins { my ( $self, $params ) = @_; - my $method = $params->{method}; + + my $method = $params->{method}; my $req_metadata = $params->{metadata} // {}; - my $dbh = C4::Context->dbh; - my $plugin_classes = $dbh->selectcol_arrayref('SELECT DISTINCT(plugin_class) FROM plugin_methods'); + my $filter = ( $method ) ? { plugin_method => $method } : undef; + + my $plugin_classes = Koha::Plugins::Methods->search( + $filter, + { columns => 'plugin_class', + distinct => 1 + } + )->_resultset->get_column('plugin_class'); + my @plugins; # Loop through all plugins that implement at least a method - foreach my $plugin_class (@$plugin_classes) { - # filter the plugin out by method - next - if $method - && !Koha::Plugins::Methods->search( - { plugin_class => $plugin_class, plugin_method => $method } )->count; + while ( my $plugin_class = $plugin_classes->next ) { load $plugin_class; - my $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} }); + my $plugin = $plugin_class->new({ + enable_plugins => $self->{'enable_plugins'} + # loads even if plugins are disabled + # FIXME: is this for testing without bothering to mock config? + }); - my $plugin_enabled = $plugin->retrieve_data('__ENABLED__'); - $plugin->{enabled} = $plugin_enabled; - - # Want all plugins. Not only enabled ones. - if ( defined($params->{all}) && $params->{all} ) { - $plugin_enabled = 1; - } - - next unless $plugin_enabled; + next unless $plugin->is_enabled or + defined($params->{all}) && $params->{all}; # filter the plugin out by metadata my $plugin_metadata = $plugin->get_metadata; @@ -120,6 +120,9 @@ This method iterates through all plugins physically present on a system. For each plugin module found, it will test that the plugin can be loaded, and if it can, will store its available methods in the plugin_methods table. +NOTE: We re-load all plugins here as a protective measure in case someone +has removed a plugin directly from the system without using the UI + =cut sub InstallPlugins { -- 2.22.0