diff options
Diffstat (limited to 'Bugzilla')
-rw-r--r-- | Bugzilla/Bug.pm | 8 | ||||
-rw-r--r-- | Bugzilla/Constants.pm | 4 | ||||
-rw-r--r-- | Bugzilla/Field.pm | 4 | ||||
-rw-r--r-- | Bugzilla/Field/Choice.pm | 237 | ||||
-rw-r--r-- | Bugzilla/Status.pm | 55 |
5 files changed, 205 insertions, 103 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 1d12ee2cf..d620f222c 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -54,7 +54,6 @@ use base qw(Bugzilla::Object Exporter); RemoveVotes CheckIfVotedConfirmed LogActivityEntry editable_bug_fields - SPECIAL_STATUS_WORKFLOW_ACTIONS ); ##################################################################### @@ -234,13 +233,6 @@ use constant MAX_LINE_LENGTH => 254; # Used in _check_comment(). Gives the max length allowed for a comment. use constant MAX_COMMENT_LENGTH => 65535; -use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw( - none - duplicate - change_resolution - clearresolution -); - ##################################################################### sub new { diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index abe1fe248..601ea52b2 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -151,6 +151,7 @@ use File::Basename; MAX_PRODUCT_SIZE MAX_MILESTONE_SIZE MAX_COMPONENT_SIZE + MAX_FIELD_VALUE_SIZE MAX_FREETEXT_LENGTH ); @@ -427,6 +428,9 @@ use constant MAX_MILESTONE_SIZE => 20; # The longest component name allowed. use constant MAX_COMPONENT_SIZE => 64; +# The maximum length for values of <select> fields. +use constant MAX_FIELD_VALUE_SIZE => 64; + # Maximum length allowed for free text fields. use constant MAX_FREETEXT_LENGTH => 255; diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index bb516b8f4..5d8e5adc0 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -75,7 +75,6 @@ use base qw(Exporter Bugzilla::Object); use Bugzilla::Constants; use Bugzilla::Error; -use Bugzilla::Field::Choice; use Bugzilla::Util; ############################### @@ -376,7 +375,8 @@ sub legal_values { my $self = shift; if (!defined $self->{'legal_values'}) { - my @values = Bugzilla::Field::Choice->get_all({ field => $self }); + require Bugzilla::Field::Choice; + my @values = Bugzilla::Field::Choice->type($self)->get_all(); $self->{'legal_values'} = \@values; } return $self->{'legal_values'}; diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm index 6ef911941..7705258eb 100644 --- a/Bugzilla/Field/Choice.pm +++ b/Bugzilla/Field/Choice.pm @@ -26,6 +26,8 @@ use base qw(Bugzilla::Object); use Bugzilla::Constants; use Bugzilla::Error; +use Bugzilla::Field; +use Bugzilla::Util qw(trim detaint_natural); use Scalar::Util qw(blessed); @@ -42,104 +44,141 @@ use constant DB_COLUMNS => qw( use constant NAME_FIELD => 'value'; use constant LIST_ORDER => 'sortkey, value'; -########################################## -# Constructors and Database Manipulation # -########################################## +use constant REQUIRED_CREATE_FIELDS => qw(value); -# When calling class methods, we aren't based on just one table, -# so we need some slightly hacky way to do DB_TABLE. We do it by overriding -# class methods and making them specify $_new_table. This is possible -# because we're always called from inside Bugzilla::Field, so it always -# has a $self to pass us which contains info about which field we're -# attached to. -# -# This isn't thread safe, but Bugzilla isn't a threaded application. -our $_new_table; -our $_current_field; -sub DB_TABLE { - my $invocant = shift; - if (blessed $invocant) { - return $invocant->field->name; +use constant VALIDATORS => { + value => \&_check_value, + sortkey => \&_check_sortkey, +}; + +use constant CLASS_MAP => { + bug_status => 'Bugzilla::Status', +}; + +################# +# Class Factory # +################# + +# Bugzilla::Field::Choice is actually an abstract base class. Every field +# type has its own dynamically-generated class for its values. This allows +# certain fields to have special types, like how bug_status's values +# are Bugzilla::Status objects. + +sub type { + my ($class, $field) = @_; + my $field_obj = blessed $field ? $field : Bugzilla::Field->check($field); + my $field_name = $field_obj->name; + + if ($class->CLASS_MAP->{$field_name}) { + return $class->CLASS_MAP->{$field_name}; } - return $_new_table; + + # For generic classes, we use a lowercase class name, so as + # not to interfere with any real subclasses we might make some day. + my $package = "Bugzilla::Field::Choice::$field_name"; + + # We check defined so that the package and the stored field are only + # created once globally (at least per request). We prefix it with + # field_ (instead of suffixing it) so that we don't somehow conflict + # with the names of custom fields. + if (!defined Bugzilla->request_cache->{"field_$package"}) { + eval <<EOC; + package $package; + use base qw(Bugzilla::Field::Choice); + use constant DB_TABLE => '$field_name'; +EOC + Bugzilla->request_cache->{"field_$package"} = $field_obj; + } + + return $package; } +################ +# Constructors # +################ + +# We just make new() enforce this, which should give developers +# the understanding that you can't use Bugzilla::Field::Choice +# without calling type(). sub new { my $class = shift; - my ($params) = @_; - _check_field_arg($params); - my $self = $class->SUPER::new($params); - _fix_return($self); - return $self; + if ($class eq 'Bugzilla::Field::Choice') { + ThrowCodeError('field_choice_must_use_type'); + } + $class->SUPER::new(@_); } -sub new_from_list { - my $class = shift; - my ($ids, $params) = @_; - _check_field_arg($params); - my $list = $class->SUPER::new_from_list(@_); - _fix_return($list); - return $list; -} +######################### +# Database Manipulation # +######################### -sub match { +# Our subclasses can take more arguments than we normally accept. +# So, we override create() to remove arguments that aren't valid +# columns. (Normally Bugzilla::Object dies if you pass arguments +# that aren't valid columns.) +sub create { my $class = shift; my ($params) = @_; - _check_field_arg($params); - my $results = $class->SUPER::match(@_); - _fix_return($results); - return $results; + foreach my $key (keys %$params) { + if (!grep {$_ eq $key} $class->DB_COLUMNS) { + delete $params->{$key}; + } + } + return $class->SUPER::create(@_); } -sub get_all { - my $class = shift; - _check_field_arg(@_); - my @list = $class->SUPER::get_all(@_); - _fix_return(\@list); - return @list; -} +############# +# Accessors # +############# -sub _check_field_arg { - my ($params) = @_; - my ($class, undef, undef, $func) = caller(1); - if (!defined $params->{field}) { - ThrowCodeError('param_required', - { function => "${class}::$func", - param => 'field' }); - } - elsif (!blessed $params->{field}) { - ThrowCodeError('bad_arg', { function => "${class}::$func", - argument => 'field' }); - } - $_new_table = $params->{field}->name; - $_current_field = $params->{field}; - delete $params->{field}; +sub sortkey { return $_[0]->{'sortkey'}; } +sub field { + my $invocant = shift; + my $class = ref $invocant || $invocant; + my $cache = Bugzilla->request_cache; + # This is just to make life easier for subclasses. Our auto-generated + # subclasses from type() already have this set. + $cache->{"field_$class"} ||= + new Bugzilla::Field({ name => $class->DB_TABLE }); + return $cache->{"field_$class"}; } -sub _fix_return { - my $retval = shift; - if (ref $retval eq 'ARRAY') { - foreach my $obj (@$retval) { - $obj->{field} = $_current_field; - } - } - elsif (defined $retval) { - $retval->{field} = $_current_field; +############## +# Validators # +############## + +sub _check_value { + my ($invocant, $value) = @_; + + my $field = $invocant->field; + + $value = trim($value); + ThrowUserError('fieldvalue_undefined') if !defined $value || $value eq ""; + ThrowUserError('fieldvalue_name_too_long', { value => $value }) + if length($value) > MAX_FIELD_VALUE_SIZE; + + my $exists = $invocant->type($field)->new({ name => $value }); + if ($exists) { + ThrowUserError('fieldvalue_already_exists', + { field => $field, value => $value }); } - # We do this just to avoid any possible bugs where $_new_table or - # $_current_field are set from a previous call. It also might save - # a little memory under mod_perl by releasing $_current_field explicitly. - undef $_new_table; - undef $_current_field; + return $value; } -############# -# Accessors # -############# +sub _check_sortkey { + my ($invocant, $value) = @_; + $value = trim($value); + return 0 if !$value; + # Store for the error message in case detaint_natural clears it. + my $orig_value = $value; + detaint_natural($value) + || ThrowUserError('fieldvalue_sortkey_invalid', + { sortkey => $orig_value, + field => $invocant->field }); + return $value; +} -sub sortkey { return $_[0]->{'sortkey'}; } -sub field { return $_[0]->{'field'}; } 1; @@ -153,27 +192,47 @@ Bugzilla::Field::Choice - A legal value for a <select>-type field. my $field = new Bugzilla::Field({name => 'bug_status'}); - my $choice = new Bugzilla::Field::Choice({ field => $field, id => 1 }); - my $choice = new Bugzilla::Field::Choice({ field => $field, name => 'NEW' }); + my $choice = new Bugzilla::Field::Choice->type($field)->new(1); - my $choices = Bugzilla::Field::Choice->new_from_list([1,2,3], - { field => $field}); - my $choices = Bugzilla::Field::Choice->get_all({ field => $field }); - my $choices = Bugzilla::Field::Choice->match({ sortkey => 10, - field => $field }); + my $choices = Bugzilla::Field::Choice->type($field)->new_from_list([1,2,3]); + my $choices = Bugzilla::Field::Choice->type($field)->get_all(); + my $choices = Bugzilla::Field::Choice->type($field->match({ sortkey => 10 }); =head1 DESCRIPTION This is an implementation of L<Bugzilla::Object>, but with a twist. -All the class methods require that you specify an additional C<field> -argument, which is a L<Bugzilla::Field> object that represents the -field whose value this is. +You can't call any class methods (such as C<new>, C<create>, etc.) +directly on C<Bugzilla::Field::Choice> itself. Instead, you have to +call C<Bugzilla::Field::Choice-E<gt>type($field)> to get the class +you're going to instantiate, and then you call the methods on that. + +We do that because each field has its own database table for its values, so +each value type needs its own class. -You can look at the L</SYNOPSIS> to see where this extra C<field> -argument goes in each function. +See the L</SYNOPSIS> for examples of how this works. =head1 METHODS +=head2 Class Factory + +In object-oriented design, a "class factory" is a method that picks +and returns the right class for you, based on an argument that you pass. + +=over + +=item C<type> + +Takes a single argument, which is either the name of a field from the +C<fielddefs> table, or a L<Bugzilla::Field> object representing a field. + +Returns an appropriate subclass of C<Bugzilla::Field::Choice> that you +can now call class methods on (like C<new>, C<create>, C<match>, etc.) + +B<NOTE>: YOU CANNOT CALL CLASS METHODS ON C<Bugzilla::Field::Choice>. You +must call C<type> to get a class you can call methods on. + +=back + =head2 Accessors These are in addition to the standard L<Bugzilla::Object> accessors. diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index f8b77331c..5f37974e7 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -22,13 +22,28 @@ use strict; package Bugzilla::Status; -use base qw(Bugzilla::Object Exporter); -@Bugzilla::Status::EXPORT = qw(BUG_STATE_OPEN is_open_state closed_bug_statuses); +use Bugzilla::Error; + +use base qw(Bugzilla::Field::Choice Exporter); +@Bugzilla::Status::EXPORT = qw( + BUG_STATE_OPEN + SPECIAL_STATUS_WORKFLOW_ACTIONS + + is_open_state + closed_bug_statuses +); ################################ ##### Initialization ##### ################################ +use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw( + none + duplicate + change_resolution + clearresolution +); + use constant DB_TABLE => 'bug_status'; use constant DB_COLUMNS => qw( @@ -39,8 +54,24 @@ use constant DB_COLUMNS => qw( is_open ); -use constant NAME_FIELD => 'value'; -use constant LIST_ORDER => 'sortkey, value'; +sub VALIDATORS { + my $invocant = shift; + my $validators = $invocant->SUPER::VALIDATORS; + $validators->{is_open} = \&Bugzilla::Object::check_boolean; + $validators->{value} = \&_check_value; + return $validators; +} + +######################### +# Database Manipulation # +######################### + +sub create { + my $class = shift; + my $self = $class->SUPER::create(@_); + add_missing_bug_status_transitions(); + return $self; +} ############################### ##### Accessors #### @@ -51,6 +82,22 @@ sub sortkey { return $_[0]->{'sortkey'}; } sub is_active { return $_[0]->{'isactive'}; } sub is_open { return $_[0]->{'is_open'}; } +############## +# Validators # +############## + +sub _check_value { + my $invocant = shift; + my $value = $invocant->SUPER::_check_value(@_); + + if (grep { lc($value) eq lc($_) } SPECIAL_STATUS_WORKFLOW_ACTIONS) { + ThrowUserError('fieldvalue_reserved_word', + { field => $invocant->field, value => $value }); + } + return $value; +} + + ############################### ##### Methods #### ############################### |