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

Bug 9915: (follow-up) use SQL placeholders 

This patch teaches C4::Reports::Guided::execute_query()
how to accept a list of query parameter values.  It then
follows-up on the main patch by simplifying how it converts
report parameters to a complete SQL query, and removes the
use of DBI->quote() and complicated regexes.

To test:

[1] Verify that using the OPAC svc/report service with
    sql_params continues to work.
[2] Verify that there are no regressions with running
    reports from the staff interface, both via the web
    service and the reports interface.
[3] Verify that prove -v /db_dependent/Reports_Guided.t passes.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
No regressions found.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This commit is contained in:
Galen Charlton 2014-04-20 16:04:27 +00:00
parent 9161911612
commit 1931d2d22f
3 changed files with 42 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
C4/Reports/Guided.pm
View File

@ -400,22 +400,25 @@ sub nb_rows {
=head2 execute_query
($results, $error) = execute_query($sql, $offset, $limit)
($sth, $error) = execute_query($sql, $offset, $limit[, \@sql_params])
When passed C<$sql>, this function returns an array ref containing a result set
suitably formatted for display in html or for output as a flat file when passed in
C<$format> and C<$id>. It also returns the C<$total> records available for the
supplied query. If passed any query other than a SELECT, or if there is a db error,
C<$errors> an array ref is returned containing the error after this manner:
This function returns a DBI statement handler from which the caller can
fetch the results of the SQL passed via C<$sql>.
If passed any query other than a SELECT, or if there is a db error,
C<$errors> an array ref is returned containing the error after this
manner:
C<$error->{'sqlerr'}> contains the offending SQL keyword.
C<$error->{'queryerr'}> contains the native db engine error returned for the query.
C<$error->{'queryerr'}> contains the native db engine error returned
for the query.
Valid values for C<$format> are 'text,' 'tab,' 'csv,' or 'url. C<$sql>, C<$type>,
C<$offset>, and C<$limit> are required parameters. If a valid C<$format> is passed
in, C<$offset> and C<$limit> are ignored for obvious reasons. A LIMIT specified by
the user in a user-supplied SQL query WILL apply in any case.
C<$offset>, and C<$limit> are required parameters.
C<\@sql_params> is an optional list of parameter values to paste in.
The caller is reponsible for making sure that C<$sql> has placeholders
and that the number placeholders matches the number of parameters.
=cut
@ -470,9 +473,11 @@ sub strip_limit {
}
}
sub execute_query ($;$$$) {
sub execute_query {
my ( $sql, $offset, $limit, $no_count ) = @_;
my ( $sql, $offset, $limit, $sql_params ) = @_;
$sql_params = [] unless defined $sql_params;
# check parameters
unless ($sql) {
@ -506,7 +511,7 @@ sub execute_query ($;$$$) {
$sql .= " LIMIT ?, ?";
my $sth = C4::Context->dbh->prepare($sql);
$sth->execute($offset, $limit);
$sth->execute(@$sql_params, $offset, $limit);
return ( $sth );
# my @xmlarray = ... ;
# my $url = "/cgi-bin/koha/reports/guided_reports.pl?phase=retrieve%20results&id=$id";

15
opac/svc/report
View File

@ -56,21 +56,12 @@ unless ($json_text) {
my $offset = 0;
my $limit = C4::Context->preference("SvcMaxReportRows") || 10;
my $sql = $report_rec->{savedsql};
if (@sql_params) {
# we have sql params need to fix the sql
my @split = split /<<|>>/, $sql;
my @tmpl_parameters;
for ( my $i = 0 ; $i < $#split / 2 ; $i++ ) {
my $quoted = C4::Context->dbh->quote( $sql_params[$i] );
# convert SQL parameters to placeholders
$sql =~ s/(<<.*?>>)/\?/g;
# if there are special regexp chars, we must \ them
$split[ $i * 2 + 1 ] =~ s/(\||\?|\.|\*|\(|\)|\%)/\\$1/g;
$sql =~ s/<<$split[$i*2+1]>>/$quoted/;
}
}
my ( $sth, $errors ) =
execute_query( $sql, $offset, $limit );
execute_query( $sql, $offset, $limit, \@sql_params );
if ($sth) {
my $lines;
if ($report_annotation) {

23
t/db_dependent/Reports_Guided.t
View File

@ -5,7 +5,7 @@
use Modern::Perl;
use Test::More tests => 12;
use Test::More tests => 14;
use C4::Context;
@ -14,8 +14,7 @@ BEGIN {
}
can_ok(
'C4::Reports::Guided',
qw(save_report
delete_report)
qw(save_report delete_report execute_query)
);
#Start transaction
@ -68,6 +67,24 @@ $count -= 2;
is( scalar( @{ get_saved_reports() } ),
$count, "Report2 and report3 have been deleted" );
my $sth = execute_query('SELECT COUNT(*) FROM systempreferences', 0, 10);
my $results = $sth->fetchall_arrayref;
is(scalar(@$results), 1, 'running a query returned a result');
my $version = C4::Context->preference('Version');
$sth = execute_query(
'SELECT value FROM systempreferences WHERE variable = ?',
0,
10,
[ 'Version' ],
);
$results = $sth->fetchall_arrayref;
is_deeply(
$results,
[ [ $version ] ],
'running a query with a parameter returned the expected result'
);
#End transaction
$dbh->rollback;