From 81d13b40fbc0dac54d0942e0a1907afe9da8aa5d Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Mon, 19 Apr 2021 14:24:30 +0200
Subject: [PATCH] Bug 28172: Optimize Koha::Plugins::GetPlugins

Instead of doing one SQL query for retrieving a list of plugin classes
(filtered or not by a method name) + one SQL query for each loaded
plugin (to look up the 'enabled' status), we can do a single SQL query,
fetching only enabled plugins, and do the method check using `can`.

It allows to use the Module::Load::Conditional cache when requesting
only enabled modules, which gives a great performance boost

Also, call get_metadata only if needed

Test plan:
1. Make sure you have some plugins enabled
2. Apply the 1st patch ("Test script for GetPlugins")
3. Run the test script several times, copy each result into a file A
4. Apply the other patch(es)
5. Run the test script several times, copy each result into another
   file B
6. Compare the results. Times in file B should be smaller than in file A
---
 Koha/Plugins.pm | 67 ++++++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm
index 36e6090415..ea8639a944 100644
--- a/Koha/Plugins.pm
+++ b/Koha/Plugins.pm
@@ -64,21 +64,20 @@ Calls a plugin method for all enabled plugins
 sub call {
     my ($class, $method, @args) = @_;
 
-    my @responses;
-    if (C4::Context->config('enable_plugins')) {
-        my @plugins = $class->new({ enable_plugins => 1 })->GetPlugins({ method => $method });
-        @plugins = grep { $_->can($method) } @plugins;
-        foreach my $plugin (@plugins) {
-            my $response = eval { $plugin->$method(@args) };
-            if ($@) {
-                warn sprintf("Plugin error (%s): %s", $plugin->get_metadata->{name}, $@);
-                next;
-            }
+    return unless C4::Context->config('enable_plugins');
 
-            push @responses, $response;
+    my @responses;
+    my @plugins = $class->new()->GetPlugins({ method => $method });
+    foreach my $plugin (@plugins) {
+        my $response = eval { $plugin->$method(@args) };
+        if ($@) {
+            warn sprintf("Plugin error (%s): %s", $plugin->get_metadata->{name}, $@);
+            next;
         }
 
+        push @responses, $response;
     }
+
     return @responses;
 }
 
@@ -103,37 +102,41 @@ sub GetPlugins {
 
     my $method       = $params->{method};
     my $req_metadata = $params->{metadata} // {};
-
-    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 $all          = $params->{all} // 0;
+    my $nocache      = 0;
+
+    my @plugin_classes;
+    if ($all) {
+        @plugin_classes = $self->plugins();
+
+        # Do not use Module::Load::Conditional cache if we want all plugins
+        # as $self->plugins can return uninstalled plugins
+        $nocache = 1;
+    } else {
+        my $rs = Koha::Database->schema->resultset('PluginData');
+        $rs = $rs->search({ plugin_key => '__ENABLED__', plugin_value => 1 });
+        @plugin_classes = $rs->get_column('plugin_class')->all();
+    }
 
     my @plugins;
+    foreach my $plugin_class (@plugin_classes) {
+        if ( can_load( modules => { $plugin_class => undef }, nocache => $nocache ) ) {
+            next unless $plugin_class->isa('Koha::Plugins::Base');
+            next if $method && !$plugin_class->can($method);
 
-    # Loop through all plugins that implement at least a method
-    while ( my $plugin_class = $plugin_classes->next ) {
-
-        if ( can_load( modules => { $plugin_class => undef }, nocache => 1 ) ) {
             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?
             });
 
-            next unless $plugin->is_enabled or
-                        defined($params->{all}) && $params->{all};
-
             # filter the plugin out by metadata
-            my $plugin_metadata = $plugin->get_metadata;
-            next
-                if $plugin_metadata
-                and %$req_metadata
-                and any { !$plugin_metadata->{$_} || $plugin_metadata->{$_} ne $req_metadata->{$_} } keys %$req_metadata;
+            if (%$req_metadata) {
+                my $plugin_metadata = $plugin->get_metadata;
+                next
+                    if $plugin_metadata
+                    and any { !$plugin_metadata->{$_} || $plugin_metadata->{$_} ne $req_metadata->{$_} } keys %$req_metadata;
+            }
 
             push @plugins, $plugin;
         } elsif ( defined($params->{errors}) && $params->{errors} ){
-- 
2.20.1