[Rt-devel] Re: TransactionBatch state rework + scrip code clean up.

Ruslan U. Zakirov cubic at acronis.ru
Tue May 4 06:12:58 EDT 2004


		Hello.
I don't know where should I put text. I added few lines to pod and below 
text which should be put somewhere.

Scrip stages:
Scrips can be applied in different stages: 'TransactionCreate' and 
'TransactionBatch'.
In TransactionCreate stage scrip applied right after transaction is 
created and can be committed more then once during one request for each 
transaction which is applicable.
In TrabsactionBatch stage scrip applied only once during request when 
all transactions are created and all scrips in TransactionCreate stage 
already applied. Scrip aplied only once even if batch contain more then 
one applicable transaction.
For backward compatibilty reasons and to allow using same scrip action 
in both stages $ScripActionObj->TransactionObj return first transaction 
which is applicable in batch. You can get full batch with 
$TicketObj->TransactionBatch call which return array reference. Array 
contain transaction objects ordered by create time.

Example:
User's request creates transactions in next order: a b c d e.
Scrip has condition which trigger b and d transaction.

If scrip in TransactionCreate stage then
1) scrip's action will be committed two times.
2) First time $TicketObj->TransactionBatch will return [a_obj b_obj] and 
$ScripActionObj->TransactionObj will return b_obj.
3) Second time it'll be [a_obj b_obj c_obj d_obj] and 
$ScripActionObj->TransactionObj will return d_obj.

If scrip in TransactionBatch stage
1) scrip's action will be committed only once.
2) $TicketObj->TransactionBatch will return [a_obj b_obj c_obj d_obj 
e_obj] and $ScripActionObj->TransactionObj will return b_obj(first 
transaction which triggered condition).


			Best regards. Ruslan.
-------------- next part --------------
=== lib/RT/Scrip_Overlay.pm
==================================================================
--- lib/RT/Scrip_Overlay.pm  (revision 222)
+++ lib/RT/Scrip_Overlay.pm  (local)
@@ -215,13 +215,12 @@
 
 sub QueueObj {
     my $self = shift;
-    
-    if (!$self->{'QueueObj'})  {
-	require RT::Queue;
-	$self->{'QueueObj'} = RT::Queue->new($self->CurrentUser);
-	$self->{'QueueObj'}->Load($self->__Value('Queue'));
-    }
-    return ($self->{'QueueObj'});
+
+    require RT::Queue;
+    my $obj = RT::Queue->new($self->CurrentUser);
+    $obj->Load( $self->__Value('Queue') );
+
+    return $obj;
 }
 
 # }}}
@@ -237,16 +236,15 @@
 
 sub ActionObj {
     my $self = shift;
-    
-    unless (defined $self->{'ScripActionObj'})  {
-	require RT::ScripAction;
-	
-	$self->{'ScripActionObj'} = RT::ScripAction->new($self->CurrentUser);
-	#TODO: why are we loading Actions with templates like this. 
-	# two seperate methods might make more sense
-	$self->{'ScripActionObj'}->Load($self->ScripAction, $self->Template);
-    }
-    return ($self->{'ScripActionObj'});
+
+    require RT::ScripAction;
+
+    my $obj = RT::ScripAction->new($self->CurrentUser);
+#TODO: why are we loading Actions with templates like this. 
+# two seperate methods might make more sense
+    $obj->Load($self->ScripAction, $self->Template);
+
+    return $obj;
 }
 
 # }}}
@@ -262,15 +260,14 @@
 sub ConditionObj {
     my $self = shift;
 
-    unless ( defined $self->{'ScripConditionObj'} ) {
-        require RT::ScripCondition;
-        $self->{'ScripConditionObj'} =
-          RT::ScripCondition->new( $self->CurrentUser );
-        if ( $self->ScripCondition ) {
-            $self->{'ScripConditionObj'}->Load( $self->ScripCondition );
-        }
+    require RT::ScripCondition;
+
+    my $obj = RT::ScripCondition->new( $self->CurrentUser );
+    if ( $self->ScripCondition ) {
+        $obj->Load( $self->ScripCondition );
     }
-    return ( $self->{'ScripConditionObj'} );
+
+    return $obj;
 }
 
 # }}}
@@ -284,13 +281,13 @@
 
 sub TemplateObj {
     my $self = shift;
-    
-    unless (defined $self->{'TemplateObj'})  {
-	require RT::Template;
-	    $self->{'TemplateObj'} = RT::Template->new($self->CurrentUser);
-	    $self->{'TemplateObj'}->Load($self->Template);
-    }
-    return ($self->{'TemplateObj'});
+
+    require RT::Template;
+
+    my $obj = RT::Template->new($self->CurrentUser);
+    $obj->Load( $self->Template );
+
+    return $obj;
 }
 
 # }}}
@@ -309,6 +306,9 @@
 Usually, the ticket and transaction objects passed to this method
 should be loaded by the SuperUser role
 
+In TransactionBatch stage scrip committed only once with first applicable
+transaction in batch, you always can get batch with $TicketObj->TransactionBatch.
+
 =cut
 
 
@@ -320,40 +320,26 @@
                  TransactionObj => undef,
                  @_ );
 
+    $self->{'TicketObj'} = $args{'TicketObj'};
+    $self->{'TransactionObj'} = $args{'TransactionObj'};
+#$RT::Logger->debug( 'Applying Scrip #'.$self->id.' at stage '.
+#$self->Stage.' on Ticket #'.$args{'TicketObj'}->id);
+
+    unless( $self->IsApplicable() ) {
+        return undef;
+    }
+
     # We want to make sure that if a scrip dies, we don't get
     # hurt
     eval {
-
-        #Load the scrip's Condition object
-        $self->ConditionObj->LoadCondition(
-                                      ScripObj       => $self,
-                                      TicketObj      => $args{'TicketObj'},
-                                      TransactionObj => $args{'TransactionObj'},
-        );
-
-        unless ( $self->IsApplicable() ) {
-            $self->ConditionObj->DESTROY;
-            return (undef);
-        }
-
-        #If it's applicable, prepare and commit it
-        $self->ActionObj->LoadAction( ScripObj       => $self,
-                                      TicketObj      => $args{'TicketObj'},
-                                      TransactionObj => $args{'TransactionObj'},
-        );
-
         unless ( $self->Prepare() ) {
             $RT::Logger->info(
                           "$self: Couldn't prepare " . $self->ActionObj->Name );
-            $self->ActionObj->DESTROY();
-            $self->ConditionObj->DESTROY();
             return (undef);
         }
         unless ( $self->Commit() ) {
             $RT::Logger->info(
                            "$self: Couldn't commit " . $self->ActionObj->Name );
-            $self->ActionObj->DESTROY();
-            $self->ConditionObj->DESTROY();
             return (undef);
         }
 
@@ -363,14 +349,11 @@
 
         #We're done with it. lets clean up.
         #TODO: something else isn't letting these get garbage collected. check em out.
-        $self->ActionObj->DESTROY();
-        $self->ConditionObj->DESTROY();
         return (1);
     };
     if ($@) {
         $RT::Logger->error( "Scrip " . $self->Id . " died. - " . $@ );
     }
-
 }
 # }}}
 
@@ -378,17 +361,83 @@
 
 =head2 IsApplicable
 
-Calls the  Condition object\'s IsApplicable method
+Calls the Condition object\'s IsApplicable method
+In TransactionBatch stage checks all transactions in batch
+for application and stops on first applicable transaction.
 
 =cut
 
 sub IsApplicable {
     my $self = shift;
-    return ($self->ConditionObj->IsApplicable(@_));
+    my $res = eval {
+	my $TicketObj = $self->{'TicketObj'};
+	unless( $TicketObj ) {
+	    $RT::Logger->info( $self->loc( 'IsApplicable called before Apply' ) );
+	    return undef;
+	}
+
+	my $CondObj = $self->ConditionObj;
+	my @Transactions;
+
+        if( !$self->Stage || $self->Stage =~ /TransactionCreate/i ) {
+	    @Transactions = ( $self->{'TransactionObj'} );
+        } elsif ( $self->Stage =~ /TransactionBatch/i && $TicketObj->TransactionBatch ) {
+            @Transactions = @{ $TicketObj->TransactionBatch };
+	}
+
+	foreach my $trx( @Transactions ) {
+	    $CondObj->LoadCondition(
+                                      ScripObj       => $self,
+                                      TicketObj      => $TicketObj,
+                                      TransactionObj => $trx,
+            );
+	    if( $CondObj->IsApplicable(@_) ) {
+		$self->{'TransactionObj'} = $trx;
+		return 1;
+	    }
+	}
+	return undef;
+    };
+    if ($@) {
+        $RT::Logger->error( "Scrip " . $self->Id . " condition died. - " . $@ );
+	return undef;
+    }
+    return $res;
 }
 
 # }}}
 
+sub _ActionObj {
+    my $self = shift;
+    my $res = eval {
+	my $TicketObj = $self->{'TicketObj'};
+	unless( $TicketObj ) {
+	    $RT::Logger->error( $self->loc( '_ActionObj called before Apply' ) );
+	    return undef;
+	}
+
+	my $TransactionObj = $self->{'TransactionObj'};
+	unless( $TransactionObj ) {
+	    $RT::Logger->error( $self->loc( '_ActionObj called before Apply' ) );
+	    return undef;
+	}
+
+	my $ActionObj = $self->ActionObj;
+	$ActionObj->LoadAction(
+                               ScripObj       => $self,
+                               TicketObj      => $TicketObj,
+                               TransactionObj => $TransactionObj,
+                              );
+
+	return $ActionObj;
+    };
+    if ($@) {
+        $RT::Logger->error( "Scrip " . $self->Id . " action loading died. - " . $@ );
+	return undef;
+    }
+    return $res;
+}
+
 # {{{ sub Prepare
 
 =head2 Prepare
@@ -399,7 +448,13 @@
 
 sub Prepare {
     my $self = shift;
-    $self->ActionObj->Prepare(@_);
+
+    my $ActionObj = $self->_ActionObj;
+    unless( $ActionObj ) {
+        return undef;
+    };
+
+    return $ActionObj->Prepare(@_)
 }
 
 # }}}
@@ -414,7 +469,13 @@
 
 sub Commit {
     my $self = shift;
-    $self->ActionObj->Commit(@_);
+
+    my $ActionObj = $self->_ActionObj;
+    unless( $ActionObj ) {
+        return undef;
+    };
+
+    return $ActionObj->Commit(@_)
 }
 
 # }}}
@@ -424,7 +485,8 @@
 # {{{ sub DESTROY
 sub DESTROY {
     my $self = shift;
-    $self->{'ActionObj'} = undef;
+    $self->{'TicketObj'} = undef;
+    $self->{'TransactionObj'} = undef;
 }
 # }}}
 
=== lib/RT/Scrips_Overlay.pm
==================================================================
--- lib/RT/Scrips_Overlay.pm  (revision 222)
+++ lib/RT/Scrips_Overlay.pm  (local)
@@ -144,6 +144,7 @@
             || $RT::Logger->err("$self couldn't load ticket $args{'Ticket'}\n");
     }
 
+#FIXME: In 'TransactionBatch' stage we shouldn't check for TransactionObj
     if ( ($TransactionObj = $args{'TransactionObj'}) ) {
         $TransactionObj->CurrentUser($self->CurrentUser);
     }


More information about the Rt-devel mailing list