Skip to content

bugzilla
bugzilla
Commit 8f33041e authored by Max Kanat-Alexander's avatar Max Kanat-Alexander
Browse files
Options

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
parent 0a4878cc
Show whitespace changes
Inline Side-by-side
Showing with 87 additions and 96 deletions
Bugzilla/Attachment.pm
View file @ 8f33041e
......@@ -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);
......
Bugzilla/Bug.pm
View file @ 8f33041e
......@@ -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;
}
......
Bugzilla/Classification.pm
View file @ 8f33041e
......@@ -40,10 +40,6 @@ use constant DB_COLUMNS => qw(
sortkey
);
use constant REQUIRED_CREATE_FIELDS => qw(
name
);
use constant UPDATE_COLUMNS => qw(
name
description
......
Bugzilla/Component.pm
View file @ 8f33041e
......@@ -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);
}
......
Bugzilla/Field.pm
View file @ 8f33041e
......@@ -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,
......
Bugzilla/Field/Choice.pm
View file @ 8f33041e
......@@ -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,
......
Bugzilla/Flag.pm
View file @ 8f33041e
......@@ -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;
}
......
Bugzilla/Group.pm
View file @ 8f33041e
......@@ -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
......
Bugzilla/Keyword.pm
View file @ 8f33041e
......@@ -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;
}
......
Bugzilla/Milestone.pm
View file @ 8f33041e
......@@ -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);
}
......
Bugzilla/Object.pm
View file @ 8f33041e
......@@ -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>
......
Bugzilla/Product.pm
View file @ 8f33041e
......@@ -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
......
Bugzilla/Search/Recent.pm
View file @ 8f33041e
......@@ -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);
}
......
Bugzilla/Search/Saved.pm
View file @ 8f33041e
......@@ -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,
......
Bugzilla/User.pm
View file @ 8f33041e
......@@ -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,
......
Bugzilla/Util.pm
View file @ 8f33041e
......@@ -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);
}
......
Bugzilla/Version.pm
View file @ 8f33041e
......@@ -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);
}
......
Bugzilla/Whine/Schedule.pm
View file @ 8f33041e
......@@ -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
......
email_in.pl
View file @ 8f33041e
......@@ -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' },
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment