From 8e808ffbf7b7b28a1cdfda3d188cc156a2e879d9 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Wed, 6 Sep 2006 02:18:26 +0000 Subject: Bug 351098: Make Bugzilla::Object able to update objects in the database, and make Bugzilla::Keyword use it Patch By Max Kanat-Alexander r=LpSolit, a=myk --- Bugzilla/Bug.pm | 62 ++++++++++++------------- Bugzilla/Keyword.pm | 40 +++++++++++----- Bugzilla/Object.pm | 128 +++++++++++++++++++++++++++++++++++++++++++++++++--- Bugzilla/User.pm | 10 ++-- 4 files changed, 187 insertions(+), 53 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index c984c8a98..e8f90dfdf 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -228,26 +228,26 @@ sub run_create_validators { my $class = shift; my $params = shift; - my $product = _check_product($params->{product}); + my $product = $class->_check_product($params->{product}); $params->{product_id} = $product->id; delete $params->{product}; ($params->{bug_status}, $params->{everconfirmed}) - = _check_bug_status($product, $params->{bug_status}); + = $class->_check_bug_status($product, $params->{bug_status}); - $params->{target_milestone} = _check_target_milestone($product, + $params->{target_milestone} = $class->_check_target_milestone($product, $params->{target_milestone}); - $params->{version} = _check_version($product, $params->{version}); + $params->{version} = $class->_check_version($product, $params->{version}); - my $component = _check_component($product, $params->{component}); + my $component = $class->_check_component($product, $params->{component}); $params->{component_id} = $component->id; delete $params->{component}; $params->{assigned_to} = - _check_assigned_to($component, $params->{assigned_to}); + $class->_check_assigned_to($component, $params->{assigned_to}); $params->{qa_contact} = - _check_qa_contact($component, $params->{qa_contact}); + $class->_check_qa_contact($component, $params->{qa_contact}); # Callers cannot set Reporter, currently. $params->{reporter} = Bugzilla->user->id; @@ -332,7 +332,7 @@ sub remove_from_db { ##################################################################### sub _check_alias { - my ($alias) = @_; + my ($invocant, $alias) = @_; $alias = trim($alias); return undef if (!Bugzilla->params->{'usebugaliases'} || !$alias); ValidateBugAlias($alias); @@ -340,7 +340,7 @@ sub _check_alias { } sub _check_assigned_to { - my ($component, $name) = @_; + my ($invocant, $component, $name) = @_; my $user = Bugzilla->user; $name = trim($name); @@ -355,21 +355,21 @@ sub _check_assigned_to { } sub _check_bug_file_loc { - my ($url) = @_; + my ($invocant, $url) = @_; # If bug_file_loc is "http://", the default, use an empty value instead. $url = '' if $url eq 'http://'; return $url; } sub _check_bug_severity { - my ($severity) = @_; + my ($invocant, $severity) = @_; $severity = trim($severity); check_field('bug_severity', $severity); return $severity; } sub _check_bug_status { - my ($product, $status) = @_; + my ($invocant, $product, $status) = @_; my $user = Bugzilla->user; my @valid_statuses = VALID_ENTRY_STATUS; @@ -396,7 +396,7 @@ sub _check_bug_status { } sub _check_cc { - my ($ccs) = @_; + my ($invocant, $ccs) = @_; return [] unless $ccs; my %cc_ids; @@ -409,7 +409,7 @@ sub _check_cc { } sub _check_comment { - my ($comment) = @_; + my ($invocant, $comment) = @_; if (!defined $comment) { ThrowCodeError('undefined_field', { field => 'comment' }); @@ -430,7 +430,7 @@ sub _check_comment { } sub _check_component { - my ($product, $name) = @_; + my ($invocant, $product, $name) = @_; $name = trim($name); $name || ThrowUserError("require_component"); my $obj = Bugzilla::Component::check_component($product, $name); @@ -441,7 +441,7 @@ sub _check_component { } sub _check_deadline { - my ($date) = @_; + my ($invocant, $date) = @_; $date = trim($date); my $tt_group = Bugzilla->params->{"timetrackinggroup"}; return undef unless $date && $tt_group @@ -455,7 +455,7 @@ sub _check_deadline { # Takes two comma/space-separated strings and returns arrayrefs # of valid bug IDs. sub _check_dependencies { - my ($depends_on, $blocks) = @_; + my ($invocant, $depends_on, $blocks) = @_; # Only editbugs users can set dependencies on bug entry. return ([], []) unless Bugzilla->user->in_group('editbugs'); @@ -482,11 +482,11 @@ sub _check_dependencies { } sub _check_estimated_time { - return _check_time($_[0], 'estimated_time'); + return $_[0]->_check_time($_[1], 'estimated_time'); } sub _check_keywords { - my ($keyword_string) = @_; + my ($invocant, $keyword_string) = @_; $keyword_string = trim($keyword_string); return [] if (!$keyword_string || !Bugzilla->user->in_group('editbugs')); @@ -501,7 +501,7 @@ sub _check_keywords { } sub _check_product { - my ($name) = @_; + my ($invocant, $name) = @_; # Check that the product exists and that the user # is allowed to enter bugs into this product. Bugzilla->user->can_enter_product($name, THROW_ERROR); @@ -515,14 +515,14 @@ sub _check_product { } sub _check_op_sys { - my ($op_sys) = @_; + my ($invocant, $op_sys) = @_; $op_sys = trim($op_sys); check_field('op_sys', $op_sys); return $op_sys; } sub _check_priority { - my ($priority) = @_; + my ($invocant, $priority) = @_; if (!Bugzilla->params->{'letsubmitterchoosepriority'}) { $priority = Bugzilla->params->{'defaultpriority'}; } @@ -533,18 +533,18 @@ sub _check_priority { } sub _check_remaining_time { - return _check_time($_[0], 'remaining_time'); + return $_[0]->_check_time($_[1], 'remaining_time'); } sub _check_rep_platform { - my ($platform) = @_; + my ($invocant, $platform) = @_; $platform = trim($platform); check_field('rep_platform', $platform); return $platform; } sub _check_short_desc { - my ($short_desc) = @_; + my ($invocant, $short_desc) = @_; # Set the parameter to itself, but cleaned up $short_desc = clean_text($short_desc) if $short_desc; @@ -554,11 +554,11 @@ sub _check_short_desc { return $short_desc; } -sub _check_status_whiteboard { return defined $_[0] ? $_[0] : ''; } +sub _check_status_whiteboard { return defined $_[1] ? $_[1] : ''; } # Unlike other checkers, this one doesn't return anything. sub _check_strict_isolation { - my ($product, $cc_ids, $assignee_id, $qa_contact_id) = @_; + my ($invocant, $product, $cc_ids, $assignee_id, $qa_contact_id) = @_; return unless Bugzilla->params->{'strict_isolation'}; @@ -588,7 +588,7 @@ sub _check_strict_isolation { } sub _check_target_milestone { - my ($product, $target) = @_; + my ($invocant, $product, $target) = @_; $target = trim($target); $target = $product->default_milestone if !defined $target; check_field('target_milestone', $target, @@ -597,7 +597,7 @@ sub _check_target_milestone { } sub _check_time { - my ($time, $field) = @_; + my ($invocant, $time, $field) = @_; my $tt_group = Bugzilla->params->{"timetrackinggroup"}; return 0 unless $tt_group && Bugzilla->user->in_group($tt_group); $time = trim($time) || 0; @@ -606,7 +606,7 @@ sub _check_time { } sub _check_qa_contact { - my ($component, $name) = @_; + my ($invocant, $component, $name) = @_; my $user = Bugzilla->user; return undef unless Bugzilla->params->{'useqacontact'}; @@ -625,7 +625,7 @@ sub _check_qa_contact { } sub _check_version { - my ($product, $version) = @_; + my ($invocant, $product, $version) = @_; $version = trim($version); check_field('version', $version, [map($_->name, @{$product->versions})]); return $version; diff --git a/Bugzilla/Keyword.pm b/Bugzilla/Keyword.pm index fead77821..2152b338d 100644 --- a/Bugzilla/Keyword.pm +++ b/Bugzilla/Keyword.pm @@ -42,6 +42,11 @@ use constant VALIDATORS => { description => \&_check_description, }; +use constant UPDATE_COLUMNS => qw( + name + description +); + ############################### #### Accessors ###### ############################### @@ -49,15 +54,22 @@ use constant VALIDATORS => { sub description { return $_[0]->{'description'}; } sub bug_count { - ($_[0]->{'bug_count'}) ||= - Bugzilla->dbh->selectrow_array('SELECT COUNT(keywords.bug_id) AS bug_count - FROM keyworddefs - LEFT JOIN keywords - ON keyworddefs.id = keywords.keywordid - WHERE keyworddefs.id=?', undef, ($_[0]->id)); - return $_[0]->{'bug_count'}; + my ($self) = @_; + return $self->{'bug_count'} if defined $self->{'bug_count'}; + ($self->{'bug_count'}) = + Bugzilla->dbh->selectrow_array( + 'SELECT COUNT(*) FROM keywords WHERE keywordid = ?', + undef, $self->id); + return $self->{'bug_count'}; } +############################### +#### Mutators ##### +############################### + +sub set_name { $_[0]->set('name', $_[1]); } +sub set_description { $_[0]->set('description', $_[1]); } + ############################### #### Subroutines ###### ############################### @@ -96,20 +108,26 @@ sub get_all_with_bug_count { ############################### sub _check_name { - my ($name) = @_; + my ($self, $name) = @_; + $name = trim($name); $name eq "" && ThrowUserError("keyword_blank_name"); if ($name =~ /[\s,]/) { ThrowUserError("keyword_invalid_name"); } - my $keyword = new Bugzilla::Keyword({ name => $name }); - ThrowUserError("keyword_already_exists", { name => $name }) if $keyword; + + # We only want to validate the non-existence of the name if + # we're creating a new Keyword or actually renaming the keyword. + if (!ref($self) || $self->name ne $name) { + my $keyword = new Bugzilla::Keyword({ name => $name }); + ThrowUserError("keyword_already_exists", { name => $name }) if $keyword; + } return $name; } sub _check_description { - my ($desc) = @_; + my ($self, $desc) = @_; $desc = trim($desc); $desc eq '' && ThrowUserError("keyword_blank_description"); return $desc; diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 67fb707e6..f89a371f2 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -12,6 +12,10 @@ # # The Original Code is the Bugzilla Bug Tracking System. # +# The Initial Developer of the Original Code is Everything Solved. +# Portions created by Everything Solved are Copyright (C) 2006 +# Everything Solved. All Rights Reserved. +# # Contributor(s): Max Kanat-Alexander use strict; @@ -115,6 +119,48 @@ sub new_from_list { sub id { return $_[0]->{'id'}; } sub name { return $_[0]->{'name'}; } +############################### +#### Methods #### +############################### + +sub set { + my ($self, $field, $value) = @_; + + # This method is protected. It's used to help implement set_ functions. + caller->isa('Bugzilla::Object') + || ThrowCodeError('protection_violation', + { caller => caller, + superclass => __PACKAGE__, + function => 'Bugzilla::Object->set' }); + + my $validators = $self->VALIDATORS; + if (exists $validators->{$field}) { + my $validator = $validators->{$field}; + $value = $self->$validator($value); + } + + $self->{$field} = $value; +} + +sub update { + my $self = shift; + + my $dbh = Bugzilla->dbh; + my $table = $self->DB_TABLE; + my $id_field = $self->ID_FIELD; + + my $columns = join(', ', map {"$_ = ?"} $self->UPDATE_COLUMNS); + my @values; + foreach my $column ($self->UPDATE_COLUMNS) { + my $value = $self->{$column}; + trick_taint($value) if defined $value; + push(@values, $value); + } + + $dbh->do("UPDATE $table SET $columns WHERE $id_field = ?", undef, + @values, $self->id); +} + ############################### #### Subroutines ###### ############################### @@ -152,7 +198,8 @@ sub run_create_validators { foreach my $field (sort keys %$params) { my $value; if (exists $validators->{$field}) { - $value = &{$validators->{$field}}($params->{$field}); + my $validator = $validators->{$field}; + $value = $class->$validator($params->{$field}); } else { $value = $params->{$field}; @@ -254,15 +301,34 @@ C. This should be an array. =item C A hashref that points to a function that will validate each param to -C. Each function in this hashref will be passed a single -argument, the value passed to C for that field. These -functions should call L if they fail. -They must return the validated value. +L. + +Validators are called both by L and L. When +they are called by L, the first argument will be the name +of the class (what we normally call C<$class>). + +When they are called by L, the first argument will be +a reference to the current object (what we normally call C<$self>). + +The second argument will be the value passed to L or +Lfor that field. + +These functions should call L if they fail. + +The validator must return the validated value. + +=item C + +A list of columns to update when L is called. +If a field can't be changed, it shouldn't be listed here. (For example, +the L usually can't be updated.) =back =head1 METHODS +=head2 Constructors + =over =item C @@ -293,7 +359,7 @@ They must return the validated value. =back -=head1 SUBROUTINES +=head2 Database Manipulation =over @@ -330,6 +396,56 @@ Returns: Two arrayrefs. The first is an array of database field names. The second is an untainted array of values that should go into those fields (in the same order). +=item C + +Saves the values currently in this object to the database. +Only the fields specified in L will be +updated. Returns nothing and takes no parameters. + +=back + +=head2 Subclass Helpers + +These functions are intended only for use by subclasses. If +you call them from anywhere else, they will throw a C. + +=over + +=item C + +=over + +=item B + +Sets a certain hash member of this class to a certain value. +Used for updating fields. Calls the validator for this field, +if it exists. Subclasses should use this function +to implement the various C mutators for their different +fields. + +See L for more information. + +=item B + +=over + +=item C<$field> - The name of the hash member to update. This should +be the same as the name of the field in L, if it exists there. + +=item C<$value> - The value that you're setting the field to. + +=back + +=item B (nothing) + +=back + +=back + +=head1 CLASS FUNCTIONS + +=over + =item C Description: Returns all objects in this table from the database. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 81bbb7fc1..4cb2c4469 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -121,13 +121,13 @@ sub new { # Validators ################################################################################ -sub _check_disable_mail { return $_[0] ? 1 : 0; } -sub _check_disabledtext { return trim($_[0]) || ''; } +sub _check_disable_mail { return $_[1] ? 1 : 0; } +sub _check_disabledtext { return trim($_[1]) || ''; } # This is public since createaccount.cgi needs to use it before issuing # a token for account creation. sub check_login_name_for_creation { - my ($name) = @_; + my ($self, $name) = @_; $name = trim($name); $name || ThrowUserError('user_login_required'); validate_email_syntax($name) @@ -138,7 +138,7 @@ sub check_login_name_for_creation { } sub _check_password { - my ($pass) = @_; + my ($self, $pass) = @_; # If the password is '*', do not encrypt it or validate it further--we # are creating a user who should not be able to log in using DB @@ -150,7 +150,7 @@ sub _check_password { return $cryptpassword; } -sub _check_realname { return trim($_[0]) || ''; } +sub _check_realname { return trim($_[1]) || ''; } ################################################################################ # Methods -- cgit v1.2.3-24-g4f1b