[Rt-commit] rt branch, 4.4-trunk, updated. rt-4.4.4-425-g23270aa967

[Jim Brandt]

The branch, 4.4-trunk has been updated
       via  23270aa9679e756c18745a268b2e1ca6103dda39 (commit)
       via  6a7f1beb71dd4484ab55eea3254e29c1a5017342 (commit)
       via  cd949cc2568c5bf9df4eb8139f9d6d8311842a6a (commit)
       via  3c3727434c2b37f36d1170c64745887e02a0d220 (commit)
       via  53725266941c2aca7522ddea7439da52c1e899d1 (commit)
       via  a6e66eff7de39dbbf257916fec1cf0f775b27739 (commit)
      from  4b7c5afbab153bae1faed2662eef484643907fca (commit)

Summary of changes:
 lib/RT/CustomRole.pm             | 20 +++++++++++---------
 lib/RT/Record.pm                 |  1 +
 lib/RT/Record/Role/Roles.pm      | 12 +++++++++++-
 t/customroles/existing-tickets.t |  1 +
 t/customroles/rights.t           |  9 ++++++---
 5 files changed, 30 insertions(+), 13 deletions(-)

- Log -----------------------------------------------------------------
commit 6a7f1beb71dd4484ab55eea3254e29c1a5017342
Merge: 4b7c5afbab cd949cc256
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri May 7 10:52:53 2021 -0400

    Merge branch '4.4/cache-roles' into 4.4-trunk

diff --cc lib/RT/CustomRole.pm
index 085c03d3a3,cdcfbcf1a1..8ec00efdd3
--- a/lib/RT/CustomRole.pm
+++ b/lib/RT/CustomRole.pm
@@@ -202,21 -201,18 +201,20 @@@ sub _RegisterAsRole 
                  return 1;
              }
  
-             # for callers not specific to any queue, e.g. ColumnMap
-             if (!ref($object)) {
-                 return 1;
-             }
+             # reload the role to avoid capturing $self across requests
+             my $role = RT::CustomRole->new(RT->SystemUser);
+             $role->Load($id);
  
 -            # custom roles apply to queues, so canonicalize a ticket
 -            # into its queue
 -            if ($object->isa('RT::Ticket')) {
 -                $object = $object->QueueObj;
 -            }
 +            if ( $object->isa('RT::Ticket') || $object->isa('RT::Queue') ) {
 +                return 0 unless $object->CurrentUserHasRight('SeeQueue');
  
 -            if ($object->isa('RT::Queue')) {
 -                return $role->IsAdded($object->Id);
 +                # custom roles apply to queues, so canonicalize a ticket
 +                # into its queue
 +                if ( $object->isa('RT::Ticket') ) {
 +                    $object = $object->QueueObj;
 +                }
 +
 +                return $role->IsAdded( $object->Id );
              }
  
              return 0;
@@@ -687,11 -684,24 +685,15 @@@ sub SetDisabled 
          return ($ok, $msg);
      }
  
 -    # we can't unconditionally re-enable all role groups because
 -    # if you add a role to queues A and B, add users and privileges and
 -    # tickets on both, remove the role from B, disable the role, then re-enable
 -    # the role, we shouldn't re-enable B because it's still removed
 -    my $queues = $self->AddedTo;
 -    while (my $queue = $queues->Next) {
 -        $self->_SetGroupsDisabledForQueue($value, $queue);
 -    }
 -
 -    $RT::Handle->Commit();
 +    RT::Principal->InvalidateACLCache();
  
      if ( $value == 0 ) {
+         $self->_RegisterAsRole;
+         RT->System->CustomRoleCacheNeedsUpdate(1);
          return (1, $self->loc("Custom role enabled"));
      } else {
+         $self->_UnregisterAsRole;
+         RT->System->CustomRoleCacheNeedsUpdate(1);
          return (1, $self->loc("Custom role disabled"));
      }
  }
diff --cc lib/RT/Record/Role/Roles.pm
index 9dcd141726,ef628e5978..4d4723a66a
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@@ -290,8 -290,15 +293,15 @@@ sub Roles 
                  $ok }
              grep { !$_->[1]{AppliesToObjectPredicate}
                   or $_->[1]{AppliesToObjectPredicate}->($self) }
 -             map { [ $_, $self->Role($_) ] }
 +             map { [ $_, $self->_ROLES->{$_} ] }
              keys %{ $self->_ROLES };
+ 
+     # Cache at ticket/queue object level mainly to reduce calls of
+     # custom role's AppliesToObjectPredicate for performance.
+     if ( ref($self) =~ /RT::(?:Ticket|Queue)/ ) {
+         $self->{_Roles}{$key} = \@roles;
+     }
+     return @roles;
  }
  
  {

commit 23270aa9679e756c18745a268b2e1ca6103dda39
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri May 7 11:21:58 2021 -0400

    Reload test tickets to clear cached role values

diff --git a/t/customroles/rights.t b/t/customroles/rights.t
index c7d71b51ef..3f546d4700 100644
--- a/t/customroles/rights.t
+++ b/t/customroles/rights.t
@@ -80,7 +80,8 @@ sub sales_has_rights_for_inbox_individual {
     my $has_right = shift;
     my $rationale = shift || '';
 
-    my $t = $inbox_individual;
+    my $t = RT::Ticket->new( RT->SystemUser );
+    $t->Load($inbox_individual->Id);
 
     if ($has_right) {
         is($t->RoleAddresses($sales->GroupType), (join ', ', sort $moss->EmailAddress, $ricky->EmailAddress), 'got salespeople');
@@ -129,7 +130,8 @@ sub sales_has_rights_for_inbox_group {
     my $has_right = shift;
     my $rationale = shift || '';
 
-    my $t = $inbox_group;
+    my $t = RT::Ticket->new( RT->SystemUser );
+    $t->Load($inbox_group->Id);
 
     if ($has_right) {
         is($t->RoleAddresses($sales->GroupType), (join ', ', sort $moss->EmailAddress, $ricky->EmailAddress, $blake->EmailAddress, $williamson->EmailAddress), 'got all salespeople');
@@ -161,7 +163,8 @@ sub sales_has_rights_for_specs_individual {
     my $has_right = shift;
     my $rationale = shift || '';
 
-    my $t = $specs_individual;
+    my $t = RT::Ticket->new( RT->SystemUser );
+    $t->Load($specs_individual->Id);
 
     if (!$has_right) {
         is($t->RoleAddresses($sales->GroupType), '', "got no salespeople $rationale");

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