Bug 351098: Make Bugzilla::Object able to update objects in the database, and make Bugzilla::Keyword use it

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=myk

12 files changed, 248 insertions, 182 deletions

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,16 +54,23 @@ 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 <mkanat@bugzilla.org>
 
 use strict;
@@ -116,6 +120,48 @@ 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<create()>. This should be an array.
 =item C<VALIDATORS>
 
 A hashref that points to a function that will validate each param to
-C<create()>. Each function in this hashref will be passed a single
-argument, the value passed to C<create()> for that field. These
-functions should call L<Bugzilla::Error/ThrowUserError> if they fail.
-They must return the validated value.
+L</create>. 
+
+Validators are called both by L</create> and L</set>. When
+they are called by L</create>, the first argument will be the name
+of the class (what we normally call C<$class>).
+
+When they are called by L</set>, 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</create> or 
+L</set>for that field. 
+
+These functions should call L<Bugzilla::Error/ThrowUserError> if they fail.
+
+The validator must return the validated value.
+
+=item C<UPDATE_COLUMNS>
+
+A list of columns to update when L</update> is called.
+If a field can't be changed, it shouldn't be listed here. (For example,
+the L</ID_FIELD> usually can't be updated.)
 
 =back
 
 =head1 METHODS
 
+=head2 Constructors
+
 =over
 
 =item C<new($param)>
@@ -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<update>
+
+Saves the values currently in this object to the database.
+Only the fields specified in L</UPDATE_COLUMNS> 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<CodeError>.
+
+=over
+
+=item C<set>
+
+=over
+
+=item B<Description>
+
+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<set_> mutators for their different
+fields.
+
+See L</VALIDATORS> for more information.
+
+=item B<Params>
+
+=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</VALIDATORS>, if it exists there.
+
+=item C<$value> - The value that you're setting the field to.
+
+=back
+
+=item B<Returns> (nothing)
+
+=back
+
+=back
+
+=head1 CLASS FUNCTIONS
+
+=over
+
 =item C<get_all>
 
  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
diff --git a/createaccount.cgi b/createaccount.cgi
index f58b8402b..1f372fe7e 100755
--- a/createaccount.cgi
+++ b/createaccount.cgi
@@ -60,7 +60,7 @@ unless ($createexp) {
 my $login = $cgi->param('login');
 
 if (defined($login)) {
-    $login = Bugzilla::User::check_login_name_for_creation($login);
+    $login = Bugzilla::User->check_login_name_for_creation($login);
     $vars->{'login'} = $login;
 
     if ($login !~ /$createexp/) {
diff --git a/editkeywords.cgi b/editkeywords.cgi
index f2403c372..bf130768e 100755
--- a/editkeywords.cgi
+++ b/editkeywords.cgi
@@ -34,34 +34,6 @@ my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
 my $vars = {};
 
-sub Validate {
-    my ($name, $description) = @_;
-    if ($name eq "") {
-        ThrowUserError("keyword_blank_name");
-    }
-    if ($name =~ /[\s,]/) {
-        ThrowUserError("keyword_invalid_name");
-    }    
-    if ($description eq "") {
-        ThrowUserError("keyword_blank_description");
-    }
-    # It is safe to detaint these values as they are only
-    # used in placeholders.
-    trick_taint($name);
-    $_[0] = $name;
-    trick_taint($description);
-    $_[1] = $description;
-}
-
-sub ValidateKeyID {
-    my $id = shift;
-
-    $id = trim($id || 0);
-    detaint_natural($id) || ThrowCodeError('invalid_keyword_id');
-    return $id;
-}
-
-
 #
 # Preliminary checks:
 #
@@ -75,7 +47,8 @@ $user->in_group('editkeywords')
                                      action => "edit",
                                      object => "keywords"});
 
-my $action  = trim($cgi->param('action')  || '');
+my $action = trim($cgi->param('action')  || '');
+my $key_id = $cgi->param('id');
 $vars->{'action'} = $action;
 
 
@@ -127,32 +100,14 @@ if ($action eq 'new') {
 #
 
 if ($action eq 'edit') {
-    my $id = ValidateKeyID(scalar $cgi->param('id'));
-
-    # get data of keyword
-    my ($name, $description) =
-        $dbh->selectrow_array('SELECT name, description FROM keyworddefs
-                               WHERE id = ?', undef, $id);
-
-    if (!$name) {
-        $vars->{'id'} = $id;
-        ThrowCodeError("invalid_keyword_id", $vars);
-    }
-
-    my $bugs = $dbh->selectrow_array('SELECT COUNT(*) FROM keywords
-                                      WHERE keywordid = ?',
-                                      undef, $id);
+    my $keyword = new Bugzilla::Keyword($key_id)
+        || ThrowCodeError('invalid_keyword_id', { id => $key_id });
 
-    $vars->{'keyword_id'} = $id;
-    $vars->{'name'} = $name;
-    $vars->{'description'} = $description;
-    $vars->{'bug_count'} = $bugs;
+    $vars->{'keyword'} = $keyword;
 
     print $cgi->header();
-
     $template->process("admin/keywords/edit.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
-
     exit;
 }
 
@@ -162,27 +117,16 @@ if ($action eq 'edit') {
 #
 
 if ($action eq 'update') {
-    my $id = ValidateKeyID(scalar $cgi->param('id'));
-
-    my $name  = trim($cgi->param('name') || '');
-    my $description  = trim($cgi->param('description')  || '');
+    my $keyword = new Bugzilla::Keyword($key_id)
+        || ThrowCodeError('invalid_keyword_id', { id => $key_id });
 
-    Validate($name, $description);
-
-    my $tmp = $dbh->selectrow_array('SELECT id FROM keyworddefs
-                                     WHERE name = ?', undef, $name);
-
-    if ($tmp && $tmp != $id) {
-        $vars->{'name'} = $name;
-        ThrowUserError("keyword_already_exists", $vars);
-    }
-
-    $dbh->do('UPDATE keyworddefs SET name = ?, description = ?
-              WHERE id = ?', undef, ($name, $description, $id));
+    $keyword->set_name($cgi->param('name'));
+    $keyword->set_description($cgi->param('description'));
+    $keyword->update();
 
     print $cgi->header();
 
-    $vars->{'name'} = $name;
+    $vars->{'keyword'} = $keyword;
     $template->process("admin/keywords/rebuild-cache.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
@@ -191,36 +135,23 @@ if ($action eq 'update') {
 
 
 if ($action eq 'delete') {
-    my $id = ValidateKeyID(scalar $cgi->param('id'));
-
-    my $name = $dbh->selectrow_array('SELECT name FROM keyworddefs
-                                      WHERE id= ?', undef, $id);
-
-    if (!$cgi->param('reallydelete')) {
-        my $bugs = $dbh->selectrow_array('SELECT COUNT(*) FROM keywords
-                                          WHERE keywordid = ?',
-                                          undef, $id);
-
-        if ($bugs) {
-            $vars->{'bug_count'} = $bugs;
-            $vars->{'keyword_id'} = $id;
-            $vars->{'name'} = $name;
-
-            print $cgi->header();
+    my $keyword =  new Bugzilla::Keyword($key_id)
+        || ThrowCodeError('invalid_keyword_id', { id => $key_id });
 
-            $template->process("admin/keywords/confirm-delete.html.tmpl", $vars)
-              || ThrowTemplateError($template->error());
+    $vars->{'keyword'} = $keyword;
 
-            exit;
-        }
+    if (!$cgi->param('reallydelete') && $keyword->bug_count) {
+        print $cgi->header();
+        $template->process("admin/keywords/confirm-delete.html.tmpl", $vars)
+            || ThrowTemplateError($template->error());
+        exit;
     }
 
-    $dbh->do('DELETE FROM keywords WHERE keywordid = ?', undef, $id);
-    $dbh->do('DELETE FROM keyworddefs WHERE id = ?', undef, $id);
+    $dbh->do('DELETE FROM keywords WHERE keywordid = ?', undef, $keyword->id);
+    $dbh->do('DELETE FROM keyworddefs WHERE id = ?', undef, $keyword->id);
 
     print $cgi->header();
 
-    $vars->{'name'} = $name;
     $template->process("admin/keywords/rebuild-cache.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
 
diff --git a/post_bug.cgi b/post_bug.cgi
index db59750f1..e906ca6ba 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -116,7 +116,7 @@ $template->process($format->{'template'}, $vars, \$comment)
 
 # Check that the product exists and that the user
 # is allowed to enter bugs into this product.
-my $product = Bugzilla::Bug::_check_product($cgi->param('product'));
+my $product = Bugzilla::Bug->_check_product($cgi->param('product'));
 
 # Set cookies
 if (defined $cgi->param('product')) {
@@ -141,26 +141,26 @@ umask 0;
 
 # This has to go somewhere after 'maketemplate' 
 # or it breaks bookmarks with no comments. 
-$comment = Bugzilla::Bug::_check_comment($cgi->param('comment'));
+$comment = Bugzilla::Bug->_check_comment($cgi->param('comment'));
 # If comment is all whitespace, it'll be null at this point. That's
 # OK except for the fact that it causes e-mail to be suppressed.
 $comment = $comment ? $comment : " ";
 
-my $cc_ids = Bugzilla::Bug::_check_cc([$cgi->param('cc')]);
-my @keyword_ids = @{Bugzilla::Bug::_check_keywords($cgi->param('keywords'))};
+my $cc_ids = Bugzilla::Bug->_check_cc([$cgi->param('cc')]);
+my @keyword_ids = @{Bugzilla::Bug->_check_keywords($cgi->param('keywords'))};
 
 # XXX These checks are only here until strict_isolation can move fully
 # into Bugzilla::Bug.
-my $component = Bugzilla::Bug::_check_component($product, 
+my $component = Bugzilla::Bug->_check_component($product, 
     $cgi->param('component'));
-my $assigned_to_id = Bugzilla::Bug::_check_assigned_to($component,
+my $assigned_to_id = Bugzilla::Bug->_check_assigned_to($component,
     $cgi->param('assigned_to'));
-my $qa_contact_id = Bugzilla::Bug::_check_qa_contact($component,
+my $qa_contact_id = Bugzilla::Bug->_check_qa_contact($component,
     $cgi->param('qa_contact'));
-Bugzilla::Bug::_check_strict_isolation($product, $cc_ids, $assigned_to_id, 
+Bugzilla::Bug->_check_strict_isolation($product, $cc_ids, $assigned_to_id, 
     $qa_contact_id);
 
-my ($depends_on_ids, $blocks_ids) = Bugzilla::Bug::_check_dependencies(
+my ($depends_on_ids, $blocks_ids) = Bugzilla::Bug->_check_dependencies(
     scalar $cgi->param('dependson'), scalar $cgi->param('blocked'));
 
 # get current time
diff --git a/template/en/default/admin/keywords/confirm-delete.html.tmpl b/template/en/default/admin/keywords/confirm-delete.html.tmpl
index bb29437ef..89123e2bf 100755
--- a/template/en/default/admin/keywords/confirm-delete.html.tmpl
+++ b/template/en/default/admin/keywords/confirm-delete.html.tmpl
@@ -18,12 +18,11 @@
   #
   # Contributor(s): Terry Weissman <terry@mozilla.org>
   #                 Vlad Dascalu <jocuri@softhome.net>
+  #                 Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# INTERFACE:
-  # name: string. The name of the keyword.
-  # keyword_id: number. The ID of the keyword.
-  # bug_count: number. The number of bugs with the keyword.
+  # keyword: A Bugzilla::Keyword object.
   #%]
 
 [% PROCESS global/header.html.tmpl
@@ -31,19 +30,19 @@
 %]
 
 <p>
-  [% IF bug_count == 1 %]
+  [% IF keyword.bug_count == 1 %]
     There is one [% terms.bug %] with this keyword set.
   [% ELSE %]
-    There are [% bug_count %] [%+terms.bugs %] with
+    There are [% keyword.bug_count FILTER html %] [%+ terms.bugs %] with
     this keyword set.
   [% END %]
   
   Are you <b>sure</b> you want to delete
-  the <code>[% name FILTER html %]</code> keyword?
+  the <code>[% keyword.name FILTER html %]</code> keyword?
 </p>
 
 <form method="post" action="editkeywords.cgi">
-  <input type="hidden" name="id" value="[% keyword_id %]">
+  <input type="hidden" name="id" value="[% keyword.id FILTER html %]">
   <input type="hidden" name="action" value="delete">
   <input type="hidden" name="reallydelete" value="1">
   <input type="submit" id="delete"
diff --git a/template/en/default/admin/keywords/edit.html.tmpl b/template/en/default/admin/keywords/edit.html.tmpl
index b6b5fc932..0d3beaf33 100755
--- a/template/en/default/admin/keywords/edit.html.tmpl
+++ b/template/en/default/admin/keywords/edit.html.tmpl
@@ -18,13 +18,11 @@
   #
   # Contributor(s): Terry Weissman <terry@mozilla.org>
   #                 Vlad Dascalu <jocuri@softhome.net>
+  #                 Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# INTERFACE:
-  # keyword_id: number. The ID of the keyword.
-  # name: string. The name of the keyword.
-  # description: string. The description of the keyword.
-  # bug_count: number. The number of bugs with the keyword.
+  # keyword: A Bugzilla::Keyword object.
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
@@ -37,7 +35,8 @@
   <table border="0" cellpadding="4" cellspacing="0">
     <tr>
       <th align="right">Name:</th>
-      <td><input size="64" maxlength="64" name="name" value="[% name FILTER html %]"></td>
+      <td><input size="64" maxlength="64" name="name" 
+                 value="[% keyword.name FILTER html %]"></td>
     </tr>
     <tr>
       <th align="right">Description:</th>
@@ -47,16 +46,16 @@
           minrows        = 4
           cols           = 64
           wrap           = 'virtual'
-          defaultcontent = description
+          defaultcontent = keyword.description
         %]
       </td>
     </tr>
     <tr>
       <th align="right">[% terms.Bugs %]:</th>
       <td>
-        [% IF bug_count > 0 %]
-          <a href="buglist.cgi?keywords=[% name FILTER url_quote %]">
-            [% bug_count %]</a>
+        [% IF keyword.bug_count > 0 %]
+          <a href="buglist.cgi?keywords=[% keyword.name FILTER url_quote %]">
+            [% keyword.bug_count FILTER html %]</a>
         [% ELSE %]
           none
         [% END %]
@@ -66,7 +65,7 @@
 
   <input type="submit" id="update" value="Update">
   <input type="hidden" name="action" value="update">
-  <input type="hidden" name="id" value="[% keyword_id %]">
+  <input type="hidden" name="id" value="[% keyword.id FILTER html %]">
 </form>
 
 <p><a href="editkeywords.cgi">Edit other keywords</a>.</p>
diff --git a/template/en/default/admin/keywords/rebuild-cache.html.tmpl b/template/en/default/admin/keywords/rebuild-cache.html.tmpl
index 84b7a7efa..f9f6f31c0 100755
--- a/template/en/default/admin/keywords/rebuild-cache.html.tmpl
+++ b/template/en/default/admin/keywords/rebuild-cache.html.tmpl
@@ -18,11 +18,12 @@
   #
   # Contributor(s): Terry Weissman <terry@mozilla.org>
   #                 Vlad Dascalu <jocuri@softhome.net>
+  #                 Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# INTERFACE:
   # action: string; the current action (either "update" or "delete").
-  # name: string; the name of the current keyword.
+  # keyword: A Bugzilla::Keyword object
   #%]
   
 [% IF action == "update" %]
@@ -35,7 +36,7 @@
 
 [% PROCESS global/header.html.tmpl %]
 
-Keyword [% name FILTER html %] [%+status FILTER html %].
+Keyword [% keyword.name FILTER html %] [%+ status FILTER html %].
 
 <p>
   <b>After you have finished deleting or modifying keywords,
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index 2723b8dd9..9a7b3d50d 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -515,16 +515,6 @@
   'classification_text', 
 ],
 
-'admin/keywords/edit.html.tmpl' => [
-  'keyword_id',
-  'bug_count',
-],
-
-'admin/keywords/confirm-delete.html.tmpl' => [
-  'keyword_id',
-  'bug_count',
-],
-
 'admin/flag-type/confirm-delete.html.tmpl' => [
   'flag_type.flag_count',
   'flag_type.id', 
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 27798491b..a66524355 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -296,6 +296,18 @@
     a <code>[% param FILTER html %]</code> argument, and that
     argument was not set.
 
+  [% ELSIF error == "protection_violation" %]
+    The function <code>[% function FILTER html %]</code> was called
+    from
+   
+    [% IF caller %]
+        <code>[%+ caller FILTER html %]</code>, which is
+    [% END %]
+
+    outside the package. This function may only be called from
+    a subclass of <code>[% superclass FILTER html %]</code>.
+
+
   [% ELSIF error == "unknown_comparison_type" %]
     Specified comparison type is not supported.