Bug 346597: Have Bugzilla::Group implement Bugzilla::Object->create, and make checksetup.pl use it

Bugzilla/DB/Schema.pm

@@ -786,7 +786,7 @@ use constant ABSTRACT_SCHEMA => {
             isbless    => {TYPE => 'BOOLEAN', NOTNULL => 1,
                            DEFAULT => 'FALSE'},
             grant_type => {TYPE => 'INT1', NOTNULL => 1,
-                           DEFAULT => '0'},
+                           DEFAULT => GRANT_DIRECT},
         ],
         INDEXES => [
             user_group_map_user_id_idx =>
@@ -807,7 +807,7 @@ use constant ABSTRACT_SCHEMA => {
             member_id  => {TYPE => 'INT3', NOTNULL => 1},
             grantor_id => {TYPE => 'INT3', NOTNULL => 1},
             grant_type => {TYPE => 'INT1', NOTNULL => 1,
-                           DEFAULT => '0'},
+                           DEFAULT => GROUP_MEMBERSHIP},
         ],
         INDEXES => [
             group_group_map_member_id_idx =>

Bugzilla/Group.pm

@@ -27,6 +27,7 @@ use strict;
 package Bugzilla::Group;
 
 use base qw(Bugzilla::Object);
+use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Error;
 
@@ -47,6 +48,15 @@ use constant DB_TABLE => 'groups';
 
 use constant LIST_ORDER => 'isbuggroup, name';
 
+use constant VALIDATORS => {
+    name        => \&_check_name,
+    description => \&_check_description,
+    userregexp  => \&_check_user_regexp,
+    isbuggroup  => \&_check_is_bug_group,
+};
+
+use constant REQUIRED_CREATE_FIELDS => qw(name description isbuggroup);
+
 ###############################
 ####      Accessors      ######
 ###############################
@@ -56,10 +66,46 @@ sub is_bug_group { return $_[0]->{'isbuggroup'};   }
 sub user_regexp  { return $_[0]->{'userregexp'};   }
 sub is_active    { return $_[0]->{'isactive'};     }
 
+###############################
+####        Methods        ####
+###############################
+
+sub _rederive_regexp {
+    my ($self) = @_;
+    RederiveRegexp($self->user_regexp, $self->id);
+}
+
 ################################
 #####  Module Subroutines    ###
 ################################
 
+sub create {
+    my $class = shift;
+    my ($params) = @_;
+    my $dbh = Bugzilla->dbh;
+
+    print "Creating group $params->{name}...\n" unless i_am_cgi();
+
+    my $group = $class->SUPER::create(@_);
+
+    # Since we created a new group, give the "admin" group all privileges
+    # initially.
+    my $admin = new Bugzilla::Group({name => 'admin'});
+    # This function is also used to create the "admin" group itself,
+    # so there's a chance it won't exist yet.
+    if ($admin) {
+        my $sth = $dbh->prepare('INSERT INTO group_group_map
+                                 (member_id, grantor_id, grant_type)
+                                 VALUES (?, ?, ?)');
+        $sth->execute($admin->id, $group->id, GROUP_MEMBERSHIP);
+        $sth->execute($admin->id, $group->id, GROUP_BLESS);
+        $sth->execute($admin->id, $group->id, GROUP_VISIBLE);
+    }
+
+    $group->_rederive_regexp() if $group->user_regexp;
+    return $group;
+}
+
 sub ValidateGroupName {
     my ($name, @users) = (@_);
     my $dbh = Bugzilla->dbh;
@@ -79,6 +125,65 @@ sub ValidateGroupName {
     return $ret;
 }
 
+# This sub is not perldoc'ed because we expect it to go away and
+# just become the _rederive_regexp private method.
+sub RederiveRegexp {
+    my ($regexp, $gid) = @_;
+    my $dbh = Bugzilla->dbh;
+    my $sth = $dbh->prepare("SELECT userid, login_name, group_id
+                               FROM profiles
+                          LEFT JOIN user_group_map
+                                 ON user_group_map.user_id = profiles.userid
+                                AND group_id = ?
+                                AND grant_type = ?
+                                AND isbless = 0");
+    my $sthadd = $dbh->prepare("INSERT INTO user_group_map
+                                 (user_id, group_id, grant_type, isbless)
+                                 VALUES (?, ?, ?, 0)");
+    my $sthdel = $dbh->prepare("DELETE FROM user_group_map
+                                 WHERE user_id = ? AND group_id = ?
+                                 AND grant_type = ? and isbless = 0");
+    $sth->execute($gid, GRANT_REGEXP);
+    while (my ($uid, $login, $present) = $sth->fetchrow_array()) {
+        if (($regexp =~ /\S+/) && ($login =~ m/$regexp/i))
+        {
+            $sthadd->execute($uid, $gid, GRANT_REGEXP) unless $present;
+        } else {
+            $sthdel->execute($uid, $gid, GRANT_REGEXP) if $present;
+        }
+    }
+}
+
+###############################
+###       Validators        ###
+###############################
+
+sub _check_name {
+    my ($invocant, $name) = @_;
+    $name = trim($name);
+    $name || ThrowUserError("empty_group_name");
+    my $exists = new Bugzilla::Group({name => $name });
+    ThrowUserError("group_exists", { name => $name }) if $exists;
+    return $name;
+}
+
+sub _check_description {
+    my ($invocant, $desc) = @_;
+    $desc = trim($desc);
+    $desc || ThrowUserError("empty_group_description");
+    return $desc;
+}
+
+sub _check_user_regexp {
+    my ($invocant, $regex) = @_;
+    $regex = trim($regex) || '';
+    ThrowUserError("invalid_regexp") unless (eval {qr/$regex/});
+    return $regex;
+}
+
+sub _check_is_bug_group {
+    return $_[1] ? 1 : 0;
+}
 1;
 
 __END__
@@ -113,6 +218,13 @@ provides, in addition to any methods documented below.
 
 =over
 
+=item C<create>
+
+Note that in addition to what L<Bugzilla::Object/create($params)>
+normally does, this function also makes the new group be inherited
+by the C<admin> group. That is, the C<admin> group will automatically
+be a member of this group.
+
 =item C<ValidateGroupName($name, @users)>
 
  Description: ValidateGroupName checks to see if ANY of the users

checksetup.pl

@@ -421,43 +421,6 @@ fix_all_file_permissions(!$silent);
 check_graphviz(!$silent) if Bugzilla->params->{'webdotbase'};
 
 ###########################################################################
-# Populate groups table
-###########################################################################
-
-sub GroupDoesExist
-{
-    my ($name) = @_;
-    my $sth = $dbh->prepare("SELECT name FROM groups WHERE name='$name'");
-    $sth->execute;
-    if ($sth->rows) {
-        return 1;
-    }
-    return 0;
-}
-
-
-#
-# This subroutine ensures that a group exists. If not, it will be created 
-# automatically, and given the next available groupid
-#
-
-sub AddGroup {
-    my ($name, $desc, $userregexp) = @_;
-    $userregexp ||= "";
-
-    return if GroupDoesExist($name);
-    
-    print "Adding group $name ...\n";
-    my $sth = $dbh->prepare('INSERT INTO groups
-                          (name, description, userregexp, isbuggroup)
-                          VALUES (?, ?, ?, ?)');
-    $sth->execute($name, $desc, $userregexp, 0);
-
-    my $last = $dbh->bz_last_key('groups', 'id');
-    return $last;
-}
-
-###########################################################################
 # Changes to the fielddefs --TABLE--
 ###########################################################################
 
@@ -474,56 +437,63 @@ Bugzilla::Field::populate_field_definitions();
 
 Bugzilla::Install::DB::update_table_definitions();        
 
-#
+###########################################################################
 # Bugzilla uses --GROUPS-- to assign various rights to its users.
-#
+###########################################################################
 
-AddGroup('tweakparams', 'Can tweak operating parameters');
-AddGroup('editusers', 'Can edit or disable users');
-AddGroup('creategroups', 'Can create and destroy groups.');
-AddGroup('editclassifications', 'Can create, destroy, and edit classifications.');
-AddGroup('editcomponents', 'Can create, destroy, and edit components.');
-AddGroup('editkeywords', 'Can create, destroy, and edit keywords.');
-AddGroup('admin', 'Administrators');
-
-if (!GroupDoesExist("editbugs")) {
-    my $id = AddGroup('editbugs', 'Can edit all bug fields.', ".*");
-    my $sth = $dbh->prepare("SELECT userid FROM profiles");
-    $sth->execute();
-    while (my ($userid) = $sth->fetchrow_array()) {
-        $dbh->do("INSERT INTO user_group_map 
-            (user_id, group_id, isbless, grant_type) 
-            VALUES ($userid, $id, 0, " . GRANT_DIRECT . ")");
-    }
-}
+my $admin_group = Bugzilla::Group->new({ name => 'admin' })
+    || Bugzilla::Group->create({ 
+           name => 'admin', description => 'Administrators', isbuggroup => 0 });
 
-if (!GroupDoesExist("canconfirm")) {
-    my $id = AddGroup('canconfirm',  'Can confirm a bug.', ".*");
-    my $sth = $dbh->prepare("SELECT userid FROM profiles");
-    $sth->execute();
-    while (my ($userid) = $sth->fetchrow_array()) {
-        $dbh->do("INSERT INTO user_group_map 
-            (user_id, group_id, isbless, grant_type) 
-            VALUES ($userid, $id, 0, " . GRANT_DIRECT . ")");
-    }
+Bugzilla::Group->create({ name => 'tweakparams', 
+    description => 'Can tweak operating parameters', isbuggroup => 0 })
+    unless new Bugzilla::Group({ name => 'tweakparams' });
 
-}
+Bugzilla::Group->create({ name => 'editusers', 
+    description => 'Can edit or disable users', isbuggroup => 0 })
+    unless new Bugzilla::Group({ name => 'editusers' });
+
+Bugzilla::Group->create({ name => 'creategroups', 
+    description => 'Can create and destroy groups.', isbuggroup => 0 })
+    unless new Bugzilla::Group({ name => 'creategroups' });
+
+Bugzilla::Group->create({ name => 'editclassifications', 
+    description => 'Can create, destroy, and edit classifications.', 
+    isbuggroup => 0 })
+    unless new Bugzilla::Group({ name => 'editclassifications' });
+
+Bugzilla::Group->create({ name => 'editcomponents', 
+    description => 'Can create, destroy, and edit components.',
+    isbuggroup => 0 })
+    unless new Bugzilla::Group({ name => 'editcomponents' });
+
+Bugzilla::Group->create({ name => 'editkeywords', 
+    description => 'Can create, destroy, and edit keywords.',
+    isbuggroup => 0 })
+    unless new Bugzilla::Group({ name => 'editkeywords' });
+
+Bugzilla::Group->create({name => 'editbugs', 
+    description => 'Can edit all bug fields.', userregexp => ".*", 
+    isbuggroup => 0 })
+    unless new Bugzilla::Group({name => "editbugs"});
+
+Bugzilla::Group->create({ name => 'canconfirm',  
+    description => 'Can confirm a bug.', userregexp => ".*", 
+    isbuggroup => 0 })
+    unless new Bugzilla::Group({name => "canconfirm"});
 
 # Create bz_canusewhineatothers and bz_canusewhines
-if (!GroupDoesExist('bz_canusewhines')) {
-    my $whine_group = AddGroup('bz_canusewhines',
-                               'User can configure whine reports for self');
-    my $whineatothers_group = AddGroup('bz_canusewhineatothers',
-                                       'Can configure whine reports for ' .
-                                       'other users');
-    my $group_exists = $dbh->selectrow_array(
-        q{SELECT 1 FROM group_group_map 
-           WHERE member_id = ? AND grantor_id = ? AND grant_type = ?},
-        undef, $whineatothers_group, $whine_group, GROUP_MEMBERSHIP);
-    $dbh->do("INSERT INTO group_group_map " .
-             "(member_id, grantor_id, grant_type) " .
-             "VALUES (${whineatothers_group}, ${whine_group}, " .
-             GROUP_MEMBERSHIP . ")") unless $group_exists;
+if (!new Bugzilla::Group({name => 'bz_canusewhines'})) {
+    my $whine = Bugzilla::Group->create({name => 'bz_canusewhines',
+       description => 'User can configure whine reports for self', 
+       isbuggroup => 0 });
+    my $whineatothers = Bugzilla::Group->create({
+        name => 'bz_canusewhineatothers',
+        description => 'Can configure whine reports for other users', 
+        isbuggroup => 0 });
+
+    $dbh->do('INSERT INTO group_group_map (grantor_id, member_id) VALUES (?,?)',
+             undef, $whine->id, $whineatothers->id);
 }
 
 # 2005-08-14 bugreport@peshkin.net -- Bug 304583
@@ -563,24 +533,14 @@ while (my ($uid, $login, $gid, $rexp, $present) = $sth->fetchrow_array()) {
 }
 
 # 2005-10-10 karl@kornel.name -- Bug 204498
-if (!GroupDoesExist('bz_sudoers')) {
-    my $sudoers_group = AddGroup('bz_sudoers',
-                                 'Can perform actions as other users');
-    my $sudo_protect_group = AddGroup('bz_sudo_protect',
-                                      'Can not be impersonated by other users');
-    my ($admin_group) = $dbh->selectrow_array('SELECT id FROM groups
-                                               WHERE name = ?', undef, 'admin');
-    
-    # Admins should be given sudo access
-    # Everyone in sudo should be in sudo_protect
-    # Admins can grant membership in both groups
-    my $sth = $dbh->prepare('INSERT INTO group_group_map 
-                             (member_id, grantor_id, grant_type) 
-                             VALUES (?, ?, ?)');
-    $sth->execute($admin_group, $sudoers_group, GROUP_MEMBERSHIP);
-    $sth->execute($sudoers_group, $sudo_protect_group, GROUP_MEMBERSHIP);
-    $sth->execute($admin_group, $sudoers_group, GROUP_BLESS);
-    $sth->execute($admin_group, $sudo_protect_group, GROUP_BLESS);
+if (!new Bugzilla::Group({name => 'bz_sudoers'})) {
+    my $sudo = Bugzilla::Group->create({name => 'bz_sudoers',
+       description => 'Can perform actions as other users', isbuggroup => 0 });
+    my $sudo_protect = Bugzilla::Group->create({name => 'bz_sudo_protect',
+        description => 'Can not be impersonated by other users',
+        isbuggroup => 0 });
+    $dbh->do('INSERT INTO group_group_map (grantor_id, member_id) VALUES (?,?)',
+             undef, $sudo_protect->id, $sudo->id);
 }
 
 ###########################################################################

editgroups.cgi

@@ -52,36 +52,6 @@ $user->in_group('creategroups')
 
 my $action = trim($cgi->param('action') || '');
 
-# RederiveRegexp: update user_group_map with regexp-based grants
-sub RederiveRegexp
-{
-    my $regexp = shift;
-    my $gid = shift;
-    my $dbh = Bugzilla->dbh;
-    my $sth = $dbh->prepare("SELECT userid, login_name, group_id
-                               FROM profiles
-                          LEFT JOIN user_group_map
-                                 ON user_group_map.user_id = profiles.userid
-                                AND group_id = ?
-                                AND grant_type = ?
-                                AND isbless = 0");
-    my $sthadd = $dbh->prepare("INSERT INTO user_group_map
-                                 (user_id, group_id, grant_type, isbless)
-                                 VALUES (?, ?, ?, 0)");
-    my $sthdel = $dbh->prepare("DELETE FROM user_group_map
-                                 WHERE user_id = ? AND group_id = ?
-                                 AND grant_type = ? and isbless = 0");
-    $sth->execute($gid, GRANT_REGEXP);
-    while (my ($uid, $login, $present) = $sth->fetchrow_array()) {
-        if (($regexp =~ /\S+/) && ($login =~ m/$regexp/i))
-        {
-            $sthadd->execute($uid, $gid, GRANT_REGEXP) unless $present;
-        } else {
-            $sthdel->execute($uid, $gid, GRANT_REGEXP) if $present;
-        }
-    }
-}
-
 # Add missing entries in bug_group_map for bugs created while
 # a mandatory group was disabled and which is now enabled again.
 sub fix_bug_permissions {
@@ -313,7 +283,7 @@ if ($action eq 'new') {
                   SELECT ?, products.id, 0, ?, ?, 0 FROM products',
                   undef, ($gid, CONTROLMAPSHOWN, CONTROLMAPNA));
     }
-    RederiveRegexp($regexp, $gid);
+    Bugzilla::Group::RederiveRegexp($regexp, $gid);
 
     print $cgi->header();
     $template->process("admin/groups/created.html.tmpl", $vars)
@@ -657,7 +627,7 @@ sub doGroupChanges {
         $chgs = 1;
         $dbh->do('UPDATE groups SET userregexp = ? WHERE id = ?',
                   undef, ($regexp, $gid));
-        RederiveRegexp($regexp, $gid);
+        Bugzilla::Group::RederiveRegexp($regexp, $gid);
     }
 
     my $sthInsert = $dbh->prepare('INSERT INTO group_group_map