Skip to content

Commit f22f1b1

Browse files
committed
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.
1 parent ffdaf3e commit f22f1b1

File tree

11 files changed

+22
-47
lines changed

11 files changed

+22
-47
lines changed

etc/schema.Oracle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ CREATE TABLE CachedGroupMembers (
199199
MemberId NUMBER(11,0),
200200
Via NUMBER(11,0),
201201
ImmediateParentId NUMBER(11,0),
202-
Disabled NUMBER(11,0) DEFAULT 0 NOT NULL
202+
Disabled NUMBER(11,0) DEFAULT 0 NOT NULL,
203+
FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE
203204
);
204205
CREATE INDEX DisGrouMem ON CachedGroupMembers (GroupId, MemberId, Disabled);
205206
CREATE INDEX CachedGroupMembers2 ON CachedGroupMembers (MemberId, GroupId, Disabled);

etc/schema.Pg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ CREATE TABLE CachedGroupMembers (
325325
Via int,
326326
ImmediateParentId int,
327327
Disabled integer NOT NULL DEFAULT 0 ,
328+
FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE,
328329
PRIMARY KEY (id)
329330

330331
);

etc/schema.SQLite

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,12 @@ create table CachedGroupMembers (
221221
MemberId int,
222222
Via int,
223223
ImmediateParentId int,
224-
Disabled int2 NOT NULL DEFAULT 0 # if this cached group member is a member of this group by way of a disabled
224+
Disabled int2 NOT NULL DEFAULT 0, # if this cached group member is a member of this group by way of a disabled
225225
# group or this group is disabled, this will be set to 1
226226
# this allows us to not find members of disabled subgroups when listing off
227227
# group members recursively.
228228
# Also, this allows us to have the ACL system elide members of disabled groups
229+
FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE
229230

230231

231232
) ;

etc/schema.mysql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ create table CachedGroupMembers (
207207
# this allows us to not find members of disabled subgroups when listing off
208208
# group members recursively.
209209
# Also, this allows us to have the ACL system elide members of disabled groups
210+
FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE,
210211
PRIMARY KEY (id)
211212
) ENGINE=InnoDB CHARACTER SET utf8mb4;
212213

etc/upgrade/5.0.6/schema.Oracle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE CachedGroupMembers ADD FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE;

etc/upgrade/5.0.6/schema.Pg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE CachedGroupMembers ADD FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE;

etc/upgrade/5.0.6/schema.mysql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE CachedGroupMembers ADD FOREIGN KEY (Via) REFERENCES CachedGroupMembers(id) ON DELETE CASCADE;

lib/RT/CachedGroupMember.pm

Lines changed: 7 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ sub Create {
9999
my %args = ( Group => '',
100100
Member => '',
101101
ImmediateParent => '',
102-
Via => '0',
102+
Via => undef,
103103
Disabled => '0',
104104
@_ );
105105

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

182187

183188

184-
=head2 Delete
185-
186-
Deletes the current CachedGroupMember from the group it's in and
187-
cascades the delete to all submembers.
188-
189-
=cut
190-
191-
sub Delete {
192-
my $self = shift;
193-
194-
195-
my $member = $self->MemberObj();
196-
if ( $member->IsGroup ) {
197-
my $deletable = RT::CachedGroupMembers->new( $self->CurrentUser );
198-
199-
$deletable->Limit( FIELD => 'id',
200-
OPERATOR => '!=',
201-
VALUE => $self->id );
202-
$deletable->Limit( FIELD => 'Via',
203-
OPERATOR => '=',
204-
VALUE => $self->id );
205-
206-
while ( my $kid = $deletable->Next ) {
207-
my ($ok, $msg) = $kid->Delete();
208-
unless ($ok) {
209-
$RT::Logger->error(
210-
"Couldn't delete CachedGroupMember " . $kid->Id );
211-
return ($ok, $msg);
212-
}
213-
}
214-
}
215-
my ($ok, $msg) = $self->SUPER::Delete();
216-
$RT::Logger->error( "Couldn't delete CachedGroupMember " . $self->Id ) unless $ok;
217-
return ($ok, $msg);
218-
}
219-
220-
221-
222189
=head2 SetDisabled
223190
224191
SetDisableds the current CachedGroupMember from the group it's in and cascades

lib/RT/GroupMember.pm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ sub _InsertCGM {
103103
Member => $self->MemberObj,
104104
Group => $self->GroupObj,
105105
ImmediateParent => $self->GroupObj,
106-
Via => '0'
106+
Via => undef,
107107
);
108108

109109

@@ -289,7 +289,7 @@ sub _StashUser {
289289
Member => $args{'Member'},
290290
Group => $args{'Group'},
291291
ImmediateParent => $args{'Group'},
292-
Via => '0'
292+
Via => undef,
293293
);
294294

295295
unless ($cached_id) {

lib/RT/Handle.pm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ sub Connect {
154154
}
155155
elsif ( $db_type eq 'SQLite' ) {
156156
$self->dbh->{sqlite_see_if_its_a_number} = 1;
157+
$self->dbh->do('PRAGMA foreign_keys = ON'); # ON DELETE CASCADE for CachedGroupMembers needs it
157158
}
158159

159160
$self->dbh->{'LongReadLen'} = RT->Config->Get('MaxAttachmentSize');

0 commit comments

Comments
 (0)