Bugzilla – Attachment 171144 Details for
Bug 23685
Exporting report may consume unlimited memory
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23685: (follow-up) Add export limit for guided reports
Bug-23685-follow-up-Add-export-limit-for-guided-re.patch (text/plain), 5.38 KB, created by
Marcel de Rooy
on 2024-09-06 13:16:52 UTC
(
hide
)
Description:
Bug 23685: (follow-up) Add export limit for guided reports
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2024-09-06 13:16:52 UTC
Size:
5.38 KB
patch
obsolete
>From 01a3079eaadce2da9472c2628ccb96f6ac54354c Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 6 Sep 2024 09:29:39 +0000 >Subject: [PATCH] Bug 23685: (follow-up) Add export limit for guided reports >Content-Type: text/plain; charset=utf-8 > >Test plan: >Set pref GuidedReportsExportLimit to some positive integer. >Test if exporting a report respects that limit. >Run t/db_dependent/Koha/Reports.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > Koha/Report.pm | 27 ++++++++++++++++++++++++- > reports/guided_reports.pl | 2 +- > t/db_dependent/Koha/Reports.t | 37 ++++++++++++++++++++++++++++++++++- > 3 files changed, 63 insertions(+), 3 deletions(-) > >diff --git a/Koha/Report.pm b/Koha/Report.pm >index 2a1adab1c6..bc6fa71ece 100644 >--- a/Koha/Report.pm >+++ b/Koha/Report.pm >@@ -168,7 +168,7 @@ for building batch action links in the template > =cut > > sub prep_report { >- my ( $self, $param_names, $sql_params ) = @_; >+ my ( $self, $param_names, $sql_params, $params ) = @_; # params keys: export > my $sql = $self->savedsql; > > # First we split out the placeholders >@@ -226,10 +226,35 @@ sub prep_report { > $sql =~ s/<<$split[$i*2+1]>>/$quoted/; > } > >+ $sql = $self->_might_add_limit($sql) if $params->{export}; >+ > $sql = "$sql /* saved_sql.id: ${\( $self->id )} */"; > return $sql, $headers; > } > >+=head3 _might_add_limit >+ >+ $sql = $self->_might_add_limit($sql); >+ >+Depending on pref GuidedReportsExportLimit. If there is a limit defined >+and the report does not contain a limit already, this routine adds >+a LIMIT to $sql. >+ >+=cut >+ >+sub _might_add_limit { >+ my ( $self, $sql ) = @_; >+ if ( C4::Context->preference('GuidedReportsExportLimit') >+ && $sql !~ /\sLIMIT\s\d+(?![^\(]*\))/i ) >+ { >+ # Note: regex tries to account for allowed limits in subqueries. >+ # This assumes that we dont have an INTO option at the end (which does >+ # not work in current Koha reporting) >+ $sql .= " LIMIT " . C4::Context->preference('GuidedReportsExportLimit'); >+ } >+ return $sql; >+} >+ > =head3 _type > > Returns name of corresponding DBIC resultset >diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl >index 1464afa929..7f1dc8a790 100755 >--- a/reports/guided_reports.pl >+++ b/reports/guided_reports.pl >@@ -624,7 +624,7 @@ elsif ($op eq 'export'){ > $reportname ? "$report_id-$reportname-reportresults.$format" : "$report_id-reportresults.$format"; > my $scrubber = C4::Scrubber->new(); > >- ($sql, undef) = $report->prep_report( \@param_names, \@sql_params ); >+ ($sql, undef) = $report->prep_report( \@param_names, \@sql_params, { export => 1 } ); > my ( $sth, $q_errors ) = execute_query( { sql => $sql, report_id => $report_id } ); > unless ($q_errors and @$q_errors) { > my ( $type, $content ); >diff --git a/t/db_dependent/Koha/Reports.t b/t/db_dependent/Koha/Reports.t >index 3f7eaacd6c..5ffdf8b1d9 100755 >--- a/t/db_dependent/Koha/Reports.t >+++ b/t/db_dependent/Koha/Reports.t >@@ -17,12 +17,13 @@ > > use Modern::Perl; > >-use Test::More tests => 7; >+use Test::More tests => 8; > > use Koha::Report; > use Koha::Reports; > use Koha::Database; > >+use t::lib::Mocks; > use t::lib::TestBuilder; > > my $schema = Koha::Database->new->schema; >@@ -136,4 +137,38 @@ subtest 'check_columns' => sub { > ); > }; > >+subtest '_might_add_limit' => sub { >+ plan tests => 10; >+ >+ my $sql; >+ >+ t::lib::Mocks::mock_preference( 'GuidedReportsExportLimit', undef ); # i.e. no limit >+ $sql = "SELECT * FROM biblio WHERE 1"; >+ is( Koha::Report->_might_add_limit($sql), $sql, 'Pref is undefined, no changes' ); >+ t::lib::Mocks::mock_preference( 'GuidedReportsExportLimit', 0 ); # i.e. no limit >+ is( Koha::Report->_might_add_limit($sql), $sql, 'Pref is zero, no changes' ); >+ t::lib::Mocks::mock_preference( 'GuidedReportsExportLimit', q{} ); # i.e. no limit >+ is( Koha::Report->_might_add_limit($sql), $sql, 'Pref is empty, no changes' ); >+ t::lib::Mocks::mock_preference( 'GuidedReportsExportLimit', 10 ); >+ like( Koha::Report->_might_add_limit($sql), qr/ LIMIT 10$/, 'Limit 10 found at the end' ); >+ $sql = "SELECT * FROM biblio WHERE 1 LIMIT 1000 "; >+ is( Koha::Report->_might_add_limit($sql), $sql, 'Already contains a limit' ); >+ $sql = "SELECT * FROM biblio WHERE 1 LIMIT 1000,2000"; >+ is( Koha::Report->_might_add_limit($sql), $sql, 'Variation, also contains a limit' ); >+ >+ # trying a subquery having a limit (testing the lookahead in regex) >+ $sql = "SELECT * FROM biblio WHERE biblionumber IN (SELECT biblionumber FROM reserves LIMIT 2)"; >+ like( Koha::Report->_might_add_limit($sql), qr/ LIMIT 10$/, 'Subquery, limit 10 found at the end' ); >+ $sql = "SELECT * FROM biblio WHERE biblionumber IN (SELECT biblionumber FROM reserves LIMIT 2, 3 ) AND 1"; >+ like( Koha::Report->_might_add_limit($sql), qr/ LIMIT 10$/, 'Subquery variation, limit 10 found at the end' ); >+ $sql = "select * from biblio where biblionumber in (select biblionumber from reserves limiT 3,4) and 1"; >+ like( Koha::Report->_might_add_limit($sql), qr/ LIMIT 10$/, 'Subquery lc variation, limit 10 found at the end' ); >+ >+ $sql = "select limit, 22 from mylimits where limit between 1 and 3"; >+ like( >+ Koha::Report->_might_add_limit($sql), qr/ LIMIT 10$/, >+ 'Query refers to limit field, limit 10 found at the end' >+ ); >+}; >+ > $schema->storage->txn_rollback; >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23685
:
168116
|
168570
|
168577
|
171136
|
171137
|
171138
|
171139
|
171140
|
171141
|
171142
|
171143
|
171144
|
171145
|
171171
|
171172
|
171173
|
171174
|
171175
|
171571
|
171572
|
171573
|
171574
|
171575