Bug 572602: Change the way that Bugzilla::Object determines what fields
are required for create(). It now assumes that any column that is NOT NULL and has not DEFAULT in the database is required. We also shift the burden of throwing errors about empty values to the validators. This fixes the bug that Bugzilla::Bug->create() wasn't populating default values for fields if they weren't specified in the create() parameters. r=timello, a=mkanat git-svn-id: svn://10.0.0.236/trunk@260542 18797224-902f-48f8-a5cc-f745e15eee43
This commit is contained in:
parent
7d0c9310fe
commit
1be00a65d9
@ -1 +1 @@
|
||||
7238
|
||||
7239
|
||||
@ -87,13 +87,9 @@ sub DB_COLUMNS {
|
||||
$dbh->sql_date_format('attachments.creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts';
|
||||
}
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
bug
|
||||
data
|
||||
description
|
||||
filename
|
||||
mimetype
|
||||
);
|
||||
use constant REQUIRED_FIELD_MAP => {
|
||||
bug_id => 'bug',
|
||||
};
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
description
|
||||
@ -515,6 +511,10 @@ sub _check_bug {
|
||||
my $user = Bugzilla->user;
|
||||
|
||||
$bug = ref $invocant ? $invocant->bug : $bug;
|
||||
|
||||
$bug || ThrowCodeError('param_required',
|
||||
{ function => "$invocant->create", param => 'bug' });
|
||||
|
||||
($user->can_see_bug($bug->id) && $user->can_edit_product($bug->product_id))
|
||||
|| ThrowUserError("illegal_attachment_edit_bug", { bug_id => $bug->id });
|
||||
|
||||
@ -526,7 +526,7 @@ sub _check_content_type {
|
||||
|
||||
$content_type = 'text/plain' if (ref $invocant && ($invocant->isurl || $invocant->ispatch));
|
||||
my $legal_types = join('|', LEGAL_CONTENT_TYPES);
|
||||
if ($content_type !~ /^($legal_types)\/.+$/) {
|
||||
if (!$content_type or $content_type !~ /^($legal_types)\/.+$/) {
|
||||
ThrowUserError("invalid_content_type", { contenttype => $content_type });
|
||||
}
|
||||
trick_taint($content_type);
|
||||
|
||||
@ -115,15 +115,6 @@ sub DB_COLUMNS {
|
||||
return @columns;
|
||||
}
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
component
|
||||
product
|
||||
short_desc
|
||||
version
|
||||
);
|
||||
|
||||
# There are also other, more complex validators that are called
|
||||
# from run_create_validators.
|
||||
sub VALIDATORS {
|
||||
|
||||
my $validators = {
|
||||
@ -290,6 +281,11 @@ use constant FIELD_MAP => {
|
||||
offset => 'OFFSET',
|
||||
};
|
||||
|
||||
use constant REQUIRED_FIELD_MAP => {
|
||||
product_id => 'product',
|
||||
component_id => 'component',
|
||||
};
|
||||
|
||||
#####################################################################
|
||||
|
||||
sub new {
|
||||
@ -1696,8 +1692,8 @@ sub _check_reporter {
|
||||
else {
|
||||
# On bug creation, the reporter is the logged in user
|
||||
# (meaning that he must be logged in first!).
|
||||
Bugzilla->login(LOGIN_REQUIRED);
|
||||
$reporter = Bugzilla->user->id;
|
||||
$reporter || ThrowCodeError('invalid_user');
|
||||
}
|
||||
return $reporter;
|
||||
}
|
||||
|
||||
@ -40,10 +40,6 @@ use constant DB_COLUMNS => qw(
|
||||
sortkey
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
name
|
||||
);
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
name
|
||||
description
|
||||
|
||||
@ -47,13 +47,6 @@ use constant DB_COLUMNS => qw(
|
||||
description
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
name
|
||||
product
|
||||
initialowner
|
||||
description
|
||||
);
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
name
|
||||
initialowner
|
||||
@ -61,6 +54,10 @@ use constant UPDATE_COLUMNS => qw(
|
||||
description
|
||||
);
|
||||
|
||||
use constant REQUIRED_FIELD_MAP => {
|
||||
product_id => 'product',
|
||||
};
|
||||
|
||||
use constant VALIDATORS => {
|
||||
create_series => \&Bugzilla::Object::check_boolean,
|
||||
product => \&_check_product,
|
||||
@ -234,6 +231,8 @@ sub _check_initialqacontact {
|
||||
|
||||
sub _check_product {
|
||||
my ($invocant, $product) = @_;
|
||||
$product || ThrowCodeError('param_required',
|
||||
{ function => "$invocant->create", param => 'product' });
|
||||
return Bugzilla->user->check_can_admin_product($product->name);
|
||||
}
|
||||
|
||||
|
||||
@ -105,8 +105,6 @@ use constant DB_COLUMNS => qw(
|
||||
is_mandatory
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(name description sortkey);
|
||||
|
||||
use constant VALIDATORS => {
|
||||
custom => \&_check_custom,
|
||||
description => \&_check_description,
|
||||
|
||||
@ -55,8 +55,6 @@ use constant UPDATE_COLUMNS => qw(
|
||||
use constant NAME_FIELD => 'value';
|
||||
use constant LIST_ORDER => 'sortkey, value';
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(value);
|
||||
|
||||
use constant VALIDATORS => {
|
||||
value => \&_check_value,
|
||||
sortkey => \&_check_sortkey,
|
||||
|
||||
@ -87,14 +87,6 @@ use constant DB_COLUMNS => qw(
|
||||
status
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
attach_id
|
||||
bug_id
|
||||
setter_id
|
||||
status
|
||||
type_id
|
||||
);
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
requestee_id
|
||||
setter_id
|
||||
@ -429,6 +421,7 @@ sub create {
|
||||
$params->{$_} = $flag->{$_} foreach @columns;
|
||||
|
||||
$params->{creation_date} = $params->{modification_date} = $timestamp;
|
||||
|
||||
$flag = $class->SUPER::create($params);
|
||||
return $flag;
|
||||
}
|
||||
|
||||
@ -60,8 +60,6 @@ use constant VALIDATORS => {
|
||||
icon_url => \&_check_icon_url,
|
||||
};
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(name description isbuggroup);
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
name
|
||||
description
|
||||
|
||||
@ -35,8 +35,6 @@ use constant DB_COLUMNS => qw(
|
||||
|
||||
use constant DB_TABLE => 'keyworddefs';
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(name description);
|
||||
|
||||
use constant VALIDATORS => {
|
||||
name => \&_check_name,
|
||||
description => \&_check_description,
|
||||
@ -106,7 +104,9 @@ sub _check_name {
|
||||
my ($self, $name) = @_;
|
||||
|
||||
$name = trim($name);
|
||||
$name eq "" && ThrowUserError("keyword_blank_name");
|
||||
if (!defined $name or $name eq "") {
|
||||
ThrowUserError("keyword_blank_name");
|
||||
}
|
||||
if ($name =~ /[\s,]/) {
|
||||
ThrowUserError("keyword_invalid_name");
|
||||
}
|
||||
@ -124,7 +124,9 @@ sub _check_name {
|
||||
sub _check_description {
|
||||
my ($self, $desc) = @_;
|
||||
$desc = trim($desc);
|
||||
$desc eq '' && ThrowUserError("keyword_blank_description");
|
||||
if (!defined $desc or $desc eq '') {
|
||||
ThrowUserError("keyword_blank_description");
|
||||
}
|
||||
return $desc;
|
||||
}
|
||||
|
||||
|
||||
@ -45,10 +45,9 @@ use constant DB_COLUMNS => qw(
|
||||
sortkey
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
value
|
||||
product
|
||||
);
|
||||
use constant REQUIRED_FIELD_MAP => {
|
||||
product_id => 'product',
|
||||
};
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
value
|
||||
@ -195,6 +194,8 @@ sub _check_sortkey {
|
||||
|
||||
sub _check_product {
|
||||
my ($invocant, $product) = @_;
|
||||
$product || ThrowCodeError('param_required',
|
||||
{ function => "$invocant->create", param => "product" });
|
||||
return Bugzilla->user->check_can_admin_product($product->name);
|
||||
}
|
||||
|
||||
|
||||
@ -39,6 +39,8 @@ use constant UPDATE_VALIDATORS => {};
|
||||
use constant NUMERIC_COLUMNS => ();
|
||||
use constant DATE_COLUMNS => ();
|
||||
use constant VALIDATOR_DEPENDENCIES => {};
|
||||
# XXX At some point, this will be joined with FIELD_MAP.
|
||||
use constant REQUIRED_FIELD_MAP => {};
|
||||
|
||||
# This allows the JSON-RPC interface to return Bugzilla::Object instances
|
||||
# as though they were hashes. In the future, this may be modified to return
|
||||
@ -447,10 +449,9 @@ sub check_required_create_fields {
|
||||
Bugzilla::Hook::process('object_before_create', { class => $class,
|
||||
params => $params });
|
||||
|
||||
foreach my $field ($class->REQUIRED_CREATE_FIELDS) {
|
||||
ThrowCodeError('param_required',
|
||||
{ function => "${class}->create", param => $field })
|
||||
if !exists $params->{$field};
|
||||
my @check_fields = $class->_required_create_fields();
|
||||
foreach my $field (@check_fields) {
|
||||
$params->{$field} = undef if !exists $params->{$field};
|
||||
}
|
||||
}
|
||||
|
||||
@ -616,6 +617,30 @@ sub _get_validators {
|
||||
return $cache->{$cache_key};
|
||||
}
|
||||
|
||||
# These are all the fields that need to be checked, always, when
|
||||
# calling create(), because they have no DEFAULT and they are marked
|
||||
# NOT NULL.
|
||||
sub _required_create_fields {
|
||||
my $class = shift;
|
||||
my $dbh = Bugzilla->dbh;
|
||||
my $table = $class->DB_TABLE;
|
||||
|
||||
my @columns = $dbh->bz_table_columns($table);
|
||||
my @required;
|
||||
foreach my $column (@columns) {
|
||||
my $def = $dbh->bz_column_info($table, $column);
|
||||
if ($def->{NOTNULL} and !defined $def->{DEFAULT}
|
||||
# SERIAL fields effectively have a DEFAULT, but they're not
|
||||
# listed as having a DEFAULT in DB::Schema.
|
||||
and $def->{TYPE} !~ /serial/i)
|
||||
{
|
||||
my $field = $class->REQUIRED_FIELD_MAP->{$column} || $column;
|
||||
push(@required, $field);
|
||||
}
|
||||
}
|
||||
return @required;
|
||||
}
|
||||
|
||||
1;
|
||||
|
||||
__END__
|
||||
@ -687,11 +712,6 @@ The order that C<new_from_list> and C<get_all> should return objects
|
||||
in. This should be the name of a database column. Defaults to
|
||||
L</NAME_FIELD>.
|
||||
|
||||
=item C<REQUIRED_CREATE_FIELDS>
|
||||
|
||||
The list of fields that B<must> be specified when the user calls
|
||||
C<create()>. This should be an array.
|
||||
|
||||
=item C<VALIDATORS>
|
||||
|
||||
A hashref that points to a function that will validate each param to
|
||||
@ -742,6 +762,15 @@ 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.)
|
||||
|
||||
=item C<REQUIRED_FIELD_MAP>
|
||||
|
||||
This is a hashref that maps database column names to L</create> argument
|
||||
names. You only need to specify values for fields where the argument passed
|
||||
to L</create> has a different name in the database than it does in the
|
||||
L</create> arguments. (For example, L<Bugzilla::Bug/create> takes a
|
||||
C<product> argument, but the column name in the C<bugs> table is
|
||||
C<product_id>.)
|
||||
|
||||
=item C<NUMERIC_COLUMNS>
|
||||
|
||||
When L</update> is called, it compares each column in the object to its
|
||||
@ -915,17 +944,13 @@ Description: Creates a new item in the database.
|
||||
are invalid.
|
||||
|
||||
Params: C<$params> - hashref - A value to put in each database
|
||||
field for this object. Certain values must be set (the
|
||||
ones specified in L</REQUIRED_CREATE_FIELDS>), and
|
||||
the function will throw a Code Error if you don't set
|
||||
them.
|
||||
field for this object.
|
||||
|
||||
Returns: The Object just created in the database.
|
||||
|
||||
Notes: In order for this function to work in your subclass,
|
||||
your subclass's L</ID_FIELD> must be of C<SERIAL>
|
||||
type in the database. Your subclass also must
|
||||
define L</REQUIRED_CREATE_FIELDS> and L</VALIDATORS>.
|
||||
type in the database.
|
||||
|
||||
Subclass Implementors: This function basically just
|
||||
calls L</check_required_create_fields>, then
|
||||
@ -940,8 +965,10 @@ Notes: In order for this function to work in your subclass,
|
||||
|
||||
=item B<Description>
|
||||
|
||||
Part of L</create>. Throws an error if any of the L</REQUIRED_CREATE_FIELDS>
|
||||
have not been specified in C<$params>
|
||||
Part of L</create>. Modifies the incoming C<$params> argument so that
|
||||
any field that does not have a database default will be checked
|
||||
later by L</run_create_validators>, even if that field wasn't specified
|
||||
as an argument to L</create>.
|
||||
|
||||
=item B<Params>
|
||||
|
||||
|
||||
@ -52,12 +52,6 @@ use constant DB_COLUMNS => qw(
|
||||
allows_unconfirmed
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
name
|
||||
description
|
||||
version
|
||||
);
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
name
|
||||
description
|
||||
|
||||
@ -34,8 +34,6 @@ use Bugzilla::Util;
|
||||
use constant DB_TABLE => 'profile_search';
|
||||
use constant LIST_ORDER => 'id';
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => ();
|
||||
|
||||
use constant DB_COLUMNS => qw(
|
||||
id
|
||||
user_id
|
||||
@ -120,7 +118,7 @@ sub _check_user_id {
|
||||
sub _check_bug_list {
|
||||
my ($invocant, $list) = @_;
|
||||
|
||||
my @bug_ids = ref($list) ? @$list : split(',', $list);
|
||||
my @bug_ids = ref($list) ? @$list : split(',', $list || '');
|
||||
detaint_natural($_) foreach @bug_ids;
|
||||
return join(',', @bug_ids);
|
||||
}
|
||||
|
||||
@ -48,8 +48,6 @@ use constant DB_COLUMNS => qw(
|
||||
query_type
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(name query);
|
||||
|
||||
use constant VALIDATORS => {
|
||||
name => \&_check_name,
|
||||
query => \&_check_query,
|
||||
|
||||
@ -102,8 +102,6 @@ use constant NAME_FIELD => 'login_name';
|
||||
use constant ID_FIELD => 'userid';
|
||||
use constant LIST_ORDER => NAME_FIELD;
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(login_name cryptpassword);
|
||||
|
||||
use constant VALIDATORS => {
|
||||
cryptpassword => \&_check_password,
|
||||
disable_mail => \&_check_disable_mail,
|
||||
|
||||
@ -592,8 +592,11 @@ sub is_7bit_clean {
|
||||
}
|
||||
|
||||
sub clean_text {
|
||||
my ($dtext) = shift;
|
||||
$dtext =~ s/[\x00-\x1F\x7F]+/ /g; # change control characters into a space
|
||||
my $dtext = shift;
|
||||
if ($dtext) {
|
||||
# change control characters into a space
|
||||
$dtext =~ s/[\x00-\x1F\x7F]+/ /g;
|
||||
}
|
||||
return trim($dtext);
|
||||
}
|
||||
|
||||
|
||||
@ -46,10 +46,9 @@ use constant DB_COLUMNS => qw(
|
||||
product_id
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(
|
||||
value
|
||||
product
|
||||
);
|
||||
use constant REQUIRED_FIELD_MAP => {
|
||||
product_id => 'product',
|
||||
};
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
value
|
||||
@ -188,6 +187,8 @@ sub _check_value {
|
||||
|
||||
sub _check_product {
|
||||
my ($invocant, $product) = @_;
|
||||
$product || ThrowCodeError('param_required',
|
||||
{ function => "$invocant->create", param => 'product' });
|
||||
return Bugzilla->user->check_can_admin_product($product->name);
|
||||
}
|
||||
|
||||
|
||||
@ -42,8 +42,6 @@ use constant DB_COLUMNS => qw(
|
||||
mailto_type
|
||||
);
|
||||
|
||||
use constant REQUIRED_CREATE_FIELDS => qw(eventid mailto mailto_type);
|
||||
|
||||
use constant UPDATE_COLUMNS => qw(
|
||||
eventid
|
||||
run_day
|
||||
|
||||
@ -152,13 +152,6 @@ sub post_bug {
|
||||
|
||||
my $user = Bugzilla->user;
|
||||
|
||||
# Bugzilla::Bug->create throws a confusing CodeError if
|
||||
# the REQUIRED_CREATE_FIELDS are missing, but much more
|
||||
# sensible errors if the fields exist but are just undef.
|
||||
foreach my $field (Bugzilla::Bug::REQUIRED_CREATE_FIELDS) {
|
||||
$fields->{$field} = undef if !exists $fields->{$field};
|
||||
}
|
||||
|
||||
my ($retval, $non_conclusive_fields) =
|
||||
Bugzilla::User::match_field({
|
||||
'assigned_to' => { 'type' => 'single' },
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user