[Rt-commit] rt branch, 4.4/lazy-role-groups, created. rt-4.4.4-107-g54b0e2a99c
Jim Brandt
jbrandt at bestpractical.com
Fri May 7 15:46:31 EDT 2021
The branch, 4.4/lazy-role-groups has been created
at 54b0e2a99c5aca4a404871852472531564f011dd (commit)
- Log -----------------------------------------------------------------
commit d2061ce0e21f24c3f7bc389934eb14d2e715ecb5
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Jun 2 03:26:33 2020 +0800
Support to create ticket/asset role groups lazily
For objects with a large number of custom roles, creating
all necessary role groups at object create time can add a
large number of additional create operations in the DB,
impacting performance. Creating them later when needed
reduces time to create the object.
Since role groups may not exist, we need to use left join in searches.
diff --git a/lib/RT/Asset.pm b/lib/RT/Asset.pm
index d795759913..28f749b0b7 100644
--- a/lib/RT/Asset.pm
+++ b/lib/RT/Asset.pm
@@ -192,6 +192,13 @@ by ID.
Any of these link types accept either a single value or arrayref of values
parseable by L<RT::URI>.
+=item LazyRoleGroups
+
+Boolean value that controls how role groups are created:
+
+1: create when necessary (lazy, default value)
+0: create immediately
+
=back
Returns a tuple of (status, msg) on failure and (id, msg, non-fatal errors) on
@@ -212,6 +219,7 @@ sub Create {
Contact => undef,
Status => undef,
+ LazyRoleGroups => 1,
@_
);
my @non_fatal_errors;
@@ -270,11 +278,13 @@ sub Create {
# Let users who just created an asset see it until the end of this method.
$self->{_object_is_readable} = 1;
- # Create role groups
- unless ($self->_CreateRoleGroups()) {
- RT->Logger->error("Couldn't create role groups for asset ". $self->id);
- RT->DatabaseHandle->Rollback();
- return (0, $self->loc("Couldn't create role groups for asset"));
+ if ( !$args{LazyRoleGroups} ) {
+ # Create role groups
+ unless ($self->_CreateRoleGroups()) {
+ RT->Logger->error("Couldn't create role groups for asset ". $self->id);
+ RT->DatabaseHandle->Rollback();
+ return (0, $self->loc("Couldn't create role groups for asset"));
+ }
}
# Figure out users for roles
diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index b973eb21b1..ff5d5bf37f 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -692,7 +692,17 @@ sub _AddRolesOnCreate {
my $changed = 0;
for my $role (keys %{$roles}) {
+ next unless @{$roles->{$role}};
+
my $group = $self->RoleGroup($role);
+ if ( !$group->id ) {
+ $group = $self->_CreateRoleGroup($role);
+ if ( !$group || !$group->id ) {
+ push @errors, $self->loc( "Couldn't create role group '[_1]'", $role );
+ next;
+ }
+ }
+
my @left;
for my $principal (@{$roles->{$role}}) {
if ($acls{$role}->($principal)) {
diff --git a/lib/RT/SearchBuilder/Role/Roles.pm b/lib/RT/SearchBuilder/Role/Roles.pm
index 5980882d31..703b2c979d 100644
--- a/lib/RT/SearchBuilder/Role/Roles.pm
+++ b/lib/RT/SearchBuilder/Role/Roles.pm
@@ -360,7 +360,7 @@ sub RoleLimit {
# positive condition case
$group_members ||= $self->_GroupMembersJoin(
- GroupsAlias => $groups, New => 1, Left => 0
+ GroupsAlias => $groups, New => 1,
);
if ($args{FIELD} eq "id") {
# Save a left join to Users, if possible
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 81224831cc..653d08d02a 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -192,6 +192,7 @@ Arguments: ARGS is a hash of named parameters. Valid parameters are:
Due -- an ISO date describing the ticket's due date and time in GMT
MIMEObj -- a MIME::Entity object with the content of the initial ticket request.
CustomField-<n> -- a scalar or array of values for the customfield with the id <n>
+ LazyRoleGroups -- boolean, create role groups immediately (0) or just when necessary (1, default)
Ticket links can be set up during create by passing the link type as a hask key and
the ticket id to be linked to as a value (or a URI when linking to other objects).
@@ -239,6 +240,7 @@ sub Create {
SLA => undef,
MIMEObj => undef,
_RecordTransaction => 1,
+ LazyRoleGroups => 1,
@_
);
@@ -439,15 +441,17 @@ sub Create {
}
# Create (empty) role groups
- my $create_groups_ret = $self->_CreateRoleGroups();
- unless ($create_groups_ret) {
- $RT::Logger->crit( "Couldn't create ticket groups for ticket "
- . $self->Id
- . ". aborting Ticket creation." );
- $RT::Handle->Rollback();
- return ( 0, 0,
- $self->loc("Ticket could not be created due to an internal error")
- );
+ if ( !$args{LazyRoleGroups} ) {
+ my $create_groups_ret = $self->_CreateRoleGroups();
+ unless ($create_groups_ret) {
+ $RT::Logger->crit( "Couldn't create ticket groups for ticket "
+ . $self->Id
+ . ". aborting Ticket creation." );
+ $RT::Handle->Rollback();
+ return ( 0, 0,
+ $self->loc("Ticket could not be created due to an internal error")
+ );
+ }
}
# Codify what it takes to add each kind of group
commit 35b0c0208f9728fb4c266231e51aac2465da3994
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Jun 2 03:32:58 2020 +0800
Default PrincipalId to 0 to avoid SQL error for not-existing role groups
The RoleGroup method returns an empty group object when current user
doesn't have right or the corresponding role group doesn't exist. This
could result in SQL syntax errors when calling methods like MembersObj:
SELECT main.* FROM GroupMembers main WHERE (main.GroupId = ) ORDER BY main.id ASC
Defaulting PrincipalId to 0 could avoid SQL errors like above and return
correct empty results.
diff --git a/lib/RT/Group.pm b/lib/RT/Group.pm
index 19a41ef7a9..350fcaee2e 100644
--- a/lib/RT/Group.pm
+++ b/lib/RT/Group.pm
@@ -1295,7 +1295,7 @@ Returns this user's PrincipalId
sub PrincipalId {
my $self = shift;
- return $self->Id;
+ return $self->Id || 0;
}
sub InstanceObj {
commit 54b0e2a99c5aca4a404871852472531564f011dd
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Jun 2 03:38:16 2020 +0800
Update tests for the lazy role groups change
diff --git a/t/api/ticket.t b/t/api/ticket.t
index cfba3e97bf..fbd18b1a16 100644
--- a/t/api/ticket.t
+++ b/t/api/ticket.t
@@ -112,7 +112,8 @@ my ($id, $msg) = $ticket->Create(Subject => "Foo",
Owner => RT->SystemUser->Id,
Status => 'open',
Requestor => ['jesse at example.com'],
- Queue => '1'
+ Queue => '1',
+ LazyRoleGroups => 0,
);
ok ($id, "Ticket $id was created");
ok(my $group = $ticket->RoleGroup('Requestor'));
@@ -343,4 +344,23 @@ diag("Test ticket types with different cases");
is($t->Type, "approval", "Approvals, the third and final internal type, are also lc'd during Create");
}
+diag("Test LazyRoleGroups");
+{
+ my $t = RT::Ticket->new( RT->SystemUser );
+ my ($ok) = $t->Create(
+ Queue => 'General',
+ Subject => 'LazyRoleGroup test',
+ Requestor => 'root at localhost',
+ );
+ ok( $t->RoleGroup($_)->Id, "$_ role group is created" ) for qw/Requestor Owner/;
+
+ for my $role (qw/Cc AdminCc/) {
+ ok( !$t->RoleGroup($role)->Id, "$role role group is not created yet" );
+ ok( $t->AddWatcher( Type => $role, Email => 'root at localhost' ) );
+ my $role_group = $t->RoleGroup($role);
+ ok( $role_group->Id, "$role role group is created" );
+ is( $role_group->MemberEmailAddressesAsString, 'root at localhost', "$role role group has the member root" );
+ }
+}
+
done_testing;
diff --git a/t/assets/roles.t b/t/assets/roles.t
index 1d8a647aa3..b6baa6cd65 100644
--- a/t/assets/roles.t
+++ b/t/assets/roles.t
@@ -4,7 +4,7 @@ use warnings;
use RT::Test::Assets tests => undef;
my $catalog = create_catalog( Name => "A catalog" );
-my $asset = create_asset( Name => "Test asset", Catalog => $catalog->id );
+my $asset = create_asset( Name => "Test asset", Catalog => $catalog->id, LazyRoleGroups => 0 );
ok $asset && $asset->id, "Created asset";
for my $object ($asset, $catalog, RT->System) {
diff --git a/t/web/download_user_info.t b/t/web/download_user_info.t
index 44155d49a7..bf41a899ba 100644
--- a/t/web/download_user_info.t
+++ b/t/web/download_user_info.t
@@ -62,9 +62,9 @@ EOF
my $transaction_info_tsv = <<EOF;
Ticket Id\tid\tCreated\tDescription\tOldValue\tNewValue\tContent
-1\t30\t$date_created\tTicket created\t\t\tThis transaction appears to have no content
-1\t32\t$date_commented\tComments added\t\t\tTest - Comment
-1\t33\t$date_correspondence\tCorrespondence added\t\t\tTest - Reply
+1\t28\t$date_created\tTicket created\t\t\tThis transaction appears to have no content
+1\t30\t$date_commented\tComments added\t\t\tTest - Comment
+1\t31\t$date_correspondence\tCorrespondence added\t\t\tTest - Reply
EOF
is $agent->content, $transaction_info_tsv,
-----------------------------------------------------------------------
More information about the rt-commit
mailing list