From ef9a3a28bde2f41379f1e168dca04c228f25c92e Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
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 <tomascohen@theke.io>
---
 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