Bug 41919: Limit number of current reports a single user can run simultaneously
Building on bug 41918, it would be good to limit the number of reports
a given user can run at a time. This control is config-file based as
it is tied to the server resources provided by the administrator.
Adds a new koha-conf.xml option total_running_reports_per_user_limit
(0 = disabled). When the configured limit is reached, the user is told
"You have too many reports running at the moment. Please wait for some
of them to finish before running this report again."
Note on implementation: the original patch on bz (attachment 198034)
performed a second pre-flight Koha::Reports->running call in
reports/guided_reports.pl. The bug 41918 QA follow-ups already pushed
the duplicate-running guard into Koha::Report::prep_report so it would
fire across the staff UI, svc/report and opac/svc/report. This patch
extends that same model-layer architecture rather than reintroducing
controller-layer enforcement:
- New Koha::Exceptions::Report::TotalRunning subclass of
Koha::Exceptions::Report.
- prep_report now performs a second check after the duplicate one and
throws TotalRunning when the user has total_running_reports_per_user_limit
reports of any kind already in flight.
- All three callers (guided_reports.pl, svc/report, opac/svc/report)
catch the new exception alongside DuplicateRunning, mapping it to a
new template error key total_running_reports_limit_exceeded for the
staff UI and a JSON {error: total_running_reports_limit_exceeded}
with HTTP 429 for the JSON endpoints.
Test Plan:
1) Set total_running_reports_per_user_limit to 3 in your koha-conf.xml.
2) Create 3 long-running reports like:
SELECT COUNT(*) FROM items i1 JOIN items i2 JOIN items i3 JOIN items i4 JOIN items i5 JOIN items i6
3) Run each report in turn by opening a new tab, browsing to the
reports module, and choosing to run that report.
4) Note the first three continue to run, but a fourth attempt gives
the new "too many reports running" alert in the staff UI.
5) Repeat against /cgi-bin/koha/svc/report?id=N and confirm an HTTP
429 response with JSON {error: total_running_reports_limit_exceeded}.
6) Set total_running_reports_per_user_limit back to 0 and confirm
normal behaviour resumes.
7) prove t/db_dependent/Koha/Reports.t -> 13 tests pass, including a
new subtest for TotalRunning.
Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
parent
158f0bbef5
commit
6f62a8fe30
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -178,9 +178,12 @@ this format as part of the public contract.
|
|||
If C<duplicate_running_reports_per_user_limit> is set in C<koha-conf.xml>
|
||||
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<Koha::Exceptions::Report::DuplicateRunning> instead of returning. All
|
||||
report-execution entry points (the staff guided-reports UI, C<svc/report>
|
||||
and C<opac/svc/report>) go through here so the limit is enforced uniformly.
|
||||
L<Koha::Exceptions::Report::DuplicateRunning> instead of returning. If
|
||||
C<total_running_reports_per_user_limit> is set and the userenv already has
|
||||
at least that many reports of any kind in flight, it throws
|
||||
L<Koha::Exceptions::Report::TotalRunning>. All report-execution entry
|
||||
points (the staff guided-reports UI, C<svc/report> and C<opac/svc/report>)
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -347,6 +347,7 @@ __END_SRU_PUBLICSERVER__
|
|||
|
||||
<!-- SQL report limits, 0 means the feature is disabled -->
|
||||
<duplicate_running_reports_per_user_limit>0</duplicate_running_reports_per_user_limit>
|
||||
<total_running_reports_per_user_limit>0</total_running_reports_per_user_limit>
|
||||
|
||||
<!-- Configuration for X-Forwarded-For -->
|
||||
<!-- Set enable_trusted_proxy_for_headers to 0 to never trust proxy headers, always use given REMOTE_ADDRESS -->
|
||||
|
|
|
|||
|
|
@ -164,6 +164,7 @@
|
|||
|
||||
<!-- SQL report limits, 0 means the feature is disabled -->
|
||||
<duplicate_running_reports_per_user_limit>0</duplicate_running_reports_per_user_limit>
|
||||
<total_running_reports_per_user_limit>0</total_running_reports_per_user_limit>
|
||||
|
||||
<!-- Configuration for X-Forwarded-For -->
|
||||
<!-- Set enable_trusted_proxy_for_headers to 0 to never trust proxy headers, always use given REMOTE_ADDRESS -->
|
||||
|
|
|
|||
|
|
@ -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 <strong>[% error.sqlerr | html %]</strong>.<br />
|
||||
Use of this keyword is not allowed in Koha reports due to security and data integrity risks. Only SELECT queries are allowed.<br />
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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' );
|
||||
|
|
|
|||
Loading…
Reference in New Issue