[Bps-public-commit] SD branch, master, updated. 66994d51da5c274dd3d34e05235f7fb40c164c50
jesse
jesse at bestpractical.com
Sat May 2 13:03:25 EDT 2009
The branch, master has been updated
via 66994d51da5c274dd3d34e05235f7fb40c164c50 (commit)
via d620eb235e15520bba143fc8d18838f51fc7c837 (commit)
via 75c1e476fc78cbcae7b27d4415848827c85d93a9 (commit)
via 066edd63e204470b4c6cd7b6baa501c53e0ada9b (commit)
from 1f899363a5fa4e3c62fc18cce7ad92fc63bd95af (commit)
Summary of changes:
lib/App/SD/ForeignReplica.pm | 25 +++--
lib/App/SD/Replica/rt.pm | 14 ---
lib/App/SD/Replica/trac.pm | 55 ++++++++---
lib/App/SD/Replica/trac/PullEncoder.pm | 28 ++++--
lib/App/SD/Replica/trac/PushEncoder.pm | 43 ++++-----
t/sd-trac/basic.t | 169 +++++++++++++++++++++++---------
6 files changed, 219 insertions(+), 115 deletions(-)
- Log -----------------------------------------------------------------
commit 066edd63e204470b4c6cd7b6baa501c53e0ada9b
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Sat May 2 12:54:51 2009 -0400
Expanded trac test suite. Working toward pushing tickets created in SD
diff --git a/t/sd-trac/basic.t b/t/sd-trac/basic.t
index df97d8d..cd1a141 100644
--- a/t/sd-trac/basic.t
+++ b/t/sd-trac/basic.t
@@ -1,32 +1,28 @@
-use warnings; use strict;
-use Prophet::Test;
+use warnings;
+use strict;
+use Prophet::Test;
use App::SD::Test;
-
-
BEGIN {
require File::Temp;
- $ENV{'PROPHET_REPO'} = $ENV{'SD_REPO'}
- = File::Temp::tempdir( CLEANUP => 1 ) . '/_svb';
+ $ENV{'PROPHET_REPO'} = $ENV{'SD_REPO'} = File::Temp::tempdir( CLEANUP => 1 ) . '/_svb';
diag "export SD_REPO=" . $ENV{'PROPHET_REPO'} . "\n";
}
-
unless (`which trac-admin`) { plan skip_all => 'You need trac installed to run the tests'; }
-unless (eval { require Net::Trac} ) { plan skip_all => 'You need Net::Trac installed to run the tests'; }
-plan tests => 25;
-
+unless ( eval { require Net::Trac } ) {
+ plan skip_all => 'You need Net::Trac installed to run the tests';
+}
+plan tests => 40;
use_ok('Net::Trac::Connection');
use_ok('Net::Trac::Ticket');
require 't/sd-trac/setup_trac.pl';
-
my $tr = Net::Trac::TestHarness->new();
-ok($tr->start_test_server(), "The server started!");
-
+ok( $tr->start_test_server(), "The server started!" );
my $trac = Net::Trac::Connection->new(
url => $tr->url,
@@ -34,72 +30,153 @@ my $trac = Net::Trac::Connection->new(
password => 'yatta'
);
-
-
-
-my $sd_trac_url = "trac:".$tr->url;
+my $sd_trac_url = "trac:" . $tr->url;
$sd_trac_url =~ s|http://|http://hiro:yatta@|;
-
isa_ok( $trac, "Net::Trac::Connection" );
-is($trac->url, $tr->url);
-my $ticket = Net::Trac::Ticket->new( connection => $trac);
-isa_ok($ticket, 'Net::Trac::Ticket');
+is( $trac->url, $tr->url );
-ok($ticket->create(summary => 'This product has only a moose, not a pony'));
-is($ticket->id, 1);
+is(count_tickets_in_trac(),0);
-can_ok($ticket, 'load');
-ok($ticket->load(1));
-like($ticket->state->{'summary'}, qr/pony/);
-like($ticket->summary, qr/moose/, "The summary looks like a moose");
-ok( $ticket->update( summary => 'The product does not contain a pony' ), "updated!");
-unlike($ticket->summary, qr/moose/, "The summary does not look like a moose");
+my $ticket = Net::Trac::Ticket->new( connection => $trac );
+isa_ok( $ticket, 'Net::Trac::Ticket' );
+
+ok( $ticket->create( summary => 'This product has only a moose, not a pony' ) );
+is( $ticket->id, 1 );
+
+is(count_tickets_in_trac(),1);
+can_ok( $ticket, 'load' );
+ok( $ticket->load(1) );
+like( $ticket->state->{'summary'}, qr/pony/ );
+like( $ticket->summary, qr/moose/, "The summary looks like a moose" );
+ok( $ticket->update( summary => 'The product does not contain a pony' ), "updated!" );
+unlike( $ticket->summary, qr/moose/, "The summary does not look like a moose" );
my $history = $ticket->history;
-ok($history, "The ticket has some history");
-my @entries = @{$history->entries};
-my $first = shift @entries;
-is($entries[0], undef, "there is only one history entry. no create txn");
-is ($first->category, 'Ticket');
+ok( $history, "The ticket has some history" );
+my @entries = @{ $history->entries };
+my $first = shift @entries;
+is( $entries[0], undef, "there is only one history entry. no create txn" );
+is( $first->category, 'Ticket' );
my ( $ret, $out, $err );
( $ret, $out, $err ) = run_script( 'sd', [ 'clone', '--from', $sd_trac_url ] );
+is(count_tickets_in_sd(),1);
+
diag($out);
diag($err);
my $pony_id;
-run_output_matches('sd',
+run_output_matches(
+ 'sd',
[ 'ticket', 'list', '--regex', '.' ],
[qr/(.*?)(?{ $pony_id = $1 }) The product does not contain a pony new/]
);
-ok($pony_id, "I got the ID of a pony - It's $pony_id");
+ok( $pony_id, "I got the ID of a pony - It's $pony_id" );
-($ret,$out,$err) = run_script('sd', ["ticket", "update", $pony_id ,"--", "status=closed"]);
-like($out, qr/^Ticket(.*)updated/);
+( $ret, $out, $err ) = run_script( 'sd', [ "ticket", "update", $pony_id, "--", "status=closed" ] );
+like( $out, qr/^Ticket(.*)updated/ );
diag($out);
diag($err);
-($ret,$out,$err) = run_script('sd' => ["ticket" ,"basics" ,$pony_id ,"--batch"]);
+( $ret, $out, $err ) = run_script( 'sd' => [ "ticket", "basics", $pony_id, "--batch" ] );
-like($out, qr/status: closed/);
+like( $out, qr/status: closed/ );
diag("The pony is $pony_id");
-my $new_ticket = Net::Trac::Ticket->new( connection => $trac);
-ok($new_ticket->load(1));
-is($new_ticket->status, 'new', "The ticket is new before we push to trac");
+my $new_ticket = Net::Trac::Ticket->new( connection => $trac );
+ok( $new_ticket->load(1) );
+is( $new_ticket->status, 'new', "The ticket is new before we push to trac" );
+
+( $ret, $out, $err ) = run_script( 'sd', [ 'push', '--to', $sd_trac_url ] );
+diag($out);
+diag($err);
+is(count_tickets_in_trac(),1);
+my $closed_ticket = Net::Trac::Ticket->new( connection => $trac );
+ok( $closed_ticket->load(1) );
+is( $closed_ticket->status, 'resolved', "The ticket is closed after we push to trac" );
( $ret, $out, $err ) = run_script( 'sd', [ 'push', '--to', $sd_trac_url ] );
diag($out);
diag($err);
+is(count_tickets_in_trac(),1);
+is(count_tickets_in_sd(),1);
+
+# create a ticket in sd
+
+my ($yatta_id, $yatta_uuid) = create_ticket_ok( '--summary', 'This ticket originated in SD');
+
+run_output_matches( 'sd', [ 'ticket',
+ 'list', '--regex', 'This ticket originated in SD' ],
+ [ qr/(\d+) This ticket originated in SD new/]
+
+);
+
+run_output_matches( 'sd', [ 'ticket', 'basics', '--batch', '--id', $yatta_id ],
+ [
+ "id: $yatta_id ($yatta_uuid)",
+ 'summary: This ticket originated in SD',
+ 'status: new',
+ 'milestone: alpha',
+ 'component: core',
+ qr/^created: \d{4}-\d{2}-\d{2}.+$/,
+ 'creator: '. $ENV{PROPHET_EMAIL},
+ 'reporter: ' . $ENV{PROPHET_EMAIL},
+ "original_replica: " . replica_uuid,
+ ]
+);
+
+is(count_tickets_in_sd(),2);
+
+
+# create a ticket in trac
+my $ticket2 = Net::Trac::Ticket->new( connection => $trac );
+isa_ok( $ticket2, 'Net::Trac::Ticket' );
+
+ok( $ticket2->create( summary => 'This product has only a moose, not a pony' ) );
+
+
+is(count_tickets_in_trac(),2);
+
+
+
+# push to trac
-my $closed_ticket = Net::Trac::Ticket->new( connection => $trac);
-ok($closed_ticket->load(1));
-is($closed_ticket->status, 'resolved', "The ticket is closed after we push to trac");
( $ret, $out, $err ) = run_script( 'sd', [ 'push', '--to', $sd_trac_url ] );
diag($out);
diag($err);
+is(count_tickets_in_trac(),3);
+
+# pull from trac
+
+( $ret, $out, $err ) = run_script( 'sd', [ 'pull', '--from', $sd_trac_url ] );
+diag($out);
+diag($err);
+
+
+
+is(count_tickets_in_sd(),3);
+
+
+
+sub count_tickets_in_sd {
+ my $self = shift;
+
+ my ( $ret, $out, $err ) = run_script( 'sd' =>
+ [ 'ticket', 'list', '--regex', '.' ],
+ );
+ my @lines = split(/\n/,$out);
+ return scalar @lines;
+}
+
+sub count_tickets_in_trac {
+ my $self = shift;
+ my $tickets = Net::Trac::TicketSearch->new( connection => $trac );
+ my $result = $tickets->query( summary => {not => 'nonsense'});
+ my $count = scalar @{$tickets->results};
+ return $count
+}
commit 75c1e476fc78cbcae7b27d4415848827c85d93a9
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Sat May 2 12:55:33 2009 -0400
The beginning of proper recording of tickets pushed to trac
diff --git a/lib/App/SD/Replica/trac.pm b/lib/App/SD/Replica/trac.pm
index c29f1f2..7bdea56 100644
--- a/lib/App/SD/Replica/trac.pm
+++ b/lib/App/SD/Replica/trac.pm
@@ -49,31 +49,62 @@ sub BUILD {
sub record_pushed_transactions {
my $self = shift;
my %args = validate( @_,
- { ticket => 1, changeset => { isa => 'Prophet::ChangeSet' } } );
+ { ticket => 1, changeset => { isa => 'Prophet::ChangeSet' }, start_time => 1} );
+
+ my $earliest_valid_txn_date;
+
# walk through every transaction on the ticket, starting with the latest
- warn "need to walk the pushed txns";
- for my $txn ( undef ){# 'find all the transactions pushed upstream') {
- last;
+ my $ticket = Net::Trac::Ticket->new( connection => $self->trac);
+ $ticket->load($args{ticket});
+
+ for my $txn ( sort {$b->date <=> $a->date } @{$ticket->history->entries}) {
+
+ warn "Recording that we pushed ".$ticket->id. " " .$txn->date;
+
+ my $oldest_changeset_for_ticket = $self->app_handle->handle->last_changeset_from_source( $args{changeset}->original_source_uuid);
+
+ # walk backwards through all transactions on the ticket we just updated
+ # Skip any transaction where the remote user isn't me, this might include any transaction
+ # RT created with a scrip on your behalf
+
+ next unless $txn->author eq $self->trac->user;
+ # XXX - are we always decoding txn author correctly?
+
+ # get the completion time _after_ we do our next round trip to rt to try to make sure
+ # a bit of lag doesn't skew us to the wrong side of a 1s boundary
+ my $txn_created_dt = $txn->date;
+ unless($txn_created_dt) {
+ die $args{ticket}. " - Couldn't parse '".$txn->created."' as a timestamp";
+ }
+ my $txn_created = $txn_created_dt->epoch;
+
+
+ # skip any transaction created more than 5 seconds before the push started.
+ if (!$earliest_valid_txn_date){
+ my $change_window = time() - $args{start_time};
+ # I can't think of any reason that number shouldn't be 1, but clocks are fickle
+ $earliest_valid_txn_date = $txn_created - ($change_window + 5);
+ }
+
+ last if $txn_created < $earliest_valid_txn_date;
+
# if the transaction id is older than the id of the last changeset
# we got from the original source of this changeset, we're done
- last if $txn->id <= $self->last_changeset_from_source(
- $args{changeset}->original_source_uuid
- );
+ last if $txn_created <= $oldest_changeset_for_ticket;
- # if the transaction from Trac is more recent than the most recent
+ # if the transaction from trac is more recent than the most recent
# transaction we got from the original source of the changeset
# then we should record that we sent that transaction upstream
- # XXX TODO - THIS IS WRONG - we should only be recording transactions we pushed
+
$self->record_pushed_transaction(
- transaction => $txn->id,
+ transaction => $txn_created,
changeset => $args{'changeset'},
record => $args{'ticket'}
);
}
}
-
=head2 uuid
Return the replica's UUID
@@ -85,7 +116,6 @@ sub uuid {
return $self->uuid_for_url( $self->remote_url);
}
-
sub remote_uri_path_for_comment {
my $self = shift;
my $id = shift;
@@ -99,6 +129,7 @@ sub remote_uri_path_for_id {
}
+
__PACKAGE__->meta->make_immutable;
no Any::Moose;
diff --git a/lib/App/SD/Replica/trac/PullEncoder.pm b/lib/App/SD/Replica/trac/PullEncoder.pm
index d24a613..97fe2d5 100644
--- a/lib/App/SD/Replica/trac/PullEncoder.pm
+++ b/lib/App/SD/Replica/trac/PullEncoder.pm
@@ -49,7 +49,11 @@ sub run {
my $ticket_initial_data = {%$ticket_data};
my $txns = $self->skip_previously_seen_transactions(
ticket => $ticket,
- transactions => $ticket->history->entries
+ transactions => $ticket->history->entries,
+ starting_transaction => $self->sync_source->app_handle->handle->last_changeset_from_source(
+ $self->sync_source->uuid_for_remote_id( $ticket->id )
+ )
+
);
# Walk transactions newest to oldest.
@@ -65,16 +69,11 @@ sub run {
unshift @changesets, $self->build_create_changeset( $ticket_initial_data, $ticket );
}
- $self->_record_upstream_last_modified_date($last_modified_date);
+ $self->sync_source->record_upstream_last_modified_date($last_modified_date);
$args{callback}->($_) for @changesets;
}
-sub _record_upstream_last_modified_date {
- my $self = shift;
- my $date = shift;
- return $self->sync_source->store_local_metadata('last_changeset_date' => $date);
-}
sub _translate_final_ticket_state {
my $self = shift;
@@ -122,7 +121,7 @@ sub find_matching_tickets {
my $last_changeset_seen_dt;
- if (my $last_changeset_seen = $self->sync_source->fetch_local_metadata('last_changeset_date') ) {
+ if (my $last_changeset_seen = $self->sync_source->upstream_last_modified_date()) {
$last_changeset_seen_dt = Net::Trac::Ticket->timestamp_to_datetime($last_changeset_seen);
}
@@ -151,14 +150,25 @@ sub skip_previously_seen_transactions {
my %args = validate( @_, { ticket => 1, transactions => 1, starting_transaction => 0 } );
my @txns;
+ $self->sync_source->log('Looking at pulled txns');
for my $txn ( sort @{ $args{transactions} } ) {
+ $self->sync_source->log('Looking at pulled txn ' . $txn);
# Skip things we know we've already pulled
next if $txn < ( $args{'starting_transaction'} ||0 );
+ $self->sync_source->log("It's after our starting txn");
# Skip things we've pushed
- next if $self->sync_source->foreign_transaction_originated_locally($txn, $args{'ticket'});
+
+ warn "Considering pulling $txn from ".$args{ticket};
+ if ($self->sync_source->foreign_transaction_originated_locally($txn, $args{'ticket'}) ) {
+ warn "YES, it did come from us";
+ next
+ } else {
+
+ $self->sync_source->log("It's not something we pushed");
push @txns, $txn;
+ }
}
return \@txns;
}
diff --git a/lib/App/SD/Replica/trac/PushEncoder.pm b/lib/App/SD/Replica/trac/PushEncoder.pm
index 1e769fa..7621715 100644
--- a/lib/App/SD/Replica/trac/PushEncoder.pm
+++ b/lib/App/SD/Replica/trac/PushEncoder.pm
@@ -8,7 +8,6 @@ has sync_source =>
( isa => 'App::SD::Replica::trac',
is => 'rw');
-
sub integrate_change {
my $self = shift;
my ( $change, $changeset ) = validate_pos(
@@ -16,41 +15,37 @@ sub integrate_change {
{ isa => 'Prophet::Change' },
{ isa => 'Prophet::ChangeSet' }
);
- my $id;
- my $record;
- # if the original_sequence_no of this changeset is <=
- # the last changeset our sync source for the original_sequence_no, we can skip it.
+ my ($id, $record);
+
+ # if the original_sequence_no of this changeset is <=
+ # the last changeset our sync source for the original_sequence_no, we can skip it.
# XXX TODO - this logic should be at the changeset level, not the cahnge level, as it applies to all
# changes in the changeset
- return if $self->sync_source->app_handle->handle->last_changeset_from_source( $changeset->original_source_uuid) >= $changeset->original_sequence_no;
+ return
+ if $self->sync_source->app_handle->handle->last_changeset_from_source(
+ $changeset->original_source_uuid
+ ) >= $changeset->original_sequence_no;
+
+ my $before_integration = time();
eval {
if ( $change->record_type eq 'ticket'
- and $change->change_type eq 'add_file' )
- {
+ and $change->change_type eq 'add_file' ) {
$id = $self->integrate_ticket_create( $change, $changeset );
$self->sync_source->record_remote_id_for_pushed_record(
uuid => $change->record_uuid,
- remote_id => $id
- );
-
+ remote_id => $id);
}
- elsif (
- $change->record_type eq 'attachment'
- and $change->change_type eq 'add_file'
-
- )
- {
+ elsif ( $change->record_type eq 'attachment'
+ and $change->change_type eq 'add_file') {
$id = $self->integrate_attachment( $change, $changeset );
}
elsif ( $change->record_type eq 'comment'
- and $change->change_type eq 'add_file' )
- {
+ and $change->change_type eq 'add_file' ) {
$id = $self->integrate_comment( $change, $changeset );
}
elsif ( $change->record_type eq 'ticket' ) {
$id = $self->integrate_ticket_update( $change, $changeset );
-
}
else {
$self->sync_source->log('I have no idea what I am doing for '.$change->record_uuid);
@@ -58,11 +53,11 @@ sub integrate_change {
}
$self->sync_source->record_pushed_transactions(
+ start_time => $before_integration,
ticket => $id,
- changeset => $changeset
- );
-
+ changeset => $changeset);
};
+
if (my $err = $@) {
$self->sync_source->log("Push error: ".$err);
}
@@ -145,7 +140,6 @@ sub integrate_attachment {
return $ticket_id;
}
-
sub _recode_props_for_integrate {
my $self = shift;
my ($change) = validate_pos( @_, { isa => 'Prophet::Change' } );
@@ -167,7 +161,6 @@ sub _recode_props_for_integrate {
return \%attr;
}
-
__PACKAGE__->meta->make_immutable;
no Any::Moose;
commit d620eb235e15520bba143fc8d18838f51fc7c837
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Sat May 2 12:56:07 2009 -0400
Extracted methods from the rt replica to the foreign replica base class
diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index cf9474d..94be5cd 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -45,15 +45,11 @@ sub record_pushed_transaction {
my %args = validate( @_,
{ transaction => 1, changeset => { isa => 'Prophet::ChangeSet' }, record => 1 } );
- $self->store_local_metadata(
- "foreign-txn-from-"
- . $self->uuid
- . '-record-'
- . $args{record} . '-txn-'
- . $args{transaction} => join( ':',
- $args{changeset}->original_source_uuid,
- $args{changeset}->original_sequence_no )
- );
+ my $key = join('-', "foreign-txn-from" , $self->uuid , 'record' , $args{record} , 'txn' , $args{transaction} );
+ my $value = join(':', $args{changeset}->original_source_uuid, $args{changeset}->original_sequence_no );
+ warn "Recording a pushed transaction: $key -> $value";
+ $self->store_local_metadata($key => $value);
+
}
=head2 foreign_transaction_originated_locally $transaction_id $foreign_record_id
@@ -203,6 +199,17 @@ sub record_remote_id_for_pushed_record {
$self->_set_remote_id_for_uuid(%args);
}
+sub record_upstream_last_modified_date {
+ my $self = shift;
+ my $date = shift;
+ return $self->store_local_metadata('last_modified_date' => $date);
+}
+
+sub upstream_last_modified_date {
+ my $self = shift;
+ return $self->fetch_local_metadata('last_modified_date');
+}
+
__PACKAGE__->meta->make_immutable;
no Any::Moose;
commit 66994d51da5c274dd3d34e05235f7fb40c164c50
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Sat May 2 12:56:45 2009 -0400
Make the RT replica no longer shadow the just-extracted methods
diff --git a/lib/App/SD/Replica/rt.pm b/lib/App/SD/Replica/rt.pm
index 636a104..f51e74c 100644
--- a/lib/App/SD/Replica/rt.pm
+++ b/lib/App/SD/Replica/rt.pm
@@ -61,10 +61,7 @@ sub record_pushed_transactions {
rt => $self->rt,
id => $args{'ticket'})->transactions->get_iterator->()) {
-
# walk backwards through all transactions on the ticket we just updated
-
-
# Skip any transaction where the remote user isn't me, this might include any transaction
# RT created with a scrip on your behalf
next unless $txn->creator eq $self->rt_username;
@@ -103,17 +100,6 @@ sub record_pushed_transactions {
}
}
-sub record_upstream_last_modified_date {
- my $self = shift;
- my $date = shift;
- return $self->store_local_metadata('last_modified_date' => $date);
-}
-
-sub upstream_last_modified_date {
- my $self = shift;
- return $self->fetch_local_metadata('last_modified_date');
-}
-
sub upstream_last_txn {
my $self = shift;
return $self->fetch_local_metadata('last_txn_id');
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list