https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21073 --- Comment #49 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 89801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89801 Bug 21073: Improve plugin performance Review of attachment 89801: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=21073&attachment=89801) ----------------------------------------------------------------- A few, hopefully minor, bits of feedback.. Failing QA whilst I await a reply. ::: Koha/Plugins.pm @@ +73,5 @@
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');
Why do we mix old dbh calls with dbic calls in this new code? Also, can we not skip a series of DB calls that are found in the loop by filtering on $method if it exists before running through the loop? @@ +119,4 @@
my $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} });
+ Koha::Plugins::Methods->search({ plugin_class => $plugin_class })->delete();
Can't make my mind up as to whether we should really rebuilding the whole db table with every/any install/upgrade of any plugin... ::: Koha/Plugins/Handler.pm @@ +63,5 @@
my $params = $args->{'params'};
+ my $has_method = Koha::Plugins::Methods->search({ plugin_class => $plugin_class, plugin_method => $plugin_method })->count(); + if ( $has_method ) { + load $plugin_class;
Is it ever possible for 'load' to fail here and if so should we catch and warn about it?.. We seem to have effectively removed a warning from the prior code.. I'm wondering if we may ever get a case where the plugin exists in the DB but has been deleted from the filesystem. @@ +68,2 @@
my $plugin = $plugin_class->new( { cgi => $cgi, enable_plugins => $args->{'enable_plugins'} } ); + my @return = $plugin->$plugin_method( $params );
The above line is never referenced right? ::: plugins/plugins-upload.pl @@ +87,4 @@
$template->param( ERRORS => [ \%errors ] ); output_html_with_http_headers $input, $cookie, $template->output; exit; + } else {
'unless else' is a weird looking construct.. as we're calling 'exit' in the unless block we can just call the 'Koha::Plugins->new()->InstallPlugins' method below the block and not in it's own else block. -- You are receiving this mail because: You are watching all bug changes.