This adds documentation to the filter methods. The preparation step replaces the Itemref properties with filters on the underlying database columns so that DBIBackend only has to deal with 'real' table columns. This also adds better support for multi-column Itemref properties. The validation is performed when building the filter. This means errors are reported close to when the invalid instructions were provided rather than when the database complains about a non-sensical WHERE clause.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45023 Signed-off-by: Francois Gouget fgouget@codeweavers.com --- This fixes the remainder of issue 9 "No Itemref support for multiple key fields". --- testbot/lib/ObjectModel/Collection.pm | 154 +++++++++++++++++++++++++- testbot/lib/ObjectModel/DBIBackEnd.pm | 34 +----- 2 files changed, 153 insertions(+), 35 deletions(-)
diff --git a/testbot/lib/ObjectModel/Collection.pm b/testbot/lib/ObjectModel/Collection.pm index 1a48eec35..9dcaf33d4 100644 --- a/testbot/lib/ObjectModel/Collection.pm +++ b/testbot/lib/ObjectModel/Collection.pm @@ -1,6 +1,6 @@ # -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*- # Copyright 2009 Ge van Geldorp -# Copyright 2012-2014, 2021 Francois Gouget +# Copyright 2012-2014, 2021-2022 Francois Gouget # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -700,11 +700,31 @@ my %ValueComparators = ( 'NOT LIKE' => 1, );
+=pod +=over 12 + +=item C<FilterValue()> + +Compares the specified property to a set of values, joining the results with +an OR operator. + +If the property is an Itemref, the only allowed comparators are '=' and '<>', +and they apply to the underlying columns. +The inequality and like comparators are not allowed on boolean properties. + +Finally note that applying comparators like ">=" to multiple values is just an +inefficient way of requiring the property to be greater than or equal to the +minimum / maximum value. + +=back +=cut + sub FilterValue($$$) { my ($PropertyName, $Comparator, $Values) = @_;
- die "unknown '$Comparator' comparator" if (!$ValueComparators{$Comparator}); + die "unknown $Comparator comparator" if (!$ValueComparators{$Comparator}); + die "invalid set of values" if (ref($Values) ne 'ARRAY' or @$Values == 0); return { Type => 'value', Property => $PropertyName, Comparator => $Comparator, @@ -719,6 +739,17 @@ sub FilterNull($) return { Type => 'null', Property => $PropertyName }; }
+=pod +=over 12 + +=item C<FilterNotNull()> + +Requires that the specified property not be NULL. +This type of check is not allowed on required properties. + +=back +=cut + sub FilterNotNull($) { my ($PropertyName) = @_; @@ -744,10 +775,127 @@ sub FilterNot($) return { Type => 'not', Term => $Term }; }
+=pod +=over 12 + +=item C<PrepareFilter()> + +Validates the filter and performs some preprocessing to remove references to +Itemrefs. + +=back +=cut + +sub PrepareFilter($$) +{ + my ($self, $Filter) = @_; + + die "undefined filter" if (!defined $Filter); + if ($Filter->{Terms}) + { + foreach my $Term (@{$Filter->{Terms}}) + { + $self->PrepareFilter($Term); + } + return; + } + if ($Filter->{Term}) + { + $self->PrepareFilter($Filter->{Term}); + return; + } + + my $PropertyName = $Filter->{Property}; + my $PropertyDescriptor = $self->GetPropertyDescriptorByName($PropertyName); + die "unknown $PropertyName property" if (!$PropertyDescriptor); + if ($PropertyDescriptor->GetClass() eq "Detailref") + { + die "$PropertyName: cannot filter on Detailref properties"; + } + + if ($Filter->{Type} eq "value") + { + if ($PropertyDescriptor->GetClass() eq "Itemref") + { + if ($Filter->{Comparator} ne "=" and $Filter->{Comparator} ne "<>") + { + die "$PropertyName: cannot perform $Filter->{Comparator} comparisons on Itemrefs"; + } + + my $RefColNames = $PropertyDescriptor->GetRefColNames(); + if (@$RefColNames == 1) + { + $Filter->{Property} = $RefColNames->[0]; + # @$RefColNames == 1 => $Value->GetKey() == column value + $Filter->{Values} = [ map { $_->GetKey() } @{$Filter->{Values}} ]; + $self->PrepareFilter($Filter); + } + elsif (@{$Filter->{Values}} == 1) + { + delete $Filter->{Property}; + delete $Filter->{Values}; + $Filter->{Type} = $Filter->{Comparator} eq "=" ? 'and' : 'or'; + + # The target table column names are unknown and thus cannot be accessed + # directly. But their values correspond to the target's key components. + my ($_ColNames, $ColValues) = $Filter->{Values}->[0]->GetMasterKey(); + foreach my $ColIndex (0..$#{$RefColNames}) + { + my $Term = FilterValue($RefColNames->[$ColIndex], $ColValues->[$ColIndex], $Filter->{Comparator}); + $self->PrepareFilter($Term); + push @{$Filter->{Terms}}, $Term; + } + } + else + { + my @Terms; + foreach my $Item (@{$Filter->{Values}}) + { + my $Term = FilterValue($PropertyName, [$Item], $Filter->{Comparator}); + $self->PrepareFilter($Term); + push @Terms, $Term; + } + delete $Filter->{Property}; + delete $Filter->{Values}; + $Filter->{Type} = 'or'; + $Filter->{Terms} = @Terms; + } + } + elsif ($Filter->{Comparator} ne "=" and $Filter->{Comparator} ne "<>" and + $PropertyDescriptor->GetClass() eq "Basic" and + $PropertyDescriptor->GetType() eq "B") + { + die "$PropertyName: cannot perform $Filter->{Comparator} comparisons on booleans"; + } + } + elsif ($Filter->{Type} =~ /null/) + { + if ($PropertyDescriptor->GetIsRequired()) + { + die "$PropertyName cannot be NULL (pointless filter)"; + } + elsif ($PropertyDescriptor->GetClass() eq "Itemref") + { + my $RefColNames = $PropertyDescriptor->GetRefColNames(); + if (@$RefColNames > 1) + { + delete $Filter->{Property}; + $Filter->{Terms} = [ map { { Type => $Filter->{Type}, Property => $_ } } @$RefColNames ]; + $Filter->{Type} = $Filter->{Type} eq 'null' ? 'and' : 'or'; + } + else + { + $Filter->{Property} = $RefColNames->[0]; + } + } + } +} + sub AddFilter($$;$$) { my $self = shift; - my $Filter = @_ == 1 ? $_[0] : FilterValue($_[0], $_[2] || "=", $_[1]); + my $Filter =@_ == 1 ? $_[0] : FilterValue($_[0], $_[2] || "=", $_[1]); + $self->PrepareFilter($Filter);
if (!$self->{Filter}) { diff --git a/testbot/lib/ObjectModel/DBIBackEnd.pm b/testbot/lib/ObjectModel/DBIBackEnd.pm index e837d47f5..d21b3f03f 100644 --- a/testbot/lib/ObjectModel/DBIBackEnd.pm +++ b/testbot/lib/ObjectModel/DBIBackEnd.pm @@ -215,31 +215,6 @@ sub BuildFieldList($$) return $Fields; }
-sub ColNameToDb($) -{ - my ($PropertyDescriptor) = @_; - - my $ColNames; - if ($PropertyDescriptor->GetClass() eq "Itemref") - { - $ColNames = $PropertyDescriptor->GetRefColNames(); - if (@$ColNames != 1) - { - # This would require building a more complex where clause, something like - # ((col1 = item1.val1 AND col2 = item1.val2...) OR - # (col1 = item2.val1 AND col2 = item2.val2...) OR ...) - # So it is not supported. Instead one should build the where clause by - # hand from the underlying columns. - die "cannot filter on the ". $PropertyDescriptor->GetName() ." Itemref because it has a multi-column key: @$ColNames"; - } - } - else - { - $ColNames = $PropertyDescriptor->GetColNames(); - } - return $ColNames->[0]; -} - =pod =over 12
@@ -273,13 +248,8 @@ sub GetFilterWhere($$$) my (@Wheres, @Data); foreach my $ColValue (@$Values) { - my $ColName = ColNameToDb($PropertyDescriptor); + my $ColName = $PropertyDescriptor->GetColNames()->[0]; push @Wheres, "$ColName $Filter->{Comparator} ?"; - - if ($PropertyDescriptor->GetClass() eq "Itemref") - { - $ColValue = $ColValue->GetKey(); - } push @Data, $self->ToDb($ColValue, $PropertyDescriptor); } return (@$Values > 1, join(" OR ", @Wheres), @Data); @@ -312,7 +282,7 @@ sub GetFilterWhere($$$) if ($Filter->{Type} eq 'null' or $Filter->{Type} eq 'not null') { my $PropertyDescriptor = $Collection->GetPropertyDescriptorByName($Filter->{Property}); - my $ColName = ColNameToDb($PropertyDescriptor); + my $ColName = $PropertyDescriptor->GetColNames()->[0]; my $Operator = ($Filter->{Type} eq 'null' ? "IS NULL" : "IS NOT NULL"); return (0, "$ColName $Operator", []); }