From 6f62a8fe300b21818da05b9c8fe55450af985773 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 27 Apr 2026 18:56:01 +0100 Subject: [PATCH] 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] Signed-off-by: Martin Renvoize Signed-off-by: Lucas Gass --- Koha/Exceptions/Report.pm | 5 +++ Koha/Report.pm | 20 ++++++++-- debian/templates/koha-conf-site.xml.in | 1 + etc/koha-conf.xml | 1 + .../modules/reports/guided_reports_start.tt | 2 + opac/svc/report | 8 +++- reports/guided_reports.pl | 8 +++- svc/report | 8 +++- t/db_dependent/Koha/Reports.t | 40 ++++++++++++++++++- 9 files changed, 86 insertions(+), 7 deletions(-) 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' );