gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 35536: Refine verbose handling in some Koha::Plugins calls 

Three routines in Plugins got the verbose parameter on 35507.
We can refine this a bit further.
The idea here is report when you are installing plugins but not
report when just calling plugins (flooding logs).

[1] GetPlugins: Most callers do not expect (or check) results for
    failing plugins. This patch makes GetPlugins only return
    errors when passing the *errors* flag (in 2 cases).
    [a] The misc/devel script prints warnings now using verbose,
        so does not need the errors flag anymore.
    [b] plugins/plugins-home is the only case left. Tiny adjustment
        to keep current behavior. Fixed colspan in template.
        Does not need verbose in favor of 'errors' (passed to
        template).
    [c] For most calls we do not want verbose. New default is 0.

[2] InstallPlugins
    [a] Disabled verbose in plugin-upload. Not really needed.
        Added a FIXME; we need to improve individual install.
    [b] misc/devel: No warnings anymore when calling InstallPlugins
        after GetPlugins.

[3] get_enabled_plugins
    [a] Plugins->call does not need verbose.
    [b] Plugins->feature_enabled does not need it too.

Test plan:

[1] See previous plan. With TestMR data but without patch, run
misc script and go to plugins-home. Do you see load errors on
commandline or form?

[2] Run plugins/plugins-upload (uploading just some file is good
enough); verify that you do not see TestMR lines in logfile.

[3] Run t/db_dependent/Koha/Plugins/Plugins.t for the additional
test on verbose and errors flag.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
(cherry picked from commit d8e04545b8)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This commit is contained in:
Marcel de Rooy 2023-12-18 08:52:20 +00:00 committed by Fridolin Somers
parent f11c05bbdd
commit b00f04a760
6 changed files with 56 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
Koha/Plugins.pm
View File

@ -81,7 +81,7 @@ sub call {
return unless C4::Context->config('enable_plugins');
my @responses;
my @plugins = $class->get_enabled_plugins();
my @plugins = $class->get_enabled_plugins( { verbose => 0 } );
@plugins = grep { $_->can($method) } @plugins;
# TODO: Remove warn when after_hold_create is removed from the codebase
@ -167,7 +167,7 @@ sub feature_enabled {
my $key = "ENABLED_PLUGIN_FEATURE_" . $method;
my $feature = Koha::Cache::Memory::Lite->get_from_cache($key);
unless ( defined($feature) ) {
my @plugins = $class->get_enabled_plugins();
my @plugins = $class->get_enabled_plugins( { verbose => 0 } );
my $enabled = any { $_->can($method) } @plugins;
Koha::Cache::Memory::Lite->set_in_cache( $key, $enabled );
}
@ -182,12 +182,20 @@ method or metadata value.
my @plugins = Koha::Plugins::GetPlugins({
method => 'some_method',
metadata => { some_key => 'some_value' },
[ verbose => 1 ],
[ all => 1, errors => 1, verbose => 1 ],
});
The method and metadata parameters are optional.
If you pass multiple keys in the metadata hash, all keys must match.
If you pass errors (only used in plugins-home), we return two arrayrefs:
( $good, $bad ) = Koha::Plugins::GetPlugins( { errors => 1 } );
If you pass verbose, you can enable or disable explicitly warnings
from Module::Load::Conditional. Disabled by default to not flood
the logs.
=cut
sub GetPlugins {
@ -195,7 +203,10 @@ sub GetPlugins {
my $method = $params->{method};
my $req_metadata = $params->{metadata} // {};
my $verbose = delete $params->{verbose} // $self->_verbose;
my $errors = $params->{errors};
# By default dont warn here unless asked to do so.
my $verbose = $params->{verbose} // 0;
my $filter = ( $method ) ? { plugin_method => $method } : undef;
@ -206,11 +217,10 @@ sub GetPlugins {
}
)->_resultset->get_column('plugin_class');
my @plugins;
# Loop through all plugins that implement at least a method
my ( @plugins, @failing );
while ( my $plugin_class = $plugin_classes->next ) {
if ( can_load( modules => { $plugin_class => undef }, verbose => $verbose, nocache => 1 ) ) {
my $plugin;
@ -241,13 +251,12 @@ sub GetPlugins {
and any { !$plugin_metadata->{$_} || $plugin_metadata->{$_} ne $req_metadata->{$_} } keys %$req_metadata;
push @plugins, $plugin;
} elsif ( defined($params->{errors}) && $params->{errors} ){
push @plugins, { error => 'cannot_load', name => $plugin_class };
} elsif( $errors ) {
push @failing, { error => 1, name => $plugin_class };
}
}
return @plugins;
return $errors ? ( \@plugins, \@failing ) : @plugins;
}
=head2 InstallPlugins

4
koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-home.tt
View File

@ -155,9 +155,9 @@
<span class="label label-warning">ERRORS</span>
</td>
[% IF ( CAN_user_plugins_configure || CAN_user_plugins_manage || CAN_user_plugins_report || CAN_user_plugins_tool ) %]
<td colspan="7">Error found whilst attempting to load plugin</td>
<td colspan="8">Error found whilst attempting to load plugin</td>
[% ELSE %]
<td colspan="6">Error found whilst attempting to load plugin</td>
<td colspan="8">Error found whilst attempting to load plugin</td>
[% END %]
</tr>
[% ELSE %]

13
misc/devel/install_plugins.pl
View File

@ -38,20 +38,15 @@ unless ( C4::Context->config("enable_plugins") ) {
}
my @existing_plugins = Koha::Plugins->new()->GetPlugins({
all => 1,
errors => 1,
all => 1,
verbose => 1, # warns about plugins failing to load
});
my $existing_plugins;
for my $existing_plugin (@existing_plugins) {
if( defined $existing_plugin->{error} ){
print "Could not load: ".$existing_plugin->{name}." any associated method will be removed\n";
} else {
$existing_plugins->{ $existing_plugin->{metadata}->{name} } =
$existing_plugin->{metadata}->{version};
}
$existing_plugins->{ $existing_plugin->{metadata}->{name} } = $existing_plugin->{metadata}->{version};
}
my @installed_plugins = Koha::Plugins->new()->InstallPlugins();
my @installed_plugins = Koha::Plugins->new()->InstallPlugins( { verbose => 0 } );
unless (@installed_plugins) {
my $plugins_dir = C4::Context->config("pluginsdir");
if ( ref($plugins_dir) eq 'ARRAY' ) {

4
plugins/plugins-home.pl
View File

@ -50,7 +50,7 @@ if ($plugins_enabled) {
method => $method,
);
my @plugins = Koha::Plugins->new()->GetPlugins(
my ( $plugins, $failures ) = Koha::Plugins->new()->GetPlugins(
{
method => $method,
all => 1,
@ -58,7 +58,7 @@ if ($plugins_enabled) {
}
);
$template->param( plugins => \@plugins, );
$template->param( plugins => [ @$plugins, @$failures ] );
$template->param( plugins_restricted => C4::Context->config('plugins_restricted') );
$template->param( can_search => C4::Context->config('plugin_repos') ? 1 : 0 );

5
plugins/plugins-upload.pl
View File

@ -124,7 +124,10 @@ if ( ( $op eq 'Upload' ) && ( $uploadfile || $uploadlocation ) ) {
exit;
}
Koha::Plugins->new()->InstallPlugins();
# Install plugins; verbose not needed, we redirect to plugins-home.
# FIXME There is no good way to verify the install below; we need an
# individual plugin install.
Koha::Plugins->new->InstallPlugins( { verbose => 0 } );
}
} elsif ( ( $op eq 'Upload' ) && !$uploadfile && !$uploadlocation ) {
warn "Problem uploading file or no file uploaded.";

26
t/db_dependent/Koha/Plugins/Plugins.t
View File

@ -24,7 +24,7 @@ use File::Temp qw( tempdir tempfile );
use FindBin qw($Bin);
use Module::Load::Conditional qw(can_load);
use Test::MockModule;
use Test::More tests => 17;
use Test::More tests => 18;
use Test::Warn;
use C4::Context;
@ -404,6 +404,30 @@ subtest 'RemovePlugins' => sub {
$schema->storage->txn_rollback;
};
subtest 'verbose and errors flag' => sub {
plan tests => 6;
$schema->storage->txn_begin;
t::lib::Mocks::mock_config( 'enable_plugins', 1 );
Koha::Plugins->RemovePlugins( { destructive => 1 } ); # clean start
our $class_basename = 'Koha::Plugin::TestMR::' . time;
my $plugin_mocks = [];
# Recreate TestMR plugins without module and mock on Module::Load::Conditional
reload_plugin( $_, $plugin_mocks ) for 1 .. 2;
warnings_like { Koha::Plugins->new->GetPlugins } [], 'Verbose was off';
warnings_like { Koha::Plugins->new->GetPlugins( { verbose => 1 } ) }
[ qr/TestMR/, qr/TestMR/ ], 'Verbose was on, two warns';
my ( $plugins, $failures ) = Koha::Plugins->new->GetPlugins( { errors => 1 } );
is( @$plugins, 0, 'No good plugins left' );
is( @$failures, 2, 'Two failing plugins' );
is( $failures->[0]->{error}, 1, 'Failure hash contains error key' );
is( $failures->[0]->{name}, "${class_basename}1", 'Failure hash contains name key' );
$schema->storage->txn_rollback;
};
$schema->storage->txn_begin; # Matching rollback at very end