[Rt-commit] rt branch, 4.0/cache-empty-preferences, created. rt-4.0.20-46-g5b707df

[Alex Vandiver]

The branch, 4.0/cache-empty-preferences has been created
        at  5b707dfe1f7d6fb7666d80ad6e6d566849a6ea84 (commit)

- Log -----------------------------------------------------------------
commit a273c51b334a3d760483ac3a0f53b049ecb389b4
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 7 10:50:19 2014 -0400

    Use cached attribute list when fetching preferences
    
    Attributes have, since their initial implementation, use
    LoadByNameAndObject.  However, when the user does not have any
    preferences stored, this can be very expensive, as
    DBIx::SearchBuilder::Record::Cachable does not cache negative results --
    that is, that the matching Attribute row does not exist.  This means
    that every request for RT->Config->Get( '...', $session{CurrentUser})
    results in an identical database query.
    
    Use the ->Attributes method, which caches the set of user attributes.
    This, in itself, is not sufficient to cause the preferences to be
    cached; the the next commit.

diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 20ce784..f8b7017 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -1346,10 +1346,8 @@ sub Preferences {
     my $name = _PrefName (shift);
     my $default = shift;
 
-    my $attr = RT::Attribute->new( $self->CurrentUser );
-    $attr->LoadByNameAndObject( Object => $self, Name => $name );
-
-    my $content = $attr->Id ? $attr->Content : undef;
+    my ($attr) = $self->Attributes->Named( $name );
+    my $content = $attr ? $attr->Content : undef;
     unless ( ref $content eq 'HASH' ) {
         return defined $content ? $content : $default;
     }
@@ -1378,9 +1376,8 @@ sub SetPreferences {
     return (0, $self->loc("No permission to set preferences"))
         unless $self->CurrentUserCanModify('Preferences');
 
-    my $attr = RT::Attribute->new( $self->CurrentUser );
-    $attr->LoadByNameAndObject( Object => $self, Name => $name );
-    if ( $attr->Id ) {
+    my ($attr) = $self->Attributes->Named( $name );
+    if ( $attr ) {
         my ($ok, $msg) = $attr->SetContent( $value );
         return (1, "No updates made")
             if $msg eq "That is already the current value";
@@ -1403,9 +1400,8 @@ sub DeletePreferences {
     return (0, $self->loc("No permission to set preferences"))
         unless $self->CurrentUserCanModify('Preferences');
 
-    my $attr = RT::Attribute->new( $self->CurrentUser );
-    $attr->LoadByNameAndObject( Object => $self, Name => $name );
-    if ( $attr->Id ) {
+    my ($attr) = $self->Attributes->Named( $name );
+    if ( $attr ) {
         return $attr->Delete;
     }
 

commit 5b707dfe1f7d6fb7666d80ad6e6d566849a6ea84
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Jul 7 11:49:17 2014 -0400

    Cache Attributes on the CurrentUser object, not an ephemeral User object
    
    The previous commit moved preferences to using the Attributes cache;
    however, this gains little because the cache is on the User object
    derived from the CurrentUser, which is derived afresh each time that
    RT->Config->Get() is called.
    
    Call the ->Preferences method directly on the CurrentUser, instead of
    the desrived User, so the cache is on the more durable CurrentUser
    object.  The cache is cleared (via ->ClearAttributes) during the ->Load
    call at the start of every request.  ->DeletePreferences is also
    switched to use ->DeleteAttribute, which explicitly purges the cache;
    otherwise, it may return stale data.
    
    This better attribute caching ensures that even negative preferences
    loads are cached for the duration of the request.  This, in turn,
    eliminates a large number of repeated database queries (such as for the
    username format preference) for users with no explicit preferences set.

diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
index d33192c..bb5174a 100644
--- a/lib/RT/Config.pm
+++ b/lib/RT/Config.pm
@@ -1013,7 +1013,6 @@ sub Get {
 
     my $res;
     if ( $user && $user->id && $META{$name}->{'Overridable'} ) {
-        $user = $user->UserObj if $user->isa('RT::CurrentUser');
         my $prefs = $user->Preferences($RT::System);
         $res = $prefs->{$name} if $prefs;
     }
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index f8b7017..f85cd4d 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -1400,12 +1400,11 @@ sub DeletePreferences {
     return (0, $self->loc("No permission to set preferences"))
         unless $self->CurrentUserCanModify('Preferences');
 
-    my ($attr) = $self->Attributes->Named( $name );
-    if ( $attr ) {
-        return $attr->Delete;
-    }
+    my ($attr) = $self->DeleteAttribute( $name );
+    return (0, $self->loc("Preferences were not found"))
+        unless $attr;
 
-    return (0, $self->loc("Preferences were not found"));
+    return 1;
 }
 
 =head2 Stylesheet

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