[Rt-commit] rt branch 5.0/ticket-acl-convert-or-to-in created. rt-5.0.3-143-g52f4823e44

[BPS Git Server]

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/ticket-acl-convert-or-to-in has been created
        at  52f4823e44272e0d9135703c72e57f654ec2c1ee (commit)

- Log -----------------------------------------------------------------
commit 52f4823e44272e0d9135703c72e57f654ec2c1ee
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Fri Nov 4 05:05:45 2022 +0800

    Convert OR'd role group names for ticket ACL check to IN
    
    E.g. if "ShowTicket" is granted to "AdminCc" and "Requestor" on queue
    "General", part of the ACL check SQL was like:
    
        (
            CachedGroupMembers_2.MemberId IS NOT NULL
            AND LOWER(Groups_1.Name) = 'admincc'
            AND main.Queue IN ('1')
        )
        OR
        (
            CachedGroupMembers_2.MemberId IS NOT NULL
            AND LOWER(Groups_1.Name) = 'requestor'
            AND main.Queue IN ('1')
        )
    
    This commit changes it to:
    
        (
            CachedGroupMembers_2.MemberId IS NOT NULL
            AND LOWER(Groups_1.Name) IN ('adminCc','requestor')
            AND main.Queue IN ('1')
        )
    
    This can simplify SQL and thus improve performance when "ShowTicket" is
    granted to a lot of roles.

diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 183932117f..c5deb89d1b 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -2868,7 +2868,43 @@ sub CurrentUserCanSee {
         $self->SUPER::_OpenParen('ACL');
         my $ea = 'AND';
         $ea = 'OR' if $limit_queues->( $ea, @direct_queues );
+
+        # Merge roles if possible, so the SQL can be tweaked from:
+        #
+        #    (
+        #        CachedGroupMembers_2.MemberId IS NOT NULL
+        #        AND LOWER(Groups_1.Name) = 'admincc'
+        #        AND main.Queue IN ('1')
+        #    )
+        #    OR
+        #    (
+        #        CachedGroupMembers_2.MemberId IS NOT NULL
+        #        AND LOWER(Groups_1.Name) = 'requestor'
+        #        AND main.Queue IN ('1')
+        #    )
+        #
+        # to:
+        #    (
+        #        CachedGroupMembers_2.MemberId IS NOT NULL
+        #        AND LOWER(Groups_1.Name) IN ('adminCc','requestor')
+        #        AND main.Queue IN ('1')
+        #    )
+        my $stringify_queues = sub {
+            my $queues = shift or return '';
+            # not array means global
+            return ref $queues ? join( ',', @$queues ) : 'global';
+        };
+
+        my %queue;
+        while ( my ( $role, $queues ) = each %roles ) {
+            next if $role eq 'Owner';
+            push @{ $queue{ $stringify_queues->($queues) } }, lc $role;
+        }
+
+        my %queue_applied;
         while ( my ($role, $queues) = each %roles ) {
+            next if $role ne 'Owner' && $queue_applied{ $stringify_queues->($queues) }++;
+
             $self->SUPER::_OpenParen('ACL');
             if ( $role eq 'Owner' ) {
                 $self->Limit(
@@ -2879,6 +2915,8 @@ sub CurrentUserCanSee {
                 );
             }
             else {
+                my $roles = $queue{ $stringify_queues->($queues) };
+
                 $self->Limit(
                     SUBCLAUSE       => 'ACL',
                     ALIAS           => $cgm_alias,
@@ -2892,7 +2930,9 @@ sub CurrentUserCanSee {
                     SUBCLAUSE       => 'ACL',
                     ALIAS           => $role_group_alias,
                     FIELD           => 'Name',
-                    VALUE           => $role,
+                    FUNCTION        => 'LOWER(?)',
+                    VALUE           => $roles,
+                    OPERATOR        => 'IN',
                     ENTRYAGGREGATOR => 'AND',
                     CASESENSITIVE   => 0,
                 );

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


hooks/post-receive
-- 
rt