From 931bd3b651ccca832a618f7b28bbc35e503665d4 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Fri, 3 Oct 2008 06:28:31 +0000 Subject: Bug 455632: Add Bugzilla::Field::Choice->create and have editvalues.cgi use it Patch By Max Kanat-Alexander r=bbaetz, a=mkanat --- Bugzilla/Field/Choice.pm | 237 +++++++++++++++++++++++++++++------------------ 1 file changed, 148 insertions(+), 89 deletions(-) (limited to 'Bugzilla/Field/Choice.pm') 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 < '$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