[Rt-commit] rt branch, 4.2/queue-summary-cache, created. rt-4.2.13-122-g40a5437
Jim Brandt
jbrandt at bestpractical.com
Thu Mar 16 17:07:55 EDT 2017
The branch, 4.2/queue-summary-cache has been created
at 40a543773eadda64fc16393b2566e5c4bc0092cb (commit)
- Log -----------------------------------------------------------------
commit ffdc242bcc93f5adb961f9fba1653c6e73f6d688
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Thu Mar 16 11:55:37 2017 -0400
Add test for contents of user session
diff --git a/t/web/session.t b/t/web/session.t
new file mode 100644
index 0000000..dedd3dc
--- /dev/null
+++ b/t/web/session.t
@@ -0,0 +1,42 @@
+
+use strict;
+use warnings;
+
+use RT::Test tests => undef;
+
+my ($baseurl, $agent) = RT::Test->started_ok;
+
+my $url = $agent->rt_base_url;
+
+# get the top page
+{
+ $agent->get($url);
+ is ($agent->status, 200, "Loaded a page");
+}
+
+# test a login
+{
+ $agent->login('root' => 'password');
+ # the field isn't named, so we have to click link 0
+ is( $agent->status, 200, "Fetched the page ok");
+ $agent->content_contains("Logout", "Found a logout link");
+}
+
+my $ids_ref = RT::Interface::Web::Session->Ids();
+
+# Should only have one session id at this point.
+is( scalar @$ids_ref, 1, 'Got just one session id');
+
+diag 'Load session for root user';
+my %session;
+tie %session, 'RT::Interface::Web::Session', $ids_ref->[0];
+is ( $session{'_session_id'}, $ids_ref->[0], 'Got session id ' . $ids_ref->[0] );
+is ( $session{'CurrentUser'}->Name, 'root', 'Session is for root user' );
+
+diag 'Test queues cache';
+ok ( $session{'SelectObject---RT::Queue---12---CreateTicket---0'}, 'Queues cached for create ticket');
+is ( $session{'SelectObject---RT::Queue---12---CreateTicket---0'}{'objects'}->[0]{'Name'},
+ 'General', 'General queue is in cached list' );
+
+undef $agent;
+done_testing;
commit 33e92ebba03acdeefcbe7eef35c40c8b80e4d90e
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Thu Mar 16 12:51:53 2017 -0400
Move session cache code to a function for easier access
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 1393d76..dc344d2 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3907,6 +3907,102 @@ sub GetPrincipalsMap {
return @map;
}
+=head3 SetObjectSessionCache
+
+Convenience method to stash per-user query results in the user session. This is used
+for rights-intensive queries that change infrequently, such as generating the list of
+queues a user has access to.
+
+The method handles populating the session cache and clearing it based on CacheNeedsUpdate.
+It returns the cache key so callers can use $session directly after it has been created
+or updated.
+
+Parameters:
+
+=over
+
+=item * ObjectType, required, the object for which to fetch values
+
+=item * CheckRight, the right to check for the current user in the query
+
+=item * ShowAll, boolean, ignores the rights check
+
+=item * Default, for dropdowns, a default selected value
+
+=item * CacheNeedsUpdate, date indicating when an update happened requiring a cache clear
+
+=cut
+
+sub SetObjectSessionCache {
+ my %args = (
+ CheckRight => undef,
+ ShowAll => 1,
+ Default => 0,
+ CacheNeedsUpdate => undef,
+ @_ );
+
+ my $ObjectType = $args{'ObjectType'};
+ $ObjectType = "RT::$ObjectType" unless $ObjectType =~ /::/;
+ my $CheckRight = $args{'CheckRight'};
+ my $ShowAll = $args{'ShowAll'};
+ my $CacheNeedsUpdate = $args{'CacheNeedsUpdate'};
+
+ my $cache_key = join "---", "SelectObject", $ObjectType,
+ $session{'CurrentUser'}->Id, $CheckRight || "", $ShowAll;
+
+ if ( defined $session{$cache_key}
+ && ref $session{$cache_key} eq 'ARRAY') {
+ delete $session{$cache_key};
+ }
+ if ( defined $session{$cache_key} && defined $CacheNeedsUpdate &&
+ $session{$cache_key}{lastupdated} <= $CacheNeedsUpdate ) {
+ delete $session{$cache_key};
+ }
+
+ if ( not defined $session{$cache_key} ) {
+ my $collection = "${ObjectType}s"->new($session{'CurrentUser'});
+ $collection->UnLimit;
+
+ $HTML::Mason::Commands::m->callback( CallbackName => 'ModifyCollection',
+ ARGSRef => \%args, Collection => $collection, ObjectType => $ObjectType );
+
+ if ( $args{'Default'} ) {
+ my $object = $ObjectType->new($session{'CurrentUser'});
+ $object->Load( $args{'Default'} );
+ unless ( $ShowAll
+ or not $CheckRight
+ or $session{CurrentUser}->HasRight( Object => $object, Right => $CheckRight ) )
+ {
+ if ( $object->id ) {
+ push @{$session{$cache_key}{objects}}, {
+ Id => $object->id,
+ Name => '#' . $object->id,
+ Description => '#' . $object->id,
+ };
+ }
+ }
+ }
+
+ while (my $object = $collection->Next) {
+ if ($ShowAll
+ or not $CheckRight
+ or $session{CurrentUser}->HasRight( Object => $object, Right => $CheckRight ))
+ {
+ push @{$session{$cache_key}{objects}}, {
+ Id => $object->Id,
+ Name => $object->Name,
+ Description => $object->_Accessible("Description" => "read") ? $object->Description : undef,
+ };
+ }
+ }
+ $session{$cache_key}{lastupdated} = time();
+ }
+
+ return $cache_key;
+}
+
+
+
=head2 _load_container_object ( $type, $id );
Instantiate container object for saving searches.
diff --git a/share/html/Elements/SelectObject b/share/html/Elements/SelectObject
index 7a4ecdf..889a496 100644
--- a/share/html/Elements/SelectObject
+++ b/share/html/Elements/SelectObject
@@ -86,56 +86,17 @@ $Class => ""
$CacheNeedsUpdate => undef
</%args>
<%init>
-$ObjectType = "RT::$ObjectType" unless $ObjectType =~ /::/;
$Class ||= "select-" . CSSClass("\L$1") if $ObjectType =~ /RT::(.+)$/;
-my $cache_key = join "---", "SelectObject", $ObjectType,
- $session{'CurrentUser'}->Id, $CheckRight || "", $ShowAll;
-
-if ( defined $session{$cache_key} && ref $session{$cache_key} eq 'ARRAY') {
- delete $session{$cache_key};
-}
-if ( defined $session{$cache_key} && defined $CacheNeedsUpdate &&
- $session{$cache_key}{lastupdated} <= $CacheNeedsUpdate ) {
- delete $session{$cache_key};
+my $cache_key;
+if ( not $Lite ) {
+ $cache_key = SetObjectSessionCache(
+ ObjectType => $ObjectType,
+ CheckRight => $CheckRight,
+ ShowAll => $ShowAll,
+ Default => $Default,
+ CacheNeedsUpdate => $CacheNeedsUpdate,
+ );
}
-if ( not defined $session{$cache_key} and not $Lite ) {
- my $collection = "${ObjectType}s"->new($session{'CurrentUser'});
- $collection->UnLimit;
-
- $m->callback( CallbackName => 'ModifyCollection', ARGSRef => \%ARGS,
- Collection => $collection, ObjectType => $ObjectType );
-
- if ( $Default ) {
- my $object = $ObjectType->new($session{'CurrentUser'});
- $object->Load( $Default );
- unless ( $ShowAll
- or not $CheckRight
- or $session{CurrentUser}->HasRight( Object => $object, Right => $CheckRight ) )
- {
- if ( $object->id ) {
- push @{$session{$cache_key}{objects}}, {
- Id => $object->id,
- Name => '#' . $object->id,
- Description => '#' . $object->id,
- };
- }
- }
- }
-
- while (my $object = $collection->Next) {
- if ($ShowAll
- or not $CheckRight
- or $session{CurrentUser}->HasRight( Object => $object, Right => $CheckRight ))
- {
- push @{$session{$cache_key}{objects}}, {
- Id => $object->Id,
- Name => $object->Name,
- Description => $object->_Accessible("Description" => "read") ? $object->Description : undef,
- };
- }
- }
- $session{$cache_key}{lastupdated} = time();
-}
</%init>
commit 40a543773eadda64fc16393b2566e5c4bc0092cb
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Thu Mar 16 15:30:49 2017 -0400
Add caching to the queue list portlet
On larger RTs with many queues, and users attached to many
tickets over time, the rights check to generate the appropriate
queue list for a user as been observed to take a significant
amount of time (20+ seconds on one system). Cache this list
much like the queue list in the "Create ticket" dropdown since
it changes infrequently from page load to page load.
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index dc344d2..6a15dd6 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3931,6 +3931,10 @@ Parameters:
=item * CacheNeedsUpdate, date indicating when an update happened requiring a cache clear
+=item * Exclude, hashref ({ Name => 1 }) of object Names to exclude from the cache
+
+=back
+
=cut
sub SetObjectSessionCache {
@@ -3939,6 +3943,7 @@ sub SetObjectSessionCache {
ShowAll => 1,
Default => 0,
CacheNeedsUpdate => undef,
+ Exclude => undef,
@_ );
my $ObjectType = $args{'ObjectType'};
@@ -3964,8 +3969,14 @@ sub SetObjectSessionCache {
$collection->UnLimit;
$HTML::Mason::Commands::m->callback( CallbackName => 'ModifyCollection',
+ CallbackPage => '/Elements/Quicksearch',
ARGSRef => \%args, Collection => $collection, ObjectType => $ObjectType );
+ # This is included for continuity in the 4.2 series. It will be removed in 4.6.
+ $HTML::Mason::Commands::m->callback( CallbackName => 'SQLFilter',
+ CallbackPage => '/Elements/QueueSummaryByLifecycle', Queues => $collection )
+ if $ObjectType eq "RT::Queue";
+
if ( $args{'Default'} ) {
my $object = $ObjectType->new($session{'CurrentUser'});
$object->Load( $args{'Default'} );
@@ -3988,10 +3999,12 @@ sub SetObjectSessionCache {
or not $CheckRight
or $session{CurrentUser}->HasRight( Object => $object, Right => $CheckRight ))
{
+ next if $args{'Exclude'} and exists $args{'Exclude'}->{$object->Name};
push @{$session{$cache_key}{objects}}, {
Id => $object->Id,
Name => $object->Name,
Description => $object->_Accessible("Description" => "read") ? $object->Description : undef,
+ Lifecycle => $object->_Accessible("Lifecycle" => "read") ? $object->Lifecycle : undef,
};
}
}
diff --git a/share/html/Elements/QueueSummaryByLifecycle b/share/html/Elements/QueueSummaryByLifecycle
index 749c0b5..842312f 100644
--- a/share/html/Elements/QueueSummaryByLifecycle
+++ b/share/html/Elements/QueueSummaryByLifecycle
@@ -62,7 +62,7 @@
<%PERL>
my $i = 0;
-for my $queue (@queues) {
+for my $queue (@$queues) {
next if lc($queue->{Lifecycle} || '') ne lc $lifecycle->Name;
$i++;
@@ -75,7 +75,7 @@ for my $queue (@queues) {
% for my $status (@cur_statuses) {
<td align="right">
- <a href="<% $link_status->($queue, $status) %>"><% $data->{$queue->{id}}->{lc $status} || '-' %></a>
+ <a href="<% $link_status->($queue, $status) %>"><% $data->{$queue->{Id}}->{lc $status} || '-' %></a>
</td>
% }
</tr>
@@ -112,25 +112,12 @@ $m->callback(
link_status => \$link_status,
);
-my $Queues = RT::Queues->new( $session{'CurrentUser'} );
-$Queues->UnLimit();
-$m->callback( CallbackName => 'SQLFilter', Queues => $Queues );
-
-my @queues = grep $queue_filter->($_), @{ $Queues->ItemsArrayRef };
-$m->callback( CallbackName => 'Filter', Queues => \@queues );
-
- at queues = map {
- { id => $_->Id,
- Name => $_->Name,
- Description => $_->Description || '',
- Lifecycle => $_->Lifecycle,
- }
-} grep $_, @queues;
-
my %lifecycle;
-for my $queue (@queues) {
+for my $queue (@$queues) {
my $cycle = RT::Lifecycle->Load( Name => $queue->{'Lifecycle'} );
+ RT::Logger->error('Unable to load lifecycle for ' . $queue->{'Lifecycle'})
+ unless $cycle;
$lifecycle{ lc $cycle->Name } = $cycle;
}
@@ -154,18 +141,18 @@ my $query =
"(".
join(" OR ", map {"Status = '$_'"} @escaped) #'
.") AND (".
- join(' OR ', map "Queue = ".$_->{id}, @queues)
+ join(' OR ', map "Queue = ".$_->{Id}, @$queues)
.")";
-$query = 'id < 0' unless @queues;
+$query = 'id < 0' unless @$queues;
$report->SetupGroupings( Query => $query, GroupBy => [qw(Status Queue)] );
while ( my $entry = $report->Next ) {
$data->{ $entry->__Value("Queue") }->{ $entry->__Value("Status") }
- = $entry->__Value('id');
+ = $entry->__Value('Id');
$statuses->{ $entry->__Value("Status") } = 1;
}
</%INIT>
<%ARGS>
-$queue_filter => undef
+$queues => undef # Arrayref of hashes with cached queue info
@statuses => ()
</%ARGS>
diff --git a/share/html/Elements/QueueSummaryByStatus b/share/html/Elements/QueueSummaryByStatus
index 526d08a..621ec35 100644
--- a/share/html/Elements/QueueSummaryByStatus
+++ b/share/html/Elements/QueueSummaryByStatus
@@ -56,7 +56,7 @@
<%PERL>
my $i = 0;
-for my $queue (@queues) {
+for my $queue (@$queues) {
$i++;
my $lifecycle = $lifecycle{ lc $queue->{'Lifecycle'} };
my @queue_statuses = grep { $lifecycle->IsValid($_) } @statuses;
@@ -72,7 +72,7 @@ for my $queue (@queues) {
if ( $lifecycle->IsValid( $status ) ) {
</%perl>
<td align="right">
- <a href="<% $link_status->($queue, $status) %>"><% $data->{$queue->{id}}->{lc $status} || '-' %></a>
+ <a href="<% $link_status->($queue, $status) %>"><% $data->{$queue->{Id}}->{lc $status} || '-' %></a>
</td>
% } else {
<td align="right">-</td>
@@ -108,25 +108,12 @@ $m->callback(
link_status => \$link_status,
);
-my $Queues = RT::Queues->new( $session{'CurrentUser'} );
-$Queues->UnLimit();
-$m->callback( CallbackName => 'SQLFilter', Queues => $Queues );
-
-my @queues = grep $queue_filter->($_), @{ $Queues->ItemsArrayRef };
-$m->callback( CallbackName => 'Filter', Queues => \@queues );
-
- at queues = map {
- { id => $_->Id,
- Name => $_->Name,
- Description => $_->Description || '',
- Lifecycle => $_->Lifecycle,
- }
-} grep $_, @queues;
-
my %lifecycle;
-for my $queue (@queues) {
+for my $queue (@$queues) {
my $cycle = RT::Lifecycle->Load( Name => $queue->{'Lifecycle'} );
+ RT::Logger->error('Unable to load lifecycle for ' . $queue->{'Lifecycle'})
+ unless $cycle;
$lifecycle{ lc $cycle->Name } = $cycle;
}
@@ -148,9 +135,9 @@ my $query =
"(".
join(" OR ", map {s{(['\\])}{\\$1}g; "Status = '$_'"} @statuses) #'
.") AND (".
- join(' OR ', map "Queue = ".$_->{id}, @queues)
+ join(' OR ', map "Queue = ".$_->{Id}, @$queues)
.")";
-$query = 'id < 0' unless @queues;
+$query = 'id < 0' unless @$queues;
$report->SetupGroupings( Query => $query, GroupBy => [qw(Status Queue)] );
while ( my $entry = $report->Next ) {
@@ -160,6 +147,6 @@ while ( my $entry = $report->Next ) {
}
</%INIT>
<%ARGS>
-$queue_filter => undef
+$queues => undef
@statuses => ()
</%ARGS>
diff --git a/share/html/Elements/Quicksearch b/share/html/Elements/Quicksearch
index 316c483..2792c3b 100644
--- a/share/html/Elements/Quicksearch
+++ b/share/html/Elements/Quicksearch
@@ -53,13 +53,22 @@
titleright_href => RT->Config->Get('WebPath').'/Prefs/Quicksearch.html',
&>
<& $comp,
- queue_filter => sub { $_->CurrentUserHasRight('ShowTicket') && !exists $unwanted->{$_->Name} },
+ queues => $session{$cache_key}{objects},
&>
</&>
</div>
<%INIT>
my $unwanted = $session{'CurrentUser'}->UserObj->Preferences('QuickSearch', {});
my $comp = $SplitByLifecycle? '/Elements/QueueSummaryByLifecycle' : '/Elements/QueueSummaryByStatus';
+
+my $cache_key = SetObjectSessionCache(
+ ObjectType => 'RT::Queue',
+ CheckRight => 'ShowTicket',
+ ShowAll => 0,
+ CacheNeedsUpdate => RT->System->QueueCacheNeedsUpdate,
+ Exclude => $unwanted,
+);
+
</%INIT>
<%ARGS>
$SplitByLifecycle => 1
diff --git a/share/html/Prefs/Quicksearch.html b/share/html/Prefs/Quicksearch.html
index 920d457..fc82d86 100644
--- a/share/html/Prefs/Quicksearch.html
+++ b/share/html/Prefs/Quicksearch.html
@@ -102,6 +102,8 @@ if ($ARGS{'Save'}) {
}
}
+ RT->System->QueueCacheNeedsUpdate(1);
+
my ($ok, $msg) = $user->SetPreferences('QuickSearch', $unwanted);
push @actions, $ok ? loc('Preferences saved.') : $msg;
}
-----------------------------------------------------------------------
More information about the rt-commit
mailing list