Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Improving SQLHelper

Browse source 
More concise and more readable
removing do local $"= using join instead
Adding Caching of queries
Adding Some tests
API between function reorganizing for more consistency
This commit is contained in:
Henri-Damien LAURENT 2009-09-19 12:52:32 +02:00
parent 7a2efb285d
commit 70a6b86c74
2 changed files with 78 additions and 84 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

138
C4/SQLHelper.pm
View file

@ -40,7 +40,7 @@ BEGIN {
UpdateInTable UpdateInTable
GetPrimaryKeys GetPrimaryKeys
); );
%EXPORT_TAGS = ( all =>[qw( InsertInTable SearchInTable UpdateInTable GetPrimaryKeys)] %EXPORT_TAGS = ( all =>[qw( InsertInTable DeleteInTable SearchInTable UpdateInTable GetPrimaryKeys)]
); );
} }
@ -90,7 +90,7 @@ $searchtype is string Can be "wide" or "exact"
sub SearchInTable{ sub SearchInTable{
my ($tablename,$filters,$orderby, $limit, $columns_out, $filter_columns,$searchtype) = @_; my ($tablename,$filters,$orderby, $limit, $columns_out, $filter_columns,$searchtype) = @_;
$searchtype||="wide"; # $searchtype||="start_with";
my $dbh = C4::Context->dbh; my $dbh = C4::Context->dbh;
$columns_out||=["*"]; $columns_out||=["*"];
my $sql = do { local $"=', '; my $sql = do { local $"=', ';
@ -98,14 +98,17 @@ sub SearchInTable{
}; };
my $row; my $row;
my $sth; my $sth;
my ($keys,$values)=_filter_fields($filters,$tablename, $searchtype,$filter_columns); my ($keys,$values)=_filter_fields($tablename,$filters,$searchtype,$filter_columns);
my @criteria=grep{defined($_) && $_ !~/^\W$/ }@$keys; if ($keys){
if ($filters) { my @criteria=grep{defined($_) && $_ !~/^\W$/ }@$keys;
$sql.= do { local $"=') AND ('; if (@criteria) {
qq{ WHERE (@criteria) } $sql.= do { local $"=') AND (';
}; qq{ WHERE (@criteria) }
} };
}
}
if ($orderby){ if ($orderby){
#Order by desc by default
my @orders=map{ "$_".($$orderby{$_}? " DESC" : "") } keys %$orderby; my @orders=map{ "$_".($$orderby{$_}? " DESC" : "") } keys %$orderby;
$sql.= do { local $"=', '; $sql.= do { local $"=', ';
qq{ ORDER BY @orders} qq{ ORDER BY @orders}
@ -116,7 +119,7 @@ sub SearchInTable{
} }
$debug && $values && warn $sql," ",join(",",@$values); $debug && $values && warn $sql," ",join(",",@$values);
$sth = $dbh->prepare($sql); $sth = $dbh->prepare_cached($sql);
$sth->execute(@$values); $sth->execute(@$values);
my $results = $sth->fetchall_arrayref( {} ); my $results = $sth->fetchall_arrayref( {} );
return $results; return $results;
@ -137,17 +140,11 @@ sub SearchInTable{
sub InsertInTable{ sub InsertInTable{
my ($tablename,$data) = @_; my ($tablename,$data) = @_;
my $dbh = C4::Context->dbh; my $dbh = C4::Context->dbh;
my ($keys,$values)=_filter_fields($data,$tablename,0); my ($keys,$values)=_filter_hash($tablename,$data,0);
map{$_=~s/\(|\)//g; $_=~s/ AND /, /g}@$keys; my $query = qq{ INSERT INTO $tablename SET }.join(", ",@$keys);
my $query = do { local $"=',';
qq{
INSERT INTO $tablename
SET @$keys
};
};
$debug && warn $query, join(",",@$values); $debug && warn $query, join(",",@$values);
my $sth = $dbh->prepare($query); my $sth = $dbh->prepare_cached($query);
$sth->execute( @$values); $sth->execute( @$values);
return $dbh->last_insert_id(undef, undef, $tablename, undef); return $dbh->last_insert_id(undef, undef, $tablename, undef);
@ -170,17 +167,14 @@ sub UpdateInTable{
my @field_ids=GetPrimaryKeys($tablename); my @field_ids=GetPrimaryKeys($tablename);
my @ids=@$data{@field_ids}; my @ids=@$data{@field_ids};
my $dbh = C4::Context->dbh; my $dbh = C4::Context->dbh;
my ($keys,$values)=_filter_fields($data,$tablename,0); my ($keys,$values)=_filter_hash($tablename,$data,0);
map{$_=~s/\(|\)//g; $_=~s/ AND /, /g}@$keys; my $query =
my $query = do { local $"=','; qq{ UPDATE $tablename
qq{ SET }.join(",",@$keys).qq{
UPDATE $tablename
SET @$keys
WHERE }.join (" AND ",map{ "$_=?" }@field_ids); WHERE }.join (" AND ",map{ "$_=?" }@field_ids);
};
$debug && warn $query, join(",",@$values,@ids); $debug && warn $query, join(",",@$values,@ids);
my $sth = $dbh->prepare($query); my $sth = $dbh->prepare_cached($query);
return $sth->execute( @$values,@ids); return $sth->execute( @$values,@ids);
} }
@ -199,21 +193,16 @@ sub UpdateInTable{
sub DeleteInTable{ sub DeleteInTable{
my ($tablename,$data) = @_; my ($tablename,$data) = @_;
my @field_ids=GetPrimaryKeys($tablename);
my @ids=$$data{@field_ids};
my $dbh = C4::Context->dbh; my $dbh = C4::Context->dbh;
my ($keys,$values)=_filter_fields($data,$tablename,0); my ($keys,$values)=_filter_fields($tablename,$data,1);
if ($keys){
my $query = do { local $"=' AND '; my $query = do { local $"=') AND (';
qq{ qq{ DELETE FROM $tablename WHERE (@$keys)};
DELETE FROM $tablename };
WHERE }.map{" $_=? "}@field_ids; $debug && warn $query, join(",",@$values);
}; my $sth = $dbh->prepare_cached($query);
$debug && warn $query, join(",",@$values,@ids); return $sth->execute( @$values);
}
my $sth = $dbh->prepare($query);
return $sth->execute( @$values,@ids);
} }
=head2 GetPrimaryKeys =head2 GetPrimaryKeys
@ -253,7 +242,7 @@ With
sub _get_columns($) { sub _get_columns($) {
my ($tablename)=@_; my ($tablename)=@_;
my $dbh=C4::Context->dbh; my $dbh=C4::Context->dbh;
my $sth=$dbh->prepare(qq{SHOW COLUMNS FROM $tablename }); my $sth=$dbh->prepare_cached(qq{SHOW COLUMNS FROM $tablename });
$sth->execute; $sth->execute;
my $columns= $sth->fetchall_hashref(qw(Field)); my $columns= $sth->fetchall_hashref(qw(Field));
} }
@ -268,7 +257,7 @@ _filter_columns($tablename,$research, $filtercolumns)
Given Given
- a tablename - a tablename
- indicator on purpose whether it is a research or not - indicator on purpose whether all fields should be returned or only non Primary keys
- array_ref to columns to limit to - array_ref to columns to limit to
Returns an array of all the fieldnames of the table Returns an array of all the fieldnames of the table
@ -311,14 +300,21 @@ and a ref to value array
=cut =cut
sub _filter_fields{ sub _filter_fields{
my ($filter_input,$tablename,$searchtype,$filtercolumns)=@_; my ($tablename,$filter_input,$searchtype,$filtercolumns)=@_;
my @keys; my @keys;
my @values; my @values;
if (ref($filter_input) eq "HASH"){ if (ref($filter_input) eq "HASH"){
return _filter_hash($filter_input,$tablename, $searchtype); my ($keys, $values) = _filter_hash($tablename,$filter_input, $searchtype);
if ($keys){
my $stringkey="(".join (") AND (",@$keys).")";
return [$stringkey],$values;
}
else {
return ();
}
} elsif (ref($filter_input) eq "ARRAY"){ } elsif (ref($filter_input) eq "ARRAY"){
foreach my $element_data (@$filter_input){ foreach my $element_data (@$filter_input){
my ($localkeys,$localvalues)=_filter_fields($element_data,$tablename,$searchtype,$filtercolumns); my ($localkeys,$localvalues)=_filter_fields($tablename,$element_data,$searchtype,$filtercolumns);
if ($localkeys){ if ($localkeys){
@$localkeys=grep{defined($_) && $_ !~/^\W*$/}@$localkeys; @$localkeys=grep{defined($_) && $_ !~/^\W*$/}@$localkeys;
my $string=do{ my $string=do{
@ -331,14 +327,14 @@ sub _filter_fields{
} }
} }
else{ else{
return _filter_string($filter_input,$tablename, $searchtype,$filtercolumns); return _filter_string($tablename,$filter_input,$searchtype,$filtercolumns);
} }
return (\@keys,\@values); return (\@keys,\@values);
} }
sub _filter_hash{ sub _filter_hash{
my ($filter_input, $tablename,$searchtype)=@_; my ($tablename,$filter_input, $searchtype)=@_;
my (@values, @keys); my (@values, @keys);
my $columns= _get_columns($tablename); my $columns= _get_columns($tablename);
my @columns_filtered= _filter_columns($tablename,$searchtype); my @columns_filtered= _filter_columns($tablename,$searchtype);
@ -349,17 +345,13 @@ sub _filter_hash{
## supposed to be a hash of simple values, hashes of arrays could be implemented ## supposed to be a hash of simple values, hashes of arrays could be implemented
$$filter_input{$field}=format_date_in_iso($$filter_input{$field}) if ($$columns{$field}{Type}=~/date/ && $$filter_input{$field} !~C4::Dates->regexp("iso")); $$filter_input{$field}=format_date_in_iso($$filter_input{$field}) if ($$columns{$field}{Type}=~/date/ && $$filter_input{$field} !~C4::Dates->regexp("iso"));
my ($tmpkeys, $localvalues)=_Process_Operands($$filter_input{$field},$field,$searchtype,$columns); my ($tmpkeys, $localvalues)=_Process_Operands($$filter_input{$field},$field,$searchtype,$columns);
if ($tmpkeys){ if (@$tmpkeys){
push @values, @$localvalues; push @values, @$localvalues;
push @keys, @$tmpkeys; push @keys, @$tmpkeys;
} }
} }
my $string=do{
local $"=") AND (";
qq{( @keys )}
};
if (@keys){ if (@keys){
return ([$string],\@values); return (\@keys,\@values);
} }
else { else {
return (); return ();
@ -367,14 +359,14 @@ sub _filter_hash{
} }
sub _filter_string{ sub _filter_string{
my ($filter_input,$tablename, $searchtype,$filtercolumns)=@_; my ($tablename,$filter_input, $searchtype,$filtercolumns)=@_;
return () unless($filter_input);
my @operands=split / /,$filter_input;
my @columns_filtered= _filter_columns($tablename,$searchtype,$filtercolumns); my @columns_filtered= _filter_columns($tablename,$searchtype,$filtercolumns);
my $columns= _get_columns($tablename); my $columns= _get_columns($tablename);
my @operands=split / /,$filter_input;
my (@values,@keys); my (@values,@keys);
my @localkeys; my @localkeys;
foreach my $operand (@operands){ foreach my $operand (@operands){
foreach my $field (@columns_filtered){ foreach my $field (@columns_filtered){
my ($tmpkeys, $localvalues)=_Process_Operands($operand,$field,$searchtype,$columns); my ($tmpkeys, $localvalues)=_Process_Operands($operand,$field,$searchtype,$columns);
if ($tmpkeys){ if ($tmpkeys){
@ -383,10 +375,7 @@ sub _filter_string{
} }
} }
} }
my $sql= do { local $"=' OR '; my $sql= join (' OR ', @localkeys);
qq{@localkeys}
};
push @keys, $sql; push @keys, $sql;
if (@keys){ if (@keys){
@ -400,24 +389,23 @@ sub _Process_Operands{
my ($operand, $field, $searchtype,$columns)=@_; my ($operand, $field, $searchtype,$columns)=@_;
my @values; my @values;
my @tmpkeys; my @tmpkeys;
my $strkeys; my @localkeys;
my @localvaluesextended=("\% $operand\%","$operand\%") ; push @tmpkeys, " $field = ? ";
$strkeys= " $field = ? "; push @values, $operand;
if ($searchtype eq "wide"){ unless ($searchtype){
if ($field=~/(?<!zip)code|number/ ){ return \@tmpkeys,\@values;
$strkeys="( $field='' OR $field IS NULL OR $strkeys ) "; }
if ($searchtype eq "start_with"){
if ($field=~/(?<!zip)code|(?<!card)number/ ){
push @tmpkeys,(" $field= '' ","$field IS NULL");
} elsif ($$columns{$field}{Type}=~/varchar|text/){ } elsif ($$columns{$field}{Type}=~/varchar|text/){
$strkeys="( $field LIKE ? OR $field LIKE ? OR $strkeys ) "; push @tmpkeys,(" $field LIKE ? ","$field LIKE ?");
my @localvaluesextended=("\% $operand\%","$operand\%") ;
push @values,@localvaluesextended; push @values,@localvaluesextended;
} }
push @tmpkeys, $strkeys;
} }
else{ push @localkeys,qq{ (}.join(" OR ",@tmpkeys).qq{) };
$strkeys= " $field = ? "; return (\@localkeys,\@values);
push @tmpkeys, $strkeys;
}
push @values, $operand;
return (\@tmpkeys,\@values);
} }
1; 1;

24
t/db_dependent/SQLHelper.t
View file

@ -9,7 +9,7 @@ use Data::Dumper;
use C4::SQLHelper qw(:all); use C4::SQLHelper qw(:all);
use Test::More tests => 10; use Test::More tests => 14;
BEGIN { BEGIN {
use_ok('C4::SQLHelper'); use_ok('C4::SQLHelper');
@ -22,19 +22,25 @@ my @branchcodes=keys %$branches;
my $borrid; my $borrid;
ok($borrid=InsertInTable("borrowers",{firstname=>"Jean",surname=>"Valjean",city=>" ",zipcode=>" ",email=>"email",categorycode=>$categories[0]->{categorycode}, branchcode=>$branchcodes[0]}),"Insert In Table"); ok($borrid=InsertInTable("borrowers",{firstname=>"Jean",surname=>"Valjean",city=>" ",zipcode=>" ",email=>"email",categorycode=>$categories[0]->{categorycode}, branchcode=>$branchcodes[0]}),"Insert In Table");
ok(my $status=UpdateInTable("borrowers",{borrowernumber=>$borrid,firstname=>"Jean",surname=>"Valjean",city=>"ma6tVaCracker ",zipcode=>" ",email=>"email", branchcode=>$branchcodes[1]}),"Update In Table"); ok(my $status=UpdateInTable("borrowers",{borrowernumber=>$borrid,firstname=>"Jean",surname=>"Valjean",city=>"ma6tVaCracker ",zipcode=>" ",email=>"email", branchcode=>$branchcodes[1]}),"Update In Table");
my $borrowers=SearchInTable("borrowers",{firstname=>"Jean"}); my $borrowers=SearchInTable("borrowers");
ok(@$borrowers>0, "Search In Table All values");
$borrowers=SearchInTable("borrowers",{firstname=>"Jean"});
ok(@$borrowers>0, "Search In Table hashref"); ok(@$borrowers>0, "Search In Table hashref");
my $borrowers=SearchInTable("borrowers","Jean"); $borrowers=SearchInTable("borrowers","Jean");
ok(@$borrowers>0, "Search In Table string"); ok(@$borrowers>0, "Search In Table string");
my $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}]); $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}]);
ok(@$borrowers>0, "Search In Table arrayref"); ok(@$borrowers>0, "Search In Table arrayref");
my $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)]); $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)]);
ok(keys %{$$borrowers[0]} ==1, "Search In Table columns out limit"); ok(keys %{$$borrowers[0]} ==1, "Search In Table columns out limit");
my $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(firstname surname title)]); $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(firstname surname title)]);
ok(@$borrowers>0, "Search In Table columns out limit"); ok(@$borrowers>0, "Search In Table columns out limit");
my $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(firstname title)]); $borrowers=SearchInTable("borrowers",["Valjean",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(firstname title)]);
ok(@$borrowers==0, "Search In Table columns filter firstname title limit Valjean not in other fields than surname "); ok(@$borrowers==0, "Search In Table columns filter firstname title limit Valjean not in other fields than surname ");
my $borrowers=SearchInTable("borrowers",["Val",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(surname)],"wide"); $borrowers=SearchInTable("borrowers",["Val",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(surname)],"start_with");
ok(@$borrowers>0, "Search In Table columns filter surname Val on a wide search found "); ok(@$borrowers>0, "Search In Table columns filter surname Val on a wide search found ");
my $borrowers=SearchInTable("borrowers",["Val",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(surname)],"exact"); $borrowers=SearchInTable("borrowers",["Val",{firstname=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(surname)],"exact");
ok(@$borrowers==0, "Search In Table columns filter surname Val in exact search not found "); ok(@$borrowers==0, "Search In Table columns filter surname Val in exact search not found ");
$borrowers=eval{SearchInTable("borrowers",["Val",{member=>"Jean"}],undef,undef,[qw(borrowernumber)],[qw(firstname title)],"exact")};
ok(@$borrowers==0 && !($@), "Search In Table fails gracefully when no correct field passed in hash");
$status=DeleteInTable("borrowers",{borrowernumber=>$borrid});
ok($status>0 && !($@), "DeleteInTable OK");