[Rt-commit] rt branch, reuse_merged, updated. rt-3.8.6-96-gd4ed191

Ruslan Zakirov ruz at bestpractical.com
Tue Nov 17 12:24:18 EST 2009


The branch, reuse_merged has been updated
       via  d4ed191b704cf494ac459177d90c8b406e302716 (commit)
       via  43a6f209352e771bfdcb98fc1ba41ddf46a1c416 (commit)
       via  c376d4f5c954fd657e22f8b7e6bfc89621496295 (commit)
       via  94347470db7586c4e06a97e482270d3a1a1ad112 (commit)
       via  11a07c3180e6fa56c2fd3a106649bfe3ac8c56e8 (commit)
       via  47cf4391e703388cb5a0f95c132d8674cb3c1041 (commit)
      from  054c5f3bb927914c91a167b8f749ef5a69937673 (commit)

Summary of changes:
 lib/RT/Interface/Web/Handler.pm |    2 +
 lib/RT/Ticket_Overlay.pm        |  119 ++++++++++++++++++++++----------------
 2 files changed, 71 insertions(+), 50 deletions(-)

- Log -----------------------------------------------------------------
commit 47cf4391e703388cb5a0f95c132d8674cb3c1041
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Nov 17 17:40:43 2009 +0300

    refactor and improve _Links sub in RT::Ticket
    
    * we can use Merged method here
    * we can use Local* fields for tickets
    * apply 'id = 0' if user has no rights to see this ticket.
      it protects us from later limits

diff --git a/lib/RT/Ticket_Overlay.pm b/lib/RT/Ticket_Overlay.pm
index a8d32ef..877dc02 100755
--- a/lib/RT/Ticket_Overlay.pm
+++ b/lib/RT/Ticket_Overlay.pm
@@ -2202,28 +2202,36 @@ sub _Links {
     my $field = shift;
     my $type  = shift || "";
 
-    unless ( $self->{"$field$type"} ) {
-        $self->{"$field$type"} = new RT::Links( $self->CurrentUser );
-        if ( $self->CurrentUserHasRight('ShowTicket') ) {
-            # Maybe this ticket is a merged ticket
-            my $Tickets = new RT::Tickets( $self->CurrentUser );
-            # at least to myself
-            $self->{"$field$type"}->Limit( FIELD => $field,
-                                           VALUE => $self->URI,
-                                           ENTRYAGGREGATOR => 'OR' );
-            $Tickets->Limit( FIELD => 'EffectiveId',
-                             VALUE => $self->EffectiveId );
-            while (my $Ticket = $Tickets->Next) {
-                $self->{"$field$type"}->Limit( FIELD => $field,
-                                               VALUE => $Ticket->URI,
-                                               ENTRYAGGREGATOR => 'OR' );
-            }
-            $self->{"$field$type"}->Limit( FIELD => 'Type',
-                                           VALUE => $type )
-              if ($type);
-        }
+    my $cache_key = "$field$type";
+    return $self->{ $cache_key } if $self->{ $cache_key };
+
+    $RT::Logger->error( "links field: $field, type: $type" );
+    my $links = $self->{ $cache_key }
+              = RT::Links->new( $self->CurrentUser );
+    unless ( $self->CurrentUserHasRight('ShowTicket') ) {
+        $links->Limit( FIELD => 'id', VALUE => 0 );
+        return $links;
     }
-    return ( $self->{"$field$type"} );
+
+    # Maybe this ticket is a merge ticket
+    my $limit_on = 'Local'. $field;
+    # at least to myself
+    $links->Limit(
+        FIELD           => $limit_on,
+        VALUE           => $self->id,
+        ENTRYAGGREGATOR => 'OR',
+    );
+    $links->Limit(
+        FIELD           => $limit_on,
+        VALUE           => $_,
+        ENTRYAGGREGATOR => 'OR',
+    ) foreach $self->Merged;
+    $links->Limit(
+        FIELD => 'Type',
+        VALUE => $type,
+    ) if $type;
+
+    return $links;
 }
 
 # }}}
@@ -2640,7 +2648,7 @@ Returns list of tickets' ids that's been merged into this ticket.
 sub Merged {
     my $self = shift;
 
-    my $mergees = new RT::Tickets( $self->CurrentUser );
+    my $mergees = RT::Tickets->new( $self->CurrentUser );
     $mergees->Limit(
         FIELD    => 'EffectiveId',
         OPERATOR => '=',

commit 11a07c3180e6fa56c2fd3a106649bfe3ac8c56e8
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Nov 17 19:51:04 2009 +0300

    refactor Load before bringing new code

diff --git a/lib/RT/Ticket_Overlay.pm b/lib/RT/Ticket_Overlay.pm
index 877dc02..5f7ef79 100755
--- a/lib/RT/Ticket_Overlay.pm
+++ b/lib/RT/Ticket_Overlay.pm
@@ -148,47 +148,41 @@ Otherwise, returns the ticket id.
 sub Load {
     my $self = shift;
     my $id   = shift;
+    $id = '' unless defined $id;
 
-    #TODO modify this routine to look at EffectiveId and do the recursive load
-    # thing. be careful to cache all the interim tickets we try so we don't loop forever.
+    # TODO: modify this routine to look at EffectiveId and
+    # do the recursive load thing. be careful to cache all
+    # the interim tickets we try so we don't loop forever.
 
     # FIXME: there is no TicketBaseURI option in config
     my $base_uri = RT->Config->Get('TicketBaseURI') || '';
     #If it's a local URI, turn it into a ticket id
-    if ( $base_uri && defined $id && $id =~ /^$base_uri(\d+)$/ ) {
+    if ( $base_uri && $id =~ /^$base_uri(\d+)$/ ) {
         $id = $1;
     }
 
-    #If it's a remote URI, we're going to punt for now
-    elsif ( $id =~ '://' ) {
+    unless ( $id =~ /^\d+$/ ) {
+        $RT::Logger->debug("Tried to load a bogus ticket id: '$id'");
         return (undef);
     }
 
-    #If we have an integer URI, load the ticket
-    if ( defined $id && $id =~ /^\d+$/ ) {
-        my ($ticketid,$msg) = $self->LoadById($id);
-
-        unless ($self->Id) {
-            $RT::Logger->debug("$self tried to load a bogus ticket: $id");
-            return (undef);
-        }
-    }
-
-    #It's not a URI. It's not a numerical ticket ID. Punt!
-    else {
-        $RT::Logger->debug("Tried to load a bogus ticket id: '$id'");
+    my ($ticketid, $msg) = $self->LoadById( $id );
+    unless ( $self->Id ) {
+        $RT::Logger->debug("$self tried to load a bogus ticket: $id");
         return (undef);
     }
 
     #If we're merged, resolve the merge.
-    if ( ( $self->EffectiveId ) and ( $self->EffectiveId != $self->Id ) ) {
-        $RT::Logger->debug ("We found a merged ticket.". $self->id ."/".$self->EffectiveId);
-        return ( $self->Load( $self->EffectiveId ) );
+    if ( ($self->EffectiveId || 0) != $self->Id ) {
+        $RT::Logger->debug(
+            "We found a merged ticket. "
+            . $self->id ."/". $self->EffectiveId
+        );
+        return $self->Load( $self->EffectiveId );
     }
 
     #Ok. we're loaded. lets get outa here.
-    return ( $self->Id );
-
+    return $self->Id;
 }
 
 # }}}
@@ -2473,8 +2467,6 @@ sub _AddLink {
 
 MergeInto take the id of the ticket to merge this ticket into.
 
-
-
 =cut
 
 sub MergeInto {

commit 94347470db7586c4e06a97e482270d3a1a1ad112
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Nov 17 20:01:14 2009 +0300

    introduce MERGE_CACHE for Merged and Load
    
    we use Merged method eight times per ticket show, which generate
    eight queries. It's better to cache this data. At least for one
    request.

diff --git a/lib/RT/Ticket_Overlay.pm b/lib/RT/Ticket_Overlay.pm
index 5f7ef79..c3e86c9 100755
--- a/lib/RT/Ticket_Overlay.pm
+++ b/lib/RT/Ticket_Overlay.pm
@@ -135,6 +135,11 @@ our %LINKDIRMAP = (
 sub LINKTYPEMAP   { return \%LINKTYPEMAP   }
 sub LINKDIRMAP   { return \%LINKDIRMAP   }
 
+our %MERGE_CACHE = (
+    effective => {},
+    merged => {},
+);
+
 # {{{ sub Load
 
 =head2 Load
@@ -2640,18 +2645,22 @@ Returns list of tickets' ids that's been merged into this ticket.
 sub Merged {
     my $self = shift;
 
+    my $id = $self->id;
+    return @{ $MERGE_CACHE{'merged'}{ $id } }
+        if $MERGE_CACHE{'merged'}{ $id };
+
     my $mergees = RT::Tickets->new( $self->CurrentUser );
     $mergees->Limit(
         FIELD    => 'EffectiveId',
-        OPERATOR => '=',
-        VALUE    => $self->Id,
+        VALUE    => $id,
     );
     $mergees->Limit(
         FIELD    => 'id',
         OPERATOR => '!=',
-        VALUE    => $self->Id,
+        VALUE    => $id,
     );
-    return map $_->id, @{ $mergees->ItemsArrayRef || [] };
+    return @{ $MERGE_CACHE{'merged'}{ $id } ||= [] }
+        = map $_->id, @{ $mergees->ItemsArrayRef || [] };
 }
 
 # }}}

commit c376d4f5c954fd657e22f8b7e6bfc89621496295
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Nov 17 20:17:13 2009 +0300

    use MERGE_CACHE in Load for quick EffectiveId lookup
    
    when we show history of a multi-merge ticket, transaction
    often belong to merged tickets and we call Load quite
    often. Just a micro-optimization.

diff --git a/lib/RT/Ticket_Overlay.pm b/lib/RT/Ticket_Overlay.pm
index c3e86c9..731711b 100755
--- a/lib/RT/Ticket_Overlay.pm
+++ b/lib/RT/Ticket_Overlay.pm
@@ -171,6 +171,9 @@ sub Load {
         return (undef);
     }
 
+    $id = $MERGE_CACHE{'effective'}{ $id }
+        if $MERGE_CACHE{'effective'}{ $id };
+
     my ($ticketid, $msg) = $self->LoadById( $id );
     unless ( $self->Id ) {
         $RT::Logger->debug("$self tried to load a bogus ticket: $id");
@@ -183,7 +186,9 @@ sub Load {
             "We found a merged ticket. "
             . $self->id ."/". $self->EffectiveId
         );
-        return $self->Load( $self->EffectiveId );
+        my $real_id = $self->Load( $self->EffectiveId );
+        $MERGE_CACHE{'effective'}{ $id } = $real_id;
+        return $real_id;
     }
 
     #Ok. we're loaded. lets get outa here.

commit 43a6f209352e771bfdcb98fc1ba41ddf46a1c416
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Nov 17 20:20:56 2009 +0300

    clean cache when data is changed

diff --git a/lib/RT/Ticket_Overlay.pm b/lib/RT/Ticket_Overlay.pm
index 731711b..ad25cd8 100755
--- a/lib/RT/Ticket_Overlay.pm
+++ b/lib/RT/Ticket_Overlay.pm
@@ -2501,6 +2501,11 @@ sub MergeInto {
         return ( 0, $self->loc("Permission Denied") );
     }
 
+    delete $MERGE_CACHE{'effective'}{ $self->id };
+    delete @{ $MERGE_CACHE{'merged'} }{
+        $ticket_id, $MergeInto->id, $self->id
+    };
+
     $RT::Handle->BeginTransaction();
 
     # We use EffectiveId here even though it duplicates information from

commit d4ed191b704cf494ac459177d90c8b406e302716
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Tue Nov 17 20:23:31 2009 +0300

    clean cache after every request

diff --git a/lib/RT/Interface/Web/Handler.pm b/lib/RT/Interface/Web/Handler.pm
index 8d17921..8d1be8d 100644
--- a/lib/RT/Interface/Web/Handler.pm
+++ b/lib/RT/Interface/Web/Handler.pm
@@ -217,6 +217,8 @@ sub CleanupRequest {
         RT::Crypt::GnuPG::UseKeyForEncryption();
         RT::Crypt::GnuPG::UseKeyForSigning( undef );
     }
+
+    %RT::Ticket::MERGE_CACHE = ( effective => {}, merged => {} );
 }
 # }}}
 

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


More information about the Rt-commit mailing list