[Bps-public-commit] SD - A distributed issue tracker branch, master, updated. 75768136db2286d854a14694024a7bef8afdd21c
jesse
jesse at bestpractical.com
Mon Feb 16 16:33:49 EST 2009
The branch, master has been updated
via 75768136db2286d854a14694024a7bef8afdd21c (commit)
via 8dd19f7a85d8ede42185b20c9e0dc7710b1cef75 (commit)
from 722e63fa42896925dfe5b3dfc8e5f28d1a9dc06c (commit)
Summary of changes:
lib/App/SD/Replica/rt/PullEncoder.pm | 91 +++++++++++++++++++----
t/sd-rt/{bogus-rt-data.t => race-condition.t} | 96 +++++++++++++++++--------
2 files changed, 142 insertions(+), 45 deletions(-)
copy t/sd-rt/{bogus-rt-data.t => race-condition.t} (55%)
- Log -----------------------------------------------------------------
commit 8dd19f7a85d8ede42185b20c9e0dc7710b1cef75
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Mon Feb 16 12:45:18 2009 -0500
Added a test for the race condition in the current push encoder
diff --git a/t/sd-rt/race-condition.t b/t/sd-rt/race-condition.t
new file mode 100644
index 0000000..d159e6b
--- /dev/null
+++ b/t/sd-rt/race-condition.t
@@ -0,0 +1,141 @@
+#!/usr/bin/perl -w
+
+# to run:
+#
+# RT_DBA_USER=root RT_DBA_PASSWORD= prove -lv -I/Users/clkao/work/bps/rt-3.7/lib t/sd-rt.t
+use strict;
+
+use Prophet::Test;
+use Path::Class;
+
+BEGIN {
+ unless ( eval 'use RT::Test; 1' ) {
+ diag $@ if $ENV{'TEST_VERBOSE'};
+ plan skip_all => 'requires RT 3.8 or newer to run tests.';
+ }
+}
+
+plan tests => 8;
+use App::SD::Test;
+
+no warnings 'once';
+
+RT::Handle->InsertData( $RT::EtcPath . '/initialdata' );
+
+BEGIN {
+ require File::Temp;
+ $ENV{'PROPHET_REPO'} = $ENV{'SD_REPO'}
+ = File::Temp::tempdir( CLEANUP => 1 ) . '/_svb';
+ diag "export SD_REPO=" . $ENV{'PROPHET_REPO'} . "\n";
+}
+
+my $IMAGE_FILE = qw|t/data/bplogo.gif|;
+
+$RT::Test::SKIP_REQUEST_WORK_AROUND = 1;
+
+
+my $reason = <<EOF;
+Before this script started passing, the RT replica type would automatically mark any change that happened before or at the same time as a push as having originated in SD, so it wouldn't pull it back from RT. This includes changes made by scrips after ticket update
+EOF
+
+diag($reason);
+
+
+my ( $url, $m ) = RT::Test->started_ok;
+
+use RT::Client::REST;
+use RT::Client::REST::Ticket;
+my $rt = RT::Client::REST->new( server => $url );
+$rt->login( username => 'root', password => 'password' );
+
+$url =~ s|http://|http://root:password@|;
+my $sd_rt_url = "rt:$url|General|Status!='resolved'";
+
+
+
+# Create a ticket in RT
+my $ticket = RT::Client::REST::Ticket->new(
+ rt => $rt,
+ queue => 'General',
+ status => 'new',
+ subject => 'Fly Man',
+)->store( text => "Initial ticket Comment" );
+
+my $flyman_rt_id = $ticket->id;
+
+ok($flyman_rt_id, "I created a new ticket in RT");
+
+
+
+
+# pull to sd
+
+
+my ( $ret, $out, $err );
+( $ret, $out, $err ) = run_script( 'sd', [ 'clone', '--from', $sd_rt_url ] );
+my ( $yatta_id, $flyman_id );
+
+
+# make sure ticket is new
+
+run_output_matches(
+ 'sd',
+ [ 'ticket', 'list', '--regex', '.' ],
+ [qr/(.*?)(?{ $flyman_id = $1 }) Fly Man new/]
+);
+
+
+
+
+
+# comment on ticket in sd
+
+( $ret, $out, $err )
+ = run_script( 'sd',
+ [ 'ticket', 'comment', $flyman_id, '--content', 'helium is a noble gas' ] );
+ok( $ret, $out );
+like( $out, qr/Created comment/ );
+
+
+
+
+
+# make sure ticket is new
+
+run_output_matches(
+ 'sd',
+ [ 'ticket', 'list', '--regex', '.' ],
+ ["$flyman_id Fly Man new"]
+);
+
+
+# push to rt
+{
+
+ my ( $ret, $out, $err ) = run_script( 'sd', [ 'push', '--to', $sd_rt_url ] );
+ diag($out);
+ diag($err);
+
+}
+
+# make sure ticket is open in rt, since after we commented the scrip popped it open.
+{
+ my $fetched_ticket = RT::Client::REST::Ticket->new(
+ rt => $rt,
+ id => $flyman_rt_id
+ )->retrieve;
+
+ is( $fetched_ticket->status, "open" );
+
+}
+
+# pull to sd
+( $ret, $out, $err ) = run_script( 'sd', [ 'pull', '--from', $sd_rt_url ] );
+
+# make sure ticket is open
+run_output_matches_unordered(
+ 'sd',
+ [ 'ticket', 'list', '--regex', '.' ],
+ [ "$flyman_id Fly Man open", ]
+);
+
commit 75768136db2286d854a14694024a7bef8afdd21c
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Mon Feb 16 16:33:16 2009 -0500
Now we only inspect txns and tickets we've never seen before.
diff --git a/lib/App/SD/Replica/rt/PullEncoder.pm b/lib/App/SD/Replica/rt/PullEncoder.pm
index fcc9252..ee9acea 100644
--- a/lib/App/SD/Replica/rt/PullEncoder.pm
+++ b/lib/App/SD/Replica/rt/PullEncoder.pm
@@ -1,10 +1,11 @@
package App::SD::Replica::rt::PullEncoder;
-use Any::Moose;
+use Any::Moose; use strict;
extends 'App::SD::ForeignReplica::PullEncoder';
use Params::Validate qw(:all);
use Memoize;
use Time::Progress;
+use HTTP::Date;
has sync_source =>
( isa => 'App::SD::Replica::rt',
@@ -19,7 +20,6 @@ sub run {
}
);
- my $first_rev = ( $args{'after'} + 1 ) || 1;
my $tickets = {};
my @transactions;
@@ -30,23 +30,29 @@ sub run {
my $counter = 0;
$self->sync_source->log("Discovering ticket history");
+
+ my ($last_modified, $last_txn);;
+
my $progress = Time::Progress->new();
$progress->attr( max => $#tickets );
+
local $| = 1;
+
+
for my $id (@tickets) {
$counter++;
print $progress->report( "%30b %p Est: %E\r", $counter );
- $self->sync_source->log(
- "Fetching ticket $id - $counter of " . scalar @tickets
- );
+ $self->sync_source->log( "Fetching ticket $id - $counter of " . scalar @tickets);
+
$tickets->{ $id } = $self->_translate_final_ticket_state(
$self->sync_source->rt->show( type => 'ticket', id => $id )
);
push @transactions, @{
$self->find_matching_transactions(
ticket => $id,
- starting_transaction => $first_rev
+ starting_transaction => (( $args{'after'} + 1 ) || 1)
+
)
};
}
@@ -54,6 +60,10 @@ sub run {
my $txn_counter = 0;
my @changesets;
for my $txn ( sort { $b->{'id'} <=> $a->{'id'} } @transactions ) {
+
+ $last_modified = $txn->{'Created'} if ( !$last_modified || ($txn->{'Created'} > $last_modified ));
+ $last_txn = $txn->{'id'} if (!$last_txn || ($txn->{id} > $last_txn));
+
$txn_counter++;
$self->sync_source->log("Transcoding transaction @{[$txn->{'id'}]} - $txn_counter of ". scalar @transactions);
my $changeset = $self->transcode_one_txn( $txn, $tickets->{ $txn->{Ticket} } );
@@ -67,8 +77,37 @@ sub run {
$self->sync_source->log("Applying changeset ".++$cs_counter . " of ".scalar @changesets);
$args{callback}->($_)
}
+
+ $self->_record_upstream_last_modified_date($last_modified) if ($last_modified > $self->_upstream_last_modified_date);
+ $self->_record_upstream_last_txn($last_txn) if ($last_txn > $self->_upstream_last_txn);
+}
+
+sub _record_upstream_last_modified_date {
+ my $self = shift;
+ my $date = shift;
+ return $self->sync_source->store_local_metadata('last_modified_date' => $date);
+}
+
+sub _upstream_last_modified_date {
+ my $self = shift;
+ return $self->sync_source->fetch_local_metadata('last_modified_date');
+}
+
+sub _upstream_last_txn {
+ my $self = shift;
+ return $self->sync_source->fetch_local_metadata('last_txn_id');
}
+sub _record_upstream_last_txn {
+ my $self = shift;
+ my $id = shift;
+ warn "Id is $id";
+ return $self->sync_source->store_local_metadata('last_txn_id' => $id);
+}
+
+
+
+
sub _translate_final_ticket_state {
my $self = shift;
my $ticket = shift;
@@ -96,7 +135,7 @@ sub _translate_final_ticket_state {
}
}
- $ticket->{$_} = $self->unix_time_to_iso( $ticket->{$_} )
+ $ticket->{$_} = $self->rt_time_to_iso( $ticket->{$_} )
for grep defined $ticket->{$_}, qw(Created Resolved Told LastUpdated Due Starts Started);
$ticket->{$_} =~ s/ minutes$//
@@ -121,6 +160,22 @@ Returns an RT::Client ticket collection for all tickets found matching your QUER
sub find_matching_tickets {
my $self = shift;
my ($query) = validate_pos(@_, 1);
+
+ # If we've ever synced, we can limit our search to only newer things
+ if ($self->_upstream_last_modified_date){
+ # last modified date is in GMT and searches are in user-time XXX -check assumption
+ # because of this, we really want to back that date down by one day to catch overlap
+ # XXX TODO we are playing FAST AND LOOSE WITH DATE MATH
+ # XXX TODO THIS WILL HURT US SOME DAY
+ # At that time, Jesse will buy you a beer.
+ my $before = HTTP::Date::str2time($self->_upstream_last_modified_date() ) - (86400 + 7200) ; # 26 hours ago deals with most any possible edge case
+
+
+ $query = "($query) AND LastUpdated >= '".HTTP::Date::time2iso($before) ."'";
+
+ $self->sync_source->log("Skipping all tickets not updated since ".HTTP::Date::time2iso($before));
+ }
+
return $self->sync_source->rt->search( type => 'ticket', query => $query );
}
@@ -137,14 +192,16 @@ sub find_matching_transactions {
my $rt_handle = $self->sync_source->rt;
+ my $latest = $self->_upstream_last_txn();
for my $txn ( sort $rt_handle->get_transaction_ids( parent_id => $args{'ticket'} ) ) {
- # Skip things we know we've already pulled
+ # Skip things calling code told us to skip
next if $txn < $args{'starting_transaction'};
-
+ # skip things we had on our last pull
+ next if $txn <= $latest;
+
# Skip things we've pushed
next if $self->sync_source->foreign_transaction_originated_locally($txn, $args{'ticket'});
-
my $txn_hash = $rt_handle->get_transaction(
parent_id => $args{'ticket'},
id => $txn,
@@ -156,8 +213,7 @@ sub find_matching_transactions {
my $id = $1;
my $a = $rt_handle->get_attachment( parent_id => $args{'ticket'}, id => $id);
- push( @{ $txn_hash->{_attachments} }, $a )
- if ( $a->{Filename} );
+ push( @{ $txn_hash->{_attachments} }, $a ) if ( $a->{Filename} );
}
@@ -174,7 +230,6 @@ sub transcode_one_txn {
unless ( $sub ) {
die "Transaction type $txn->{Type} (for transaction $txn->{id}) not implemented yet";
}
-
my $changeset = Prophet::ChangeSet->new(
{ original_source_uuid => $self->sync_source->uuid,
original_sequence_no => $txn->{'id'},
@@ -205,6 +260,9 @@ sub transcode_one_txn {
return $changeset;
}
+
+{ # Recoding RT transactions
+
sub _recode_attachment_create {
my $self = shift;
my %args = validate( @_, { ticket => 1, txn => 1, changeset => 1, attachment => 1 } );
@@ -312,6 +370,7 @@ sub _recode_txn_Create {
$self->_recode_content_update(%args); # add the create content txn as a seperate change in this changeset
}
+*_recode_txn_Link = \&_recode_txn_AddLink;
sub _recode_txn_AddLink {
# XXX, TODO: syncing links doesn't work
return;
@@ -438,6 +497,9 @@ sub _recode_txn_CustomField {
);
}
+}
+
+
sub resolve_user_id_to {
my $self = shift;
my $attr = shift;
@@ -463,9 +525,8 @@ sub resolve_user_id_to {
memoize 'resolve_user_id_to';
-use HTTP::Date;
-sub unix_time_to_iso {
+sub rt_time_to_iso {
my $self = shift;
my $date = shift;
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list