[Rt-commit] rt branch, 4.0/shredded-transactionbatch, created. rt-4.0.6-251-gd29975b

[Alex Vandiver]

The branch, 4.0/shredded-transactionbatch has been created
        at  d29975bf9778e9208b81ee4f12612824e473bf23 (commit)

- Log -----------------------------------------------------------------
commit bc3212fe21431fa9825bbd021bb746602f4b0266
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jul 31 15:54:52 2012 -0400

    Warn if TransactionBatch would be triggered on shredded tickets
    
    Since 1e280ff, we switch the current user by creating a new RT::Ticket
    object and load the current ticket into it.  However, this also has a
    subtle behavior change -- previously, if a ticket had ending
    transactions and was removed from the underlying database, appropriate
    scrips would still be triggered for it, despite the ticket no longer
    existing.  Depending on the content of the scrips, this may have led to
    unexpected results.  In the new behavior, the ticket object passed to
    the scrips unexpectedly has no id.
    
    Before running _ApplyTransactionBatch, detect if the ticket record is no
    longer valid, and return, dropping the transactions; in doing so, warn
    of the confluence of events that led to this scenario, rare as it is.
    
    While this is technically a behavior change (from both before and after
    1e280ff), it almost certainly only arises during testing, and previously
    generated subtly incorrect or inconsistent results.  The new behavior
    reports the failure, maintains consistency, and suggests a solution.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 1a3514c..91711e4 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -3345,6 +3345,19 @@ sub _ApplyTransactionBatch {
 
     return if $self->RanTransactionBatch;
     $self->RanTransactionBatch(1);
+
+    my $still_exists = RT::Ticket->new( RT->SystemUser );
+    $still_exists->Load( $self->Id );
+    if (not $still_exists->Id) {
+        # The ticket has been removed from the database, but we still
+        # have pending TransactionBatch txns for it.  Unfortunately,
+        # because it isn't in the DB anymore, attempting to run scrips
+        # on it may produce unpredictable results; simply drop the
+        # batched transactions.
+        $RT::Logger->warning("TransactionBatch was fired on a ticket that no longer exists; unable to run scrips!  Call ->ApplyTransactionBatch before shredding the ticket, for consistent results.");
+        return;
+    }
+
     my $batch = $self->TransactionBatch;
 
     my %seen;

commit d29975bf9778e9208b81ee4f12612824e473bf23
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Jul 31 16:08:02 2012 -0400

    Update shredder tests to heed the warning added in bc3212f
    
    Due to the shredder tests not using RT::Test, no warning was actually
    output (and no test was failed) by code in the previous commit.
    Regardless, update the code to cope with the failure case as suggested.

diff --git a/t/shredder/03plugin_tickets.t b/t/shredder/03plugin_tickets.t
index 092b570..e63eef8 100644
--- a/t/shredder/03plugin_tickets.t
+++ b/t/shredder/03plugin_tickets.t
@@ -34,6 +34,7 @@ use_ok('RT::Tickets');
     my $child = RT::Ticket->new( RT->SystemUser );
     my ($cid) = $child->Create( Subject => 'child', Queue => 1, MemberOf => $pid );
     ok( $cid, "created new ticket" );
+    $_->ApplyTransactionBatch for $parent, $child;
 
     my $plugin = RT::Shredder::Plugin::Tickets->new;
     isa_ok($plugin, 'RT::Shredder::Plugin::Tickets');
@@ -77,6 +78,8 @@ cmp_deeply( dump_current_and_savepoint('clean'), "current DB equal to savepoint"
     my ($status, $msg) = $child->AddLink( Target => $pid, Type => 'DependsOn' );
     ok($status, "added reqursive link") or diag "error: $msg";
 
+    $_->ApplyTransactionBatch for $parent, $child;
+
     my $plugin = RT::Shredder::Plugin::Tickets->new;
     isa_ok($plugin, 'RT::Shredder::Plugin::Tickets');
 
@@ -121,6 +124,8 @@ cmp_deeply( dump_current_and_savepoint('clean'), "current DB equal to savepoint"
     ok( $cid2, "created new ticket" );
     $child2->SetStatus('resolved');
 
+    $_->ApplyTransactionBatch for $parent, $child1, $child2;
+
     my $plugin = RT::Shredder::Plugin::Tickets->new;
     isa_ok($plugin, 'RT::Shredder::Plugin::Tickets');
 

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