summaryrefslogtreecommitdiffstats
path: root/extensions/PhabBugz/lib
diff options
context:
space:
mode:
authorDylan William Hardison <dylan@hardison.net>2018-10-14 18:19:50 +0200
committerDylan William Hardison <dylan@hardison.net>2018-10-14 18:19:50 +0200
commitce00a61057535d49aa0e83181a1d317d7842571b (patch)
tree280243de9ff791449fb2c82f3f0f2b9bd931d5b2 /extensions/PhabBugz/lib
parent6367a26da4093a8379e370ef328e9507c98b2e7e (diff)
parent6657fa9f5210d5b5a9b14c0cba6882bd56232054 (diff)
downloadbugzilla-ce00a61057535d49aa0e83181a1d317d7842571b.tar.gz
bugzilla-ce00a61057535d49aa0e83181a1d317d7842571b.tar.xz
Merge remote-tracking branch 'bmo/master'
Diffstat (limited to 'extensions/PhabBugz/lib')
-rw-r--r--extensions/PhabBugz/lib/Constants.pm2
-rw-r--r--extensions/PhabBugz/lib/Feed.pm182
-rw-r--r--extensions/PhabBugz/lib/Util.pm77
-rw-r--r--extensions/PhabBugz/lib/WebService.pm105
4 files changed, 60 insertions, 306 deletions
diff --git a/extensions/PhabBugz/lib/Constants.pm b/extensions/PhabBugz/lib/Constants.pm
index 1692f8fb9..19987de25 100644
--- a/extensions/PhabBugz/lib/Constants.pm
+++ b/extensions/PhabBugz/lib/Constants.pm
@@ -19,6 +19,7 @@ our @EXPORT = qw(
PHAB_FEED_POLL_SECONDS
PHAB_USER_POLL_SECONDS
PHAB_GROUP_POLL_SECONDS
+ PHAB_TIMEOUT
);
use constant PHAB_ATTACHMENT_PATTERN => qr/^phabricator-D(\d+)/;
@@ -27,5 +28,6 @@ use constant PHAB_CONTENT_TYPE => 'text/x-phabricator-request';
use constant PHAB_FEED_POLL_SECONDS => $ENV{PHAB_FEED_POLL} // 5;
use constant PHAB_USER_POLL_SECONDS => $ENV{PHAB_USER_POLL} // 60;
use constant PHAB_GROUP_POLL_SECONDS => $ENV{PHAB_GROUP_POLL} // 300;
+use constant PHAB_TIMEOUT => $ENV{PHAB_TIMEOUT} // 60;
1;
diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index f2a440bb1..71e6aa827 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -11,10 +11,10 @@ use 5.10.1;
use IO::Async::Timer::Periodic;
use IO::Async::Loop;
+use IO::Async::Signal;
use List::Util qw(first);
use List::MoreUtils qw(any uniq);
use Moo;
-use Scalar::Util qw(blessed);
use Try::Tiny;
use Type::Params qw( compile );
use Type::Utils;
@@ -48,6 +48,13 @@ my $Invocant = class_type { class => __PACKAGE__ };
sub start {
my ($self) = @_;
+ my $sig_alarm = IO::Async::Signal->new(
+ name => 'ALRM',
+ on_receipt => sub {
+ FATAL("Timeout reached");
+ exit;
+ },
+ );
# Query for new revisions or changes
my $feed_timer = IO::Async::Timer::Periodic->new(
first_interval => 0,
@@ -56,13 +63,17 @@ sub start {
on_tick => sub {
try {
with_writable_database {
+ alarm(PHAB_TIMEOUT);
$self->feed_query();
};
}
catch {
FATAL($_);
+ }
+ finally {
+ alarm(0);
+ Bugzilla->_cleanup();
};
- Bugzilla->_cleanup();
},
);
@@ -74,13 +85,17 @@ sub start {
on_tick => sub {
try {
with_writable_database {
+ alarm(PHAB_TIMEOUT);
$self->user_query();
};
}
catch {
FATAL($_);
+ }
+ finally {
+ alarm(0);
+ Bugzilla->_cleanup();
};
- Bugzilla->_cleanup();
},
);
@@ -92,13 +107,18 @@ sub start {
on_tick => sub {
try {
with_writable_database {
+ alarm(PHAB_TIMEOUT);
$self->group_query();
};
}
catch {
FATAL($_);
+ }
+ finally {
+ alarm(0);
+ Bugzilla->_cleanup();
};
- Bugzilla->_cleanup();
+
},
);
@@ -106,6 +126,7 @@ sub start {
$loop->add($feed_timer);
$loop->add($user_timer);
$loop->add($group_timer);
+ $loop->add($sig_alarm);
$feed_timer->start;
$user_timer->start;
$group_timer->start;
@@ -221,6 +242,11 @@ sub feed_query {
$delete_build_target->execute($target->{name}, $target->{value});
}
+
+ if (Bugzilla->datadog) {
+ my $dd = Bugzilla->datadog();
+ $dd->increment('bugzilla.phabbugz.feed_query_count');
+ }
}
sub user_query {
@@ -261,6 +287,11 @@ sub user_query {
};
$self->save_last_id($user_id, 'user');
}
+
+ if (Bugzilla->datadog) {
+ my $dd = Bugzilla->datadog();
+ $dd->increment('bugzilla.phabbugz.user_query_count');
+ }
}
sub group_query {
@@ -359,6 +390,11 @@ sub group_query {
INFO( "Project " . $project->name . " updated" );
}
}
+
+ if (Bugzilla->datadog) {
+ my $dd = Bugzilla->datadog();
+ $dd->increment('bugzilla.phabbugz.group_query_count');
+ }
}
sub process_revision_change {
@@ -501,105 +537,6 @@ sub process_revision_change {
$attachment->update($timestamp);
}
- # REVIEWER STATUSES
-
- my (@accepted, @denied);
- foreach my $review (@{ $revision->reviews }) {
- push @accepted, $review->{user} if $review->{status} eq 'accepted';
- push @denied, $review->{user} if $review->{status} eq 'rejected';
- }
-
- my @accepted_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @accepted;
- my @denied_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @denied;
- my %reviewers_hash = map { $_->{user}->name => 1 } @{ $revision->reviews };
-
- foreach my $attachment (@attachments) {
- my ($attach_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
- next if $revision->id != $attach_revision_id;
-
- # Clear old accepted review flags if no longer accepted
- my (@denied_flags, @new_flags, @removed_flags, %accepted_done, $flag_type);
- foreach my $flag (@{ $attachment->flags }) {
- next if $flag->type->name ne 'review';
- $flag_type = $flag->type if $flag->type->is_active;
- next if $flag->status ne '+';
- if (any { $flag->setter->id == $_ } @denied_user_ids) {
- INFO('Denying review flag set by ' . $flag->setter->name);
- push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
- }
- if (any { $flag->setter->id == $_ } @accepted_user_ids) {
- INFO('Skipping as review+ already set by ' . $flag->setter->name);
- $accepted_done{$flag->setter->id}++;
- }
- if (!any { $flag->setter->id == $_ } (@accepted_user_ids, @denied_user_ids)) {
- INFO('Clearing review+ flag set by ' . $flag->setter->name);
- push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
- }
- }
-
- $flag_type ||= first { $_->name eq 'review' && $_->is_active } @{ $attachment->flag_types };
-
- die "Unable to find review flag!" unless $flag_type;
-
- # Create new flags
- foreach my $user_id (@accepted_user_ids) {
- next if $accepted_done{$user_id};
- my $user = Bugzilla::User->check({ id => $user_id, cache => 1 });
- INFO('Setting new review+ flag for ' . $user->name);
- push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' });
- }
-
- # Process each flag change by updating the flag and adding a comment
- foreach my $flag_data (@new_flags) {
- my $comment = $flag_data->{setter}->name . " has approved the revision.";
- $self->add_flag_comment(
- {
- bug => $bug,
- attachment => $attachment,
- comment => $comment,
- user => $flag_data->{setter},
- old_flags => [],
- new_flags => [$flag_data],
- timestamp => $timestamp
- }
- );
- }
- foreach my $flag_data (@denied_flags) {
- my $comment = $flag_data->{setter}->name . " has requested changes to the revision.\n";
- $self->add_flag_comment(
- {
- bug => $bug,
- attachment => $attachment,
- comment => $comment,
- user => $flag_data->{setter},
- old_flags => [$flag_data],
- new_flags => [],
- timestamp => $timestamp
- }
- );
- }
- foreach my $flag_data (@removed_flags) {
- my $comment;
- if ( exists $reviewers_hash{ $flag_data->{setter}->name } ) {
- $comment = "Flag set by " . $flag_data->{setter}->name . " is no longer active.\n";
- }
- else {
- $comment = $flag_data->{setter}->name . " has been removed from the revision.\n";
- }
- $self->add_flag_comment(
- {
- bug => $bug,
- attachment => $attachment,
- comment => $comment,
- user => $flag_data->{setter},
- old_flags => [$flag_data],
- new_flags => [],
- timestamp => $timestamp
- }
- );
- }
- }
-
# FINISH UP
$bug->update($timestamp);
@@ -736,6 +673,11 @@ sub process_new_user {
foreach my $attachment (@attachments) {
my ($revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
+ if (!$revision_id) {
+ WARN("Skipping " . $attachment->filename . " on bug $bug_id. Filename should be fixed.");
+ next;
+ }
+
INFO("Processing revision D$revision_id");
my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query(
@@ -839,38 +781,4 @@ sub get_group_members {
);
}
-sub add_flag_comment {
- state $check = compile(
- $Invocant,
- Dict [
- bug => Bug,
- attachment => Attachment,
- comment => Str,
- user => User,
- old_flags => ArrayRef,
- new_flags => ArrayRef,
- timestamp => Str,
- ],
- );
- my ( $self, $params ) = $check->(@_);
- my ( $bug, $attachment, $comment, $user, $old_flags, $new_flags, $timestamp )
- = @$params{qw(bug attachment comment user old_flags new_flags timestamp)};
-
- # when this function returns, Bugzilla->user will return to its previous value.
- my $restore_prev_user = Bugzilla->set_user($user, scope_guard => 1);
-
- INFO("Flag comment: $comment");
- $bug->add_comment(
- $comment,
- {
- isprivate => $attachment->isprivate,
- type => CMT_ATTACHMENT_UPDATED,
- extra_data => $attachment->id
- }
- );
-
- $attachment->set_flags( $old_flags, $new_flags );
- $attachment->update($timestamp);
-}
-
1;
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index a93533e75..32f860413 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -20,14 +20,14 @@ use Bugzilla::Util qw(trim);
use Bugzilla::Extension::PhabBugz::Constants;
use Bugzilla::Extension::PhabBugz::Types qw(:types);
-use JSON::XS qw(encode_json decode_json);
use List::Util qw(first);
-use LWP::UserAgent;
use Taint::Util qw(untaint);
use Try::Tiny;
use Type::Params qw( compile );
use Type::Utils;
use Types::Standard qw( :types );
+use Mojo::UserAgent;
+use Mojo::JSON qw(encode_json);
use base qw(Exporter);
@@ -35,7 +35,6 @@ our @EXPORT = qw(
create_revision_attachment
get_attachment_revisions
get_bug_role_phids
- get_needs_review
intersect
is_attachment_phab_revision
request
@@ -160,11 +159,10 @@ sub request {
my $ua = $request_cache->{phabricator_ua};
unless ($ua) {
- $ua = $request_cache->{phabricator_ua} = LWP::UserAgent->new(timeout => 10);
+ $ua = $request_cache->{phabricator_ua} = Mojo::UserAgent->new;
if ($params->{proxy_url}) {
- $ua->proxy('https', $params->{proxy_url});
+ $ua->proxy($params->{proxy_url});
}
- $ua->default_header('Content-Type' => 'application/x-www-form-urlencoded');
}
my $phab_api_key = $params->{phabricator_api_key};
@@ -176,25 +174,16 @@ sub request {
$data->{__conduit__} = { token => $phab_api_key };
- my $response = $ua->post($full_uri, { params => encode_json($data) });
-
+ my $response = $ua->post($full_uri => form => { params => encode_json($data) })->result;
ThrowCodeError('phabricator_api_error', { reason => $response->message })
if $response->is_error;
- my $result;
- my $result_ok = eval {
- my $content = $response->content;
- untaint($content);
- $result = decode_json( $content );
- 1;
- };
- if (!$result_ok || $result->{error_code}) {
- ThrowCodeError('phabricator_api_error',
- { reason => 'JSON decode failure' }) if !$result_ok;
- ThrowCodeError('phabricator_api_error',
- { code => $result->{error_code},
- reason => $result->{error_info} }) if $result->{error_code};
- }
+ my $result = $response->json;
+ ThrowCodeError('phabricator_api_error',
+ { reason => 'JSON decode failure' }) if !defined($result);
+ ThrowCodeError('phabricator_api_error',
+ { code => $result->{error_code},
+ reason => $result->{error_info} }) if $result->{error_code};
return $result;
}
@@ -206,48 +195,4 @@ sub set_phab_user {
return Bugzilla->set_user($user, scope_guard => 1);
}
-sub get_needs_review {
- my ($user) = @_;
- $user //= Bugzilla->user;
- return unless $user->id;
-
- my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query(
- {
- ids => [ $user->id ]
- }
- );
-
- return [] unless $phab_user;
-
- my $diffs = request(
- 'differential.revision.search',
- {
- attachments => {
- reviewers => 1,
- },
- constraints => {
- reviewerPHIDs => [$phab_user->phid],
- statuses => ["open()"],
- },
- order => 'newest',
- }
- );
- ThrowCodeError('phabricator_api_error', { reason => 'Malformed Response' })
- unless exists $diffs->{result}{data};
-
- my @revisions;
- foreach my $revision ( @{ $diffs->{result}{data} } ) {
- foreach my $reviewer ( @{ $revision->{attachments}->{reviewers}->{reviewers} } ) {
- if ( $reviewer->{reviewerPHID} eq $phab_user->phid
- && $reviewer->{status} =~ /^(?:added|blocking)$/ )
- {
- push @revisions, $revision;
- last;
- }
- }
- }
-
- return \@revisions;
-}
-
1;
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index fa9306667..19a758a70 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -16,29 +16,23 @@ use base qw(Bugzilla::WebService);
use Bugzilla::Constants;
use Bugzilla::Error;
use Bugzilla::User;
-use Bugzilla::Util qw(detaint_natural datetime_from time_ago trick_taint);
+use Bugzilla::Util qw(detaint_natural trick_taint);
use Bugzilla::WebService::Constants;
use Bugzilla::Extension::PhabBugz::Constants;
-use Bugzilla::Extension::PhabBugz::Util qw(
- get_needs_review
-);
-use DateTime ();
-use List::Util qw(first uniq);
+use List::Util qw(first);
use List::MoreUtils qw(any);
use MIME::Base64 qw(decode_base64);
use constant READ_ONLY => qw(
check_user_enter_bug_permission
check_user_permission_for_bug
- needs_review
);
use constant PUBLIC_METHODS => qw(
check_user_enter_bug_permission
check_user_permission_for_bug
- needs_review
set_build_target
);
@@ -99,95 +93,6 @@ sub check_user_enter_bug_permission {
};
}
-sub needs_review {
- my ($self, $params) = @_;
-
- $self->_check_phabricator();
-
- my $user = Bugzilla->login(LOGIN_REQUIRED);
- my $dbh = Bugzilla->dbh;
-
- my $reviews = get_needs_review();
-
- my $authors = Bugzilla::Extension::PhabBugz::User->match({
- phids => [
- uniq
- grep { defined }
- map { $_->{fields}{authorPHID} }
- @$reviews
- ]
- });
-
- my %author_phab_to_id = map { $_->phid => $_->bugzilla_user->id } @$authors;
- my %author_id_to_user = map { $_->bugzilla_user->id => $_->bugzilla_user } @$authors;
-
- # bug data
- my $visible_bugs = $user->visible_bugs([
- uniq
- grep { $_ }
- map { $_->{fields}{'bugzilla.bug-id'} }
- @$reviews
- ]);
-
- # get all bug statuses and summaries in a single query to avoid creation of
- # many bug objects
- my %bugs;
- if (@$visible_bugs) {
- #<<<
- my $bug_rows =$dbh->selectall_arrayref(
- 'SELECT bug_id, bug_status, short_desc ' .
- ' FROM bugs ' .
- ' WHERE bug_id IN (' . join(',', ('?') x @$visible_bugs) . ')',
- { Slice => {} },
- @$visible_bugs
- );
- #>>>
- %bugs = map { $_->{bug_id} => $_ } @$bug_rows;
- }
-
- # build result
- my $datetime_now = DateTime->now(time_zone => $user->timezone);
- my @result;
- foreach my $review (@$reviews) {
- my $review_flat = {
- id => $review->{id},
- title => $review->{fields}{title},
- url => Bugzilla->params->{phabricator_base_uri} . 'D' . $review->{id},
- };
-
- # show date in user's timezone
- my $datetime = DateTime->from_epoch(
- epoch => $review->{fields}{dateModified},
- time_zone => 'UTC'
- );
- $datetime->set_time_zone($user->timezone);
- $review_flat->{updated} = $datetime->strftime('%Y-%m-%d %T %Z');
- $review_flat->{updated_fancy} = time_ago($datetime, $datetime_now);
-
- # review requester
- if (my $author = $author_id_to_user{$author_phab_to_id{ $review->{fields}{authorPHID} }}) {
- $review_flat->{author_name} = $author->name;
- $review_flat->{author_email} = $author->email;
- }
- else {
- $review_flat->{author_name} = 'anonymous';
- $review_flat->{author_email} = 'anonymous';
- }
-
- # referenced bug
- if (my $bug_id = $review->{fields}{'bugzilla.bug-id'}) {
- my $bug = $bugs{$bug_id};
- $review_flat->{bug_id} = $bug_id;
- $review_flat->{bug_status} = $bug->{bug_status};
- $review_flat->{bug_summary} = $bug->{short_desc};
- }
-
- push @result, $review_flat;
- }
-
- return { result => \@result };
-}
-
sub set_build_target {
my ( $self, $params ) = @_;
@@ -257,12 +162,6 @@ sub rest_resources {
},
},
},
- # Review requests
- qw{^/phabbugz/needs_review$}, {
- GET => {
- method => 'needs_review',
- },
- }
];
}