Mailing List Archive

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".

The branch, 5.0/cgm-cascade-delete has been created
at 7b87e463224e650348e7685103278ca18864842c (commit)

- Log -----------------------------------------------------------------
commit 7b87e463224e650348e7685103278ca18864842c
Author: sunnavy <sunnavy@bestpractical.com>
Date: Thu Dec 7 15:48:17 2023 -0500

Test cascaded deletion of cached group members

diff --git a/t/api/group.t b/t/api/group.t
index 13389567ee..0f5a997748 100644
--- a/t/api/group.t
+++ b/t/api/group.t
@@ -201,4 +201,33 @@ diag "Ticket role group members";
ok( !$CGM->id, 'No CGM record for admincc <-> bob still' );
}

+
+diag "Cascade delete cached group members";
+{
+ my $test1 = RT::Test->load_or_create_group('cascade test 1');
+ my $test2 = RT::Test->load_or_create_group('cascade test 2');
+ my $test3 = RT::Test->load_or_create_group('cascade test 3');
+ my $user = RT::Test->load_or_create_user( Name => 'User 1' );
+
+ ok( $test3->AddMember( $user->PrincipalId ), 'Add User 1 to test 3' );
+ ok( $test2->AddMember( $test3->PrincipalId ), 'Add test 3 to test 2' );
+ ok( $test1->AddMember( $test2->PrincipalId ), 'Add test 2 to test 1' );
+
+ ok( $test1->HasMemberRecursively( $test2->PrincipalId ), "Group test 1 has test 2 recursively" );
+ ok( $test1->HasMemberRecursively( $test3->PrincipalId ), "Group test 1 has test 3 recursively" );
+ ok( $test1->HasMemberRecursively( $user->PrincipalId ), "Group test 1 has User 1 recursively" );
+
+ my $cgms = RT::CachedGroupMembers->new( RT->SystemUser );
+ $cgms->Limit( FIELD => 'GroupId', VALUE => $test1->Id );
+ is( $cgms->Count, 4, 'Group test has 4 CachedGroupMembers rows' );
+
+ ok( $test1->DeleteMember( $test2->PrincipalId ), 'Delete test 2 from test' );
+ $cgms->RedoSearch;
+ is( $cgms->Count, 1, 'Group test has 1 CachedGroupMembers row' );
+
+ ok( !$test1->HasMemberRecursively( $test2->PrincipalId ), "Group test 1 does not have test 2 recursively" );
+ ok( !$test1->HasMemberRecursively( $test3->PrincipalId ), "Group test 1 does not have test 3 recursively" );
+ ok( !$test1->HasMemberRecursively( $user->PrincipalId ), "Group test 1 does not have User 1 recursively" );
+}
+
done_testing;

commit f22f1b1622c32b9909c7509fcd192294854ab2bc
Author: sunnavy <sunnavy@bestpractical.com>
Date: Thu Dec 7 15:38:57 2023 -0500

Implement cascaded deletion of cached group members on DB level

The performance is obviously better to clean up stuff on DB level. As
Via is a foreign key, it can't be 0 any more.

diff --git a/etc/schema.Oracle b/etc/schema.Oracle
index d0893a74c9..28c59f6357 100644
--- a/etc/schema.Oracle
+++ b/etc/schema.Oracle
@@ -199,7 +199,8 @@ CREATE TABLE CachedGroupMembers (
MemberId NUMBER(11,0),
Via NUMBER(11,0),
ImmediateParentId NUMBER(11,0),
- Disabled NUMBER(11,0) DEFAULT 0 NOT NULL
+ Disabled NUMBER(11,0) DEFAULT 0 NOT NULL,
+ FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE
);
CREATE INDEX DisGrouMem ON CachedGroupMembers (GroupId, MemberId, Disabled);
CREATE INDEX CachedGroupMembers2 ON CachedGroupMembers (MemberId, GroupId, Disabled);
diff --git a/etc/schema.Pg b/etc/schema.Pg
index 2e7a983062..2440171ee1 100644
--- a/etc/schema.Pg
+++ b/etc/schema.Pg
@@ -325,6 +325,7 @@ CREATE TABLE CachedGroupMembers (
Via int,
ImmediateParentId int,
Disabled integer NOT NULL DEFAULT 0 ,
+ FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE,
PRIMARY KEY (id)

);
diff --git a/etc/schema.SQLite b/etc/schema.SQLite
index be08cfd925..3df66dd98f 100644
--- a/etc/schema.SQLite
+++ b/etc/schema.SQLite
@@ -221,11 +221,12 @@ create table CachedGroupMembers (
MemberId int,
Via int,
ImmediateParentId int,
- Disabled int2 NOT NULL DEFAULT 0 # if this cached group member is a member of this group by way of a disabled
+ 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
+ FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE


) ;
diff --git a/etc/schema.mysql b/etc/schema.mysql
index 0c21d2b8b4..aa18b45a77 100644
--- a/etc/schema.mysql
+++ b/etc/schema.mysql
@@ -207,6 +207,7 @@ create table CachedGroupMembers (
# 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
+ FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE,
PRIMARY KEY (id)
) ENGINE=InnoDB CHARACTER SET utf8mb4;

diff --git a/etc/upgrade/5.0.6/schema.Oracle b/etc/upgrade/5.0.6/schema.Oracle
new file mode 100644
index 0000000000..1f33351140
--- /dev/null
+++ b/etc/upgrade/5.0.6/schema.Oracle
@@ -0,0 +1 @@
+ALTER TABLE CachedGroupMembers ADD FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE;
diff --git a/etc/upgrade/5.0.6/schema.Pg b/etc/upgrade/5.0.6/schema.Pg
new file mode 100644
index 0000000000..1f33351140
--- /dev/null
+++ b/etc/upgrade/5.0.6/schema.Pg
@@ -0,0 +1 @@
+ALTER TABLE CachedGroupMembers ADD FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE;
diff --git a/etc/upgrade/5.0.6/schema.mysql b/etc/upgrade/5.0.6/schema.mysql
new file mode 100644
index 0000000000..1f33351140
--- /dev/null
+++ b/etc/upgrade/5.0.6/schema.mysql
@@ -0,0 +1 @@
+ALTER TABLE CachedGroupMembers ADD FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE;
diff --git a/lib/RT/CachedGroupMember.pm b/lib/RT/CachedGroupMember.pm
index dc2feeed18..9debea6d56 100644
--- a/lib/RT/CachedGroupMember.pm
+++ b/lib/RT/CachedGroupMember.pm
@@ -99,7 +99,7 @@ sub Create {
my %args = ( Group => '',
Member => '',
ImmediateParent => '',
- Via => '0',
+ Via => undef,
Disabled => '0',
@_ );

@@ -141,7 +141,12 @@ sub Create {
. $args{'Via'} );
return (undef); #this will percolate up and bail out of the transaction
}
- if ( $self->__Value('Via') == 0 ) {
+
+ # Check $args{Via} instead of __Value('Via') to avoid cache issues on
+ # SQLite that may reuse ids: t/validator/group_members.t fails with
+ # __Value('Via')
+
+ if ( !$args{Via} ) {
my ( $vid, $vmsg ) = $self->__Set( Field => 'Via', Value => $id );
unless ($vid) {
$RT::Logger->warning( "Due to a via error, couldn't create "
@@ -181,44 +186,6 @@ sub Create {



-=head2 Delete
-
-Deletes the current CachedGroupMember from the group it's in and
-cascades the delete to all submembers.
-
-=cut
-
-sub Delete {
- my $self = shift;
-
-
- my $member = $self->MemberObj();
- if ( $member->IsGroup ) {
- my $deletable = RT::CachedGroupMembers->new( $self->CurrentUser );
-
- $deletable->Limit( FIELD => 'id',
- OPERATOR => '!=',
- VALUE => $self->id );
- $deletable->Limit( FIELD => 'Via',
- OPERATOR => '=',
- VALUE => $self->id );
-
- while ( my $kid = $deletable->Next ) {
- my ($ok, $msg) = $kid->Delete();
- unless ($ok) {
- $RT::Logger->error(
- "Couldn't delete CachedGroupMember " . $kid->Id );
- return ($ok, $msg);
- }
- }
- }
- my ($ok, $msg) = $self->SUPER::Delete();
- $RT::Logger->error( "Couldn't delete CachedGroupMember " . $self->Id ) unless $ok;
- return ($ok, $msg);
-}
-
-
-
=head2 SetDisabled

SetDisableds the current CachedGroupMember from the group it's in and cascades
diff --git a/lib/RT/GroupMember.pm b/lib/RT/GroupMember.pm
index 41b083561b..44286aa7a2 100644
--- a/lib/RT/GroupMember.pm
+++ b/lib/RT/GroupMember.pm
@@ -103,7 +103,7 @@ sub _InsertCGM {
Member => $self->MemberObj,
Group => $self->GroupObj,
ImmediateParent => $self->GroupObj,
- Via => '0'
+ Via => undef,
);


@@ -289,7 +289,7 @@ sub _StashUser {
Member => $args{'Member'},
Group => $args{'Group'},
ImmediateParent => $args{'Group'},
- Via => '0'
+ Via => undef,
);

unless ($cached_id) {
diff --git a/lib/RT/Handle.pm b/lib/RT/Handle.pm
index 02e4667d7f..a7af75533e 100644
--- a/lib/RT/Handle.pm
+++ b/lib/RT/Handle.pm
@@ -154,6 +154,7 @@ sub Connect {
}
elsif ( $db_type eq 'SQLite' ) {
$self->dbh->{sqlite_see_if_its_a_number} = 1;
+ $self->dbh->do('PRAGMA foreign_keys = ON'); # ON DELETE CASCADE for CachedGroupMembers needs it
}

$self->dbh->{'LongReadLen'} = RT->Config->Get('MaxAttachmentSize');
diff --git a/lib/RT/Migrate/Importer.pm b/lib/RT/Migrate/Importer.pm
index f73240e522..6da135c5d0 100644
--- a/lib/RT/Migrate/Importer.pm
+++ b/lib/RT/Migrate/Importer.pm
@@ -632,7 +632,7 @@ sub CloseStream {
# Groups
$self->RunSQL(<<'EOF');
INSERT INTO CachedGroupMembers (GroupId, MemberId, Via, ImmediateParentId, Disabled)
- SELECT Groups.id, Groups.id, 0, Groups.id, Principals.Disabled FROM Groups
+ SELECT Groups.id, Groups.id, NULL, Groups.id, Principals.Disabled FROM Groups
LEFT JOIN Principals ON ( Groups.id = Principals.id )
LEFT JOIN CachedGroupMembers ON (
Groups.id = CachedGroupMembers.GroupId
@@ -645,7 +645,7 @@ EOF
# GroupMembers
$self->RunSQL(<<'EOF');
INSERT INTO CachedGroupMembers (GroupId, MemberId, Via, ImmediateParentId, Disabled)
- SELECT GroupMembers.GroupId, GroupMembers.MemberId, 0, GroupMembers.GroupId, Principals.Disabled FROM GroupMembers
+ SELECT GroupMembers.GroupId, GroupMembers.MemberId, NULL, GroupMembers.GroupId, Principals.Disabled FROM GroupMembers
LEFT JOIN Principals ON ( GroupMembers.GroupId = Principals.id )
LEFT JOIN CachedGroupMembers ON (
GroupMembers.GroupId = CachedGroupMembers.GroupId
@@ -657,7 +657,7 @@ EOF

# Fixup Via
$self->RunSQL(<<'EOF');
-UPDATE CachedGroupMembers SET Via=id WHERE Via=0
+UPDATE CachedGroupMembers SET Via=id WHERE Via IS NULL
EOF

# Cascaded GroupMembers, use the same SQL in rt-validator

commit ffdaf3ef61bbc2029d0ddb9114431ed15e94ee4c
Author: sunnavy <sunnavy@bestpractical.com>
Date: Thu Dec 7 15:37:40 2023 -0500

Index Via column of CachedGroupMembers

When we delete CachedGroupMember rows, we need to find and delete all
derived rows, by means of the Via column. Without the index, the search
could be quite slow.

diff --git a/etc/schema.Oracle b/etc/schema.Oracle
index 53210b145b..d0893a74c9 100644
--- a/etc/schema.Oracle
+++ b/etc/schema.Oracle
@@ -204,6 +204,7 @@ CREATE TABLE CachedGroupMembers (
CREATE INDEX DisGrouMem ON CachedGroupMembers (GroupId, MemberId, Disabled);
CREATE INDEX CachedGroupMembers2 ON CachedGroupMembers (MemberId, GroupId, Disabled);
CREATE INDEX CachedGroupMembers3 on CachedGroupMembers (MemberId, ImmediateParentId);
+CREATE INDEX CachedGroupMembers4 ON CachedGroupMembers (Via);


CREATE SEQUENCE USERS_seq;
diff --git a/etc/schema.Pg b/etc/schema.Pg
index b614793b07..2e7a983062 100644
--- a/etc/schema.Pg
+++ b/etc/schema.Pg
@@ -332,6 +332,7 @@ CREATE TABLE CachedGroupMembers (
CREATE INDEX CachedGroupMembers2 on CachedGroupMembers (MemberId, GroupId, Disabled);
CREATE INDEX DisGrouMem on CachedGroupMembers (GroupId,MemberId,Disabled);
CREATE INDEX CachedGroupMembers3 on CachedGroupMembers (MemberId,ImmediateParentId);
+CREATE INDEX CachedGroupMembers4 ON CachedGroupMembers (Via);



diff --git a/etc/schema.SQLite b/etc/schema.SQLite
index 4b64f3349f..be08cfd925 100644
--- a/etc/schema.SQLite
+++ b/etc/schema.SQLite
@@ -233,6 +233,7 @@ create table CachedGroupMembers (
CREATE INDEX CachedGroupMembers1 ON CachedGroupMembers (GroupId, MemberId, Disabled);
CREATE INDEX CachedGroupMembers2 ON CachedGroupMembers (MemberId, GroupId, Disabled);
CREATE INDEX CachedGroupMembers3 ON CachedGroupMembers (MemberId, ImmediateParentId);
+CREATE INDEX CachedGroupMembers4 ON CachedGroupMembers (Via);

--- }}}

diff --git a/etc/schema.mysql b/etc/schema.mysql
index 382a10fc43..0c21d2b8b4 100644
--- a/etc/schema.mysql
+++ b/etc/schema.mysql
@@ -213,6 +213,7 @@ create table CachedGroupMembers (
CREATE INDEX DisGrouMem on CachedGroupMembers (GroupId,MemberId,Disabled);
CREATE INDEX CachedGroupMembers2 on CachedGroupMembers (MemberId, GroupId, Disabled);
CREATE INDEX CachedGroupMembers3 on CachedGroupMembers (MemberId, ImmediateParentId);
+CREATE INDEX CachedGroupMembers4 ON CachedGroupMembers (Via);



diff --git a/etc/upgrade/5.0.6/indexes b/etc/upgrade/5.0.6/indexes
new file mode 100644
index 0000000000..ae14e76e5a
--- /dev/null
+++ b/etc/upgrade/5.0.6/indexes
@@ -0,0 +1,9 @@
+use strict;
+use warnings;
+
+$RT::Handle->MakeSureIndexExists(
+ Table => 'CachedGroupMembers',
+ Columns => ['Via'],
+);
+
+1;

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


hooks/post-receive
--
rt
_______________________________________________
rt-commit mailing list
rt-commit@lists.bestpractical.com
https://lists.bestpractical.com/mailman/listinfo/rt-commit