[Rt-commit] rt branch, 4.4/optimize-cgm-table, created. rt-4.2.0-39-g2e24527

Alex Vandiver alexmv at bestpractical.com
Fri Oct 11 18:12:34 EDT 2013


The branch, 4.4/optimize-cgm-table has been created
        at  2e2452761ac8277bf28a1d91cf0a67dc205d3811 (commit)

- Log -----------------------------------------------------------------
commit e3d942c361d8080e184508d01f3b6438a7383290
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sun Mar 27 19:15:35 2011 +0400

    drop all CGM management from GroupMember class

diff --git a/lib/RT/GroupMember.pm b/lib/RT/GroupMember.pm
index 93e65a8..02facbb 100644
--- a/lib/RT/GroupMember.pm
+++ b/lib/RT/GroupMember.pm
@@ -95,58 +95,6 @@ Both Group and Member are expected to be RT::Principal objects
 
 =cut
 
-sub _InsertCGM {
-    my $self = shift;
-
-    my $cached_member = RT::CachedGroupMember->new( $self->CurrentUser );
-    my $cached_id     = $cached_member->Create(
-        Member          => $self->MemberObj,
-        Group           => $self->GroupObj,
-        ImmediateParent => $self->GroupObj,
-        Via             => '0'
-    );
-
-
-    #When adding a member to a group, we need to go back
-    #and popuplate the CachedGroupMembers of all the groups that group is part of .
-
-    my $cgm = RT::CachedGroupMembers->new( $self->CurrentUser );
-
-    # find things which have the current group as a member. 
-    # $group is an RT::Principal for the group.
-    $cgm->LimitToGroupsWithMember( $self->GroupId );
-    $cgm->Limit(
-        SUBCLAUSE => 'filter', # dont't mess up with prev condition
-        FIELD => 'MemberId',
-        OPERATOR => '!=',
-        VALUE => 'main.GroupId',
-        QUOTEVALUE => 0,
-        ENTRYAGGREGATOR => 'AND',
-    );
-
-    while ( my $parent_member = $cgm->Next ) {
-        my $parent_id = $parent_member->MemberId;
-        my $via       = $parent_member->Id;
-        my $group_id  = $parent_member->GroupId;
-
-        my $other_cached_member =
-            RT::CachedGroupMember->new( $self->CurrentUser );
-        my $other_cached_id = $other_cached_member->Create(
-            Member          => $self->MemberObj,
-                      Group => $parent_member->GroupObj,
-            ImmediateParent => $parent_member->MemberObj,
-            Via             => $parent_member->Id
-        );
-        unless ($other_cached_id) {
-            $RT::Logger->err( "Couldn't add " . $self->MemberId
-                  . " as a submember of a supergroup" );
-            return;
-        }
-    }
-
-    return $cached_id;
-}
-
 sub Create {
     my $self = shift;
     my %args = (
@@ -176,6 +124,8 @@ sub Create {
         return (undef);
     }
 
+    my $gid = $args{'Group'}->id;
+    my $mid = $args{'Member'}->id;
 
     #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
     # TODO what about the groups key cache?
@@ -195,7 +145,7 @@ sub Create {
             $RT::Handle->Rollback() unless ($args{'InsideTransaction'});
             return(undef);
         }
-        elsif ( $args{'Member'}->Id == $args{'Group'}->Id) {
+        elsif ( $mid == $gid) {
             $RT::Logger->debug("Can't add a group to itself");
             $RT::Handle->Rollback() unless ($args{'InsideTransaction'});
             return(undef);
@@ -204,8 +154,8 @@ sub Create {
 
 
     my $id = $self->SUPER::Create(
-        GroupId  => $args{'Group'}->Id,
-        MemberId => $args{'Member'}->Id
+        GroupId  => $gid,
+        MemberId => $mid
     );
 
     unless ($id) {
@@ -213,10 +163,11 @@ sub Create {
         return (undef);
     }
 
-    my $clone = RT::GroupMember->new( $self->CurrentUser );
-    $clone->Load( $id );
-    my $cached_id = $clone->_InsertCGM;
-
+    my $cached_member = RT::CachedGroupMember->new( $self->CurrentUser );
+    my $cached_id     = $cached_member->Create(
+        Group           => $args{'Group'},
+        Member          => $args{'Member'},
+    );
     unless ($cached_id) {
         $RT::Handle->Rollback() unless ($args{'InsideTransaction'});
         return (undef);
@@ -299,59 +250,34 @@ Expects to be called _outside_ a transaction
 sub Delete {
     my $self = shift;
 
-
     $RT::Handle->BeginTransaction();
 
-    # Find all occurrences of this member as a member of this group
-    # in the cache and nuke them, recursively.
-
-    # The following code will delete all Cached Group members
-    # where this member's group is _not_ the primary group 
-    # (Ie if we're deleting C as a member of B, and B happens to be 
-    # a member of A, will delete C as a member of A without touching
-    # C as a member of B
-
-    my $cached_submembers = RT::CachedGroupMembers->new( $self->CurrentUser );
-
-    $cached_submembers->Limit(
-        FIELD    => 'MemberId',
-        OPERATOR => '=',
-        VALUE    => $self->MemberObj->Id
-    );
-
-    $cached_submembers->Limit(
-        FIELD    => 'ImmediateParentId',
-        OPERATOR => '=',
-        VALUE    => $self->GroupObj->Id
-    );
-
-
-
-
-
-    while ( my $item_to_del = $cached_submembers->Next() ) {
-        my $del_err = $item_to_del->Delete();
-        unless ($del_err) {
-            $RT::Handle->Rollback();
-            $RT::Logger->warning("Couldn't delete cached group submember ".$item_to_del->Id);
-            return (undef);
-        }
-    }
-
     my ($err, $msg) = $self->SUPER::Delete();
     unless ($err) {
-            $RT::Logger->warning("Couldn't delete cached group submember ".$self->Id);
+        $RT::Logger->error("Couldn't delete cached group submember ".$self->Id);
         $RT::Handle->Rollback();
         return (undef);
     }
 
+    my $cgm = RT::CachedGroupMember->new( $self->CurrentUser );
+    $cgm->LoadByCols( GroupId => $self->GroupId, MemberId => $self->MemberId, );
+    if ( $cgm->id ) {
+        my $status = $cgm->Delete;
+        unless ($status) {
+            $RT::Logger->error("Couldn't delete cached group member");
+            $RT::Handle->Rollback;
+            return (undef);
+        }
+    } else {
+        $RT::Logger->warning("There was no CGM record matching GM record");
+    }
+
     #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
     # TODO what about the groups key cache?
     RT::Principal->InvalidateACLCache();
 
     $RT::Handle->Commit();
     return ($err);
-
 }
 
 
@@ -525,12 +451,6 @@ sub PreInflate {
     return 1;
 }
 
-sub PostInflate {
-    my $self = shift;
-
-    $self->_InsertCGM;
-}
-
 RT::Base->_ImportOverlays();
 
 1;

commit b6c05005930dc8916dd99855c0636b17dede24f6
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sun Mar 27 19:18:24 2011 +0400

    insert all CGM records in one query + update
    
    deal with users and groups differently
    
    Users have no (U, U) records like groups, but at the same
    time they can not have descandants. Query becomes special
    because of the absent record, but less complex as it's a
    leaf.

diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index 04cdb22..53d9138 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -96,12 +96,11 @@ Create takes a hash of values and creates a row in the database:
 
 sub Create {
     my $self = shift;
-    my %args = ( Group           => '',
-                 Member          => '',
-                 ImmediateParent => '',
-                 Via             => '0',
-                 Disabled        => '0',
-                 @_ );
+    my %args = (
+        Group           => undef,
+        Member          => undef,
+        @_
+    );
 
     unless (    $args{'Member'}
              && UNIVERSAL::isa( $args{'Member'}, 'RT::Principal' )
@@ -115,71 +114,93 @@ sub Create {
         $RT::Logger->debug("$self->Create: bogus Group argument");
     }
 
-    unless (    $args{'ImmediateParent'}
-             && UNIVERSAL::isa( $args{'ImmediateParent'}, 'RT::Principal' )
-             && $args{'ImmediateParent'}->Id ) {
-        $RT::Logger->debug("$self->Create: bogus ImmediateParent argument");
-    }
-
-    # If the parent group for this group member is disabled, it's disabled too, along with all its children
-    if ( $args{'ImmediateParent'}->Disabled ) {
-        $args{'Disabled'} = $args{'ImmediateParent'}->Disabled;
-    }
+    $args{'Disabled'} = $args{'Group'}->Disabled? 1 : 0;
 
-    my $id = $self->SUPER::Create(
-                              GroupId           => $args{'Group'}->Id,
-                              MemberId          => $args{'Member'}->Id,
-                              ImmediateParentId => $args{'ImmediateParent'}->Id,
-                              Disabled          => $args{'Disabled'},
-                              Via               => $args{'Via'}, );
+    $self->LoadByCols(
+        GroupId           => $args{'Group'}->Id,
+        MemberId          => $args{'Member'}->Id,
+    );
 
-    unless ($id) {
-        $RT::Logger->warning( "Couldn't create "
-                           . $args{'Member'}
-                           . " as a cached member of "
-                           . $args{'Group'}->Id . " via "
-                           . $args{'Via'} );
-        return (undef);  #this will percolate up and bail out of the transaction
-    }
-    if ( $self->__Value('Via') == 0 ) {
-        my ( $vid, $vmsg ) = $self->__Set( Field => 'Via', Value => $id );
-        unless ($vid) {
-            $RT::Logger->warning( "Due to a via error, couldn't create "
-                               . $args{'Member'}
-                               . " as a cached member of "
-                               . $args{'Group'}->Id . " via "
-                               . $args{'Via'} );
-            return (undef)
-              ;          #this will percolate up and bail out of the transaction
+    my $id;
+    if ( $id = $self->id ) {
+        if ( $self->Disabled != $args{'Disabled'} && $args{'Disabled'} == 0 ) {
+            my ($status) = $self->SetDisabled( 0 );
+            return undef unless $status;
         }
+        return $id;
     }
 
+    ($id) = $self->SUPER::Create(
+        GroupId           => $args{'Group'}->Id,
+        MemberId          => $args{'Member'}->Id,
+        Disabled          => $args{'Disabled'},
+    );
+    unless ($id) {
+        $RT::Logger->warning(
+            "Couldn't create ". $args{'Member'} ." as a cached member of "
+            . $args{'Group'} ." via ". $args{'Via'}
+        );
+        return (undef);
+    }
     return $id if $args{'Member'}->id == $args{'Group'}->id;
 
-    if ( $args{'Member'}->IsGroup() ) {
-        my $GroupMembers = $args{'Member'}->Object->MembersObj();
-        while ( my $member = $GroupMembers->Next() ) {
-            my $cached_member =
-              RT::CachedGroupMember->new( $self->CurrentUser );
-            my $c_id = $cached_member->Create(
-                                             Group  => $args{'Group'},
-                                             Member => $member->MemberObj,
-                                             ImmediateParent => $args{'Member'},
-                                             Disabled => $args{'Disabled'},
-                                             Via      => $id );
-            unless ($c_id) {
-                return (undef);    #percolate the error upwards.
-                     # the caller will log an error and abort the transaction
-            }
-
-        }
+    my $table = $self->Table;
+    if ( !$args{'Disabled'} && $args{'Member'}->IsGroup ) {
+        # update existing records, in case we activated some paths
+        my $query = "
+            SELECT CGM3.id FROM
+                $table CGM1 CROSS JOIN $table CGM2
+                JOIN $table CGM3
+                    ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+            WHERE
+                CGM1.MemberId = ? AND (CGM1.GroupId != CGM1.MemberId OR CGM1.MemberId = ?)
+                AND CGM2.GroupId = ? AND (CGM2.GroupId != CGM2.MemberId OR CGM2.GroupId = ?)
+                AND CGM1.Disabled = 0 AND CGM2.Disabled = 0 AND CGM3.Disabled > 0
+        ";
+        $RT::Handle->SimpleUpdateFromSelect(
+            $table, { Disabled => 0 }, $query,
+            $args{'Group'}->id, $args{'Group'}->id,
+            $args{'Member'}->id, $args{'Member'}->id
+        ) or return undef;
     }
 
-    return ($id);
+    my @binds;
 
-}
+    my $disabled_clause;
+    if ( $args{'Disabled'} ) {
+        $disabled_clause = '?';
+        push @binds, $args{'Disabled'};
+    } else {
+        $disabled_clause = 'CASE WHEN CGM1.Disabled + CGM2.Disabled > 0 THEN 1 ELSE 0 END';
+    }
 
+    my $query = "SELECT CGM1.GroupId, CGM2.MemberId, $disabled_clause FROM
+        $table CGM1 CROSS JOIN $table CGM2
+        LEFT JOIN $table CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+        WHERE
+            CGM1.MemberId = ? AND (CGM1.GroupId != CGM1.MemberId OR CGM1.MemberId = ?)
+            AND CGM3.id IS NULL
+    ";
+    push @binds, $args{'Group'}->id, $args{'Group'}->id;
+
+    if ( $args{'Member'}->IsGroup ) {
+        $query .= "
+            AND CGM2.GroupId = ?
+            AND (CGM2.GroupId != CGM2.MemberId OR CGM2.GroupId = ?)
+        ";
+        push @binds, $args{'Member'}->id, $args{'Member'}->id;
+    }
+    else {
+        $query .= " AND CGM2.id = ?";
+        push @binds, $id;
+    }
+    $RT::Handle->InsertFromSelect(
+        $table, ['GroupId', 'MemberId', 'Disabled'], $query, @binds,
+    );
 
+    return $id;
+}
 
 =head2 Delete
 

commit 3083b53b44d217df482fbec3cbe95393e6499252
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sun Mar 27 19:22:10 2011 +0400

    CGM->Delete method
    
    separate groups and users
    
    insert distinct set of records, new records we insert may
    have alternative paths, so we have to insert something
    consistent in Disabled column and update it later

diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index 53d9138..887cb59 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -213,36 +213,144 @@ mysql supported foreign keys with cascading deletes.
 sub Delete {
     my $self = shift;
 
-    
-    my $member = $self->MemberObj();
-    if ( $member->IsGroup ) {
-        my $deletable = RT::CachedGroupMembers->new( $self->CurrentUser );
+    if ( $self->MemberId == $self->GroupId ) {
+        # deleting self-referenced means that we're deleting a principal
+        # itself and all records where it's a parent or member should
+        # be deleted beforehead
+        return $self->SUPER::Delete( @_ );
+    }
 
-        $deletable->Limit( FIELD    => 'id',
-                           OPERATOR => '!=',
-                           VALUE    => $self->id );
-        $deletable->Limit( FIELD    => 'Via',
-                           OPERATOR => '=',
-                           VALUE    => $self->id );
+    my $table = $self->Table;
 
-        while ( my $kid = $deletable->Next ) {
-            my $kid_err = $kid->Delete();
-            unless ($kid_err) {
-                $RT::Logger->error(
-                              "Couldn't delete CachedGroupMember " . $kid->Id );
-                return (undef);
-            }
-        }
+    my $member_is_group = $self->MemberObj->IsGroup;
+
+    my $query;
+    if ( $member_is_group ) {
+        $query = "
+            SELECT CGM1.id FROM
+                CachedGroupMembers CGM1
+                JOIN CachedGroupMembers CGMA ON CGMA.MemberId = ?
+                JOIN CachedGroupMembers CGMD ON CGMD.GroupId = ?
+                LEFT JOIN GroupMembers GM1
+                    ON GM1.GroupId = CGM1.GroupId AND GM1.MemberId = CGM1.MemberId
+            WHERE
+                CGM1.GroupId = CGMA.GroupId AND CGM1.MemberId = CGMD.MemberId
+                AND CGM1.GroupId != CGM1.MemberId
+                AND GM1.id IS NULL
+        ";
     }
-    my $ret = $self->SUPER::Delete();
-    unless ($ret) {
-        $RT::Logger->error( "Couldn't delete CachedGroupMember " . $self->Id );
-        return (undef);
+    else {
+        $query = "
+            SELECT CGM1.id FROM
+                CachedGroupMembers CGM1
+                JOIN CachedGroupMembers CGMA ON CGMA.MemberId = ?
+                LEFT JOIN GroupMembers GM1
+                    ON GM1.GroupId = CGM1.GroupId AND GM1.MemberId = CGM1.MemberId
+            WHERE
+                CGM1.GroupId = CGMA.GroupId
+                AND CGM1.MemberId = ?
+                AND GM1.id IS NULL
+        ";
     }
-    return $ret;
-}
 
+    my $res = $RT::Handle->DeleteFromSelect(
+        $table, $query,
+        $self->GroupId, $self->MemberId,
+    );
+    return $res unless $res;
 
+    my @binds;
+    if ( $member_is_group ) {
+        $query =
+            "SELECT DISTINCT CGM1.GroupId, CGM2.MemberId, 1
+            FROM $table CGM1 CROSS JOIN $table CGM2
+            JOIN $table CGM3 ON CGM3.GroupId != CGM3.MemberId AND CGM3.GroupId = CGM1.GroupId
+            JOIN $table CGM4 ON CGM4.GroupId != CGM4.MemberId AND CGM4.MemberId = CGM2.MemberId
+                AND CGM3.MemberId = CGM4.GroupId
+            LEFT JOIN $table CGM5
+                ON CGM5.GroupId = CGM1.GroupId AND CGM5.MemberId = CGM2.MemberId
+            WHERE
+                CGM1.MemberId = ?
+                AND CGM2.GroupId = ?
+                AND CGM5.id IS NULL
+        ";
+        @binds = ($self->GroupId, $self->MemberId);
+
+    } else {
+        $query =
+            "SELECT DISTINCT CGM1.GroupId, ?, 1
+            FROM $table CGM1
+            JOIN $table CGM3 ON CGM3.GroupId != CGM3.MemberId AND CGM3.GroupId = CGM1.GroupId
+            JOIN $table CGM4 ON CGM4.GroupId != CGM4.MemberId AND CGM4.MemberId = ?
+                AND CGM3.MemberId = CGM4.GroupId
+            LEFT JOIN $table CGM5
+                ON CGM5.GroupId = CGM1.GroupId AND CGM5.MemberId = ?
+            WHERE
+                CGM1.MemberId = ?
+                AND CGM5.id IS NULL
+        ";
+        @binds = (
+            ($self->MemberId)x3,
+            $self->GroupId,
+        );
+    }
+
+    $res = $RT::Handle->InsertFromSelect(
+        $table, ['GroupId', 'MemberId', 'Disabled'], $query, @binds
+    );
+    return $res unless $res;
+
+    if ( $res > 0 && $member_is_group ) {
+        $query =
+            "SELECT main.id
+            FROM $table main
+            JOIN $table CGMA ON CGMA.MemberId = ?
+            JOIN $table CGMD ON CGMD.GroupId = ?
+
+            JOIN $table CGM3 ON CGM3.GroupId != CGM3.MemberId
+                AND CGM3.GroupId = main.GroupId
+                AND CGM3.Disabled = 0
+            JOIN $table CGM4 ON CGM4.GroupId != CGM4.MemberId
+                AND CGM4.MemberId = main.MemberId
+                AND CGM4.Disabled = 0
+                AND CGM3.MemberId = CGM4.GroupId
+            WHERE
+                main.GroupId = CGMA.GroupId
+                AND main.MemberId = CGMD.MemberId
+                AND main.Disabled = 1
+        ";
+    }
+    elsif ( $res > 0 ) {
+        $query =
+            "SELECT main.id
+            FROM $table main
+            JOIN $table CGMA ON CGMA.MemberId = ?
+
+            JOIN $table CGM3 ON CGM3.GroupId != CGM3.MemberId
+                AND CGM3.GroupId = main.GroupId
+                AND CGM3.Disabled = 0
+            JOIN $table CGM4 ON CGM4.GroupId != CGM4.MemberId
+                AND CGM4.MemberId = main.MemberId
+                AND CGM4.Disabled = 0
+                AND CGM3.MemberId = CGM4.GroupId
+            WHERE
+                main.GroupId = CGMA.GroupId
+                AND main.MemberId = ?
+                AND main.Disabled = 1
+        ";
+    }
+
+    $res = $RT::Handle->SimpleUpdateFromSelect(
+        $table, { Disabled => 0 }, $query,
+        $self->GroupId,
+        $self->MemberId,
+    ) if $res > 0;
+    return $res unless $res;
+
+    if ( my $m = $self->can('_FlushKeyCache') ) { $m->($self) };
+
+    return 1;
+}
 
 =head2 SetDisabled
 

commit 6e3df1e720c6698d028e54c4eebaa907db1710ad
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Mar 29 01:41:40 2011 +0400

    change CGM schema
    
    * Group and Member can not be NULLs in CGM
    * drop Via and ImmediateParentId columns
    * (GroupId, MemberId) pair in CGM becomes unique

diff --git a/etc/schema.Oracle b/etc/schema.Oracle
index effefc5..4804e8e 100755
--- a/etc/schema.Oracle
+++ b/etc/schema.Oracle
@@ -198,15 +198,12 @@ CREATE SEQUENCE CachedGroupMembers_seq;
 CREATE TABLE CachedGroupMembers (
         id              NUMBER(11,0) 
                 CONSTRAINT CachedGroupMembers_Key PRIMARY KEY,
-        GroupId         NUMBER(11,0),
-        MemberId        NUMBER(11,0),
-        Via             NUMBER(11,0),
-        ImmediateParentId       NUMBER(11,0),
+        GroupId         NUMBER(11,0) NOT NULL,
+        MemberId        NUMBER(11,0) NOT NULL,
         Disabled        NUMBER(11,0) DEFAULT 0 NOT NULL
 );
-CREATE INDEX DisGrouMem ON CachedGroupMembers (GroupId, MemberId, Disabled);
-CREATE INDEX CachedGroupMembers2 ON CachedGroupMembers (MemberId, GroupId, Disabled);
-CREATE INDEX CachedGroupMembers3 on CachedGroupMembers (MemberId, ImmediateParentId);
+CREATE UNIQUE INDEX CGM1 ON CachedGroupMembers (GroupId, MemberId, Disabled);
+CREATE UNIQUE INDEX CGM2 ON CachedGroupMembers (MemberId, GroupId, Disabled);
 
 
 CREATE SEQUENCE USERS_seq;
diff --git a/etc/schema.Pg b/etc/schema.Pg
index e5e2a04..2d50bb8 100755
--- a/etc/schema.Pg
+++ b/etc/schema.Pg
@@ -326,22 +326,15 @@ CREATE SEQUENCE cachedgroupmembers_id_seq;
 
 CREATE TABLE CachedGroupMembers (
         id int DEFAULT nextval('cachedgroupmembers_id_seq'),
-        GroupId int, 
-        MemberId int, 
-        Via int, 
-        ImmediateParentId int, 
+        GroupId int NOT NULL,
+        MemberId int NOT NULL,
         Disabled integer NOT NULL DEFAULT 0 , 
         PRIMARY KEY (id)
 
 );
 
-CREATE INDEX CachedGroupMembers2 on CachedGroupMembers (MemberId, GroupId, Disabled);
-CREATE INDEX DisGrouMem  on CachedGroupMembers (GroupId,MemberId,Disabled);
-CREATE INDEX CachedGroupMembers3  on CachedGroupMembers (MemberId,ImmediateParentId);
-
-
-
-
+CREATE UNIQUE INDEX CGM1 ON CachedGroupMembers (GroupId,MemberId);
+CREATE INDEX CGM2 ON CachedGroupMembers (MemberId);
 
 
 
diff --git a/etc/schema.SQLite b/etc/schema.SQLite
index c50e5b1..1feebe2 100755
--- a/etc/schema.SQLite
+++ b/etc/schema.SQLite
@@ -216,22 +216,18 @@ CREATE UNIQUE INDEX GroupMembers1 ON GroupMembers(GroupId, MemberId);
 
 create table CachedGroupMembers (
         id integer primary key ,
-        GroupId int, 
-        MemberId int, 
-        Via int, 
-        ImmediateParentId int,
+        GroupId int NOT NULL,
+        MemberId int NOT NULL,
         Disabled int2 NOT NULL DEFAULT 0  # if this cached group member is a member of this group by way of a disabled
                                            # group or this group is disabled, this will be set to 1
                                            # this allows us to not find members of disabled subgroups when listing off
                                            # group members recursively.
                                            # Also, this allows us to have the ACL system elide members of disabled groups
 
-        
 ) ;
 
-CREATE INDEX CachedGroupMembers1 ON CachedGroupMembers (GroupId, MemberId, Disabled);
-CREATE INDEX CachedGroupMembers2 ON CachedGroupMembers (MemberId, GroupId, Disabled);
-CREATE INDEX CachedGroupMembers3 ON CachedGroupMembers (MemberId, ImmediateParentId);
+CREATE UNIQUE INDEX CGM1 ON CachedGroupMembers (GroupId,MemberId,Disabled);
+CREATE UNIQUE INDEX CGM2 ON CachedGroupMembers (MemberId,GroupId,Disabled);
 
 --- }}}
 
diff --git a/etc/schema.mysql b/etc/schema.mysql
index 3669fb3..0fe7465 100755
--- a/etc/schema.mysql
+++ b/etc/schema.mysql
@@ -200,12 +200,8 @@ CREATE UNIQUE INDEX GroupMembers1 on GroupMembers (GroupId, MemberId);
 
 create table CachedGroupMembers (
         id int auto_increment,
-        GroupId int, # foreign key to Principals
-        MemberId int, # foreign key to Principals
-        Via int, #foreign key to CachedGroupMembers. (may point to $self->id)
-        ImmediateParentId int, #foreign key to prinicpals.
-                               # this points to the group that the member is
-                               # a member of, for ease of deletes.
+        GroupId int NOT NULL, # foreign key to Principals
+        MemberId int NOT NULL, # foreign key to Principals
         Disabled int2 NOT NULL DEFAULT 0 , # if this cached group member is a member of this group by way of a disabled
                                            # group or this group is disabled, this will be set to 1
                                            # this allows us to not find members of disabled subgroups when listing off
@@ -214,10 +210,10 @@ create table CachedGroupMembers (
         PRIMARY KEY (id)
 ) ENGINE=InnoDB CHARACTER SET utf8;
 
-CREATE INDEX DisGrouMem  on CachedGroupMembers (GroupId,MemberId,Disabled);
-CREATE INDEX CachedGroupMembers2 on CachedGroupMembers (MemberId, GroupId, Disabled);
-CREATE INDEX CachedGroupMembers3 on CachedGroupMembers (MemberId, ImmediateParentId);
-
+# we can create UNIQUE ON (GroupId,MemberId), but it only a few bytes shorter than
+# the following and there is no need to do that
+CREATE UNIQUE INDEX CGM1 ON CachedGroupMembers (GroupId,MemberId,Disabled);
+CREATE UNIQUE INDEX CGM2 ON CachedGroupMembers (MemberId,GroupId,Disabled);
 
 
 CREATE TABLE Users (

commit 9a361aed2024b65ec03ef26eeb0c75d0150d0894
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Mar 29 04:41:09 2011 +0400

    documentation for CGM table management and SQL

diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index 887cb59..bde7316 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -542,6 +542,369 @@ Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
 =cut
 
 
+=head1 FOR DEVELOPERS
+
+=head2 New structure without Via and ImmediateParent
+
+We have id, GroupId, MemberId, Disabled. In this schema
+we have unique index on GroupId and MemberId that will
+improve selects.
+
+Disabled column is complex as it's reflects all possible
+paths between group and member. If at least one active path
+exists then the record is active.
+
+When a GM record is added we do only two queries: insert
+new CGM records and update Disabled on old paths.
+
+When a GM record is deleted we update CGM in two steps:
+delete all potential candidates and re-insert them. We
+do this within one transaction.
+
+=head2 SQL behind maintaining CGM table
+
+=head3 Terminology
+
+=over 4
+
+=item * An(E) - all ancestors of E including E itself
+
+=item * De(E) - all descendants of E including E itself
+
+=back
+
+=head3 Adding a (G -> M) record
+
+When a new (G -> M) record added we should connect all An(G)
+to all De(M). The following select fetches all new records:
+
+    SELECT CGM1.GroupId, CGM2.MemberId FROM
+        CachedGroupMembers CGM1
+        CROSS JOIN CachedGroupMembers CGM2
+    WHERE
+        CGM1.MemberId = G
+        AND CGM2.GroupId = M
+    ;
+
+It handles G and M itself as we always have (E->E) records
+for groups.
+
+Some of this records may exist in the table, so we should skip existing:
+
+    SELECT CGM1.GroupId, CGM2.MemberId FROM
+        CachedGroupMembers CGM1
+        CROSS JOIN CachedGroupMembers CGM2
+        LEFT JOIN CachedGroupMembers CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+    WHERE
+        CGM1.MemberId = G
+        AND CGM2.GroupId = M
+        AND CGM3.id IS NULL
+    ;
+
+In order to do less checks we should skip (E->E) records, but not those
+that touch our G and M:
+
+    SELECT CGM1.GroupId, CGM2.MemberId FROM
+        CachedGroupMembers CGM1
+        CROSS JOIN CachedGroupMembers CGM2
+        LEFT JOIN CachedGroupMembers CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+    WHERE
+        CGM1.MemberId = G AND (CGM1.GroupId != CGM1.MemberId OR CGM1.MemberId = G)
+        AND CGM2.GroupId = M AND (CGM2.GroupId != CGM2.MemberId OR CGM2.GroupId = M)
+        AND CGM3.id IS NULL
+    ;
+
+=head4 Disabled column on insert
+
+We should handle properly Disabled column.
+
+If the GM record we're adding is disabled then all new paths we add as well
+disabled and existing one are not affected.
+
+Otherwise activity of new paths depends on entries that got connected and existing
+paths have to be updated.
+
+New paths:
+
+    SELECT CGM1.GroupId, CGM2.MemberId, IF(CGM1.Disabled+CGM2.Disabled > 0, 1, 0) FROM
+    ...
+
+Updating old paths, the following records should be activated:
+
+    SELECT CGM3.id FROM
+        CachedGroupMembers CGM1
+        CROSS JOIN CachedGroupMembers CGM2
+        JOIN CachedGroupMembers CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+    WHERE
+        CGM1.MemberId = G AND (CGM1.GroupId != CGM1.MemberId OR CGM1.MemberId = G)
+        AND CGM2.GroupId = M AND (CGM2.GroupId != CGM2.MemberId OR CGM2.GroupId = M)
+        AND CGM1.Disabled = 0 AND CGM2.Disabled = 0 AND CGM3.Disabled > 0
+    ;
+
+It's better to do this before we insert new records, so we scan less records
+to find things we need updating.
+
+=head3 mysql performance
+
+Sample results:
+
+    10k  - 0.4x seconds
+    100k - 4.x seconds
+    1M   - 4x.x seconds
+
+As long as innodb_buffer_pool_size is big enough to store insert buffer,
+and MIN(tmp_table_size, max_heap_table_size) allow us to store tmp table
+in the memory. For 100k records we need less than 15 MBytes. Disk I/O
+heavily degrades performance.
+
+=head2 Deleting a (G->M) record
+
+In case record is deleted from GM table we should re-evaluate records in CGM.
+
+Candidates for deletion are any records An(G) -> De(M):
+
+    SELECT CGM3.id FROM
+        CachedGroupMembers CGM1
+        CROSS JOIN CachedGroupMembers CGM2
+        JOIN CachedGroupMembers CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+    WHERE
+        CGM1.MemberId = G
+        AND CGM2.GroupId = M
+    ;
+
+Some of these records may still have alternative routes. A candidate (G', M')
+stays in the table if following records exist in GM and CGM tables.
+(G', X) in CGM, (X,Y) in GM and (Y,M') in CGM, where X ~ An(G) and Y !~ An(G).
+And here is SQL to select records that should be deleted:
+
+    SELECT CGM3.id FROM
+        CachedGroupMembers CGM1
+        CROSS JOIN CachedGroupMembers CGM2
+        JOIN CachedGroupMembers CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+
+    WHERE
+        CGM1.MemberId = G
+        AND CGM2.GroupId = M
+        AND NOT EXISTS (
+            SELECT CGM4.GroupId FROM
+                CachedGroupMembers CGM4
+                    ON CGM4.GroupId = CGM3.GroupId
+                JOIN GroupMembers GM1
+                    ON GM1.GroupId = CGM4.MemberId
+                JOIN GroupMembers CGM5
+                    ON CGM4.GroupId = GM1.MemberId
+                    AND CGM4.MemberId = CGM3.MemberId
+                JOIN CachedGroupMembers CGM6
+                    ON CGM6.GroupId = CGM4.MemberId
+                    AND CGM6.MemberId = G
+                LEFT JOIN CachedGroupMembers CGM7
+                    ON CGM7.GroupId = CGM5.GroupId
+                    AND CGM7.MemberId = G
+            WHERE
+                CGM7.id IS NULL
+        )
+    ;
+
+Fun.
+
+=head3 mysql performance
+
+    10k  - 4.x seconds
+    100k - 13x seconds
+    1M   - not tested
+
+Sadly this query perform much worth comparing to the insert operation. Problem is
+in the select.
+
+=head3 Delete all candidates and re-insert missing (our method)
+
+We can delete all candidates (An(G)->De(M)) from CGM table that are not
+real GM records: then insert records once again.
+
+    SELECT CGM1.id FROM
+        CachedGroupMembers CGM1
+        JOIN CachedGroupMembers CGMA ON CGMA.MemberId = G
+        JOIN CachedGroupMembers CGMD ON CGMD.GroupId = M
+        LEFT JOIN GroupMembers GM1
+            ON GM1.GroupId = CGM1.GroupId AND GM1.MemberId = CGM1.MemberId
+    WHERE
+        CGM1.GroupId = CGMA.GroupId AND CGM1.MemberId = CGMD.MemberId
+        AND CGM1.GroupId != CGM1.MemberId
+        AND GM1.id IS NULL
+    ;
+
+Then we can re-insert data back with insert from select described above.
+
+=head4 Disabled column on delete
+
+We delete all (An(G)->De(M)) and then re-insert survivors, so no other
+records except inserted can gain or loose activity. See this is the same
+as how we deal with it during insert.
+
+=head4 mysql performance
+
+This solution is faster than previous variant, 4-5 times slower than
+create operation, behaves linear.
+
+=head3 Recursive delete
+
+Alternative solution.
+
+Again, some (An(G), De(M)) pairs should be deleted, but some may stay. If
+delete any pair from the set then An(G) and De(M) sets don't change, so
+we can delete things step by step. Run delete operation, if any was deleted
+then run it once again, do it until operation deletes no rows. We shouldn't
+delete records where:
+
+=over 4
+
+=item * GroupId == MemberId
+
+=item * exists matching GM
+
+=item * exists equivalent GM->CGM pair
+
+=item * exists equivalent CGM->GM pair
+
+=back
+
+Query with most conditions in one NOT EXISTS subquery:
+
+    SELECT CGM1.id FROM
+        CachedGroupMembers CGM1
+        JOIN CachedGroupMembers CGMA ON CGMA.MemberId = G
+        JOIN CachedGroupMembers CGMD ON CGMD.GroupId = M
+    WHERE
+        CGM1.GroupId = CGMA.GroupId AND CGM1.MemberId = CGMD.MemberId
+        AND CGM1.GroupId != CGM1.MemberId
+        AND NOT EXISTS (
+            SELECT * FROM
+                CachedGroupMembers CGML
+                CROSS JOIN GroupMembers GM
+                CROSS JOIN CachedGroupMembers CGMR
+            WHERE
+                CGML.GroupId = CGM1.GroupId
+                AND GM.GroupId = CGML.MemberId
+                AND CGMR.GroupId = GM.MemberId
+                AND CGMR.MemberId = CGM1.MemberId
+                AND (
+                    (CGML.GroupId = CGML.MemberId AND CGMR.GroupId != CGMR.MemberId)
+                    OR 
+                    (CGML.GroupId != CGML.MemberId AND CGMR.GroupId = CGMR.MemberId)
+                )
+        )
+    ;
+
+=head4 mysql performance
+
+It's better than first solution, but still it's not linear. Problem is that
+NOT EXISTS means that for every link that should be deleted we have to check too
+many conditions (too many rows to scan). Still delete + insert behave better and
+linear.
+
+=head3 Alternative ways
+
+Store additional info in a table, similar to Via and IP we had. Then we can
+do iterative delete like in the last solution. However, this will slowdown
+insert, probably not that much as I suspect we would be able to push new data
+in one query.
+
+=head2 Disabling a (G->G) record
+
+We're interested only in (G->G) records as CGM path is disabled if group
+is disabled. Disabled users don't affect CGM records.
+
+When (G->G) gets Disabled, 1) (G->De(G)) gets Disabled 2) all active
+(An(G)->De(G)) get disabled unless record has an alternative active path.
+
+First can be done without much problem:
+
+    UPDATE CGM SET Disabled => 1 WHERE GroupId = G;
+
+Second part is harder. Finding an alternative path is harder and similar to
+performing delete in one query.
+
+Instead we disable all candidates and then re-enable required. Selecting
+candidates is simple:
+
+    SELECT main.id FROM CachedGroupMembers main
+        JOIN CachedGroupMembers CGM1 ON main.GroupId = CGM1.GroupId AND CGM1.MemberId = G
+        JOIN CachedGroupMembers CGM2 ON main.MemberId = CGM2.MemberId AND CGM2.GroupId = G
+    WHERE main.Disabled = 0;
+
+We can narrow it down. If (G'->G) is disabled where G'~An(G) then activity
+of (G'->M') where M'~De(G) isn't affected by activity of (G->G):
+
+    SELECT main.id FROM CachedGroupMembers main
+        JOIN CachedGroupMembers CGM1 ON main.GroupId = CGM1.GroupId AND CGM1.MemberId = G
+            AND CGM1.Disabled = 0
+        JOIN CachedGroupMembers CGM2 ON main.MemberId = CGM2.MemberId AND CGM2.GroupId = G
+    WHERE main.Disabled = 0;
+
+Now we can re-enable disabled records which still have active alternative paths:
+
+    SELECT main.id FROM CachedGroupMembers main
+        JOIN CachedGroupMembers CGM1 ON main.GroupId = CGM1.GroupId AND CGM1.MemberId = G
+            AND CGM1.Disabled = 0
+        JOIN CachedGroupMembers CGM2 ON main.MemberId = CGM2.MemberId AND CGM2.GroupId = G
+
+        JOIN CachedGroupMembers CGM3 ON CGM3.Disabled = 0 AND main.GroupId = CGM3.GroupID
+        JOIN CachedGroupMembers CGM4 ON CGM4.Disabled = 0 AND main.MemberId = CGM4.MemberId
+            AND CGM4.GroupId = CGM3.MemberId
+
+    WHERE main.Disabled = 1;
+
+Enabling records is much easier, just update all candidates.
+
+=head2 INDEXING
+
+=head3 Access patterns
+
+We either have group and want members, have member and want groups or
+have both and check existance.
+
+Disabled column has low selectivity.
+
+=head3 Index access without table access
+
+Some databases can access index by prefix and use rest as data source, so
+multi column indexes improve performance.
+
+This works on L<mysql (see "using index")|http://dev.mysql.com/doc/refman/5.1/en/explain-output.html#explain-output-columns>
+and L<Oracle|http://docs.oracle.com/cd/A58617_01/server.804/a58246/access.htm#2174>.
+
+This doesn't work for Pg, but L<comes in 9.2|http://rhaas.blogspot.com/2011/10/fast-counting.html>.
+
+=head3 Indexes
+
+For Oracle, mysql and SQLite:
+
+    UNIQUE ON (GroupId, MemberId, Disabled)
+    UNIQUE ON (MemberId, GroupId, Disabled)
+
+For Pg:
+
+    UNIQUE ON (GroupId, MemberId)
+    (MemberId)
+
+=head2 What's next
+
+We don't create self-referencing records for users and it complicates
+a few code paths in this module. However, we have ACL equiv groups for
+every user and these groups have (G->G) records and (G->U) record. So
+we have one additional group per user and two CGM records.
+
+We can give user's id to ACL equiv group, so G.id = U.id. In this case
+we get (G, G) pair that is at the same time (U->U) and (G->U) pairs.
+It simplifies code in this module and CGM table smaller by one record
+per user.
+
+=cut
 
 sub _CoreAccessible {
     {

commit def21be77e124983e633ecb0c3ca6b8a731bad90
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed Mar 14 14:49:33 2012 +0400

    drop most mentions of ImmediateParent and Via

diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index bde7316..f39d921 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -82,14 +82,6 @@ Create takes a hash of values and creates a row in the database:
   'Member' is the RT::Principal  of the user or group we're adding to 
   the cache.
 
-  'ImmediateParent' is the RT::Principal of the group that this 
-  principal belongs to to get here
-
-  int(11) 'Via' is an internal reference to CachedGroupMembers->Id of
-  the "parent" record of this cached group member. It should be empty if 
-  this member is a "direct" member of this group. (In that case, it will 
-  be set to this cached group member's id after creation)
-
   This routine should _only_ be called by GroupMember->Create
 
 =cut
@@ -138,7 +130,7 @@ sub Create {
     unless ($id) {
         $RT::Logger->warning(
             "Couldn't create ". $args{'Member'} ." as a cached member of "
-            . $args{'Group'} ." via ". $args{'Via'}
+            . $args{'Group'}
         );
         return (undef);
     }
@@ -208,7 +200,7 @@ Deletes the current CachedGroupMember from the group it's in and cascades
 the delete to all submembers. This routine could be completely excised if
 mysql supported foreign keys with cascading deletes.
 
-=cut 
+=cut
 
 sub Delete {
     my $self = shift;
@@ -358,12 +350,12 @@ SetDisableds the current CachedGroupMember from the group it's in and cascades
 the SetDisabled to all submembers. This routine could be completely excised if
 mysql supported foreign keys with cascading SetDisableds.
 
-=cut 
+=cut
 
 sub SetDisabled {
     my $self = shift;
     my $val = shift;
- 
+
     # if it's already disabled, we're good.
     return (1) if ( $self->__Value('Disabled') == $val);
     my $err = $self->_Set(Field => 'Disabled', Value => $val);
@@ -372,7 +364,7 @@ sub SetDisabled {
         $RT::Logger->error( "Couldn't SetDisabled CachedGroupMember " . $self->Id .": $msg");
         return ($err);
     }
-    
+
     my $member = $self->MemberObj();
     if ( $member->IsGroup ) {
         my $deletable = RT::CachedGroupMembers->new( $self->CurrentUser );
@@ -408,22 +400,7 @@ sub GroupObj {
 
 
 
-=head2 ImmediateParentObj  
-
-Returns the RT::Principal object for this group ImmediateParent
-
-=cut
-
-sub ImmediateParentObj {
-    my $self      = shift;
-    my $principal = RT::Principal->new( $self->CurrentUser );
-    $principal->Load( $self->ImmediateParentId );
-    return ($principal);
-}
-
-
-
-=head2 MemberObj  
+=head2 MemberObj
 
 Returns the RT::Principal object for this group member
 
@@ -487,58 +464,17 @@ Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
 
 =cut
 
-
-=head2 Via
-
-Returns the current value of Via.
-(In the database, Via is stored as int(11).)
-
-
-
-=head2 SetVia VALUE
-
-
-Set Via to VALUE.
-Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
-(In the database, Via will be stored as a int(11).)
-
-
-=cut
-
-
-=head2 ImmediateParentId
-
-Returns the current value of ImmediateParentId.
-(In the database, ImmediateParentId is stored as int(11).)
-
-
-
-=head2 SetImmediateParentId VALUE
-
-
-Set ImmediateParentId to VALUE.
-Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
-(In the database, ImmediateParentId will be stored as a int(11).)
-
-
-=cut
-
-
 =head2 Disabled
 
 Returns the current value of Disabled.
 (In the database, Disabled is stored as smallint(6).)
 
-
-
 =head2 SetDisabled VALUE
 
-
 Set Disabled to VALUE.
 Returns (1, 'Status message') on success and (0, 'Error Message') on failure.
 (In the database, Disabled will be stored as a smallint(6).)
 
-
 =cut
 
 
@@ -908,21 +844,15 @@ per user.
 
 sub _CoreAccessible {
     {
-
         id =>
                 {read => 1, sql_type => 4, length => 11,  is_blob => 0,  is_numeric => 1,  type => 'int(11)', default => ''},
         GroupId =>
                 {read => 1, write => 1, sql_type => 4, length => 11,  is_blob => 0,  is_numeric => 1,  type => 'int(11)', default => ''},
         MemberId =>
                 {read => 1, write => 1, sql_type => 4, length => 11,  is_blob => 0,  is_numeric => 1,  type => 'int(11)', default => ''},
-        Via =>
-                {read => 1, write => 1, sql_type => 4, length => 11,  is_blob => 0,  is_numeric => 1,  type => 'int(11)', default => ''},
-        ImmediateParentId =>
-                {read => 1, write => 1, sql_type => 4, length => 11,  is_blob => 0,  is_numeric => 1,  type => 'int(11)', default => ''},
         Disabled =>
                 {read => 1, write => 1, sql_type => 5, length => 6,  is_blob => 0,  is_numeric => 1,  type => 'smallint(6)', default => '0'},
-
- }
+    }
 };
 
 sub Serialize {
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 2bec67b..e170d7c 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -434,7 +434,7 @@ sub _Create {
     # in the ordinary case, this would fail badly because it would recurse and add all the members of this group as 
     # cached members. thankfully, we're creating the group now...so it has no members.
     my $cgm = RT::CachedGroupMember->new($self->CurrentUser);
-    $cgm->Create(Group =>$self->PrincipalObj, Member => $self->PrincipalObj, ImmediateParent => $self->PrincipalObj);
+    $cgm->Create( Group => $self->PrincipalObj, Member => $self->PrincipalObj );
 
 
     if ( $args{'_RecordTransaction'} ) {
@@ -795,9 +795,14 @@ This routine finds all the cached group members that are members of this group
     # a member of A, will delete C as a member of A without touching
     # C as a member of B
 
-    my $cached_submembers = RT::CachedGroupMembers->new( $self->CurrentUser );
-
-    $cached_submembers->Limit( FIELD    => 'ImmediateParentId', OPERATOR => '=', VALUE    => $self->Id);
+    my $cgm = RT::CachedGroupMember->new( $self->CurrentUser );
+    $cgm->LoadByCols( MemberId => $self->id, GroupId => $self->id );
+    my ($status) = $cgm->SetDisabled($val);
+    unless ( $status ) {
+        $RT::Handle->Rollback;
+        $RT::Logger->warning("Couldn't disable cached group member #". $cgm->Id);
+        return (undef);
+    }
 
     #Clear the key cache. TODO someday we may want to just clear a little bit of the keycache space. 
     # TODO what about the groups key cache?
@@ -805,15 +810,6 @@ This routine finds all the cached group members that are members of this group
 
 
 
-    while ( my $item = $cached_submembers->Next() ) {
-        my $del_err = $item->SetDisabled($val);
-        unless ($del_err) {
-            $RT::Handle->Rollback();
-            $RT::Logger->warning("Couldn't disable cached group submember ".$item->Id);
-            return (undef);
-        }
-    }
-
     $self->_NewTransaction( Type => ($val == 1) ? "Disabled" : "Enabled" );
 
     $RT::Handle->Commit();
@@ -822,7 +818,6 @@ This routine finds all the cached group members that are members of this group
     } else {
         return (1, $self->loc("Group enabled"));
     }
-
 }
 
 
diff --git a/lib/RT/GroupMember.pm b/lib/RT/GroupMember.pm
index 02facbb..80a14e4 100644
--- a/lib/RT/GroupMember.pm
+++ b/lib/RT/GroupMember.pm
@@ -225,8 +225,6 @@ sub _StashUser {
     my $cached_id     = $cached_member->Create(
         Member          => $args{'Member'},
         Group           => $args{'Group'},
-        ImmediateParent => $args{'Group'},
-        Via             => '0'
     );
 
     unless ($cached_id) {

commit 01bce0fb14f29dbd0ba4adcb4a46461711d1ed82
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Mar 15 23:11:48 2012 +0400

    implement SetDisabled

diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index f39d921..9b3001d 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -355,32 +355,75 @@ mysql supported foreign keys with cascading SetDisableds.
 sub SetDisabled {
     my $self = shift;
     my $val = shift;
+    $val = $val ? 1 : 0;
 
     # if it's already disabled, we're good.
-    return (1) if ( $self->__Value('Disabled') == $val);
-    my $err = $self->_Set(Field => 'Disabled', Value => $val);
-    my ($retval, $msg) = $err->as_array();
-    unless ($retval) {
-        $RT::Logger->error( "Couldn't SetDisabled CachedGroupMember " . $self->Id .": $msg");
-        return ($err);
-    }
+    return (1) if $self->__Value('Disabled') == $val;
+
+    if ( $val ) {
+        unless ( $self->GroupId == $self->MemberId ) {
+            $RT::Logger->error("SetDisabled should only be applied to (G->G) records");
+            return undef;
+        }
 
-    my $member = $self->MemberObj();
-    if ( $member->IsGroup ) {
-        my $deletable = RT::CachedGroupMembers->new( $self->CurrentUser );
+        my $query = "SELECT main.id FROM CachedGroupMembers main
+            WHERE main.Disabled = 0 AND main.GroupId = ?";
+
+        $RT::Handle->SimpleUpdateFromSelect(
+            $self->Table, { Disabled => 1 }, $query,
+            $self->GroupId,
+        ) or return undef;
+
+        $query = "SELECT main.id FROM CachedGroupMembers main
+            JOIN CachedGroupMembers CGM1 ON main.GroupId = CGM1.GroupId
+                AND CGM1.MemberId = ?
+            JOIN CachedGroupMembers CGM2 ON main.MemberId = CGM2.MemberId
+                AND CGM2.GroupId = ? AND CGM2.GroupId != CGM2.MemberId
+
+            WHERE main.Disabled = 0
+                AND NOT EXISTS (
+                    SELECT CGM3.id
+                    FROM CachedGroupMembers CGM3, CachedGroupMembers CGM4
+                    WHERE CGM3.Disabled = 0 AND CGM4.Disabled = 0
+                        AND CGM3.GroupId = main.GroupId
+                        AND CGM3.MemberId = CGM4.GroupId
+                        AND CGM4.MemberId = main.MemberId
+                        AND CGM3.id != main.id
+                        AND CGM4.id != main.id
+                )
+        ";
 
-        $deletable->Limit( FIELD    => 'Via', OPERATOR => '=', VALUE    => $self->id );
-        $deletable->Limit( FIELD    => 'id', OPERATOR => '!=', VALUE    => $self->id );
 
-        while ( my $kid = $deletable->Next ) {
-            my $kid_err = $kid->SetDisabled($val );
-            unless ($kid_err) {
-                $RT::Logger->error( "Couldn't SetDisabled CachedGroupMember " . $kid->Id );
-                return ($kid_err);
-            }
+
+        $RT::Handle->SimpleUpdateFromSelect(
+            $self->Table, { Disabled => 1 }, $query,
+            ($self->GroupId)x2,
+        ) or return undef;
+    }
+    else {
+        my ($status, $msg) = $self->_Set(Field => 'Disabled', Value => $val);
+        unless ( $status ) {
+            $RT::Logger->error(
+                "Couldn't SetDisabled CachedGroupMember #" . $self->Id .": $msg"
+            );
+            return $status;
         }
+        REDO:
+        my $query = "SELECT main.id FROM CachedGroupMembers main
+            JOIN CachedGroupMembers CGM1 ON main.GroupId = CGM1.GroupId
+                AND CGM1.MemberId = ?
+            JOIN CachedGroupMembers CGM2 ON main.MemberId = CGM2.MemberId
+                AND CGM2.GroupId = ?
+            WHERE main.Disabled = 1";
+
+        my $res = $RT::Handle->SimpleUpdateFromSelect(
+            $self->Table, { Disabled => 0 }, $query,
+            $self->GroupId, $self->MemberId
+        ) or return undef;
+        goto REDO if $res > 0;
     }
-    return ($err);
+    if ( my $m = $self->can('_FlushKeyCache') ) { $m->($self) };
+    return (1);
 }
 
 

commit 5a6ebbb23e1d0047cb64a25e2ff50896faf6df85
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Mar 15 23:15:02 2012 +0400

    return id on success from Group->_AddMember

diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index e170d7c..ce05638 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1122,11 +1122,11 @@ sub _AddMember {
         }
     }
 
-    return (1, $self->loc("[_1] set to [_2]",
+    return ($id, $self->loc("[_1] set to [_2]",
                           $self->loc($self->Name), $new_member_obj->Object->Name) )
         if $self->SingleMemberRoleGroup;
 
-    return ( 1, $self->loc("Member added: [_1]", $new_member_obj->Object->Name) );
+    return ($id, $self->loc("Member added: [_1]", $new_member_obj->Object->Name) );
 }
 
 

commit 2e48ed276a193d97678e61681769a85183b8d547
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Mar 15 23:16:53 2012 +0400

    call Create in array context
    
    to make sure we always get first element and not last
    which is usually error message.
    
    yes, now some Create methods return scalar, but most of
    them return list and tomorrow we might want to changes
    that minority as well. Don't want to hunt for bugs after
    such change.

diff --git a/lib/RT/GroupMember.pm b/lib/RT/GroupMember.pm
index 80a14e4..aaa360e 100644
--- a/lib/RT/GroupMember.pm
+++ b/lib/RT/GroupMember.pm
@@ -153,18 +153,17 @@ sub Create {
     }
 
 
-    my $id = $self->SUPER::Create(
+    my ($id) = $self->SUPER::Create(
         GroupId  => $gid,
         MemberId => $mid
     );
-
     unless ($id) {
         $RT::Handle->Rollback() unless ($args{'InsideTransaction'});
         return (undef);
     }
 
     my $cached_member = RT::CachedGroupMember->new( $self->CurrentUser );
-    my $cached_id     = $cached_member->Create(
+    my ($cached_id)     = $cached_member->Create(
         Group           => $args{'Group'},
         Member          => $args{'Member'},
     );

commit 17c83f90be17083bbaf0ae658fa0f5703e5f255c
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sat Mar 17 00:14:37 2012 +0400

    replace group.t with group_members.t
    
    former was all about checking membership, new file
    checks much more and even does random rounds

diff --git a/t/api/group-rename.t b/t/api/group-rename.t
new file mode 100644
index 0000000..29564f1
--- /dev/null
+++ b/t/api/group-rename.t
@@ -0,0 +1,23 @@
+
+use strict;
+use warnings;
+use RT;
+use RT::Test nodata => 1, tests => undef;
+
+
+
+{
+    my $u = RT::Group->new(RT->SystemUser);
+    my ( $id, $msg ) = $u->CreateUserDefinedGroup( Name => 'TestGroup' );
+    ok( $u->id, 'Created TestGroup' );
+    ok( $u->SetName('testgroup'), 'rename to lower cased version: testgroup' );
+    ok( $u->SetName('TestGroup'), 'rename back' );
+
+    my $u2 = RT::Group->new( RT->SystemUser );
+    ( $id, $msg ) = $u2->CreateUserDefinedGroup( Name => 'TestGroup' );
+    ok( !$id, "can't create duplicated group: $msg" );
+    ( $id, $msg ) = $u2->CreateUserDefinedGroup( Name => 'testgroup' );
+    ok( !$id, "can't create duplicated group even case is different: $msg" );
+}
+
+done_testing;
diff --git a/t/api/group.t b/t/api/group.t
deleted file mode 100644
index 9ba3257..0000000
--- a/t/api/group.t
+++ /dev/null
@@ -1,111 +0,0 @@
-
-use strict;
-use warnings;
-use RT;
-use RT::Test nodata => 1, tests => undef;
-
-
-{
-
-ok (require RT::Group);
-
-ok (my $group = RT::Group->new(RT->SystemUser), "instantiated a group object");
-ok (my ($id, $msg) = $group->CreateUserDefinedGroup( Name => 'TestGroup', Description => 'A test group',
-                    ), 'Created a new group');
-isnt ($id , 0, "Group id is $id");
-is ($group->Name , 'TestGroup', "The group's name is 'TestGroup'");
-my $ng = RT::Group->new(RT->SystemUser);
-
-ok($ng->LoadUserDefinedGroup('TestGroup'), "Loaded testgroup");
-is($ng->id , $group->id, "Loaded the right group");
-
-
-ok (($id,$msg) = $ng->AddMember('1'), "Added a member to the group");
-ok($id, $msg);
-ok (($id,$msg) = $ng->AddMember('2' ), "Added a member to the group");
-ok($id, $msg);
-ok (($id,$msg) = $ng->AddMember('3' ), "Added a member to the group");
-ok($id, $msg);
-
-# Group 1 now has members 1, 2 ,3
-
-my $group_2 = RT::Group->new(RT->SystemUser);
-ok (my ($id_2, $msg_2) = $group_2->CreateUserDefinedGroup( Name => 'TestGroup2', Description => 'A second test group'), , 'Created a new group');
-isnt ($id_2 , 0, "Created group 2 ok- $msg_2 ");
-ok (($id,$msg) = $group_2->AddMember($ng->PrincipalId), "Made TestGroup a member of testgroup2");
-ok($id, $msg);
-ok (($id,$msg) = $group_2->AddMember('1' ), "Added  member RT_System to the group TestGroup2");
-ok($id, $msg);
-
-# Group 2 how has 1, g1->{1, 2,3}
-
-my $group_3 = RT::Group->new(RT->SystemUser);
-ok (my ($id_3, $msg_3) = $group_3->CreateUserDefinedGroup( Name => 'TestGroup3', Description => 'A second test group'), 'Created a new group');
-isnt ($id_3 , 0, "Created group 3 ok - $msg_3");
-ok (($id,$msg) =$group_3->AddMember($group_2->PrincipalId), "Made TestGroup a member of testgroup2");
-ok($id, $msg);
-
-# g3 now has g2->{1, g1->{1,2,3}}
-
-my $principal_1 = RT::Principal->new(RT->SystemUser);
-$principal_1->Load('1');
-
-my $principal_2 = RT::Principal->new(RT->SystemUser);
-$principal_2->Load('2');
-
-ok (($id,$msg) = $group_3->AddMember('1' ), "Added  member RT_System to the group TestGroup2");
-ok($id, $msg);
-
-# g3 now has 1, g2->{1, g1->{1,2,3}}
-
-is($group_3->HasMember($principal_2), undef, "group 3 doesn't have member 2");
-ok($group_3->HasMemberRecursively($principal_2), "group 3 has member 2 recursively");
-ok($ng->HasMember($principal_2) , "group ".$ng->Id." has member 2");
-my ($delid , $delmsg) =$ng->DeleteMember($principal_2->Id);
-isnt ($delid ,0, "Sucessfully deleted it-".$delid."-".$delmsg);
-
-#Gotta reload the group objects, since we've been messing with various internals.
-# we shouldn't need to do this.
-#$ng->LoadUserDefinedGroup('TestGroup');
-#$group_2->LoadUserDefinedGroup('TestGroup2');
-#$group_3->LoadUserDefinedGroup('TestGroup');
-
-# G1 now has 1, 3
-# Group 2 how has 1, g1->{1, 3}
-# g3 now has  1, g2->{1, g1->{1, 3}}
-
-ok(!$ng->HasMember($principal_2)  , "group ".$ng->Id." no longer has member 2");
-is($group_3->HasMemberRecursively($principal_2), undef, "group 3 doesn't have member 2");
-is($group_2->HasMemberRecursively($principal_2), undef, "group 2 doesn't have member 2");
-is($ng->HasMember($principal_2), undef, "group 1 doesn't have member 2");
-is($group_3->HasMemberRecursively($principal_2), undef, "group 3 has member 2 recursively");
-
-
-
-}
-
-{
-
-ok(my $u = RT::Group->new(RT->SystemUser));
-ok($u->Load(4), "Loaded the first user");
-is($u->PrincipalObj->id , 4, "user 4 is the fourth principal");
-is($u->PrincipalObj->PrincipalType , 'Group' , "Principal 4 is a group");
-
-
-}
-
-{
-    my $u = RT::Group->new(RT->SystemUser);
-    $u->LoadUserDefinedGroup('TestGroup');
-    ok( $u->id, 'loaded TestGroup' );
-    ok( $u->SetName('testgroup'), 'rename to lower cased version: testgroup' );
-    ok( $u->SetName('TestGroup'), 'rename back' );
-
-    my $u2 = RT::Group->new( RT->SystemUser );
-    my ( $id, $msg ) = $u2->CreateUserDefinedGroup( Name => 'TestGroup' );
-    ok( !$id, "can't create duplicated group: $msg" );
-    ( $id, $msg ) = $u2->CreateUserDefinedGroup( Name => 'testgroup' );
-    ok( !$id, "can't create duplicated group even case is different: $msg" );
-}
-
-done_testing;
diff --git a/t/api/group_members.t b/t/api/group_members.t
new file mode 100644
index 0000000..b85e0dc
--- /dev/null
+++ b/t/api/group_members.t
@@ -0,0 +1,391 @@
+use strict;
+use warnings;
+
+use RT::Test nodata => 1, tests => 376;
+
+my %GROUP;
+foreach my $name (qw(A B C D)) {
+    my $group = $GROUP{$name} = RT::Group->new( RT->SystemUser );
+    my ($status, $msg) = $group->CreateUserDefinedGroup( Name => $name );
+    ok $status, "created a group '$name'" or diag "error: $msg";
+}
+
+my %USER;
+foreach my $name (qw(a b c d)) {
+    my $user = $USER{$name} = RT::User->new( RT->SystemUser );
+    my ($status, $msg) = $user->Create( Name => $name );
+    ok $status, "created an user '$name'" or diag "error: $msg";
+}
+
+my %DISABLED;
+
+{
+    add_members_ok( A => qw(a b c) );
+    check_membership( A => [qw(a b c)] );
+
+    add_members_ok( B => qw(A) );
+    add_members_ok( C => qw(B) );
+    check_membership( A => [qw(a b c)], B => [qw(A)], C => [qw(B)] );
+
+    del_members_ok( A => 'b' );
+    check_membership( A => [qw(a c)], B => [qw(A)], C => [qw(B)] );
+
+    add_members_ok( A => qw(b) );
+    add_members_ok( B => qw(b) );
+    check_membership( A => [qw(a b c)], B => [qw(A b)], C => [qw(B)] );
+
+    del_members_ok( A => 'b' );
+    check_membership( A => [qw(a c)], B => [qw(A b)], C => [qw(B)] );
+
+    random_delete( A => [qw(a c)], B => [qw(A b)], C => [qw(B)] );
+}
+
+{
+    add_members_ok( A => qw(B C) );
+    add_members_ok( B => qw(D) );
+    add_members_ok( C => qw(D) );
+    add_members_ok( A => qw(D) );
+    check_membership( A => [qw(B C D)], B => [qw(D)], C => [qw(D)] );
+
+    del_members_ok( A => qw(D) );
+    check_membership( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+    random_delete( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+}
+
+{
+    add_members_ok( A => qw(B C) );
+    add_members_ok( B => qw(d) );
+    add_members_ok( C => qw(d) );
+    add_members_ok( A => qw(d) );
+    check_membership( A => [qw(B C d)], B => [qw(d)], C => [qw(d)] );
+
+    del_members_ok( A => qw(d) );
+    check_membership( A => [qw(B C)], B => [qw(d)], C => [qw(d)] );
+    random_delete( A => [qw(B C)], B => [qw(d)], C => [qw(d)] );
+}
+
+for (1..3) {
+    random_delete( random_build() );
+}
+
+{
+# variations of ruby:
+#   A
+# / | \
+# B | C
+# \ | /
+#   D
+    add_members_ok( A => qw(B C) );
+    add_members_ok( B => qw(D) );
+    add_members_ok( C => qw(D) );
+
+    # disable B with A -> C -> D active alternative for A->D
+    disable_group_ok( 'B' );
+    check_cgm_activity( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+
+    # disable C, no more active alternative for A->D
+    disable_group_ok( 'C' );
+    check_cgm_activity( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+
+    # add direct active (A->D) link
+    add_members_ok( A => qw(D) );
+    check_cgm_activity( A => [qw(B C D)], B => [qw(D)], C => [qw(D)] );
+
+    # delete active (A->D) when all alternative are disabled
+    del_members_ok( A => 'D' );
+    check_cgm_activity( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+
+    # enable C that enables A->D
+    enable_group_ok( 'C' );
+    check_cgm_activity( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+
+    # delete (A->B) and add back, B is disabled
+    del_members_ok( A => 'B' );
+    add_members_ok( A => qw(B) );
+    check_cgm_activity( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+
+    random_delete( A => [qw(B C)], B => [qw(D)], C => [qw(D)] );
+
+    enable_group_ok( 'B' );
+}
+
+{
+# variations of triangle
+#   A
+# /   \
+# B -> C
+    add_members_ok( A => qw(B C) );
+    disable_group_ok( 'B' );
+
+    # add member to disabled group
+    add_members_ok( B => qw(C) );
+    check_cgm_activity( A => [qw(B C)], B => [qw(C)] );
+
+    random_delete( A => [qw(B C)], B => [qw(C)] );
+    enable_group_ok( 'B' );
+}
+
+sub random_build {
+    my (%GM, %RCGM);
+
+    my @groups = keys %GROUP;
+
+    my $i = 9;
+    while ( $i-- ) {
+        REPICK:
+        my $g = $groups[int rand @groups];
+        my @members = (keys %GROUP, keys %USER);
+        substract_list(
+            \@members,
+            $g,
+            $GM{$g}? @{$GM{$g}} : (),
+            $RCGM{$g}? @{$RCGM{$g}} : (),
+        );
+        unless ( @members ) {
+            substract_list(\@groups, $g);
+            die "boo" unless @groups;
+            goto REPICK;
+        }
+
+        my $m = $members[int rand @members];
+
+        my $error = "($g -> $m) to ". describe_state(%GM);
+        diag "going to add $error";
+
+        add_members_ok( $g => $m );
+        push @{ $GM{ $g }||=[] }, $m;
+        unless ( check_membership( %GM ) ) {
+            Test::More::diag("were adding $error") unless $ENV{'TEST_VERBOSE'};
+        }
+
+        %RCGM = reverse_gm( gm_to_cgm(%GM) );
+    }
+    return %GM;
+}
+
+sub random_delete {
+    my %GM = @_;
+
+    while ( my @groups = keys %GM ) {
+        my $g = $groups[ int rand @groups ];
+        my $m = $GM{ $g }->[ int rand @{ $GM{ $g } } ];
+
+        my $error = "($g -> $m) from ". describe_state(%GM);
+        diag "going to delete $error";
+
+        del_members_ok( $g => $m );
+        @{ $GM{ $g } } = grep $_ ne $m, @{ $GM{ $g } };
+        delete $GM{ $g } unless @{ $GM{ $g } };
+
+        unless ( check_membership( %GM ) ) {
+            Test::More::diag("were deleting $error") unless $ENV{'TEST_VERBOSE'};
+        }
+    }
+}
+
+sub describe_state {
+    my %GM = @_;
+    return '('. join(
+        ', ',
+        map { "$_ -> [". join( ' ', @{ $GM{ $_ } } )."]" } sort keys %GM
+    ) .')';
+}
+
+sub check_membership {
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+    my %GM = @_;
+    my $res = _check_membership( HasMember => %GM );
+    my %CGM = gm_to_cgm(%GM);
+    $res &&= _check_membership( HasMemberRecursively => %CGM );
+    $res &&= check_cgm_activity( %GM );
+    return $res;
+}
+
+sub gm_to_cgm {
+    my %GM = @_;
+
+    my $flat;
+    $flat = sub {
+        return unless $GM{ $_[0] };
+        return map { $_, $flat->($_) } @{ $GM{ $_[0] } };
+    };
+
+    my %CGM;
+    $CGM{ $_ } = [ $flat->( $_ ) ] foreach keys %GM;
+    return %CGM;
+}
+
+sub gm_to_activity {
+    my %GM = @_;
+
+    my $flat;
+    $flat = sub {
+        return if $DISABLED{ $_[0] };
+        my @self_ref = $GROUP{$_[0]} && !$DISABLED{$_[0]}? ($_[0]) : ();
+        return @self_ref unless $GM{ $_[0] };
+        return @self_ref, map { $_, $flat->($_) } @{ $GM{ $_[0] } };
+    };
+
+    my %CGM;
+    $CGM{ $_ } = [ $flat->( $_ ) ] foreach keys %GROUP;
+    return %CGM;
+}
+
+sub reverse_gm {
+    my %GM = @_;
+    my %res = @_;
+
+    foreach my $g ( keys %GM ) {
+        push @{ $res{$_}||=[] }, $g foreach @{ $GM{ $g } };
+    }
+    return %res;
+}
+
+sub check_cgm_activity {
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+    my %GM = @_;
+
+    my $id_to_name = sub {
+        my $p = RT::Principal->new( RT->SystemUser );
+        $p->Load($_[0]);
+        $p->Object->Name;
+    };
+
+    my $data = $RT::Handle->dbh->selectall_arrayref(
+        "SELECT GroupId, MemberId FROM CachedGroupMembers WHERE Disabled = 0"
+        .' AND GroupId IN ('. join( ', ', map $_->id, values %GROUP, values %USER ) .')'
+        .' AND MemberId IN ('. join( ', ', map $_->id, values %GROUP, values %USER ) .')'
+    );
+
+    my %got;
+    foreach (@$data) {
+        my ($g, $m) = (map $id_to_name->($_), @$_);
+        push @{ $got{$g} ||= []}, $m;
+    }
+
+    my %expected = gm_to_activity( %GM );
+
+    foreach my $hash (\%got, \%expected) {
+        foreach ( values %$hash ) {
+            my %seen;
+            @$_ = sort grep !$seen{$_}++, @$_;
+        }
+        delete $hash->{$_} foreach grep !@{$hash->{$_}}, keys %$hash;
+    }
+    use Data::Dumper;
+    is_deeply(\%got, \%expected, 'activity of the records is correct')
+        or diag Dumper( \%got, \%expected );
+}
+
+sub _check_membership {
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+    my $method = shift;
+    my %GM = @_;
+
+    my $not_ok = 0;
+    foreach my $gname ( keys %GROUP ) {
+        foreach my $mname ( grep $gname ne $_, keys %USER, keys %GROUP ) {
+            my $ok;
+            if ( $GM{$gname} && grep $mname eq $_, @{$GM{$gname}} ) {
+                #note "checking ($gname -> $mname) for presence";
+                unless ( $GROUP{$gname}->$method( ($USER{$mname}||$GROUP{$mname})->PrincipalObj ) ) {
+                    $not_ok = 1;
+                    note "Group $gname has no member $mname, but should";
+                }
+            } else {
+                #note "checking ($gname -> $mname) for absence";
+                if ( $GROUP{$gname}->$method( ($USER{$mname}||$GROUP{$mname})->PrincipalObj ) ) {
+                    $not_ok = 1;
+                    note "Group $gname has member $mname, but should not";
+                }
+            }
+        }
+    }
+    return ok !$not_ok, "$method is ok";
+}
+
+sub add_members_ok {
+    my ($g, @members) = @_;
+    foreach my $m (@members) {
+        my ($status, $msg) = $GROUP{$g}->AddMember( ($USER{$m}||$GROUP{$m})->PrincipalId );
+        ok $status, $msg;
+    }
+}
+sub del_members_ok {
+    my ($g, @members) = @_;
+    foreach my $m (@members) {
+        my ($status, $msg) = $GROUP{$g}->DeleteMember( ($USER{$m}||$GROUP{$m})->PrincipalId );
+        ok $status, $msg;
+    }
+}
+
+sub dump_gm {
+    my ($G, $M) = @_;
+    my $dbh = $RT::Handle->dbh;
+
+    my $gm_id = sub {
+        my ($G, $M) = @_;
+        return ($dbh->selectrow_array(
+            "SELECT id FROM GroupMembers WHERE GroupId = $G AND MemberId = $M"
+        ))[0] || 0;
+    };
+    my $cgm_id = sub {
+        my ($G, $M) = @_;
+        return ($dbh->selectrow_array(
+            "SELECT id FROM CachedGroupMembers WHERE GroupId = $G AND MemberId = $M"
+        ))[0] || 0;
+    };
+    my $anc = sub {
+        my $M = shift;
+        return @{$dbh->selectcol_arrayref(
+            "SELECT GroupId FROM CachedGroupMembers WHERE MemberId = $M"
+        )};
+    };
+    my $des = sub {
+        my $G = shift;
+        return @{$dbh->selectcol_arrayref(
+            "SELECT MemberId FROM CachedGroupMembers WHERE GroupId = $G"
+        )};
+    };
+    my $anc_des_pairs = sub {
+        my ($G,$M) = @_;
+
+        foreach my $A ( $anc->($G) ) {
+            foreach my $D ( $des->($M) ) {
+                next unless my $id = $cgm_id->($A, $D);
+                diag "\t($A,$D) (#$id)(GM#". $gm_id->($A, $D).")";
+            }
+        }
+    };
+
+    my $id;
+    diag "Dumping GM ($G, $M) (#". $gm_id->($G, $M) .')';
+    diag "CGM ($G, $M) (#". $cgm_id->($G, $M) .')';
+    diag "An($G): ". join ',', map "$_ (GM#". $gm_id->($_, $G) .")", $anc->($G);
+    diag "De($M): ". join ',', map "$_ (GM#". $gm_id->($M, $_) .")", $des->($M);
+    diag "(An($G), De($M)): ";
+    $anc_des_pairs->($G, $M);
+}
+
+sub disable_group_ok {
+    my $g = shift;
+    my ($status, $msg) = $GROUP{ $g }->SetDisabled(1);
+    ok $status, "disabled group '$g'" or diag "error: $msg";
+    $DISABLED{$g} = $GROUP{ $g }->Disabled;
+}
+sub enable_group_ok {
+    my $g = shift;
+    my ($status, $msg) = $GROUP{ $g }->SetDisabled(0);
+    ok $status, "enabled group '$g'" or diag "error: $msg";
+    $DISABLED{$g} = $GROUP{ $g }->Disabled;
+}
+
+sub substract_list {
+    my $list = shift;
+    foreach my $e ( @_ ) {
+        @$list = grep $_ ne $e, @$list;
+    }
+}

commit 52d977bfbf23c2f6e595642b8a47dcc29b63bc6b
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Fri Mar 23 15:46:31 2012 +0400

    make test more verbose about what it checks

diff --git a/t/validator/group_members.t b/t/validator/group_members.t
index 7a37ed5..237d9b5 100644
--- a/t/validator/group_members.t
+++ b/t/validator/group_members.t
@@ -98,8 +98,10 @@ RT::Test->db_is_valid;
     isnt($ecode, 0, 'non-zero exit code');
 
     for ( my $i = 1; $i < @groups; $i++ ) {
-        ok $groups[$i]->HasMember( $groups[$i-1]->id ), "has member";
-        ok $groups[$i]->HasMemberRecursively( $groups[$_]->id ), "has member"
+        ok $groups[$i]->HasMember( $groups[$i-1]->id ),
+            "G #". $groups[$i]->id ." has member #". $groups[$i-1]->id;
+        ok $groups[$i]->HasMemberRecursively( $groups[$_]->id ),
+            "G #". $groups[$i]->id ." has member #". $groups[$_]->id
             foreach 0..$i-1;
     }
 

commit 9b40560684c47fbe40a8cb7b008e039b49bf58e7
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Fri Mar 23 15:47:46 2012 +0400

    change validator to support new CGM table

diff --git a/sbin/rt-validator.in b/sbin/rt-validator.in
index 6eaac17..ea03785 100644
--- a/sbin/rt-validator.in
+++ b/sbin/rt-validator.in
@@ -423,7 +423,6 @@ push @CHECKS, 'CGM vs. GM' => sub {
     $res *= check_integrity(
         GroupMembers       => ['GroupId', 'MemberId'],
         CachedGroupMembers => ['GroupId', 'MemberId'],
-        join_condition     => 't.ImmediateParentId = t.GroupId AND t.Via = t.id',
         action => sub {
             my $id = shift;
             return unless prompt(
@@ -436,33 +435,14 @@ push @CHECKS, 'CGM vs. GM' => sub {
             die "Couldn't load GM record #$id" unless $gm->id;
             my $cgm = create_record( 'CachedGroupMembers',
                 GroupId => $gm->GroupId, MemberId => $gm->MemberId,
-                ImmediateParentId => $gm->GroupId, Via => undef,
-                Disabled => 0, # XXX: we should check integrity of Disabled field
             );
-            update_records( "CachedGroupMembers", { id => $cgm }, { Via => $cgm } );
         },
     );
-    # all first level CGM records should have a GM record
-    $res *= check_integrity(
-        CachedGroupMembers => ['GroupId', 'MemberId'],
-        GroupMembers       => ['GroupId', 'MemberId'],
-        condition     => 's.ImmediateParentId = s.GroupId AND s.Via = s.id AND s.GroupId != s.MemberId',
-        action => sub {
-            my $id = shift;
-            return unless prompt(
-                'Delete',
-                "Found a record in CachedGroupMembers for a (Group, Member) pair"
-                ." that doesn't exist in the GroupMembers table."
-            );
 
-            delete_record( 'CachedGroupMembers', $id );
-        },
-    );
     # each group should have a CGM record where MemberId == GroupId
     $res *= check_integrity(
         Groups => ['id', 'id'],
         CachedGroupMembers => ['GroupId', 'MemberId'],
-        join_condition     => 't.ImmediateParentId = t.GroupId AND t.Via = t.id',
         action => sub {
             my $id = shift;
             return unless prompt(
@@ -477,13 +457,9 @@ push @CHECKS, 'CGM vs. GM' => sub {
             die "Loaded group by $id has id ". $g->id  unless $g->id == $id;
             my $cgm = create_record( 'CachedGroupMembers',
                 GroupId => $id, MemberId => $id,
-                ImmediateParentId => $id, Via => undef,
-                Disabled => $g->Disabled,
             );
-            update_records( "CachedGroupMembers", { id => $cgm }, { Via => $cgm } );
         },
     );
-
     # and back, each record in CGM with MemberId == GroupId without exceptions
     # should reference a group
     $res *= check_integrity(
@@ -500,97 +476,223 @@ push @CHECKS, 'CGM vs. GM' => sub {
             delete_record( 'CachedGroupMembers', $id );
         },
     );
-    # Via
-    $res *= check_integrity(
-        CachedGroupMembers => 'Via',
-        CachedGroupMembers => 'id',
-        action => sub {
-            my $id = shift;
-            return unless prompt(
+
+    # a CGM record (where MemberId != GroupId) should either match GM record
+    # or two CGM records
+    {
+        my $query = <<END;
+SELECT CGM.id
+FROM
+    CachedGroupMembers CGM
+WHERE
+    CGM.GroupId != CGM.MemberId
+    AND NOT EXISTS (
+        SELECT 1 FROM GroupMembers GM
+        WHERE GM.GroupId = CGM.GroupId
+            AND GM.MemberId = CGM.MemberId
+    )
+    AND NOT EXISTS (
+        SELECT 1 FROM CachedGroupMembers CGML, CachedGroupMembers CGMR
+        WHERE
+                CGML.GroupId = CGM.GroupId
+            AND CGML.MemberId = CGMR.GroupId
+            AND CGMR.MemberId = CGM.MemberId
+
+            AND CGML.GroupId != CGML.MemberId
+            AND CGMR.GroupId != CGMR.MemberId
+    )
+END
+
+        my $sth = execute_query( $query );
+        while ( my ($id) = $sth->fetchrow_array ) {
+            $res = 0;
+            print STDERR "CGM #$id has no corresponding record in GM or pair in CGM table\n";
+            next unless prompt(
                 'Delete',
-                "Found a record in CachedGroupMembers with Via that references a nonexistent record."
+                "Found records in CachedGroupMembers table that have no origin."
             );
-
             delete_record( 'CachedGroupMembers', $id );
-        },
-    );
+        }
+    }
 
-    # for every CGM where ImmediateParentId != GroupId there should be
-    # matching parent record (first level) 
+    # now, when we are sure all CGM records have ground then we can check missing
+    # records based on other CGM records
+    {
+         my $query = <<END;
+SELECT CGML.GroupId, CGMR.MemberId, CGML.Disabled + CGMR.Disabled
+FROM CachedGroupMembers CGML, CachedGroupMembers CGMR
+WHERE
+    CGML.MemberId = CGMR.GroupId
+    AND CGML.GroupId != CGML.MemberId
+    AND CGMR.GroupId != CGMR.MemberId
+    AND NOT EXISTS (
+        SELECT 1 FROM CachedGroupMembers CGM
+        WHERE CGM.GroupId = CGML.GroupId
+            AND CGM.MemberId = CGMR.MemberId
+    )
+END
+        my $sth = execute_query( $query );
+        while ( my ($g, $m, $d) = $sth->fetchrow_array ) {
+            $res = 0;
+            print STDERR "CGM table has no record ($g, $m), but ($g -> X -> $m) pair exist in CGM\n";
+            next unless prompt(
+                'Create',
+                "Missing records in CachedGroupMembers."
+            );
+            create_record( 'CachedGroupMembers',
+                GroupId => $g, MemberId => $m, Disabled => $d? 1 : 0,
+            );
+        }
+    }
+
+    return $res;
+};
+
+# Disabled in CGM
+push @CHECKS, 'Disabled in CGM' => sub {
+    my $res = 1;
+    # make sure every disabled group forms only disabled CGM records
     $res *= check_integrity(
-        CachedGroupMembers => ['ImmediateParentId', 'MemberId'],
-        CachedGroupMembers => ['GroupId', 'MemberId'],
-        join_condition => 't.Via = t.id',
-        condition => 's.ImmediateParentId != s.GroupId',
-        action => sub {
+        Principals         => ['id'],
+        CachedGroupMembers => ['GroupId'],
+        condition          => "s.PrincipalType = ? AND s.Disabled != 0",
+        join_condition     => "t.Disabled != 0",
+        bind_values        => ['Group'],
+        action             => sub {
             my $id = shift;
             return unless prompt(
-                'Delete',
-                "Found a record in CachedGroupMembers that references a nonexistent record in CachedGroupMembers table."
+                'Update',
+                "Found not disabled record in CachedGroupMembers for a disabled group."
             );
 
-            delete_record( 'CachedGroupMembers', $id );
+            update_records('CachedGroupMembers', { GroupId => $id }, { Disabled => 1 });
         },
     );
-
-    # for every CGM where ImmediateParentId != GroupId there should be
-    # matching "grand" parent record
+    # make sure every enabled group has enabled (G,G) record in CGM
     $res *= check_integrity(
-        CachedGroupMembers => ['GroupId', 'ImmediateParentId', 'Via'],
-        CachedGroupMembers => ['GroupId', 'MemberId', 'id'],
-        condition => 's.ImmediateParentId != s.GroupId',
-        action => sub {
+        Principals         => ['id', 'id'],
+        CachedGroupMembers => ['GroupId', 'MemberId'],
+        condition          => "s.PrincipalType = ? AND s.Disabled = 0",
+        join_condition     => "t.Disabled = 0",
+        bind_values        => ['Group'],
+        action             => sub {
             my $id = shift;
             return unless prompt(
-                'Delete',
-                "Found a record in CachedGroupMembers that references a nonexistent record in CachedGroupMembers table."
+                'Update',
+                "Found enabled group when loop record in CachedGroupMembers is disabled."
             );
 
-            delete_record( 'CachedGroupMembers', $id );
+            update_records('CachedGroupMembers', { GroupId => $id, MemberId => $id }, { Disabled => 0 });
         },
     );
-
-    # CHECK recursive records:
-    # if we have CGM1 (G1,M1,V1,IP1) then for every GM2(G2, M2), where G2 == M1,
-    # we should have CGM3 where G3 = G1, M3 = M2, V3 = ID1, IP3 = M1
+    # make sure every GM record with enabled group has enabled CGM record
     {
-        my $query = <<END;
-SELECT cgm1.GroupId, gm2.MemberId, cgm1.id AS Via,
-    cgm1.MemberId AS ImmediateParentId, cgm1.Disabled
-FROM
-    CachedGroupMembers cgm1
-    CROSS JOIN GroupMembers gm2
-    LEFT JOIN CachedGroupMembers cgm3 ON (
-            cgm3.GroupId           = cgm1.GroupId
-        AND cgm3.MemberId          = gm2.MemberId
-        AND cgm3.Via               = cgm1.id
-        AND cgm3.ImmediateParentId = cgm1.MemberId )
-WHERE cgm1.GroupId != cgm1.MemberId
-AND gm2.GroupId = cgm1.MemberId
-AND cgm3.id IS NULL
+         my $query = <<END;
+SELECT GM.GroupId, GM.MemberId
+FROM GroupMembers GM, Principals p
+WHERE   p.id = GM.GroupId
+    AND p.Disabled = 0
+    AND NOT EXISTS (
+        SELECT 1 FROM CachedGroupMembers CGM
+        WHERE CGM.GroupId = GM.GroupId
+            AND CGM.MemberId = GM.MemberId
+            AND CGM.Disabled = 0
+    )
 END
-
-        my $action = sub {
-            my %props = @_;
-            return unless prompt(
-                'Create',
-                "Found records in CachedGroupMembers table without recursive duplicates."
+        my $sth = execute_query( $query );
+        while ( my ($g, $m) = $sth->fetchrow_array ) {
+            $res = 0;
+            print STDERR "CGM ($g, $m) is not active while it should be\n";
+            next unless prompt(
+                'Update',
+                "Missing records in CachedGroupMembers."
+            );
+            update_records('CachedGroupMembers',
+                { GroupId => $g, MemberId => $m },
+                { Disabled => 0 }
             );
-            my $cgm = create_record( 'CachedGroupMembers', %props );
-        };
+        }
+    }
 
+    # make sure every active CGM that is not real GM has
+    # active joint
+    {
+         my $query = <<END;
+SELECT CGM.GroupId, CGM.MemberId
+FROM CachedGroupMembers CGM
+WHERE   CGM.Disabled = 0
+    AND CGM.GroupId != CGM.MemberId
+    AND NOT EXISTS (
+        SELECT 1 FROM GroupMembers GM
+        WHERE GM.GroupId = CGM.GroupId
+            AND GM.MemberId = CGM.MemberId
+    )
+    AND NOT EXISTS (
+        SELECT 1 FROM CachedGroupMembers CGML, CachedGroupMembers CGMR
+        WHERE
+                CGML.GroupId = CGM.GroupId
+            AND CGML.MemberId = CGMR.GroupId
+            AND CGMR.MemberId = CGM.MemberId
+
+            AND CGML.Disabled = 0
+            AND CGMR.Disabled = 0
+            AND CGML.GroupId != CGML.MemberId
+            AND CGMR.GroupId != CGMR.MemberId
+    )
+END
         my $sth = execute_query( $query );
-        while ( my ($g, $m, $via, $ip, $dis) = $sth->fetchrow_array ) {
+        while ( my ($g, $m) = $sth->fetchrow_array ) {
             $res = 0;
-            print STDERR "Principal #$m is member of #$ip when #$ip is member of #$g,";
-            print STDERR " but there is no cached GM record that $m is member of #$g.\n";
-            $action->(
-                GroupId => $g, MemberId => $m, Via => $via,
-                ImmediateParentId => $ip, Disabled => $dis,
+            print STDERR "CGM ($g, $m) is active while it should not be\n";
+            next unless prompt(
+                'Update',
+                "Active records in CachedGroupMembers."
+            );
+            update_records('CachedGroupMembers',
+                { GroupId => $g, MemberId => $m },
+                { Disabled => 1 }
             );
         }
     }
 
+    # escape if we had problems
+    return $res unless $res;
+
+    # make sure every inactive CGM don't have active joint
+    {
+         my $query = <<END;
+SELECT CGM.GroupId, CGM.MemberId
+FROM CachedGroupMembers CGM
+WHERE   CGM.Disabled != 0
+    AND CGM.GroupId != CGM.MemberId
+    AND EXISTS (
+        SELECT 1 FROM CachedGroupMembers CGML, CachedGroupMembers CGMR
+        WHERE
+                CGML.GroupId = CGM.GroupId
+            AND CGML.MemberId = CGMR.GroupId
+            AND CGMR.MemberId = CGM.MemberId
+
+            AND CGML.Disabled = 0
+            AND CGMR.Disabled = 0
+
+            AND CGML.GroupId != CGML.MemberId
+            AND CGMR.GroupId != CGMR.MemberId
+    )
+END
+        my $sth = execute_query( $query );
+        while ( my ($g, $m) = $sth->fetchrow_array ) {
+            $res = 0;
+            print STDERR "CGM ($g, $m) is not active while it should be\n";
+            next unless prompt(
+                'Update',
+                "Inactive records in CachedGroupMembers."
+            );
+            update_records('CachedGroupMembers',
+                { GroupId => $g, MemberId => $m },
+                { Disabled => 0 }
+            );
+        }
+    }
     return $res;
 };
 

commit 87a3e4306b98e9cc8444d801673823d5f6d68efd
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sat Nov 24 23:59:37 2012 +0400

    upgrade script

diff --git a/etc/upgrade/4.1.0/content b/etc/upgrade/4.1.0/content
index 2a02c68..485e486 100644
--- a/etc/upgrade/4.1.0/content
+++ b/etc/upgrade/4.1.0/content
@@ -38,6 +38,73 @@ our @Initial = (
         $settings->{sidebar} = delete $settings->{summary};
         $default_portlets->SetContent($settings);
     },
+    sub {
+        # delete disabled duplicate CGM records in favor of active
+        my $status = $RT::Handle->DeleteFromSelect('CachedGroupMembers', <<END);
+SELECT main.id FROM CachedGroupMembers main
+WHERE
+    main.Disabled != 0
+    NOT EXISTS (
+        SELECT 1 FROM CachedGroupMembers CGM
+        WHERE CGM.GroupId = main.GroupId
+            AND CGM.MemberId = main.MemberId
+            AND CGM.id != main.id
+            AND CGM.Disabled = 0
+    )
+END
+        unless ( $status ) {
+            RT->Logger->error("Couldn't delete CGM records");
+            return;
+        }
+
+        # delete other duplicates
+        $status = $RT::Handle->DeleteFromSelect('CachedGroupMembers', <<END);
+SELECT main.id FROM CachedGroupMembers main
+WHERE EXISTS (
+    SELECT 1 FROM CachedGroupMembers CGM
+    WHERE CGM.GroupId = main.GroupId
+        AND CGM.MemberId = main.MemberId
+        AND CGM.id < main.id
+)
+END
+        unless ( $status ) {
+            RT->Logger->error("Couldn't delete CGM records");
+            return;
+        }
+
+        my $dbh = $RT::Handle->dbh;
+        local $dbh->{'RaiseError'} = 0;
+        local $dbh->{'PrintError'} = 1;
+
+        foreach my $index (qw(
+            CachedGroupMembers2 CachedGroupMembers3
+            DisGrouMem GrouMem
+        )) {
+            $dbh->do("DROP INDEX $index ON CachedGroupMembers")
+                or $dbh->do("ALTER TABLE CachedGroupMembers DROP INDEX $index");
+        }
+
+        # see CGM.pm for explanation
+        unless ( RT->Config->Get('DatabaseType') eq 'Pg' ) {
+            $dbh->do(
+                "CREATE UNIQUE INDEX CGM1"
+                ." ON CachedGroupMembers(GroupId, MemberId, Disabled)"
+            );
+            $dbh->do(
+                "CREATE UNIQUE INDEX CGM2"
+                ." ON CachedGroupMembers(MemberId, GroupId, Disabled)"
+            );
+        } else {
+            $dbh->do(
+                "CREATE UNIQUE INDEX CGM1"
+                ." ON CachedGroupMembers(GroupId, MemberId)"
+            );
+            $dbh->do(
+                "CREATE INDEX CGM2"
+                ." ON CachedGroupMembers(MemberId)"
+            );
+        }
+    },
 );
 
 
diff --git a/etc/upgrade/4.1.0/schema.Oracle b/etc/upgrade/4.1.0/schema.Oracle
new file mode 100644
index 0000000..135719e
--- /dev/null
+++ b/etc/upgrade/4.1.0/schema.Oracle
@@ -0,0 +1,2 @@
+ALTER TABLE CachedGroupMembers DROP COLUMN Via;
+ALTER TABLE CachedGroupMembers DROP COLUMN ImmediateParentId;
\ No newline at end of file
diff --git a/etc/upgrade/4.1.0/schema.Pg b/etc/upgrade/4.1.0/schema.Pg
new file mode 100644
index 0000000..135719e
--- /dev/null
+++ b/etc/upgrade/4.1.0/schema.Pg
@@ -0,0 +1,2 @@
+ALTER TABLE CachedGroupMembers DROP COLUMN Via;
+ALTER TABLE CachedGroupMembers DROP COLUMN ImmediateParentId;
\ No newline at end of file
diff --git a/etc/upgrade/4.1.0/schema.mysql b/etc/upgrade/4.1.0/schema.mysql
new file mode 100644
index 0000000..135719e
--- /dev/null
+++ b/etc/upgrade/4.1.0/schema.mysql
@@ -0,0 +1,2 @@
+ALTER TABLE CachedGroupMembers DROP COLUMN Via;
+ALTER TABLE CachedGroupMembers DROP COLUMN ImmediateParentId;
\ No newline at end of file

commit 7f296764966e10037477835a5e4839951a62fda0
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed Mar 28 23:56:50 2012 +0400

    we don't have rights delegations anymore

diff --git a/lib/RT/Shredder/GroupMember.pm b/lib/RT/Shredder/GroupMember.pm
index ab34537..ec07998 100644
--- a/lib/RT/Shredder/GroupMember.pm
+++ b/lib/RT/Shredder/GroupMember.pm
@@ -75,8 +75,6 @@ sub __DependsOn
     $objs->Limit( FIELD => 'ImmediateParentId', VALUE => $self->GroupId );
     push( @$list, $objs );
 
-    # XXX: right delegations should be cleaned here
-
     $deps->_PushDependencies(
             BaseObject => $self,
             Flags => DEPENDS_ON,

commit f0cbdc88683d11ece834410e0ee0706d2abaea42
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Mon Apr 9 18:53:34 2012 +0400

    adopt shredder to new API and CGM schema
    
    Actual delete is performed by API via GM record,
    CGM.pm only deals with dumping what was deleted.

diff --git a/lib/RT/Shredder/CachedGroupMember.pm b/lib/RT/Shredder/CachedGroupMember.pm
index 192c61d..5a4185e 100644
--- a/lib/RT/Shredder/CachedGroupMember.pm
+++ b/lib/RT/Shredder/CachedGroupMember.pm
@@ -57,45 +57,66 @@ use RT::Shredder::Constants;
 use RT::Shredder::Exceptions;
 use RT::Shredder::Dependency;
 
-
-sub __DependsOn
+sub _AsInsertQuery
 {
     my $self = shift;
-    my %args = (
-            Shredder => undef,
-            Dependencies => undef,
-            @_,
-           );
-    my $deps = $args{'Dependencies'};
-    my $list = [];
-
-# deep memebership
-    my $objs = RT::CachedGroupMembers->new( $self->CurrentUser );
-    $objs->Limit( FIELD => 'Via', VALUE => $self->Id );
-    $objs->Limit( FIELD => 'id', OPERATOR => '!=', VALUE => $self->Id );
-    push( @$list, $objs );
-
-# principal lost group membership and lost some rights which he could delegate to
-# some body
-
-# XXX: Here is problem cause HasMemberRecursively would return true allways
-# cause we didn't delete anything yet. :(
-    # if pricipal is not member anymore(could be via other groups) then proceed
-    if( $self->GroupObj->Object->HasMemberRecursively( $self->MemberObj ) ) {
-        my $acl = RT::ACL->new( $self->CurrentUser );
-        $acl->LimitToPrincipal( Id => $self->GroupId );
+    return $self->SUPER::_AsInsertQuery( @_ )
+        if $self->MemberId == $self->GroupId;
+
+    my $table = $self->Table;
+    my $dbh = $RT::Handle->dbh;
+    my @quoted = ( map $dbh->quote($self->$_()), qw(GroupId MemberId Disabled) );
+
+    my $query =
+        "SELECT ". join( ', ', @quoted ) .' WHERE NOT EXISTS ('
+            ."SELECT id FROM $table WHERE GroupId = $quoted[0] AND MemberId = $quoted[1]"
+        .')'
+    ;
+    my $res = $self->BuildInsertFromSelectQuery( $query ) ."\n";
+
+    $query = "SELECT CGM1.GroupId, CGM2.MemberId, CASE WHEN CGM1.Disabled + CGM2.Disabled > 0 THEN 1 ELSE 0 END FROM
+        $table CGM1 CROSS JOIN $table CGM2
+        LEFT JOIN $table CGM3
+            ON CGM3.GroupId = CGM1.GroupId AND CGM3.MemberId = CGM2.MemberId
+        WHERE
+            CGM1.MemberId = $quoted[0] AND (CGM1.GroupId != CGM1.MemberId OR CGM1.MemberId = $quoted[1])
+            AND CGM3.id IS NULL
+    ";
 
+    if ( $self->MemberObj->IsGroup ) {
+        $query .= "
+            AND CGM2.GroupId = $quoted[1]
+            AND (CGM2.GroupId != CGM2.MemberId OR CGM2.GroupId = $quoted[1])
+        ";
     }
+    else {
+        $query .= " AND CGM2.GroupId = $quoted[0] AND CGM2.MemberId = $quoted[1]";
+    }
+    $res .= $self->BuildInsertFromSelectQuery( $query ) ."\n";
+
+    return $res;
+}
 
+sub BuildInsertFromSelectQuery {
+    my $self = shift;
+    my $query = shift;
 
-    $deps->_PushDependencies(
-            BaseObject => $self,
-            Flags => DEPENDS_ON,
-            TargetObjects => $list,
-            Shredder => $args{'Shredder'}
-        );
+    my $table = $self->Table;
+    if ( RT->Config->Get('DatabaseType') eq 'Oracle' ) {
+        $query = "(SELECT ${table}_seq.nextval, insert_from.* FROM ($query) insert_from)";
+    }
+    return "INSERT INTO $table(GroupId, MemberId, Disabled) $query;";
+}
 
-    return $self->SUPER::__DependsOn( %args );
+sub __Wipeout {
+    my $self = shift;
+    return $self->SUPER::__Wipeout( @_ )
+        if $self->MemberId == $self->GroupId;
+
+    # GroupMember takes care of wiping other records
+    return 1;
 }
 
+
+
 1;
diff --git a/lib/RT/Shredder/GroupMember.pm b/lib/RT/Shredder/GroupMember.pm
index ec07998..207b497 100644
--- a/lib/RT/Shredder/GroupMember.pm
+++ b/lib/RT/Shredder/GroupMember.pm
@@ -71,8 +71,8 @@ sub __DependsOn
     my $list = [];
 
     my $objs = RT::CachedGroupMembers->new( $self->CurrentUser );
+    $objs->Limit( FIELD => 'GroupId', VALUE => $self->GroupId );
     $objs->Limit( FIELD => 'MemberId', VALUE => $self->MemberId );
-    $objs->Limit( FIELD => 'ImmediateParentId', VALUE => $self->GroupId );
     push( @$list, $objs );
 
     $deps->_PushDependencies(
@@ -126,4 +126,12 @@ sub __DependsOn
     return $self->SUPER::__DependsOn( %args );
 }
 
+sub __Wipeout
+{
+    my $self = shift;
+    my $msg = $self->_AsString ." wiped out";
+    $self->Delete;
+    $RT::Logger->info( $msg );
+    return;
+}
 1;

commit 2e2452761ac8277bf28a1d91cf0a67dc205d3811
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Mon Apr 9 23:50:33 2012 +0400

    update shredder's docs

diff --git a/lib/RT/Shredder.pm b/lib/RT/Shredder.pm
index b1450e5..19a571d 100644
--- a/lib/RT/Shredder.pm
+++ b/lib/RT/Shredder.pm
@@ -133,7 +133,14 @@ SQL commands to re-insert your objects into the RT database.
 
     mysql -u your_rt_user -p your_rt_database < /path/to/rt/var/data/shredder/dump.sql
 
-That's it.i This will restore everything you'd deleted during a
+3) Validate database with the following command. This is required as
+   DB may be inconsistent because of changes since shredding, also some
+   restore operations are hard to implement in pure SQL that will work
+   on any DB RT runs on.
+
+   rt-validator --check
+
+That's it. This will restore everything you'd deleted during a
 shredding session when the file had been created.
 
 =head1 CONFIGURATION

-----------------------------------------------------------------------


More information about the Rt-commit mailing list