[Rt-commit] rt branch, 4.2/fix-organization-for-all-links, created. rt-4.2.3-14-gc7dc9aa
Jim Brandt
jbrandt at bestpractical.com
Thu Feb 27 13:23:15 EST 2014
The branch, 4.2/fix-organization-for-all-links has been created
at c7dc9aa791b5192389e3e16fbd98ef1f10bdc6bf (commit)
- Log -----------------------------------------------------------------
commit fb647e0f74487c40b6b39b7478bb789f52368e45
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Wed Feb 26 15:30:51 2014 -0500
Update tests to reflect API consistent with ticket links
Also expand test coverage to handle additional failure cases.
diff --git a/t/articles/uri-articles.t b/t/articles/uri-articles.t
index b6a1c8d..9ad4b07 100644
--- a/t/articles/uri-articles.t
+++ b/t/articles/uri-articles.t
@@ -2,7 +2,8 @@
use strict;
use warnings;
-use RT::Test tests => 9;
+use RT::Test tests => undef;
+use Test::Warn;
use_ok "RT::URI::fsck_com_article";
my $uri = RT::URI::fsck_com_article->new( $RT::SystemUser );
@@ -12,7 +13,7 @@ isa_ok $uri, 'RT::URI::fsck_com_article';
isa_ok $uri, 'RT::URI::base';
isa_ok $uri, 'RT::Base';
-is $uri->LocalURIPrefix, 'fsck.com-article://example.com/article/';
+is $uri->LocalURIPrefix, 'fsck.com-article://example.com';
my $class = RT::Class->new( $RT::SystemUser );
$class->Create( Name => 'URItest - '. $$ );
@@ -26,5 +27,24 @@ my ($id, $msg) = $article->Create(
ok($id,$msg);
$uri = RT::URI::fsck_com_article->new( $article->CurrentUser );
-is $uri->LocalURIPrefix . $article->id, $uri->URIForObject( $article );
+is $uri->URIForObject( $article ),
+ 'fsck.com-article://example.com/article/' . $article->id,
+ 'Got correct URIForObject';
+my $article_id = $article->Id;
+ok ($uri->ParseURI("fsck.com-article://example.com/article/$article_id"),
+ 'Parsed URI');
+ok ($article->Delete(), 'Deleted article');
+
+my $ret;
+warning_like {
+ $ret = $uri->ParseURI("fsck.com-article://example.com/article/$article_id");
+} qr/Unable to load article for id $article_id. It may have been deleted/,
+ "Warned about missing article";
+
+ok (!$ret, 'Returned false on missing article');
+
+ok (!$uri->ParseURI("fsck.com-article://foo.com/article/$article_id"),
+ 'ParseURI returned false with incorrect Organization');
+
+done_testing();
commit 6d5a07560c656c7d1e0cd1f0b6b372de943d15e3
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Wed Feb 26 15:31:07 2014 -0500
Update article URI API to be consistent with ticket URI API
Previously the article implementation of LocalURIPrefix added
/article/ to the returned prefix where the ticket version
implemented in fsck_com_rt.pm did not add /ticket/ in
LocalURIPrefix but rather in URIForObject. LocalURIPrefix is
largely an internal method used by URIForObject, however
rt-validator uses it when fixing links for Organization
changes. Make the APIs consistent to avoid special case code
elsewhere.
diff --git a/lib/RT/URI/fsck_com_article.pm b/lib/RT/URI/fsck_com_article.pm
index 0c61623..031f504 100644
--- a/lib/RT/URI/fsck_com_article.pm
+++ b/lib/RT/URI/fsck_com_article.pm
@@ -63,8 +63,7 @@ Returns the prefix for a local article URI
sub LocalURIPrefix {
my $self = shift;
- my $prefix = $self->Scheme. "://". RT->Config->Get('Organization')
- . "/article/";
+ my $prefix = $self->Scheme. "://". RT->Config->Get('Organization');
return ($prefix);
}
@@ -79,7 +78,7 @@ sub URIForObject {
my $self = shift;
my $obj = shift;
- return ($self->LocalURIPrefix. $obj->Id);
+ return ($self->LocalURIPrefix . "/article/" . $obj->Id);
}
@@ -114,7 +113,7 @@ sub ParseURI {
#If it's a local URI, load the article object and return its URI
if ( $self->IsLocal) {
my $local_uri_prefix = $self->LocalURIPrefix;
- if ($self->{'uri'} =~ /^$local_uri_prefix(\d+)$/) {
+ if ($self->{'uri'} =~ /^$local_uri_prefix\/article\/(\d+)$/) {
my $id = $1;
$article = RT::Article->new( $self->CurrentUser );
$article->Load($id);
commit d4a4317cfbbbe919881da3131c4a4b738741aa92
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Wed Feb 26 17:26:17 2014 -0500
Warn if we parse an id but can't load the article
diff --git a/lib/RT/URI/fsck_com_article.pm b/lib/RT/URI/fsck_com_article.pm
index 031f504..5819ab6 100644
--- a/lib/RT/URI/fsck_com_article.pm
+++ b/lib/RT/URI/fsck_com_article.pm
@@ -116,10 +116,14 @@ sub ParseURI {
if ($self->{'uri'} =~ /^$local_uri_prefix\/article\/(\d+)$/) {
my $id = $1;
$article = RT::Article->new( $self->CurrentUser );
- $article->Load($id);
+ my ($ret, $msg) = $article->Load($id);
#If we couldn't find a article, return undef.
- unless ( defined $article->Id ) {
+ unless ( $article and $article->Id ) {
+ # We got an id, but couldn't load it, so warn that it may
+ # have been deleted.
+ RT::Logger->warning("Unable to load article for id $id. It may"
+ . " have been deleted: $msg");
return undef;
}
} else {
commit 71ab3bfd749c9f70ce4cd97e23e67d79a843ec05
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Wed Feb 26 16:55:51 2014 -0500
Move article object validation to cover entire method
Add a check on the $article object to the end of the method to
catch the case where Organization may have changed. Previously
this would result in a "Can't call method "Id" on an undefined
value" warning.
diff --git a/lib/RT/URI/fsck_com_article.pm b/lib/RT/URI/fsck_com_article.pm
index 5819ab6..931c55f 100644
--- a/lib/RT/URI/fsck_com_article.pm
+++ b/lib/RT/URI/fsck_com_article.pm
@@ -131,6 +131,11 @@ sub ParseURI {
}
}
+ #If we couldn't find a article, return undef.
+ unless ( $article and $article->Id ) {
+ return undef;
+ }
+
$self->{'object'} = $article;
return ($article->Id);
}
commit 11f646aeb63e79804a4898865969d5377b9dcfef
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Tue Feb 25 15:23:04 2014 -0500
Validate article and other types of links
Check non-ticket links for a changed Organization and
for links to missing objects like Articles. Use %INC
to find loaded RT::URI subclasses to catch link types that
might be loaded from an extension like Assets.
Add Article to the models checked since Articles are part of
RT now.
diff --git a/sbin/rt-validator.in b/sbin/rt-validator.in
index db89dce..d24b777 100644
--- a/sbin/rt-validator.in
+++ b/sbin/rt-validator.in
@@ -115,6 +115,7 @@ my %TYPE = (
my @models = qw(
ACE
+ Article
Attachment
Attribute
CachedGroupMember
@@ -930,7 +931,10 @@ push @CHECKS, 'Links: wrong organization' => sub {
{ model => 'Link', column => 'Base' },
);
- my $rt_uri = RT::URI::fsck_com_rt->new( $RT::SystemUser );
+ my @rt_uris = rt_uri_modules();
+ foreach my $package (@rt_uris) {
+
+ my $rt_uri = $package->new( $RT::SystemUser );
my $scheme = $rt_uri->Scheme;
my $prefix = $rt_uri->LocalURIPrefix;
@@ -972,6 +976,7 @@ push @CHECKS, 'Links: wrong organization' => sub {
last; # plenty of chances we covered all cases with one update
}
}
+ } # end foreach my $package (@rt_uris)
return $res;
};
@@ -1055,7 +1060,10 @@ push @CHECKS, 'Links: missing object' => sub {
{ model => 'Link', column => 'Base' },
);
- my $rt_uri = RT::URI::fsck_com_rt->new( $RT::SystemUser );
+ my @rt_uris = rt_uri_modules();
+ foreach my $package (@rt_uris) {
+
+ my $rt_uri = $package->new( $RT::SystemUser );
my $scheme = $rt_uri->Scheme;
my $prefix = $rt_uri->LocalURIPrefix;
@@ -1069,6 +1077,8 @@ push @CHECKS, 'Links: missing object' => sub {
my $tprefix = $prefix .'/'. ($tclass eq 'RT::Ticket'? 'ticket' : $tclass) .'/';
+ $tprefix = $prefix . '/article/' if $tclass eq 'RT::Article';
+
my $query = "SELECT s.id FROM $stable s LEFT JOIN $ttable t "
." ON t.id = ". sql_str2int("SUBSTR(s.$scolumn, ?)")
." WHERE s.$scolumn LIKE ? AND t.id IS NULL";
@@ -1096,6 +1106,7 @@ push @CHECKS, 'Links: missing object' => sub {
}
}
}
+ } # end foreach my $package (@rt_uris)
return $res;
};
@@ -1375,6 +1386,20 @@ sub prompt_integer {
return $cached_answer{ $token } = $a;
} }
+# Find all RT::URI modules RT has loaded
+
+sub rt_uri_modules {
+ my @uris = grep /^RT\/URI\/.+\.pm$/, keys %INC;
+ my @uri_modules;
+ foreach my $uri_path (@uris){
+ next if $uri_path =~ /base\.pm$/; # Skip base RT::URI object
+ $uri_path = substr $uri_path, 0, -3; # chop off .pm
+ push @uri_modules, join '::', split '/', $uri_path;
+ }
+
+ return @uri_modules;
+}
+
1;
__END__
commit 19cfd1305912ada718b891ccbfe089fafdc842bf
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Tue Feb 25 16:44:11 2014 -0500
Add indent for new foreach loops
diff --git a/sbin/rt-validator.in b/sbin/rt-validator.in
index d24b777..e60328d 100644
--- a/sbin/rt-validator.in
+++ b/sbin/rt-validator.in
@@ -934,48 +934,48 @@ push @CHECKS, 'Links: wrong organization' => sub {
my @rt_uris = rt_uri_modules();
foreach my $package (@rt_uris) {
- my $rt_uri = $package->new( $RT::SystemUser );
- my $scheme = $rt_uri->Scheme;
- my $prefix = $rt_uri->LocalURIPrefix;
-
- foreach my $use ( @URI_USES ) {
- my $table = m2t( $use->{'model'} );
- my $column = $use->{'column'};
-
- my $query = "SELECT id, $column FROM $table WHERE"
- . " $column LIKE ? AND $column NOT LIKE ?";
- my @binds = ($scheme ."://%", $prefix ."%");
+ my $rt_uri = $package->new( $RT::SystemUser );
+ my $scheme = $rt_uri->Scheme;
+ my $prefix = $rt_uri->LocalURIPrefix;
- while ( my ($k, $v) = each %{ $use->{'Additional'} || {} } ) {
- $query .= " AND $k = ?";
- push @binds, $v;
- }
- my $sth = execute_query( $query, @binds );
- while ( my ($id, $value) = $sth->fetchrow_array ) {
- $res = 0;
- print STDERR "Record #$id in $table. Value of $column column most probably is an incorrect link\n";
- my ($wrong_org) = ( $value =~ m{^\Q$scheme\E://(.+)/[^/]+/[0-9]*$} );
- next unless my $replace_with = prompt(
- 'Replace',
- "Column $column in $table is a link. Local links has scheme '$scheme'"
- ." followed by organization name from the config file. There is record"
- ." #$id that has scheme '$scheme', but organization is '$wrong_org'."
- ." Most probably you changed organization, but didn't update links."
- ." It's ok to replace these wrong links.\n",
- "Links: wrong organization $wrong_org"
- );
+ foreach my $use ( @URI_USES ) {
+ my $table = m2t( $use->{'model'} );
+ my $column = $use->{'column'};
- print "Updating record(s) in $table\n" if $opt{'verbose'};
- my $wrong_prefix = $scheme . '://'. $wrong_org;
- my $query = "UPDATE $table SET $column = ". sql_concat('?', "SUBSTR($column, ?)")
- ." WHERE $column LIKE ?";
- execute_query( $query, $prefix, length($wrong_prefix)+1, $wrong_prefix .'/%' );
+ my $query = "SELECT id, $column FROM $table WHERE"
+ . " $column LIKE ? AND $column NOT LIKE ?";
+ my @binds = ($scheme ."://%", $prefix ."%");
- $redo_check{'Links: wrong organization'} = 1;
- $redo_check{'Links: LocalX for non-ticket'} = 1;
- last; # plenty of chances we covered all cases with one update
+ while ( my ($k, $v) = each %{ $use->{'Additional'} || {} } ) {
+ $query .= " AND $k = ?";
+ push @binds, $v;
+ }
+ my $sth = execute_query( $query, @binds );
+ while ( my ($id, $value) = $sth->fetchrow_array ) {
+ $res = 0;
+ print STDERR "Record #$id in $table. Value of $column column most probably is an incorrect link\n";
+ my ($wrong_org) = ( $value =~ m{^\Q$scheme\E://(.+)/[^/]+/[0-9]*$} );
+ next unless my $replace_with = prompt(
+ 'Replace',
+ "Column $column in $table is a link. Local links has scheme '$scheme'"
+ ." followed by organization name from the config file. There is record"
+ ." #$id that has scheme '$scheme', but organization is '$wrong_org'."
+ ." Most probably you changed organization, but didn't update links."
+ ." It's ok to replace these wrong links.\n",
+ "Links: wrong organization $wrong_org"
+ );
+
+ print "Updating record(s) in $table\n" if $opt{'verbose'};
+ my $wrong_prefix = $scheme . '://'. $wrong_org;
+ my $query = "UPDATE $table SET $column = ". sql_concat('?', "SUBSTR($column, ?)")
+ ." WHERE $column LIKE ?";
+ execute_query( $query, $prefix, length($wrong_prefix)+1, $wrong_prefix .'/%' );
+
+ $redo_check{'Links: wrong organization'} = 1;
+ $redo_check{'Links: LocalX for non-ticket'} = 1;
+ last; # plenty of chances we covered all cases with one update
+ }
}
- }
} # end foreach my $package (@rt_uris)
return $res;
};
@@ -1063,49 +1063,49 @@ push @CHECKS, 'Links: missing object' => sub {
my @rt_uris = rt_uri_modules();
foreach my $package (@rt_uris) {
- my $rt_uri = $package->new( $RT::SystemUser );
- my $scheme = $rt_uri->Scheme;
- my $prefix = $rt_uri->LocalURIPrefix;
+ my $rt_uri = $package->new( $RT::SystemUser );
+ my $scheme = $rt_uri->Scheme;
+ my $prefix = $rt_uri->LocalURIPrefix;
- foreach my $use ( @URI_USES ) {
- my $stable = m2t( $use->{'model'} );
- my $scolumn = $use->{'column'};
+ foreach my $use ( @URI_USES ) {
+ my $stable = m2t( $use->{'model'} );
+ my $scolumn = $use->{'column'};
- foreach my $tmodel ( @models ) {
- my $tclass = 'RT::'. $tmodel;
- my $ttable = m2t($tmodel);
+ foreach my $tmodel ( @models ) {
+ my $tclass = 'RT::'. $tmodel;
+ my $ttable = m2t($tmodel);
- my $tprefix = $prefix .'/'. ($tclass eq 'RT::Ticket'? 'ticket' : $tclass) .'/';
+ my $tprefix = $prefix .'/'. ($tclass eq 'RT::Ticket'? 'ticket' : $tclass) .'/';
- $tprefix = $prefix . '/article/' if $tclass eq 'RT::Article';
+ $tprefix = $prefix . '/article/' if $tclass eq 'RT::Article';
- my $query = "SELECT s.id FROM $stable s LEFT JOIN $ttable t "
- ." ON t.id = ". sql_str2int("SUBSTR(s.$scolumn, ?)")
- ." WHERE s.$scolumn LIKE ? AND t.id IS NULL";
- my @binds = (length($tprefix) + 1, $tprefix.'%');
+ my $query = "SELECT s.id FROM $stable s LEFT JOIN $ttable t "
+ ." ON t.id = ". sql_str2int("SUBSTR(s.$scolumn, ?)")
+ ." WHERE s.$scolumn LIKE ? AND t.id IS NULL";
+ my @binds = (length($tprefix) + 1, $tprefix.'%');
- while ( my ($k, $v) = each %{ $use->{'Additional'} || {} } ) {
- $query .= " AND s.$k = ?";
- push @binds, $v;
- }
-
- my $sth = execute_query( $query, @binds );
- while ( my ($sid) = $sth->fetchrow_array ) {
- $res = 0;
- print STDERR "Link in $scolumn column in record #$sid in $stable table points"
- ." to not existing object.\n";
- next unless prompt(
- 'Delete',
- "Column $scolumn in $stable table is a link to an object that doesn't exist."
- ." You can delete such records, however make sure there is no other"
- ." errors with links.\n",
- 'Link to a missing object in $ttable'
- );
+ while ( my ($k, $v) = each %{ $use->{'Additional'} || {} } ) {
+ $query .= " AND s.$k = ?";
+ push @binds, $v;
+ }
- delete_record($stable, $sid);
+ my $sth = execute_query( $query, @binds );
+ while ( my ($sid) = $sth->fetchrow_array ) {
+ $res = 0;
+ print STDERR "Link in $scolumn column in record #$sid in $stable table points"
+ ." to not existing object.\n";
+ next unless prompt(
+ 'Delete',
+ "Column $scolumn in $stable table is a link to an object that doesn't exist."
+ ." You can delete such records, however make sure there is no other"
+ ." errors with links.\n",
+ 'Link to a missing object in $ttable'
+ );
+
+ delete_record($stable, $sid);
+ }
}
}
- }
} # end foreach my $package (@rt_uris)
return $res;
};
commit c7dc9aa791b5192389e3e16fbd98ef1f10bdc6bf
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Wed Feb 26 17:36:02 2014 -0500
Fix indent. Whitespace-only change
diff --git a/lib/RT/URI/fsck_com_article.pm b/lib/RT/URI/fsck_com_article.pm
index 931c55f..2667480 100644
--- a/lib/RT/URI/fsck_com_article.pm
+++ b/lib/RT/URI/fsck_com_article.pm
@@ -126,9 +126,9 @@ sub ParseURI {
. " have been deleted: $msg");
return undef;
}
- } else {
- return undef;
- }
+ } else {
+ return undef;
+ }
}
#If we couldn't find a article, return undef.
-----------------------------------------------------------------------
More information about the rt-commit
mailing list