From 7f16a9065617dd087712f9daad630967f4695585 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Mon, 4 Feb 2008 18:23:47 +0000 Subject: Bug 413772: Eliminate sqlify_criteria() in Bugzilla::Flag and replace match() there with Bugzilla::Object::match() - Patch by Max Kanat-Alexander r/a=LpSolit --- Bugzilla/Attachment.pm | 4 +- Bugzilla/Bug.pm | 4 +- Bugzilla/Flag.pm | 100 +++++++++++++------------------------------------ attachment.cgi | 4 +- editusers.cgi | 4 +- post_bug.cgi | 2 +- process_bug.cgi | 2 +- 7 files changed, 35 insertions(+), 85 deletions(-) diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 954765f03..b4ac9e6f0 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -426,7 +426,7 @@ sub flags { my $self = shift; return $self->{flags} if exists $self->{flags}; - $self->{flags} = Bugzilla::Flag::match({ 'attach_id' => $self->id }); + $self->{flags} = Bugzilla::Flag->match({ 'attach_id' => $self->id }); return $self->{flags}; } @@ -918,7 +918,7 @@ sub insert_attachment_for_bug { Bugzilla->error_mode(ERROR_MODE_DIE); eval { Bugzilla::Flag::validate($cgi, $bug->bug_id, -1, SKIP_REQUESTEE_ON_ERROR); - Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi, $hr_vars); + Bugzilla::Flag->process($bug, $attachment, $timestamp, $cgi, $hr_vars); }; Bugzilla->error_mode($error_mode_cache); if ($@) { diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index bc50ccff4..364f5730c 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -2312,7 +2312,7 @@ sub flag_types { 'component_id' => $self->{'component_id'} }); foreach my $flag_type (@$flag_types) { - $flag_type->{'flags'} = Bugzilla::Flag::match( + $flag_type->{'flags'} = Bugzilla::Flag->match( { 'bug_id' => $self->bug_id, 'type_id' => $flag_type->{'id'}, 'target_type' => 'bug' }); @@ -2432,7 +2432,7 @@ sub show_attachment_flags { { 'target_type' => 'attachment', 'product_id' => $self->{'product_id'}, 'component_id' => $self->{'component_id'} }); - my $num_attachment_flags = Bugzilla::Flag::count( + my $num_attachment_flags = Bugzilla::Flag->count( { 'target_type' => 'attachment', 'bug_id' => $self->bug_id }); diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 9dc6a2ef5..dc298634d 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -202,16 +202,21 @@ and returns an array of matching records. =cut sub match { + my $class = shift; my ($criteria) = @_; - my $dbh = Bugzilla->dbh; - - my @criteria = sqlify_criteria($criteria); - $criteria = join(' AND ', @criteria); - my $flag_ids = $dbh->selectcol_arrayref("SELECT id FROM flags - WHERE $criteria"); + # If the caller specified only bug or attachment flags, + # limit the query to those kinds of flags. + if (my $type = delete $criteria->{'target_type'}) { + if ($type eq 'attachment') { + $criteria->{'attach_id'} = NOT_NULL; + } + else { + $criteria->{'attach_id'} = IS_NULL; + } + } - return Bugzilla::Flag->new_from_list($flag_ids); + return $class->SUPER::match(@_); } =pod @@ -229,15 +234,8 @@ and returns an array of matching records. =cut sub count { - my ($criteria) = @_; - my $dbh = Bugzilla->dbh; - - my @criteria = sqlify_criteria($criteria); - $criteria = join(' AND ', @criteria); - - my $count = $dbh->selectrow_array("SELECT COUNT(*) FROM flags WHERE $criteria"); - - return $count; + my $class = shift; + return scalar @{$class->match(@_)}; } ###################################################################### @@ -485,10 +483,10 @@ sub _validate { } sub snapshot { - my ($bug_id, $attach_id) = @_; + my ($class, $bug_id, $attach_id) = @_; - my $flags = match({ 'bug_id' => $bug_id, - 'attach_id' => $attach_id }); + my $flags = $class->match({ 'bug_id' => $bug_id, + 'attach_id' => $attach_id }); my @summaries; foreach my $flag (@$flags) { my $summary = $flag->type->name . $flag->status; @@ -517,7 +515,7 @@ object used to obtain the flag fields that the user submitted. =cut sub process { - my ($bug, $attachment, $timestamp, $cgi, $hr_vars) = @_; + my ($class, $bug, $attachment, $timestamp, $cgi, $hr_vars) = @_; my $dbh = Bugzilla->dbh; # Make sure the bug (and attachment, if given) exists and is accessible @@ -533,7 +531,7 @@ sub process { $timestamp ||= $dbh->selectrow_array('SELECT NOW()'); # Take a snapshot of flags before any changes. - my @old_summaries = snapshot($bug_id, $attach_id); + my @old_summaries = $class->snapshot($bug_id, $attach_id); # Cancel pending requests if we are obsoleting an attachment. if ($attachment && $cgi->param('isobsolete')) { @@ -586,7 +584,7 @@ sub process { } # Take a snapshot of flags after changes. - my @new_summaries = snapshot($bug_id, $attach_id); + my @new_summaries = $class->snapshot($bug_id, $attach_id); update_activity($bug_id, $attach_id, $timestamp, \@old_summaries, \@new_summaries); } @@ -865,7 +863,7 @@ sub retarget { foreach my $flagtype (@$flagtypes) { # Get the number of flags of this type already set for this target. - my $has_flags = count( + my $has_flags = __PACKAGE__->count( { 'type_id' => $flagtype->id, 'bug_id' => $bug->bug_id, 'attach_id' => $flag->attach_id }); @@ -989,7 +987,7 @@ sub FormToNewFlags { next unless scalar(grep { $_ == $type_id } @type_ids); # Get the number of flags of this type already set for this target. - my $has_flags = count( + my $has_flags = __PACKAGE__->count( { 'type_id' => $type_id, 'target_type' => $attachment ? 'attachment' : 'bug', 'bug_id' => $bug->bug_id, @@ -1111,7 +1109,8 @@ sub CancelRequests { return if (!scalar(@$request_ids)); # Take a snapshot of flags before any changes. - my @old_summaries = snapshot($bug->bug_id, $attachment->id) if ($timestamp); + my @old_summaries = __PACKAGE__->snapshot($bug->bug_id, $attachment->id) + if ($timestamp); my $flags = Bugzilla::Flag->new_from_list($request_ids); foreach my $flag (@$flags) { clear($flag, $bug, $attachment) } @@ -1119,60 +1118,11 @@ sub CancelRequests { return unless ($timestamp); # Take a snapshot of flags after any changes. - my @new_summaries = snapshot($bug->bug_id, $attachment->id); + my @new_summaries = __PACKAGE__->snapshot($bug->bug_id, $attachment->id); update_activity($bug->bug_id, $attachment->id, $timestamp, \@old_summaries, \@new_summaries); } -###################################################################### -# Private Functions -###################################################################### - -=begin private - -=head1 PRIVATE FUNCTIONS - -=over - -=item C - -Converts a hash of criteria into a list of SQL criteria. - -=back - -=cut - -sub sqlify_criteria { - # a reference to a hash containing the criteria (field => value) - my ($criteria) = @_; - - # the generated list of SQL criteria; "1=1" is a clever way of making sure - # there's something in the list so calling code doesn't have to check list - # size before building a WHERE clause out of it - my @criteria = ("1=1"); - - # If the caller specified only bug or attachment flags, - # limit the query to those kinds of flags. - if (defined($criteria->{'target_type'})) { - if ($criteria->{'target_type'} eq 'bug') { push(@criteria, "attach_id IS NULL") } - elsif ($criteria->{'target_type'} eq 'attachment') { push(@criteria, "attach_id IS NOT NULL") } - } - - # Go through each criterion from the calling code and add it to the query. - foreach my $field (keys %$criteria) { - my $value = $criteria->{$field}; - next unless defined($value); - if ($field eq 'type_id') { push(@criteria, "type_id = $value") } - elsif ($field eq 'bug_id') { push(@criteria, "bug_id = $value") } - elsif ($field eq 'attach_id') { push(@criteria, "attach_id = $value") } - elsif ($field eq 'requestee_id') { push(@criteria, "requestee_id = $value") } - elsif ($field eq 'setter_id') { push(@criteria, "setter_id = $value") } - elsif ($field eq 'status') { push(@criteria, "status = '$value'") } - } - - return @criteria; -} - =head1 SEE ALSO =over diff --git a/attachment.cgi b/attachment.cgi index 65a8aa539..449abde65 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -423,7 +423,7 @@ sub edit { 'product_id' => $product_id , 'component_id' => $component_id }); foreach my $flag_type (@$flag_types) { - $flag_type->{'flags'} = Bugzilla::Flag::match({ 'type_id' => $flag_type->id, + $flag_type->{'flags'} = Bugzilla::Flag->match({ 'type_id' => $flag_type->id, 'attach_id' => $attachment->id }); } $vars->{'flag_types'} = $flag_types; @@ -529,7 +529,7 @@ sub update { # to attachments so that we can delete pending requests if the user # is obsoleting this attachment without deleting any requests # the user submits at the same time. - Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi, $vars); + Bugzilla::Flag->process($bug, $attachment, $timestamp, $cgi, $vars); # Update the attachment record in the database. $dbh->do("UPDATE attachments diff --git a/editusers.cgi b/editusers.cgi index 6dda0e97a..11bd62bde 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -484,14 +484,14 @@ if ($action eq 'search') { foreach (@$buglist) { my ($bug_id, $attach_id) = @$_; - my @old_summaries = Bugzilla::Flag::snapshot($bug_id, $attach_id); + my @old_summaries = Bugzilla::Flag->snapshot($bug_id, $attach_id); if ($attach_id) { $sth_flagupdate_attachment->execute($bug_id, $attach_id, $otherUserID); } else { $sth_flagupdate_bug->execute($bug_id, $otherUserID); } - my @new_summaries = Bugzilla::Flag::snapshot($bug_id, $attach_id); + my @new_summaries = Bugzilla::Flag->snapshot($bug_id, $attach_id); # Let update_activity do all the dirty work, including setting # the bug timestamp. Bugzilla::Flag::update_activity($bug_id, $attach_id, $timestamp, diff --git a/post_bug.cgi b/post_bug.cgi index 0f23e7d98..89edca2f4 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -230,7 +230,7 @@ my $error_mode_cache = Bugzilla->error_mode; Bugzilla->error_mode(ERROR_MODE_DIE); eval { Bugzilla::Flag::validate($cgi, $id, undef, SKIP_REQUESTEE_ON_ERROR); - Bugzilla::Flag::process($bug, undef, $timestamp, $cgi, $vars); + Bugzilla::Flag->process($bug, undef, $timestamp, $cgi, $vars); }; Bugzilla->error_mode($error_mode_cache); if ($@) { diff --git a/process_bug.cgi b/process_bug.cgi index 8eb7aaf33..7c5902e73 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -550,7 +550,7 @@ foreach my $bug (@bug_objects) { } # Set and update flags. - Bugzilla::Flag::process($bug, undef, $timestamp, $cgi, $vars); + Bugzilla::Flag->process($bug, undef, $timestamp, $cgi, $vars); $dbh->bz_commit_transaction(); -- cgit v1.2.3-24-g4f1b