Bug 304583: Remove all remaining need to rederive inherited groups

Bugzilla/Auth/Login/WWW/Env.pm

@@ -38,6 +38,7 @@ sub login {
     my $matched_userid    = '';
     my $matched_extern_id = '';
     my $disabledtext      = '';
+    my $new_login_name = 0;
 
     my $dbh = Bugzilla->dbh;
     my $sth;
@@ -122,6 +123,7 @@ sub login {
                                      ") VALUES ( ?, ?, ?, '' )");
                 $sth->execute($env_email, '*', $env_realname);
                 $matched_userid = $dbh->bz_last_key('profiles', 'userid');
+                $new_login_name = $matched_userid;
             }
         }
     }
@@ -147,9 +149,16 @@ sub login {
                           ($env_realname || $this_realname),
                           $matched_userid);
             $sth->execute;
+            $new_login_name = $matched_userid;
         }
     }
 
+    # If the login name may be new, make sure the regexp groups are current
+    if ($new_login_name) {
+        my $userprofile = new Bugzilla::User($matched_userid);
+        $userprofile->derive_regexp_groups;
+    }
+
     # Now we throw an error if the user has been disabled
     if ($disabledtext) {
         ThrowUserError("account_disabled",

Bugzilla/Bug.pm

@@ -541,25 +541,22 @@ sub groups {
     # user_group_map record putting the user in that group.
     # The LEFT JOINs are checking for record existence.
     #
+    my $grouplist = Bugzilla->user->groups_as_string;
     my $sth = $dbh->prepare(
              "SELECT DISTINCT groups.id, name, description," .
              " bug_group_map.group_id IS NOT NULL," .
-             " user_group_map.group_id IS NOT NULL," .
+             " CASE WHEN groups.id IN($grouplist) THEN 1 ELSE 0 END," .
              " isactive, membercontrol, othercontrol" .
              " FROM groups" . 
              " LEFT JOIN bug_group_map" .
              " ON bug_group_map.group_id = groups.id" .
              " AND bug_id = ?" .
-             " LEFT JOIN user_group_map" .
-             " ON user_group_map.group_id = groups.id" .
-             " AND user_id = ?" .
-             " AND isbless = 0" .
              " LEFT JOIN group_control_map" .
              " ON group_control_map.group_id = groups.id" .
              " AND group_control_map.product_id = ? " .
              " WHERE isbuggroup = 1" .
              " ORDER BY description");
-    $sth->execute($self->{'bug_id'}, Bugzilla->user->id,
+    $sth->execute($self->{'bug_id'},
                   $self->{'product_id'});
 
     while (my ($groupid, $name, $description, $ison, $ingroup, $isactive,

Bugzilla/Constants.pm

@@ -53,7 +53,6 @@ use base qw(Exporter);
     LOGOUT_KEEP_CURRENT
 
     GRANT_DIRECT
-    GRANT_DERIVED
     GRANT_REGEXP
 
     GROUP_MEMBERSHIP
@@ -68,8 +67,6 @@ use base qw(Exporter);
 
     COMMENT_COLS
 
-    DERIVE_GROUPS_TABLES_ALREADY_LOCKED
-
     UNLOCK_ABORT
     
     RELATIONSHIPS
@@ -157,7 +154,6 @@ use constant contenttypes =>
   };
 
 use constant GRANT_DIRECT => 0;
-use constant GRANT_DERIVED => 1;
 use constant GRANT_REGEXP => 2;
 
 use constant GROUP_MEMBERSHIP => 0;
@@ -180,10 +176,6 @@ use constant DEFAULT_QUERY_NAME => '(Default query)';
 # The column length for displayed (and wrapped) bug comments.
 use constant COMMENT_COLS => 80;
 
-# Used to indicate to User::new and User::new_from_login calls
-# that the derive_groups tables are already locked
-use constant DERIVE_GROUPS_TABLES_ALREADY_LOCKED => 1;
-
 # used by Bugzilla::DB to indicate that tables are being unlocked
 # because of error
 use constant UNLOCK_ABORT => 1;

Bugzilla/Flag.pm

@@ -906,9 +906,7 @@ sub notify {
     {
         my @new_cc_list;
         foreach my $cc (split(/[, ]+/, $flag->{'type'}->{'cc_list'})) {
-            my $ccuser = Bugzilla::User->new_from_login($cc,
-                                                        DERIVE_GROUPS_TABLES_ALREADY_LOCKED)
-              || next;
+            my $ccuser = Bugzilla::User->new_from_login($cc) || next;
 
             next if $flag->{'target'}->{'bug'}->{'restricted'}
               && !$ccuser->can_see_bug($flag->{'target'}->{'bug'}->{'id'});

Bugzilla/User.pm

@@ -111,8 +111,6 @@ sub _create {
     # We're checking for validity here, so any value is OK
     trick_taint($val);
 
-    my $tables_locked_for_derive_groups = shift;
-
     my $dbh = Bugzilla->dbh;
 
     my ($id,
@@ -137,26 +135,6 @@ sub _create {
     $self->{'disabledtext'}   = $disabledtext;
     $self->{'showmybugslink'} = $mybugslink;
 
-    # Now update any old group information if needed
-    my $result = $dbh->selectrow_array(q{SELECT 1
-                                           FROM profiles, groups
-                                          WHERE userid=?
-                                            AND profiles.refreshed_when <=
-                                                  groups.last_changed},
-                                       undef,
-                                       $id);
-
-    if ($result) {
-        my $is_main_db;
-        unless ($is_main_db = Bugzilla->dbwritesallowed()) {
-            Bugzilla->switch_to_main_db();
-        }
-        $self->derive_groups($tables_locked_for_derive_groups);
-        unless ($is_main_db) {
-            Bugzilla->switch_to_shadow_db();
-        }
-    }
-
     return $self;
 }
 
@@ -283,11 +261,36 @@ sub groups {
     # The above gives us an arrayref [name, id, name, id, ...]
     # Convert that into a hashref
     my %groups = @$groups;
+    my $sth;
+    my @groupidstocheck = values(%groups);
+    my %groupidschecked = ();
+    $sth = $dbh->prepare("SELECT groups.name, groups.id
+                            FROM group_group_map
+                      INNER JOIN groups
+                              ON groups.id = grantor_id
+                           WHERE member_id = ? 
+                             AND grant_type = " . GROUP_MEMBERSHIP);
+    while (my $node = shift @groupidstocheck) {
+        $sth->execute($node);
+        my ($member_name, $member_id);
+        while (($member_name, $member_id) = $sth->fetchrow_array) {
+            if (!$groupidschecked{$member_id}) {
+                $groupidschecked{$member_id} = 1;
+                push @groupidstocheck, $member_id;
+                $groups{$member_name} = $member_id;
+            }
+        }
+    }
     $self->{groups} = \%groups;
 
     return $self->{groups};
 }
 
+sub groups_as_string {
+    my $self = shift;
+    return (join(',',values(%{$self->groups})) || '-1');
+}
+
 sub bless_groups {
     my $self = shift;
 
@@ -308,8 +311,6 @@ sub bless_groups {
         # Get all groups for the user where:
         #    + They have direct bless privileges
         #    + They are a member of a group that inherits bless privs.
-        # Because of the second requirement, derive_groups must be up-to-date
-        # for this to function properly in all circumstances.
         $query = q{
             SELECT DISTINCT groups.id, groups.name, groups.description
                        FROM groups, user_group_map, group_group_map AS ggm
@@ -318,8 +319,9 @@ sub bless_groups {
                               AND groups.id=user_group_map.group_id)
                              OR (groups.id = ggm.grantor_id
                                  AND ggm.grant_type = ?
-                                 AND user_group_map.group_id = ggm.member_id
-                                 AND user_group_map.isbless = 0))};
+                                 AND ggm.member_id IN(} .
+                                 $self->groups_as_string . 
+                               q{)))};
         $connector = 'AND';
         @bindValues = ($self->id, GROUP_BLESS);
     }
@@ -340,26 +342,13 @@ sub bless_groups {
 
 sub in_group {
     my ($self, $group) = @_;
+    return exists $self->groups->{$group} ? 1 : 0;
+}
 
-    # If we already have the info, just return it.
-    return defined($self->{groups}->{$group}) if defined $self->{groups};
-    return 0 unless $self->id;
-
-    # Otherwise, go check for it
-
-    my $dbh = Bugzilla->dbh;
-
-    my ($res) = $dbh->selectrow_array(q{SELECT 1
-                                  FROM groups, user_group_map
-                                 WHERE groups.id=user_group_map.group_id
-                                   AND user_group_map.user_id=?
-                                   AND isbless=0
-                                   AND groups.name=?},
-                              undef,
-                              $self->id,
-                              $group);
-
-    return defined($res);
+sub in_group_id {
+    my ($self, $id) = @_;
+    my %j = reverse(%{$self->groups});
+    return exists $j{$id} ? 1 : 0;
 }
 
 sub can_see_user {
@@ -405,7 +394,7 @@ sub can_see_bug {
                              LEFT JOIN bug_group_map 
                                ON bugs.bug_id = bug_group_map.bug_id
                                AND bug_group_map.group_ID NOT IN(" .
-                               join(',',(-1, values(%{$self->groups}))) .
+                               $self->groups_as_string .
                                ") WHERE bugs.bug_id = ? 
                                AND creation_ts IS NOT NULL " .
                              $dbh->sql_group_by('bugs.bug_id', 'reporter, ' .
@@ -445,7 +434,7 @@ sub get_selectable_products {
                   CONTROLMAPMANDATORY . " ";
     }
     $query .= "AND group_id NOT IN(" . 
-               join(',', (-1,values(%{Bugzilla->user->groups}))) . ") " .
+               $self->groups_as_string . ") " .
               "WHERE group_id IS NULL ORDER BY name";
     my $dbh = Bugzilla->dbh;
     my $sth = $dbh->prepare($query);
@@ -501,8 +490,8 @@ sub visible_groups_as_string {
     return join(', ', @{$self->visible_groups_inherited()});
 }
 
-sub derive_groups {
-    my ($self, $already_locked) = @_;
+sub derive_regexp_groups {
+    my ($self) = @_;
 
     my $id = $self->id;
     return unless $id;
@@ -511,71 +500,32 @@ sub derive_groups {
 
     my $sth;
 
-    $dbh->bz_lock_tables('profiles WRITE', 'user_group_map WRITE',
-                         'group_group_map READ',
-                         'groups READ') unless $already_locked;
-
     # avoid races, we are only up to date as of the BEGINNING of this process
     my $time = $dbh->selectrow_array("SELECT NOW()");
 
-    # first remove any old derived stuff for this user
-    $dbh->do(q{DELETE FROM user_group_map
-                      WHERE user_id = ?
-                        AND grant_type != ?},
-             undef,
-             $id,
-             GRANT_DIRECT);
-
-    my %groupidsadded = ();
     # add derived records for any matching regexps
 
-    $sth = $dbh->prepare("SELECT id, userregexp FROM groups WHERE userregexp != ''");
-    $sth->execute;
-
-    my $group_insert;
-    while (my $row = $sth->fetch) {
-        if ($self->{login} =~ m/$row->[1]/i) {
-            $group_insert ||= $dbh->prepare(q{INSERT INTO user_group_map
-                                              (user_id, group_id, isbless, grant_type)
-                                              VALUES (?, ?, 0, ?)});
-            $groupidsadded{$row->[0]} = 1;
-            $group_insert->execute($id, $row->[0], GRANT_REGEXP);
-        }
-    }
-
-    # Get a list of the groups of which the user is a member.
-    my %groupidschecked = ();
-
-    my @groupidstocheck = @{$dbh->selectcol_arrayref(q{SELECT group_id
-                                                         FROM user_group_map
-                                                        WHERE user_id=?},
-                                                     undef,
-                                                     $id)};
-
-    # Each group needs to be checked for inherited memberships once.
-    my $group_sth;
-    while (@groupidstocheck) {
-        my $group = shift @groupidstocheck;
-        if (!defined($groupidschecked{"$group"})) {
-            $groupidschecked{"$group"} = 1;
-            $group_sth ||= $dbh->prepare(q{SELECT grantor_id
-                                             FROM group_group_map
-                                            WHERE member_id=?
-                                              AND grant_type = } .
-                                                  GROUP_MEMBERSHIP);
-            $group_sth->execute($group);
-            while (my ($groupid) = $group_sth->fetchrow_array) {
-                if (!defined($groupidschecked{"$groupid"})) {
-                    push(@groupidstocheck,$groupid);
-                }
-                if (!$groupidsadded{$groupid}) {
-                    $groupidsadded{$groupid} = 1;
-                    $group_insert ||= $dbh->prepare(q{INSERT INTO user_group_map
-                                                      (user_id, group_id, isbless, grant_type)
-                                                      VALUES (?, ?, 0, ?)});
-                    $group_insert->execute($id, $groupid, GRANT_DERIVED);
-                }
-            }
+    $sth = $dbh->prepare("SELECT id, userregexp, user_group_map.group_id
+                            FROM groups
+                       LEFT JOIN user_group_map
+                              ON groups.id = user_group_map.group_id
+                             AND user_group_map.user_id = ?
+                             AND user_group_map.grant_type = ?");
+    $sth->execute($id, GRANT_REGEXP);
+
+    my $group_insert = $dbh->prepare(q{INSERT INTO user_group_map
+                                       (user_id, group_id, isbless, grant_type)
+                                       VALUES (?, ?, 0, ?)});
+    my $group_delete = $dbh->prepare(q{DELETE FROM user_group_map
+                                       WHERE user_id = ?
+                                         AND group_id = ?
+                                         AND isbless = 0
+                                         AND grant_type = ?});
+    while (my ($group, $regexp, $present) = $sth->fetchrow_array()) {
+        if (($regexp ne '') && ($self->{login} =~ m/$regexp/i)) {
+            $group_insert->execute($id, $group, GRANT_REGEXP) unless $present;
+        } else {
+            $group_delete->execute($id, $group, GRANT_REGEXP) if $present;
         }
     }
 
@@ -585,7 +535,6 @@ sub derive_groups {
              undef,
              $time,
              $id);
-    $dbh->bz_unlock_tables() unless $already_locked;
 }
 
 sub product_responsibilities {
@@ -690,8 +639,8 @@ sub match {
             $query .= "AND user_group_map.user_id = userid " .
                       "AND isbless = 0 " .
                       "AND group_id IN(" .
-                      join(', ', (-1, @{$user->visible_groups_inherited})) . ") " .
-                      "AND grant_type <> " . GRANT_DERIVED;
+                      join(', ', (-1, @{$user->visible_groups_inherited})) . 
+                      ")";
         }
         $query    .= " AND disabledtext = '' " if $exclude_disabled;
         $query    .= "ORDER BY namelength ";
@@ -743,8 +692,7 @@ sub match {
             $query .= " AND user_group_map.user_id = userid" .
                       " AND isbless = 0" .
                       " AND group_id IN(" .
-                join(', ', (-1, @{$user->visible_groups_inherited})) . ")" .
-                      " AND grant_type <> " . GRANT_DERIVED;
+                join(', ', (-1, @{$user->visible_groups_inherited})) . ")";
         }
         $query     .= " AND disabledtext = ''" if $exclude_disabled;
         $query     .= " ORDER BY namelength";
@@ -1151,8 +1099,7 @@ sub get_userlist {
         $query .= "LEFT JOIN user_group_map " .
                   "ON user_group_map.user_id = userid AND isbless = 0 " .
                   "AND group_id IN(" .
-                  join(', ', (-1, @{$self->visible_groups_inherited})) . ") " .
-                  "AND grant_type <> " . GRANT_DERIVED;
+                  join(', ', (-1, @{$self->visible_groups_inherited})) . ")";
     }
     $query    .= " WHERE disabledtext = '' ";
     $query    .= $dbh->sql_group_by('userid', 'login_name, realname');
@@ -1271,7 +1218,7 @@ sub login_to_id {
 }
 
 sub UserInGroup {
-    return defined Bugzilla->user->groups->{$_[0]} ? 1 : 0;
+    return exists Bugzilla->user->groups->{$_[0]} ? 1 : 0;
 }
 
 1;
@@ -1346,10 +1293,6 @@ C<undef> if no matching user is found.
 This routine should not be required in general; most scripts should be using
 userids instead.
 
-This routine and C<new> both take an extra optional argument, which is
-passed as the argument to C<derive_groups> to avoid locking. See that
-routine's documentation for details.
-
 =end undocumented
 
 =item C<id>
@@ -1439,12 +1382,19 @@ are the names of the groups, whilst the values are the respective group ids.
 (This is so that a set of all groupids for groups the user is in can be
 obtained by C<values(%{$user-E<gt>groups})>.)
 
+=item C<groups_as_string>
+
+Returns a string containing a comma-seperated list of numeric group ids.  If
+the user is not a member of any groups, returns "-1". This is most often used
+within an SQL IN() function.
+
 =item C<in_group>
 
-Determines whether or not a user is in the given group. This method is mainly
-intended for cases where we are not looking at the currently logged in user,
-and only need to make a quick check for the group, where calling C<groups>
-and getting all of the groups would be overkill.
+Determines whether or not a user is in the given group by name. 
+
+=item C<in_group_id>
+
+Determines whether or not a user is in the given group by id. 
 
 =item C<bless_groups>
 
@@ -1464,7 +1414,7 @@ Returns 1 if the specified user account exists and is visible to the user,
 
 Determines if the user can see the specified bug.
 
-=item C<derive_groups>
+=item C<derive_regexp_groups>
 
 Bugzilla allows for group inheritance. When data about the user (or any of the
 groups) changes, the database must be updated. Handling updated groups is taken
@@ -1508,23 +1458,6 @@ Returns a list of groups that the user is aware of.
 Returns the result of C<visible_groups_direct> as a string (a comma-separated
 list).
 
-=begin undocumented
-
-This routine takes an optional argument. If true, then this routine will not
-lock the tables, but will rely on the caller to have done so itsself.
-
-This is required because mysql will only execute a query if all of the tables
-are locked, or if none of them are, not a mixture. If the caller has already
-done some locking, then this routine would fail. Thus the caller needs to lock
-all the tables required by this method, and then C<derive_groups> won't do
-any locking.
-
-This is a really ugly solution, and when Bugzilla supports transactions
-instead of using the explicit table locking we were forced to do when thats
-all MySQL supported, this will go away.
-
-=end undocumented
-
 =item C<product_responsibilities>
 
 Retrieve user's product responsibilities as a list of hashes.

checksetup.pl

@@ -31,6 +31,7 @@
 #                 Erik Stambaugh <erik@dasbistro.com>
 #                 Dave Lawrence <dkl@redhat.com>
 #                 Max Kanat-Alexander <mkanat@bugzilla.org>
+#                 Joel Peshkin <bugreport@peshkin.net>
 #
 #
 #
@@ -3557,11 +3558,8 @@ AddFDef("owner_idle_time", "Time Since Assignee Touched", 0);
 if ($dbh->bz_column_info("user_group_map", "isderived")) {
     $dbh->bz_add_column('user_group_map', 'grant_type', 
         {TYPE => 'INT1', NOTNULL => 1, DEFAULT => '0'});
-    $dbh->do("UPDATE user_group_map SET grant_type = " .
-                             "IF(isderived, " . GRANT_DERIVED . ", " .
-                             GRANT_DIRECT . ")");
-    $dbh->do("DELETE FROM user_group_map 
-              WHERE isbless = 0 AND grant_type != " . GRANT_DIRECT);
+    $dbh->do("DELETE FROM user_group_map WHERE isderived != 0");
+    $dbh->do("UPDATE user_group_map SET grant_type = " . GRANT_DIRECT);
     $dbh->bz_drop_column("user_group_map", "isderived");
 
     $dbh->bz_drop_index('user_group_map', 'user_group_map_user_id_idx');
@@ -3569,21 +3567,6 @@ if ($dbh->bz_column_info("user_group_map", "isderived")) {
         {TYPE => 'UNIQUE', 
          FIELDS => [qw(user_id group_id grant_type isbless)]});
 
-    # Evaluate regexp-based group memberships
-    my $sth = $dbh->prepare("SELECT profiles.userid, profiles.login_name,
-                             groups.id, groups.userregexp 
-                             FROM profiles, groups
-                             WHERE userregexp != ''");
-    $sth->execute();
-    my $sth2 = $dbh->prepare("INSERT IGNORE INTO user_group_map 
-                           (user_id, group_id, isbless, grant_type) 
-                           VALUES(?, ?, 0, " . GRANT_REGEXP . ")");
-    while (my ($uid, $login, $gid, $rexp) = $sth->fetchrow_array()) {
-        if ($login =~ m/$rexp/i) {
-            $sth2->execute($uid, $gid);
-        }
-    }
-
     # Make sure groups get rederived
     $dbh->do("UPDATE groups SET last_changed = NOW() WHERE name = 'admin'");
 }
@@ -4090,6 +4073,42 @@ if (!GroupDoesExist('bz_canusewhines')) {
              GROUP_MEMBERSHIP . ")") unless $group_exists;
 }
 
+# 2005-08-14 bugreport@peshkin.net -- Bug 304583
+use constant GRANT_DERIVED => 1;
+# Get rid of leftover DERIVED group permissions
+$dbh->do("DELETE FROM user_group_map WHERE grant_type = " . GRANT_DERIVED);
+# Evaluate regexp-based group memberships
+$sth = $dbh->prepare("SELECT profiles.userid, profiles.login_name,
+                         groups.id, groups.userregexp,
+                         user_group_map.group_id
+                         FROM profiles
+                         CROSS JOIN groups
+                         LEFT JOIN user_group_map
+                         ON user_group_map.user_id = profiles.userid
+                         AND user_group_map.group_id = groups.id
+                         AND user_group_map.grant_type = ?
+                         WHERE (userregexp != ''
+                         OR user_group_map.group_id IS NOT NULL)");
+
+my $sth_add = $dbh->prepare("INSERT INTO user_group_map 
+                       (user_id, group_id, isbless, grant_type) 
+                       VALUES(?, ?, 0, " . GRANT_REGEXP . ")");
+
+my $sth_del = $dbh->prepare("DELETE FROM user_group_map 
+                       WHERE user_id  = ? 
+                       AND group_id = ? 
+                       AND isbless = 0
+                       AND grant_type = " . GRANT_REGEXP);
+
+$sth->execute(GRANT_REGEXP);
+while (my ($uid, $login, $gid, $rexp, $present) = $sth->fetchrow_array()) {
+    if ($login =~ m/$rexp/i) {
+        $sth_add->execute($uid, $gid) unless $present;
+    } else {
+        $sth_del->execute($uid, $gid) if $present;
+    }
+}
+
 ###########################################################################
 # Create --SETTINGS-- users can adjust
 ###########################################################################

editgroups.cgi

@@ -56,20 +56,21 @@ sub RederiveRegexp
     my $regexp = shift;
     my $gid = shift;
     my $dbh = Bugzilla->dbh;
-    my $sth = $dbh->prepare("SELECT userid, login_name FROM profiles");
-    my $sthqry = $dbh->prepare("SELECT 1 FROM user_group_map
-                                 WHERE user_id = ? AND group_id = ?
-                                 AND grant_type = ? and isbless = 0");
+    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();
-    while (my ($uid, $login) = $sth->fetchrow_array()) {
-        my $present = $dbh->selectrow_array($sthqry, undef,
-                                            $uid, $gid, GRANT_REGEXP);
+    $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;

editusers.cgi

@@ -185,6 +185,8 @@ if ($action eq 'search') {
     insert_new_user($login, $realname, $password, $disabledtext);
     $otherUserID = $dbh->bz_last_key('profiles', 'userid');
     $dbh->bz_unlock_tables();
+    my $newprofile = new Bugzilla::User($otherUserID);
+    $newprofile->derive_regexp_groups();
     userDataToVars($otherUserID);
 
     $vars->{'message'} = 'account_created';
@@ -290,6 +292,12 @@ if ($action eq 'search') {
                      'WHERE userid = ?',
                      undef, @values);
             # XXX: should create profiles_activity entries.
+            #
+            # We create a new user object here because it needs to
+            # read information that may have changed since this
+            # script started.
+            my $newprofile = new Bugzilla::User($otherUserID);
+            $newprofile->derive_regexp_groups();
         }
     }
 
@@ -639,7 +647,7 @@ sub userDataToVars {
     my $query;
     my $dbh = Bugzilla->dbh;
 
-    $otheruser->derive_groups();
+    my $grouplist = $otheruser->groups_as_string;
 
     $vars->{'otheruser'} = $otheruser;
     $vars->{'groups'} = $user->bless_groups();
@@ -648,7 +656,7 @@ sub userDataToVars {
         qq{SELECT id,
                   COUNT(directmember.group_id) AS directmember,
                   COUNT(regexpmember.group_id) AS regexpmember,
-                  COUNT(derivedmember.group_id) AS derivedmember,
+                  CASE WHEN groups.id IN ($grouplist) THEN 1 ELSE 0 END,
                   COUNT(directbless.group_id) AS directbless
            FROM groups
            LEFT JOIN user_group_map AS directmember
@@ -661,11 +669,6 @@ sub userDataToVars {
                  AND regexpmember.user_id = ?
                  AND regexpmember.isbless = 0
                  AND regexpmember.grant_type = ?
-           LEFT JOIN user_group_map AS derivedmember
-                  ON derivedmember.group_id = id
-                 AND derivedmember.user_id = ?
-                 AND derivedmember.isbless = 0
-                 AND derivedmember.grant_type = ?
            LEFT JOIN user_group_map AS directbless
                   ON directbless.group_id = id
                  AND directbless.user_id = ?
@@ -675,20 +678,17 @@ sub userDataToVars {
         'id', undef,
         ($otheruserid, GRANT_DIRECT,
          $otheruserid, GRANT_REGEXP,
-         $otheruserid, GRANT_DERIVED,
          $otheruserid, GRANT_DIRECT));
 
     # Find indirect bless permission.
     $query = qq{SELECT groups.id
-                FROM groups, user_group_map AS ugm, group_group_map AS ggm
-                WHERE ugm.user_id = ?
-                  AND groups.id = ggm.grantor_id
-                  AND ggm.member_id = ugm.group_id
-                  AND ugm.isbless = 0
+                FROM groups, group_group_map AS ggm
+                WHERE groups.id = ggm.grantor_id
+                  AND ggm.member_id IN ($grouplist)
                   AND ggm.grant_type = ?
                } . $dbh->sql_group_by('id');
     foreach (@{$dbh->selectall_arrayref($query, undef,
-                                        ($otheruserid, GROUP_BLESS))}) {
+                                        (GROUP_BLESS))}) {
         # Merge indirect bless permissions into permission variable.
         $vars->{'permissions'}{${$_}[0]}{'indirectbless'} = 1;
     }

post_bug.cgi

@@ -335,11 +335,7 @@ foreach my $b (grep(/^bit-\d*$/, $cgi->param())) {
             $vars->{'bit'} = $v;
             ThrowCodeError("inactive_group");
         }
-        SendSQL("SELECT user_id FROM user_group_map 
-                 WHERE user_id = $::userid
-                 AND group_id = $v
-                 AND isbless = 0");
-        my ($permit) = FetchSQLData();
+        my ($permit) = $user->in_group_id($v);
         if (!$permit) {
             SendSQL("SELECT othercontrol FROM group_control_map
                      WHERE group_id = $v AND product_id = $product_id");

process_bug.cgi

@@ -73,6 +73,7 @@ use vars qw(@legal_product
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
 my $whoid = $user->id;
+my $grouplist = $user->groups_as_string;
 
 my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
@@ -704,10 +705,9 @@ sub ChangeResolution {
 my @groupAdd = ();
 my @groupDel = ();
 
-SendSQL("SELECT groups.id, isactive FROM groups INNER JOIN user_group_map " .
-        "ON groups.id = user_group_map.group_id " .
-        "WHERE user_group_map.user_id = $whoid " .
-        "AND isbless = 0 AND isbuggroup = 1");
+SendSQL("SELECT groups.id, isactive FROM groups " .
+        "WHERE id IN($grouplist) " .
+        "AND isbuggroup = 1");
 while (my ($b, $isactive) = FetchSQLData()) {
     # The multiple change page may not show all groups a bug is in
     # (eg product groups when listing more than one product)
@@ -1560,7 +1560,7 @@ foreach my $id (@idlist) {
         # - Is the bug in this group?
         SendSQL("SELECT DISTINCT groups.id, isactive, " .
                 "oldcontrolmap.membercontrol, newcontrolmap.membercontrol, " .
-                "user_group_map.user_id IS NOT NULL, " .
+                "CASE WHEN groups_id IN ($grouplist) THEN 1 ELSE 0 END, " .
                 "bug_group_map.group_id IS NOT NULL " .
                 "FROM groups " .
                 "LEFT JOIN group_control_map AS oldcontrolmap " .
@@ -1569,10 +1569,6 @@ foreach my $id (@idlist) {
                 " LEFT JOIN group_control_map AS newcontrolmap " .
                 "ON newcontrolmap.group_id = groups.id " .
                 "AND newcontrolmap.product_id = $newproduct_id " .
-                "LEFT JOIN user_group_map " .
-                "ON user_group_map.group_id = groups.id " .
-                "AND user_group_map.user_id = $whoid " .
-                "AND user_group_map.isbless = 0 " .
                 "LEFT JOIN bug_group_map " .
                 "ON bug_group_map.group_id = groups.id " .
                 "AND bug_group_map.bug_id = $id "

sanitycheck.cgi

@@ -118,66 +118,6 @@ if (defined $cgi->param('rebuildvotecache')) {
 }
 
 ###########################################################################
-# Fix group derivations
-###########################################################################
-
-if (defined $cgi->param('rederivegroups')) {
-    Status("OK, All users' inherited permissions will be rechecked when " .
-           "they next access Bugzilla.");
-    SendSQL("UPDATE groups SET last_changed = NOW() " . $dbh->sql_limit(1));
-}
-
-# rederivegroupsnow is REALLY only for testing.
-# If it wasn't, then we'd do this the faster way as a per-group
-# thing rather than per-user for group inheritance
-if (defined $cgi->param('rederivegroupsnow')) {
-    require Bugzilla::User;
-    Status("OK, now rederiving groups.");
-    SendSQL("SELECT userid FROM profiles");
-    while ((my $id) = FetchSQLData()) {
-        my $user = new Bugzilla::User($id);
-        $user->derive_groups();
-        Status("User $id");
-    }
-}
-
-if (defined $cgi->param('cleangroupsnow')) {
-    Status("OK, now cleaning stale groups.");
-    # Only users that were out of date already long ago should be cleaned
-    # and the cleaning is done with tables locked.  This is require in order
-    # to keep another session from proceeding with permission checks
-    # after the groups have been cleaned unless it first had an opportunity
-    # to get the groups up to date.
-    # If any page starts taking longer than one hour to load, this interval
-    # should be revised.
-    SendSQL("SELECT MAX(last_changed) FROM groups WHERE last_changed < NOW() - " . 
-            $dbh->sql_interval('1 HOUR'));
-    (my $cutoff) = FetchSQLData();
-    Status("Cutoff is $cutoff");
-    SendSQL("SELECT COUNT(*) FROM user_group_map");
-    (my $before) = FetchSQLData();
-    $dbh->bz_lock_tables('user_group_map WRITE', 'profiles WRITE');
-    SendSQL("SELECT userid FROM profiles " .
-            "WHERE refreshed_when > 0 " .
-            "AND refreshed_when < " . SqlQuote($cutoff) . " " .
-            $dbh->sql_limit(1000));
-    my $count = 0;
-    while ((my $id) = FetchSQLData()) {
-        $count++;
-        PushGlobalSQLState();
-        SendSQL("DELETE FROM user_group_map WHERE " .
-            "user_id = $id AND isderived = 1 AND isbless = 0");
-        SendSQL("UPDATE profiles SET refreshed_when = 0 WHERE userid = $id");
-        PopGlobalSQLState();
-    }
-    $dbh->bz_unlock_tables();
-    SendSQL("SELECT COUNT(*) FROM user_group_map");
-    (my $after) = FetchSQLData();
-    Status("Cleaned table for $count users " .
-           "- reduced from $before records to $after records");
-}
-
-###########################################################################
 # Create missing group_control_map entries
 ###########################################################################
 

token.cgi

@@ -268,7 +268,7 @@ sub changeEmail {
 
     # The email address has been changed, so we need to rederive the groups
     my $user = new Bugzilla::User($userid);
-    $user->derive_groups;
+    $user->derive_regexp_groups;
 
     # Return HTTP response headers.
     print $cgi->header();
@@ -313,7 +313,7 @@ sub cancelChangeEmail {
             # issue
 
             my $user = new Bugzilla::User($userid);
-            $user->derive_groups;
+            $user->derive_regexp_groups;
 
             $vars->{'message'} = "email_change_cancelled_reinstated";
         } 

userprefs.cgi

@@ -311,11 +311,9 @@ sub DoPermissions {
     my (@has_bits, @set_bits);
     
     SendSQL("SELECT DISTINCT name, description FROM groups " .
-            "INNER JOIN user_group_map " .
-            "ON user_group_map.group_id = groups.id " .
-            "WHERE user_id = $::userid " .
-            "AND isbless = 0 " .
-            "ORDER BY name");
+            "WHERE id IN (" . 
+            Bugzilla->user->groups_as_string .
+            ") ORDER BY name");
     while (MoreSQLData()) {
         my ($nam, $desc) = FetchSQLData();
         push(@has_bits, {"desc" => $desc, "name" => $nam});

whine.pl

@@ -240,8 +240,7 @@ sub get_next_event {
         return undef unless $fetched;
         my ($eventid, $owner_id, $subject, $body) = @{$fetched};
 
-        my $owner = Bugzilla::User->new($owner_id,
-                                        DERIVE_GROUPS_TABLES_ALREADY_LOCKED);
+        my $owner = Bugzilla::User->new($owner_id);
 
         my $whineatothers = $owner->in_group('bz_canusewhineatothers');
 
@@ -275,10 +274,13 @@ sub get_next_event {
                     my $group_id = Bugzilla::Group::ValidateGroupName(
                         $groupname, $owner);
                     if ($group_id) {
+                        my $glist = join(',',
+                            Bugzilla::User->flatten_group_membership(
+                            $group_id));
                         $sth = $dbh->prepare("SELECT user_id FROM " .
                                              "user_group_map " .
-                                             "WHERE group_id=?");
-                        $sth->execute($group_id);
+                                             "WHERE group_id IN ($glist)");
+                        $sth->execute();
                         for my $row (@{$sth->fetchall_arrayref}) {
                             if (not defined $user_objects{$row->[0]}) {
                                 $user_objects{$row->[0]} =