Bug 339381: Make Bugzilla::Classification use Bugzilla::Object - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=LpSolit

2 files changed, 59 insertions, 126 deletions

diff --git a/Bugzilla/Classification.pm b/Bugzilla/Classification.pm
index 37ae3a0cc..2eb253aa7 100644
--- a/Bugzilla/Classification.pm
+++ b/Bugzilla/Classification.pm
@@ -13,7 +13,7 @@
 # The Original Code is the Bugzilla Bug Tracking System.
 #
 # Contributor(s): Tiago R. Mello <timello@async.com.br>
-#
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 use strict;
 
@@ -23,68 +23,76 @@ use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::Product;
 
+use base qw(Bugzilla::Object);
+
 ###############################
 ####    Initialization     ####
 ###############################
 
+use constant DB_TABLE => 'classifications';
+
 use constant DB_COLUMNS => qw(
-    classifications.id
-    classifications.name
-    classifications.description
-    classifications.sortkey
+    id
+    name
+    description
+    sortkey
+);
+
+use constant REQUIRED_CREATE_FIELDS => qw(
+    name
+);
+
+use constant UPDATE_COLUMNS => qw(
+    name
+    description
+    sortkey
 );
 
-our $columns = join(", ", DB_COLUMNS);
+use constant VALIDATORS => {
+    name        => \&_check_name,
+    description => \&_check_description,
+    sortkey     => \&_check_sortkey,
+};
 
 ###############################
-####       Methods         ####
+####      Validators       ####
 ###############################
 
-sub new {
-    my $invocant = shift;
-    my $class = ref($invocant) || $invocant;
-    my $self = {};
-    bless($self, $class);
-    return $self->_init(@_);
-}
+sub _check_name {
+    my ($invocant, $name) = @_;
 
-sub _init {
-    my $self = shift;
-    my ($param) = @_;
-    my $dbh = Bugzilla->dbh;
+    $name = trim($name);
+    $name || ThrowUserError('classification_not_specified');
 
-    my $id = $param unless (ref $param eq 'HASH');
-    my $classification;
+    my $classification = new Bugzilla::Classification({name => $name});
+    if ($classification && (!ref $invocant || $classification->id != $invocant->id)) {
+        ThrowUserError("classification_already_exists", { name => $classification->name });
+    }
+    return $name;
+}
 
-    if (defined $id) {
-        detaint_natural($id)
-          || ThrowCodeError('param_must_be_numeric',
-                            {function => 'Bugzilla::Classification::_init'});
+sub _check_description {
+    my ($invocant, $description) = @_;
 
-        $classification = $dbh->selectrow_hashref(qq{
-            SELECT $columns FROM classifications
-            WHERE id = ?}, undef, $id);
+    $description  = trim($description || '');
+    return $description;
+}
 
-    } elsif (defined $param->{'name'}) {
+sub _check_sortkey {
+    my ($invocant, $sortkey) = @_;
 
-        trick_taint($param->{'name'});
-        $classification = $dbh->selectrow_hashref(qq{
-            SELECT $columns FROM classifications
-            WHERE name = ?}, undef, $param->{'name'});
-    } else {
-        ThrowCodeError('bad_arg',
-            {argument => 'param',
-             function => 'Bugzilla::Classification::_init'});
-    }
+    $sortkey ||= 0;
+    my $stored_sortkey = $sortkey;
+    detaint_natural($sortkey)
+      || ThrowUserError('classification_invalid_sortkey', { 'sortkey' => $stored_sortkey });
 
-    return undef unless (defined $classification);
-
-    foreach my $field (keys %$classification) {
-        $self->{$field} = $classification->{$field};
-    }
-    return $self;
+    return $sortkey;
 }
 
+###############################
+####       Methods         ####
+###############################
+
 sub product_count {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
@@ -116,46 +124,9 @@ sub products {
 ####      Accessors        ####
 ###############################
 
-sub id          { return $_[0]->{'id'};          }
-sub name        { return $_[0]->{'name'};        }
 sub description { return $_[0]->{'description'}; }
 sub sortkey     { return $_[0]->{'sortkey'};     }
 
-###############################
-####      Subroutines      ####
-###############################
-
-sub get_all_classifications {
-    my $dbh = Bugzilla->dbh;
-
-    my $ids = $dbh->selectcol_arrayref(q{
-        SELECT id FROM classifications ORDER BY sortkey, name});
-
-    my @classifications;
-    foreach my $id (@$ids) {
-        push @classifications, new Bugzilla::Classification($id);
-    }
-    return @classifications;
-}
-
-sub check_classification {
-    my ($class_name) = @_;
-
-    unless ($class_name) {
-        ThrowUserError("classification_not_specified");
-    }
-
-    my $classification =
-        new Bugzilla::Classification({name => $class_name});
-
-    unless ($classification) {
-        ThrowUserError("classification_doesnt_exist",
-                       { name => $class_name });
-    }
-    
-    return $classification;
-}
-
 1;
 
 __END__
@@ -174,18 +145,18 @@ Bugzilla::Classification - Bugzilla classification class.
     my $id = $classification->id;
     my $name = $classification->name;
     my $description = $classification->description;
+    my $sortkey = $classification->sortkey;
     my $product_count = $classification->product_count;
     my $products = $classification->products;
 
-    my $hash_ref = Bugzilla::Classification::get_all_classifications();
-    my $classification = $hash_ref->{1};
-
-    my $classification =
-        Bugzilla::Classification::check_classification('AcmeClass');
-
 =head1 DESCRIPTION
 
-Classification.pm represents a Classification object.
+Classification.pm represents a classification object. It is an
+implementation of L<Bugzilla::Object>, and thus provides all methods
+that L<Bugzilla::Object> provides.
+
+The methods that are specific to C<Bugzilla::Classification> are listed
+below.
 
 A Classification is a higher-level grouping of Products.
 
@@ -193,20 +164,6 @@ A Classification is a higher-level grouping of Products.
 
 =over
 
-=item C<new($param)>
-
- Description: The constructor is used to load an existing
-              classification by passing a classification
-              id or classification name using a hash.
-
- Params:      $param - If you pass an integer, the integer is the
-                      classification_id from the database that we
-                      want to read in. If you pass in a hash with
-                      'name' key, then the value of the name key
-                      is the name of a classification from the DB.
-
- Returns:     A Bugzilla::Classification object.
-
 =item C<product_count()>
 
  Description: Returns the total number of products that belong to
@@ -226,27 +183,4 @@ A Classification is a higher-level grouping of Products.
 
 =back
 
-=head1 SUBROUTINES
-
-=over
-
-=item C<get_all_classifications()>
-
- Description: Returns all classifications.
-
- Params:      none.
-
- Returns:     Bugzilla::Classification object list.
-
-=item C<check_classification($classification_name)>
-
- Description: Checks if the classification name passed in is a
-              valid classification.
-
- Params:      $classification_name - String with a classification name.
-
- Returns:     Bugzilla::Classification object.
-
-=back
-
 =cut
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index 7a560a3e6..396aaa346 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -409,8 +409,7 @@ sub _check_classification {
 
     my $classification_id = 1;
     if (Bugzilla->params->{'useclassification'}) {
-        my $classification =
-            Bugzilla::Classification::check_classification($classification_name);
+        my $classification = Bugzilla::Classification->check($classification_name);
         $classification_id = $classification->id;
     }
     return $classification_id;