[Koha-bugs] [Bug 21073] Improve plugin performance

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21073

--- Comment #49 from Martin Renvoize <martin.renvoize at 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.