From 570f09ad165fa8accbdf9e01a3e9d6fdc43bcbd2 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 21 Sep 2018 09:15:08 +0200 Subject: [PATCH] Bug 21281: (QA follow-up) Introduce _add_backtics Content-Type: text/plain; charset=utf-8 Added a (initially trivial) test in Creators/Lib.t too. Sometimes you start something, but you end somewhere else ;) The test seems a bit slower now (regex, lookahead, etc). But this should not hurt label creators in daily use. Advantages: code is even more solid, consolidated in one place and can be tested on its own. Signed-off-by: Marcel de Rooy --- C4/Creators/Lib.pm | 33 +++++++++++++++++---------------- t/db_dependent/Creators/Lib.t | 12 +++++++++++- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/C4/Creators/Lib.pm b/C4/Creators/Lib.pm index f5616a5..fc40c7b 100644 --- a/C4/Creators/Lib.pm +++ b/C4/Creators/Lib.pm @@ -67,11 +67,8 @@ C4::Creators::Lib sub _SELECT { my @params = @_; - my $fields_list = $params[0]; - if (index($fields_list, ' ')==-1 && index($fields_list,',')==-1 && $fields_list ne '*') { - $fields_list = "`$fields_list`"; - } - my $query = "SELECT $fields_list FROM $params[1]"; + my $fieldname = _add_backtics($params[0]); + my $query = "SELECT $fieldname FROM $params[1]"; $params[2] ? $query .= " WHERE $params[2];" : $query .= ';'; my $sth = C4::Context->dbh->prepare($query); # $sth->{'TraceLevel'} = 3; @@ -87,6 +84,19 @@ sub _SELECT { return $record_set; } +sub _add_backtics { + my ( @args ) = @_; + s/(?:^|\b)(\w+)(?:\b|$)/`$1`/g for @args; + # Too bad that we need to correct a few exceptions: aggregate functions + my @aggregates = ( 'COUNT', 'MAX', 'MIN', 'SUM' ); # add when needed.. + foreach my $aggr (@aggregates) { + s/`$aggr`\(/$aggr\(/gi for @args; + } + # And correct aliases + s/(`|\))\s+`AS`\s+`/$1 AS `/gi for @args; + return wantarray ? @args : $args[0]; +} + my $barcode_types = [ {type => 'CODE39', name => 'Code 39', desc => 'Translates the characters 0-9, A-Z, \'-\', \'*\', \'+\', \'$\', \'%\', \'/\', \'.\' and \' \' to a barcode pattern.', selected => 0}, {type => 'CODE39MOD', name => 'Code 39 + Modulo43', desc => 'Translates the characters 0-9, A-Z, \'-\', \'*\', \'+\', \'$\', \'%\', \'/\', \'.\' and \' \' to a barcode pattern. Encodes Mod 43 checksum.', selected => 0}, @@ -148,17 +158,8 @@ my $output_formats = [ sub _build_query { my ( $params, $table ) = @_; - my @fields = exists $params->{fields} ? @{ $params->{fields} } : (); - my @fields2 = (); - foreach my $field_name (@fields) { - if (index($field_name,' ')==-1 && $field_name ne '*') { - push @fields2, "`$field_name`"; - } else { - push @fields2, $field_name; - } - } - @fields = @fields2; - my $query = "SELECT " . ( @fields ? join(', ', @fields ) : '*' ) . " FROM $table"; + my @fields = exists $params->{fields} ? _add_backtics( @{ $params->{fields} } ) : ('*'); + my $query = "SELECT " . join(', ', @fields ) . " FROM $table"; my @where_args; if ( exists $params->{filters} ) { $query .= ' WHERE 1 '; diff --git a/t/db_dependent/Creators/Lib.t b/t/db_dependent/Creators/Lib.t index d180850..288257e 100644 --- a/t/db_dependent/Creators/Lib.t +++ b/t/db_dependent/Creators/Lib.t @@ -18,7 +18,7 @@ use Modern::Perl; use Graphics::Magick; -use Test::More tests => 645; +use Test::More tests => 646; use Test::MockModule; use t::lib::Mocks; use t::lib::TestBuilder; @@ -1437,6 +1437,16 @@ is( $records->[0]->{cardnumber}, $cardnumber1, 'cardnumber is good' ); is( $records->[0]->{branchcode}, $branchcode, 'branchcode is good' ); is( $records->[0]->{categorycode}, $categorycode, 'categorycode is good' ); +subtest '_add_backtics' => sub { + plan tests => 7; + my @a = ( 'rows', 'rows ', ' rows ', 'table.rows,field2', 'table.field1, table.field_2_a', 'table1.*, *', 'COUNT(id) AS mycount, count(*) as mycount2' ); + my @a_exp = ( '`rows`', '`rows` ', ' `rows` ', '`table`.`rows`,`field2`', '`table`.`field1`, `table`.`field_2_a`', '`table1`.*, *', 'COUNT(`id`) AS `mycount`, COUNT(*) AS `mycount2`' ); # expected results + my @b = C4::Creators::Lib::_add_backtics(@a); + while( my $f = shift @a_exp ) { + is( shift @b, $f, (shift @a). ' became '. $f ); + } +}; + # ---------- Sub ------------------------------------------ my %preferences; -- 2.1.4