diff --git a/Koha/Exceptions/Report.pm b/Koha/Exceptions/Report.pm index 141003d31e..d23c7113fb 100644 --- a/Koha/Exceptions/Report.pm +++ b/Koha/Exceptions/Report.pm @@ -9,6 +9,11 @@ use Exception::Class ( description => "The configured per-user limit on simultaneous runs of this report has been reached", fields => [ 'report_id', 'user_id', 'limit' ], }, + 'Koha::Exceptions::Report::TotalRunning' => { + isa => 'Koha::Exceptions::Report', + description => "The configured per-user limit on total simultaneous report runs has been reached", + fields => [ 'user_id', 'limit' ], + }, ); 1; diff --git a/Koha/Report.pm b/Koha/Report.pm index 57ecfb21af..fdad5cff92 100644 --- a/Koha/Report.pm +++ b/Koha/Report.pm @@ -178,9 +178,12 @@ this format as part of the public contract. If C is set in C to a non-zero value and the current userenv already has at least that many runs of the same report in flight, this method throws -L instead of returning. All -report-execution entry points (the staff guided-reports UI, C -and C) go through here so the limit is enforced uniformly. +L instead of returning. If +C is set and the userenv already has +at least that many reports of any kind in flight, it throws +L. All report-execution entry +points (the staff guided-reports UI, C and C) +go through here so the limits are enforced uniformly. =cut @@ -263,6 +266,17 @@ sub prep_report { } } + my $total_limit = C4::Context->config('total_running_reports_per_user_limit'); + if ( $total_limit && $user_id ) { + my $running = Koha::Reports->running( { user_id => $user_id } ); + if ( $running->count >= $total_limit ) { + Koha::Exceptions::Report::TotalRunning->throw( + user_id => $user_id, + limit => $total_limit, + ); + } + } + $sql .= " /* { saved_sql.id: $report_id } { user_id: $user_id } */"; return $sql, $headers; diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in index 1a6979185b..03fffb6317 100644 --- a/debian/templates/koha-conf-site.xml.in +++ b/debian/templates/koha-conf-site.xml.in @@ -347,6 +347,7 @@ __END_SRU_PUBLICSERVER__ 0 + 0 diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml index 4949b4c0ec..f811a0d378 100644 --- a/etc/koha-conf.xml +++ b/etc/koha-conf.xml @@ -164,6 +164,7 @@ 0 + 0 diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt index 8e08b53447..a1e8d5b8c4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt @@ -1640,6 +1640,8 @@ [% FOREACH error IN errors %] [% IF ( error.duplicate_running_report ) %] This report is already running. Please wait for it to finish before running it again. + [% ELSIF ( error.total_running_reports_limit_exceeded ) %] + You have too many reports running at the moment. Please wait for some of them to finish before running this report again. [% ELSIF ( error.sqlerr ) %] This report contains the SQL keyword [% error.sqlerr | html %].
Use of this keyword is not allowed in Koha reports due to security and data integrity risks. Only SELECT queries are allowed.
diff --git a/opac/svc/report b/opac/svc/report index 2eba1b1567..28aae40b44 100755 --- a/opac/svc/report +++ b/opac/svc/report @@ -66,12 +66,18 @@ unless ($json_text) { my ( $sql, undef ); eval { ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params ); }; if ( my $e = $@ ) { + my $error_key; if ( ref($e) eq 'Koha::Exceptions::Report::DuplicateRunning' ) { + $error_key = 'duplicate_running_report'; + } elsif ( ref($e) eq 'Koha::Exceptions::Report::TotalRunning' ) { + $error_key = 'total_running_reports_limit_exceeded'; + } + if ($error_key) { print $query->header( -type => 'application/json', -status => '429 Too Many Requests', ); - print encode_json( { error => 'duplicate_running_report' } ); + print encode_json( { error => $error_key } ); exit 0; } die $e; diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index c7c2a759ac..d9753e6619 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -1024,7 +1024,13 @@ if ( $op eq 'run' ) { my ( $sql, $header_types ); eval { ( $sql, $header_types ) = $report->prep_report( \@param_names, \@sql_params ); }; if ( my $e = $@ ) { + my $error_key; if ( ref($e) eq 'Koha::Exceptions::Report::DuplicateRunning' ) { + $error_key = 'duplicate_running_report'; + } elsif ( ref($e) eq 'Koha::Exceptions::Report::TotalRunning' ) { + $error_key = 'total_running_reports_limit_exceeded'; + } + if ($error_key) { $template->param( 'sql' => $original_sql, 'original_sql' => $original_sql, @@ -1032,7 +1038,7 @@ if ( $op eq 'run' ) { 'execute' => 1, 'name' => $name, 'notes' => $notes, - 'errors' => [ { duplicate_running_report => 1 } ], + 'errors' => [ { $error_key => 1 } ], 'sql_params' => \@sql_params, 'param_names' => \@param_names, ); diff --git a/svc/report b/svc/report index 838f948f0a..281d595771 100755 --- a/svc/report +++ b/svc/report @@ -68,12 +68,18 @@ unless ($json_text) { my ( $sql, undef ); eval { ( $sql, undef ) = $report_rec->prep_report( \@param_names, \@sql_params ); }; if ( my $e = $@ ) { + my $error_key; if ( ref($e) eq 'Koha::Exceptions::Report::DuplicateRunning' ) { + $error_key = 'duplicate_running_report'; + } elsif ( ref($e) eq 'Koha::Exceptions::Report::TotalRunning' ) { + $error_key = 'total_running_reports_limit_exceeded'; + } + if ($error_key) { print $query->header( -type => 'application/json', -status => '429 Too Many Requests', ); - print encode_json( { error => 'duplicate_running_report' } ); + print encode_json( { error => $error_key } ); exit 0; } die $e; diff --git a/t/db_dependent/Koha/Reports.t b/t/db_dependent/Koha/Reports.t index c29a310078..0fa30592cc 100755 --- a/t/db_dependent/Koha/Reports.t +++ b/t/db_dependent/Koha/Reports.t @@ -20,7 +20,7 @@ use Modern::Perl; use Test::Exception; use Test::MockModule; use Test::NoWarnings; -use Test::More tests => 12; +use Test::More tests => 13; use Koha::Report; use Koha::Reports; @@ -140,6 +140,44 @@ subtest 'prep_report throws when duplicate-running limit is exceeded' => sub { is( $exception->limit, 1, 'exception carries the configured limit' ); }; +subtest 'prep_report throws when total-running limit is exceeded' => sub { + plan tests => 3; + + my $report = Koha::Report->new( { report_name => 'total_running_test', savedsql => 'SELECT 1' } )->store; + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + t::lib::Mocks::mock_userenv( { patron => $patron } ); + + # The duplicate-limit check runs first, so neutralise it for these + # assertions and only exercise the total-limit branch. + t::lib::Mocks::mock_config( 'duplicate_running_reports_per_user_limit', 0 ); + + # Pretend the user has three reports of various ids in flight whenever + # running() is called without a report_id filter (ie. the "total" check). + my $reports_mock = Test::MockModule->new('Koha::Reports'); + $reports_mock->mock( + 'running', + sub { + my ( $class, $params ) = @_; + return $class->search( { id => undef } ) if $params->{report_id}; + return $class->search( { id => $report->id } ); + } + ); + + t::lib::Mocks::mock_config( 'total_running_reports_per_user_limit', 5 ); + lives_ok { $report->prep_report( [], [] ) } 'total limit not yet reached, prep_report returns normally'; + + t::lib::Mocks::mock_config( 'total_running_reports_per_user_limit', 1 ); + my $exception; + eval { $report->prep_report( [], [] ); }; + $exception = $@; + isa_ok( + $exception, 'Koha::Exceptions::Report::TotalRunning', + 'total limit reached -> TotalRunning exception' + ); + is( $exception->limit, 1, 'exception carries the configured total limit' ); +}; + subtest 'is_sql_valid' => sub { plan tests => 3 + 6 * 2; my @badwords = ( 'UPDATE', 'DELETE', 'DROP', 'INSERT', 'SHOW', 'CREATE' );