[Bps-public-commit] SD branch, master, updated. 523a3cb433fb3629dcc6fa4631575d1a1df9a284
jesse
jesse at bestpractical.com
Tue May 5 18:44:20 EDT 2009
The branch, master has been updated
via 523a3cb433fb3629dcc6fa4631575d1a1df9a284 (commit)
via d181f47561a42f1883067ad70ffe8c41a97d108e (commit)
via 9a266ef120c77acbdc5f4c4a1ac269c16973c71e (commit)
via 969f864ed4c398226db23208f6e2c9f7f7820f7d (commit)
via 35e12d44978131a7d5b2ce0437998885ae2cce8e (commit)
via a382579520082151174f46b17d40a9c6dbe70a44 (commit)
via 4aa7fafe0993c5b7048f54c0c80144894a1bb6b2 (commit)
via 7b093f9ad01cbcbb366a8247a16ad5df3400b82f (commit)
from 2351a4e4ec9748c34eae06c0179f778e99e0ea4f (commit)
Summary of changes:
Makefile.PL | 4 +-
lib/App/SD/ForeignReplica.pm | 26 +++++++++------
lib/App/SD/ForeignReplica/PullEncoder.pm | 29 +++++++++++++++++
lib/App/SD/Replica/rt/PullEncoder.pm | 17 +---------
lib/App/SD/Replica/trac/PullEncoder.pm | 51 ++++++++++++------------------
t/sd-redmine/basic.t | 5 ++-
t/sd-rt/basic.t | 4 +--
t/sd-rt/sd-rt-permission.t | 9 +-----
8 files changed, 74 insertions(+), 71 deletions(-)
- Log -----------------------------------------------------------------
commit 7b093f9ad01cbcbb366a8247a16ad5df3400b82f
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 13:31:54 2009 -0400
Enabled author tests
diff --git a/Makefile.PL b/Makefile.PL
index f89e1d1..d2f497a 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -43,6 +43,6 @@ install_share('share');
# Include subdirectory tests too.
tests("t/*.t t/*/*.t");
-
+extra_tests;
auto_install;
&WriteAll;
commit 4aa7fafe0993c5b7048f54c0c80144894a1bb6b2
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 13:32:58 2009 -0400
skip redmine tests if you don't have redmine
diff --git a/t/sd-redmine/basic.t b/t/sd-redmine/basic.t
index 7d9fe79..aa52cd9 100644
--- a/t/sd-redmine/basic.t
+++ b/t/sd-redmine/basic.t
@@ -8,7 +8,10 @@ require File::Temp;
$ENV{'PROPHET_REPO'} = $ENV{'SD_REPO'} = File::Temp::tempdir( CLEANUP => 1 ) . '/_svb';
diag "export SD_REPO=" . $ENV{'PROPHET_REPO'} . "\n";
-use Net::Redmine;
+unless ( eval { require Net::Redmine } ) {
+ plan skip_all => 'You need Net::Trac installed to run the tests';
+ }
+
require 't/sd-redmine/net_redmine_test.pl';
my $r = new_redmine();
commit a382579520082151174f46b17d40a9c6dbe70a44
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 14:37:57 2009 -0400
Removed numbering tests from sd-rt-permission.t since the new reporting callback can't do that
diff --git a/t/sd-rt/sd-rt-permission.t b/t/sd-rt/sd-rt-permission.t
index 10f6ca8..de1e624 100644
--- a/t/sd-rt/sd-rt-permission.t
+++ b/t/sd-rt/sd-rt-permission.t
@@ -22,7 +22,7 @@ BEGIN {
}
-plan tests => 23;
+plan tests => 17;
use App::SD::Test;
use RT::Client::REST;
use RT::Client::REST::Ticket;
@@ -67,7 +67,6 @@ as_alice {
run_script( 'sd', [ 'init']);
($ret, $out, $err) = run_script('sd', ['pull', '--from', $alice_rt_url, '--force']);
ok($ret);
- like($out, qr/No new changesets/);
like($err, qr/No tickets found/);
};
@@ -83,7 +82,6 @@ my $flyman_id;
as_alice {
($ret, $out, $err) = run_script('sd', ['pull', '--from', $alice_rt_url]);
ok($ret);
- like($out, qr/Merged one changeset/);
run_output_matches( 'sd', [ 'ticket', 'list', '--regex', '.' ],
[qr/(.*?)(?{ $flyman_id = $1 }) Fly Man new/] );
@@ -106,7 +104,6 @@ as_alice {
# we should know exactly how many changesets there are.. used to be 1,
# now it's 13. one must fail to be merged but we can still report that
# the others (up to but excluding the failure) were successfully merged
- unlike($out, qr/Merged 12 changesets/);
}
# try again to make sure we still have pending changesets
@@ -114,7 +111,6 @@ as_alice {
TODO: {
local $TODO = "we mark all changesets as merged even if some failed";
- unlike($out, qr/No new changesets/, "there are still pending changesets");
}
};
@@ -134,7 +130,6 @@ as_alice {
ok($ret);
TODO: {
local $TODO = "Prophet thinks it already merged this changeset!";
- like($out, qr/Merged one changeset/);
}
};
@@ -159,7 +154,6 @@ $ticket = RT::Client::REST::Ticket->new(
as_alice {
($ret, $out, $err) = run_script('sd', ['pull', '--from', $alice_rt_url]);
ok($ret);
- like($out, qr/No new changesets/);
run_output_matches( 'sd', [ 'ticket', 'list', '--regex', '.' ],
[qr/Fly Man new/] );
@@ -177,7 +171,6 @@ as_alice {
($ret, $out, $err) = run_script('sd', ['push', '--to', $alice_rt_url]);
ok($ret);
- like($out, qr/Merged one changeset/, "pushed the 'resolve' changeset");
};
$ticket = RT::Client::REST::Ticket->new(
commit 35e12d44978131a7d5b2ce0437998885ae2cce8e
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 14:40:53 2009 -0400
* fixes for new reporting callback
diff --git a/t/sd-rt/basic.t b/t/sd-rt/basic.t
index 7500644..6664aff 100644
--- a/t/sd-rt/basic.t
+++ b/t/sd-rt/basic.t
@@ -15,7 +15,7 @@ BEGIN {
}
}
-plan tests => 43;
+plan tests => 41;
use App::SD::Test;
no warnings 'once';
@@ -276,7 +276,6 @@ like( $out, qr/Created comment/ );
( $ret, $out, $err ) = run_script( 'sd', [ 'pull', '--from', $sd_rt_url ] );
ok( $ret, $out );
- like( $out, qr/No new changesets/ );
my $fetched_ticket = RT::Client::REST::Ticket->new(
rt => $rt,
@@ -297,7 +296,6 @@ like( $out, qr/Created comment/ );
diag($out);
( $ret, $out, $err ) = run_script( 'sd', [ 'pull', '--from', $sd_rt_url ] );
ok( $ret, $out );
- like( $out, qr/No new changesets/ );
my $fetched_ticket = RT::Client::REST::Ticket->new(
rt => $rt,
commit 969f864ed4c398226db23208f6e2c9f7f7820f7d
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 18:39:38 2009 -0400
Minor refactoring to unify codepaths
diff --git a/lib/App/SD/ForeignReplica.pm b/lib/App/SD/ForeignReplica.pm
index 94be5cd..2ba121e 100644
--- a/lib/App/SD/ForeignReplica.pm
+++ b/lib/App/SD/ForeignReplica.pm
@@ -47,7 +47,7 @@ sub record_pushed_transaction {
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);
}
@@ -110,32 +110,36 @@ sub uuid_for_remote_id {
my ( $self, $id ) = @_;
return $self->_lookup_uuid_for_remote_id($id)
- || $self->uuid_for_url( $self->remote_url . $self->remote_uri_path_for_id($id) );
+ ||$self->_url_based_uuid_for_remote_ticket_id( $id);
}
sub _lookup_uuid_for_remote_id {
my $self = shift;
my ($id) = validate_pos( @_, 1 );
-
-
- return $self->fetch_local_metadata('local_uuid_for_'.
- $self->uuid_for_url( $self->remote_url . $self->remote_uri_path_for_id($id))
- );
+ return $self->fetch_local_metadata('local_uuid_for_'. $self->_url_based_uuid_for_remote_ticket_id( $id));
}
sub _set_uuid_for_remote_id {
my $self = shift;
my %args = validate( @_, { uuid => 1, remote_id => 1 } );
return $self->store_local_metadata('local_uuid_for_'.
- $self->uuid_for_url(
- $self->remote_url
- . $self->remote_uri_path_for_id( $args{'remote_id'} )
- ),
+ $self->_url_based_uuid_for_remote_ticket_id( $args{'remote_id'} ),
$args{uuid}
);
}
+sub _url_based_uuid_for_remote_ticket_id {
+ my $self = shift;
+ my $id = shift;
+ return $self->uuid_for_url(
+ $self->remote_url
+ . $self->remote_uri_path_for_id( $id)
+ );
+
+}
+
+
# This mapping table stores uuids for tickets we've synced from a remote database
# Basically, if we created the ticket to begin with, then we'll know its uuid
# if we pulled the ticket from the foreign replica then its uuid will be generated
commit 9a266ef120c77acbdc5f4c4a1ac269c16973c71e
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 18:40:20 2009 -0400
* extract a method used for skipping transactions we know we don't care about
diff --git a/lib/App/SD/ForeignReplica/PullEncoder.pm b/lib/App/SD/ForeignReplica/PullEncoder.pm
index 97563f5..20448f4 100644
--- a/lib/App/SD/ForeignReplica/PullEncoder.pm
+++ b/lib/App/SD/ForeignReplica/PullEncoder.pm
@@ -1,5 +1,6 @@
package App::SD::ForeignReplica::PullEncoder;
use Any::Moose;
+use App::SD::Util;
sub warp_list_to_old_value {
my $self = shift;
@@ -13,6 +14,34 @@ sub warp_list_to_old_value {
return join( ", ", @old );
}
+=head2 _only_pull_tickets_modified_after
+
+If we've previously pulled from this sync source, this routine will
+return a datetime object. It's safe not to evaluate any ticket last
+modified before that datetime
+
+=cut
+
+sub _only_pull_tickets_modified_after {
+ my $self = shift;
+
+ # 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 $last_pull = $self->sync_source->upstream_last_modified_date();
+ return undef unless $last_pull;
+ my $before = App::SD::Util::string_to_datetime($last_pull);
+ die "Failed to parse '" . $self->sync_source->upstream_last_modified_date() . "' as a timestamp"
+ unless ($before);
+
+ # 26 hours ago deals with most any possible timezone/dst edge case
+ $before->subtract( hours => 26 );
+
+ return $before;
+}
+
__PACKAGE__->meta->make_immutable;
no Any::Moose;
1;
diff --git a/lib/App/SD/Replica/rt/PullEncoder.pm b/lib/App/SD/Replica/rt/PullEncoder.pm
index 2426bf5..0be57e2 100644
--- a/lib/App/SD/Replica/rt/PullEncoder.pm
+++ b/lib/App/SD/Replica/rt/PullEncoder.pm
@@ -148,28 +148,15 @@ sub find_matching_tickets {
my ($query) = validate_pos(@_, 1);
# If we've ever synced, we can limit our search to only newer things
- if ($self->sync_source->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 = App::SD::Util::string_to_datetime($self->sync_source->upstream_last_modified_date());
- die "Failed to parse '".$self->sync_source->upstream_last_modified_date() ."' as a timestamp" unless ($before);
- ; # 26 hours ago deals with most any possible edge case
- $before->subtract(hours => 26);
-
-
+ if (my $before = $self->_only_pull_tickets_modified_after) {
$query = "($query) AND LastUpdated >= '".$before->ymd('-'). " " .$before->hms(':') ."'";
-
-
- warn $query;
$self->sync_source->log("Skipping all tickets not updated since ".$before->iso8601);
}
return $self->sync_source->rt->search( type => 'ticket', query => $query );
}
+
=head2 find_matching_transactions { ticket => $id, starting_transaction => $num }
Returns a reference to an array of all transactions (as hashes) on ticket $id after transaction $num.
commit d181f47561a42f1883067ad70ffe8c41a97d108e
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 18:43:43 2009 -0400
Now that we have a create changeset in trac history, use it
diff --git a/lib/App/SD/Replica/trac/PullEncoder.pm b/lib/App/SD/Replica/trac/PullEncoder.pm
index 97fe2d5..d511566 100644
--- a/lib/App/SD/Replica/trac/PullEncoder.pm
+++ b/lib/App/SD/Replica/trac/PullEncoder.pm
@@ -55,7 +55,7 @@ sub run {
)
);
-
+
# Walk transactions newest to oldest.
for my $txn ( sort { $b->date <=> $a->date } @$txns ) {
$self->sync_source->log( $ticket->id . " - Transcoding transaction @{[$txn->date]} " );
@@ -65,13 +65,10 @@ sub run {
grep {defined} $self->transcode_one_txn( $txn, $ticket_initial_data, $ticket_data );
}
- # create is oldest of all
- unshift @changesets, $self->build_create_changeset( $ticket_initial_data, $ticket );
}
- $self->sync_source->record_upstream_last_modified_date($last_modified_date);
-
$args{callback}->($_) for @changesets;
+ $self->sync_source->record_upstream_last_modified_date($last_modified_date);
}
@@ -118,17 +115,10 @@ Returns a Trac::TicketSearch collection for all tickets found matching your QUER
sub find_matching_tickets {
my $self = shift;
my %query = (@_);
-
-
- my $last_changeset_seen_dt;
- 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);
- }
-
+ my $last_changeset_seen_dt = $self->_only_pull_tickets_modified_after();
$self->sync_source->log("Searching for tickets");
- my $search
- = Net::Trac::TicketSearch->new( connection => $self->sync_source->trac, limit => 500 );
+ my $search = Net::Trac::TicketSearch->new( connection => $self->sync_source->trac, limit => 9999 );
$search->query(%query);
my @results = @{$search->results};
$self->sync_source->log("Trimming things after our last pull");
@@ -150,26 +140,18 @@ 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);
+ my $txn_date = $txn->date->epoch;
# 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");
+ next if $txn_date < ( $args{'starting_transaction'} ||0 );
# Skip things we've pushed
-
- 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");
+ next if ($self->sync_source->foreign_transaction_originated_locally($txn_date, $args{'ticket'}->id) );
+
+ # ok. it didn't originate locally. we might want to integrate it
push @txns, $txn;
- }
}
+ $self->sync_source->log('Done looking at pulled txns');
return \@txns;
}
@@ -197,10 +179,12 @@ sub build_initial_ticket_state {
return \%initial_state;
}
-sub build_create_changeset {
+sub transcode_create_txn {
my $self = shift;
+ my $txn = shift;
my $create_data = shift;
- my $ticket = shift;
+ my $final_data = shift;
+ my $ticket = $txn->ticket;
# this sequence_no only works because trac tickets only allow one update
# per ticket per second.
# we decrement by 1 on the off chance that someone created and
@@ -208,7 +192,7 @@ sub build_create_changeset {
my $changeset = Prophet::ChangeSet->new(
{ original_source_uuid => $self->sync_source->uuid_for_remote_id( $ticket->id ),
original_sequence_no => ( $ticket->created->epoch-1),
- creator => $self->resolve_user_id_to( email_address => $ticket->reporter ),
+ creator => $self->resolve_user_id_to( email_address => $create_data->{reporter} ),
created => $ticket->created->ymd ." ".$ticket->created->hms
}
);
@@ -237,6 +221,11 @@ sub build_create_changeset {
sub transcode_one_txn {
my ( $self, $txn, $ticket, $ticket_final ) = (@_);
+
+ if ($txn->is_create) {
+ return $self->transcode_create_txn(@_);
+ }
+
my $ticket_uuid = $self->sync_source->uuid_for_remote_id( $ticket->{ $self->sync_source->uuid . '-id' } );
my $changeset = Prophet::ChangeSet->new(
commit 523a3cb433fb3629dcc6fa4631575d1a1df9a284
Author: Jesse Vincent <jesse at bestpractical.com>
Date: Tue May 5 18:44:09 2009 -0400
bump the Net::Trac dep
diff --git a/Makefile.PL b/Makefile.PL
index d2f497a..33c15ce 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -33,7 +33,7 @@ recommends 'Email::Address';
feature 'Trac sync' =>
-default => 0,
'LWP::Simple' => 0,
- 'Net::Trac' => 0.08;
+ 'Net::Trac' => 0.13;
recommends 'Net::Trac';
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list