From ef9a3a28bde2f41379f1e168dca04c228f25c92e Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 10 May 2024 11:38:14 -0300 Subject: [PATCH] Bug 34978: (QA follow-up) Simplify code and make it stricter This patch simplifies the code a bit, removing duplicated pieces of code and making it slightly more readable. The main change is that it now just filters the `@plugin_classes` list as required, and leaves the original codebase for dealing with loading the plugins untouched. A BIG change it implements is that it now requires full class names. I don't see the use case for partial names, and I generally think we should be stricter in this area. If there's the need for partial class names, this should be handled with specific parameters, etc. It also makes it explicit that the `include` and `exclude` parameters are mutually exclusive. A minor idiomatic issue is fixed as well. Signed-off-by: Tomas Cohen Arazi --- Koha/Plugins.pm | 56 ++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm index c8a02ed7055..636a4d56925 100644 --- a/Koha/Plugins.pm +++ b/Koha/Plugins.pm @@ -21,7 +21,7 @@ use Modern::Perl; use Array::Utils qw( array_minus ); use Class::Inspector; -use List::MoreUtils qw( any ); +use List::MoreUtils qw( any none ); use Module::Load::Conditional qw( can_load ); use Module::Load; use Module::Pluggable search_path => ['Koha::Plugin'], except => qr/::Edifact(|::Line|::Message|::Order|::Segment|::Transport)$/; @@ -32,6 +32,7 @@ use C4::Context; use C4::Output; use Koha::Cache::Memory::Lite; +use Koha::Exceptions; use Koha::Exceptions::Plugin; use Koha::Plugins::Datas; use Koha::Plugins::Methods; @@ -296,46 +297,43 @@ sub InstallPlugins { my $verbose = $params->{verbose} // $self->_verbose; my @plugin_classes = $self->plugins(); - my ( @plugins, @classes_filters ); + my @plugins; - my $has_filters = defined( $params->{include} ) || defined( $params->{exclude} ); + Koha::Exceptions::BadParameter->throw("Only one of 'include' and 'exclude' can be passed") + if ( $params->{exclude} && $params->{include} ); - # Warn user if the specified classes doesn't exist and return nothing - if ($has_filters) { - @classes_filters = defined( $params->{include} ) ? @{ $params->{include} } : @{ $params->{exclude} }; + if ( defined( $params->{include} ) || defined( $params->{exclude} ) ) { + my @classes_filters = + defined( $params->{include} ) + ? @{ $params->{include} } + : @{ $params->{exclude} }; - foreach my $classes_filter (@classes_filters) { - my $is_found = 0; - - foreach my $plugin_class (@plugin_classes) { - $is_found = 1 - if $plugin_class =~ ":$classes_filter\$|^$classes_filter\$" - || ( $classes_filter =~ "^::" && $plugin_class =~ "$classes_filter\$" ); - } - unless ($is_found) { - warn "$classes_filter have not been found, try a different name"; + # Warn user if the specified classes doesn't exist and return nothing + foreach my $class_name (@classes_filters) { + unless ( any { $class_name eq $_ } @plugin_classes ) { + warn "$class_name has not been found, try a different name"; return; } } + + # filter things + if ( $params->{include} ) { + @plugin_classes = grep { + my $plugin_class = $_; + any { $plugin_class eq $_ } @classes_filters + } @plugin_classes; + } else { # exclude + @plugin_classes = grep { + my $plugin_class = $_; + none { $plugin_class eq $_ } @classes_filters + } @plugin_classes; + } } foreach my $plugin_class (@plugin_classes) { if ( can_load( modules => { $plugin_class => undef }, verbose => $verbose, nocache => 1 ) ) { next unless $plugin_class->isa('Koha::Plugins::Base'); - # Apply the filters - if ($has_filters) { - my $is_found = 0; - foreach my $classes_filter (@classes_filters) { - $is_found = 1 - if $plugin_class =~ ":$classes_filter\$|^$classes_filter\$" - || ( $classes_filter =~ "^::" && $plugin_class =~ "$classes_filter\$" ); - } - next - if ( defined( $params->{include} ) && !$is_found ) - || ( defined( $params->{exclude} ) && $is_found ); - } - my $plugin; my $failed_instantiation; -- 2.45.0