Merge remote-tracking branch 'bmo/master'

5 files changed, 50 insertions, 78 deletions

diff --git a/extensions/Push/lib/Backoff.pm b/extensions/Push/lib/Backoff.pm
index f0116a2a7..0436cdf14 100644
--- a/extensions/Push/lib/Backoff.pm
+++ b/extensions/Push/lib/Backoff.pm
@@ -19,6 +19,7 @@ use constant AUDIT_REMOVES => 0;
 use constant USE_MEMCACHED => 0;
 
 use Bugzilla;
+use Bugzilla::Logging;
 use Bugzilla::Util;
 
 #
@@ -67,9 +68,7 @@ sub reset {
     my ($self) = @_;
     $self->{next_attempt_ts} = Bugzilla->dbh->selectrow_array('SELECT NOW()');
     $self->{attempts} = 0;
-    Bugzilla->push_ext->logger->debug(
-        sprintf("resetting backoff for %s", $self->connector)
-    );
+    INFO( sprintf 'resetting backoff for %s', $self->connector );
 }
 
 sub inc {
@@ -82,8 +81,8 @@ sub inc {
 
     $self->{next_attempt_ts} = $date;
     $self->{attempts} = $attempts;
-    Bugzilla->push_ext->logger->debug(
-        sprintf("setting next attempt for %s to %s (attempt %s)", $self->connector, $date, $attempts)
+    INFO(
+        sprintf 'setting next attempt for %s to %s (attempt %s)', $self->connector, $date, $attempts
     );
 }
 
diff --git a/extensions/Push/lib/Config.pm b/extensions/Push/lib/Config.pm
index 337c2696d..2db95b972 100644
--- a/extensions/Push/lib/Config.pm
+++ b/extensions/Push/lib/Config.pm
@@ -11,7 +11,7 @@ use 5.10.1;
 use strict;
 use warnings;
 
-use Bugzilla;
+use Bugzilla::Logging;
 use Bugzilla::Constants;
 use Bugzilla::Extension::Push::Option;
 use Crypt::CBC;
@@ -52,7 +52,6 @@ sub option {
 sub load {
     my ($self) = @_;
     my $config = {};
-    my $logger = Bugzilla->push_ext->logger;
 
     # prime $config with defaults
     foreach my $rh ($self->options) {
@@ -81,7 +80,7 @@ sub load {
     # done, update self
     foreach my $name (keys %$config) {
         my $value = $self->option($name)->{type} eq 'password' ? '********' : $config->{$name};
-        $logger->debug(sprintf("%s: set %s=%s\n", $self->{_name}, $name, $value || ''));
+        TRACE( sprintf "%s: set %s=%s\n", $self->{_name}, $name, $value || '' );
         $self->{$name} = $config->{$name};
     }
 }
diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm
index 5da64901a..e59ba6c0d 100644
--- a/extensions/Push/lib/Connector/Phabricator.pm
+++ b/extensions/Push/lib/Connector/Phabricator.pm
@@ -15,24 +15,18 @@ use base 'Bugzilla::Extension::Push::Connector::Base';
 
 use Bugzilla::Bug;
 use Bugzilla::Constants;
-use Bugzilla::Error;
-use Bugzilla::User;
 
 use Bugzilla::Extension::PhabBugz::Constants;
+use Bugzilla::Extension::PhabBugz::Policy;
+use Bugzilla::Extension::PhabBugz::Project;
+use Bugzilla::Extension::PhabBugz::Revision;
 use Bugzilla::Extension::PhabBugz::Util qw(
-  add_comment_to_revision
   add_security_sync_comments
-  create_private_revision_policy
-  edit_revision_policy
   get_attachment_revisions
   get_bug_role_phids
-  get_project_phid
   get_security_sync_groups
-  intersect
-  make_revision_public
-  make_revision_private
-  set_revision_subscribers
 );
+
 use Bugzilla::Extension::Push::Constants;
 use Bugzilla::Extension::Push::Util qw(is_public);
 
@@ -76,72 +70,55 @@ sub send {
 
     my @set_groups = get_security_sync_groups($bug);
 
-    my @revisions = get_attachment_revisions($bug);
-
-    if (!$is_public && !@set_groups) {
-        foreach my $revision (@revisions) {
-            Bugzilla->audit(sprintf(
-              'Making revision %s for bug %s private due to unkown Bugzilla groups: %s',
-              $revision->{id},
-              $bug->id,
-              join(', ', @set_groups)
-            ));
-            make_revision_private( $revision->{phid} );
-        }
-
-        add_security_sync_comments(\@revisions, $bug);
-
-        return PUSH_RESULT_OK;
-    }
+    my $revisions = get_attachment_revisions($bug);
 
     my $group_change =
       ($message->routing_key =~ /^(?:attachment|bug)\.modify:.*\bbug_group\b/)
       ? 1
       : 0;
 
-    my $subscribers;
-    if ( !$is_public ) {
-        $subscribers = get_bug_role_phids($bug);
-    }
-
-    my $secure_project_phid = get_project_phid('secure-revision');
-
-    foreach my $revision (@revisions) {
-        my $revision_phid = $revision->{phid};
-
-        my $rev_obj = Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] });
-        my $revision_updated;
-
+    foreach my $revision (@$revisions) {
         if ( $is_public && $group_change ) {
             Bugzilla->audit(sprintf(
               'Making revision %s public for bug %s',
-              $revision->{id},
+              $revision->id,
               $bug->id
             ));
-            make_revision_public($revision_phid);
-            $rev_obj->remove_project($secure_project_phid);
-            $revision_updated = 1;
+            $revision->make_public();
+        }
+        elsif ( !$is_public && !@set_groups ) {
+            Bugzilla->audit(sprintf(
+              'Making revision %s for bug %s private due to unkown Bugzilla groups: %s',
+              $revision->id,
+              $bug->id,
+              join(', ', @set_groups)
+            ));
+            $revision->make_private(['secure-revision']);
+            add_security_sync_comments([$revision], $bug);
         }
         elsif ( !$is_public && $group_change ) {
             Bugzilla->audit(sprintf(
               'Giving revision %s a custom policy for bug %s',
-              $revision->{id},
+              $revision->id,
               $bug->id
             ));
-            my $policy_phid = create_private_revision_policy( $bug, \@set_groups );
-            edit_revision_policy( $revision_phid, $policy_phid, $subscribers );
-            $rev_obj->add_project($secure_project_phid);
-            $revision_updated = 1;
+            my @set_project_names = map { "bmo-" . $_ } @set_groups;
+            $revision->make_private(\@set_project_names);
         }
-        elsif ( !$is_public && !$group_change ) {
+
+        # Subscriber list of the private revision should always match
+        # the bug roles such as assignee, qa contact, and cc members.
+        if (!$is_public) {
             Bugzilla->audit(sprintf(
               'Updating subscribers for %s for bug %s',
-              $revision->{id},
+              $revision->id,
               $bug->id
             ));
-            set_revision_subscribers( $revision_phid, $subscribers );
+            my $subscribers = get_bug_role_phids($bug);
+            $revision->set_subscribers($subscribers) if $subscribers;
         }
-        $rev_obj->update() if $revision_updated;
+
+        $revision->update();
     }
 
     return PUSH_RESULT_OK;
diff --git a/extensions/Push/lib/Connectors.pm b/extensions/Push/lib/Connectors.pm
index 75a5083ff..d3c55d3ca 100644
--- a/extensions/Push/lib/Connectors.pm
+++ b/extensions/Push/lib/Connectors.pm
@@ -11,10 +11,12 @@ use 5.10.1;
 use strict;
 use warnings;
 
+use Bugzilla::Logging;
 use Bugzilla::Extension::Push::Util;
 use Bugzilla::Constants;
 use Bugzilla::Util qw(trick_taint);
 use File::Basename;
+use Try::Tiny;
 
 sub new {
     my ($class) = @_;
@@ -25,16 +27,15 @@ sub new {
     $self->{objects} = {};
     $self->{path} = bz_locations->{'extensionsdir'} . '/Push/lib/Connector';
 
-    my $logger = Bugzilla->push_ext->logger;
     foreach my $file (glob($self->{path} . '/*.pm')) {
         my $name = basename($file);
         $name =~ s/\.pm$//;
         next if $name eq 'Base';
         if (length($name) > 32) {
-            $logger->info("Ignoring connector '$name': Name longer than 32 characters");
+            WARN("Ignoring connector '$name': Name longer than 32 characters");
         }
         push @{$self->{names}}, $name;
-        $logger->debug("Found connector '$name'");
+        TRACE("Found connector '$name'");
     }
 
     return $self;
@@ -44,7 +45,6 @@ sub _load {
     my ($self) = @_;
     return if scalar keys %{$self->{objects}};
 
-    my $logger = Bugzilla->push_ext->logger;
     foreach my $name (@{$self->{names}}) {
         next if exists $self->{objects}->{$name};
         my $file = $self->{path} . "/$name.pm";
@@ -52,34 +52,30 @@ sub _load {
         require $file;
         my $package = "Bugzilla::Extension::Push::Connector::$name";
 
-        $logger->debug("Loading connector '$name'");
+        TRACE("Loading connector '$name'");
         my $old_error_mode = Bugzilla->error_mode;
         Bugzilla->error_mode(ERROR_MODE_DIE);
-        eval {
+        try {
             my $connector = $package->new();
             $connector->load_config();
             $self->{objects}->{$name} = $connector;
+        } catch {
+            ERROR("Connector '$name' failed to load: " . clean_error($_));
         };
-        if ($@) {
-            $logger->error("Connector '$name' failed to load: " . clean_error($@));
-        }
         Bugzilla->error_mode($old_error_mode);
     }
 }
 
 sub stop {
     my ($self) = @_;
-    my $logger = Bugzilla->push_ext->logger;
     foreach my $connector ($self->list) {
         next unless $connector->enabled;
-        $logger->debug("Stopping '" . $connector->name . "'");
-        eval {
+        TRACE("Stopping '" . $connector->name . "'");
+        try {
             $connector->stop();
+        } catch {
+            ERROR("Connector '" . $connector->name . "' failed to stop: " . clean_error($_));
         };
-        if ($@) {
-            $logger->error("Connector '" . $connector->name . "' failed to stop: " . clean_error($@));
-            $logger->debug("Connector '" . $connector->name . "' failed to stop: $@");
-        }
     }
 }
 
diff --git a/extensions/Push/lib/Push.pm b/extensions/Push/lib/Push.pm
index 45b9b05dd..670b2aa56 100644
--- a/extensions/Push/lib/Push.pm
+++ b/extensions/Push/lib/Push.pm
@@ -11,6 +11,7 @@ use 5.10.1;
 use strict;
 use warnings;
 
+use Bugzilla::Logging;
 use Bugzilla::Extension::Push::BacklogMessage;
 use Bugzilla::Extension::Push::Config;
 use Bugzilla::Extension::Push::Connectors;
@@ -107,7 +108,7 @@ sub push {
 
             # if the connector is backlogged, push to the backlog queue
             if ($is_backlogged) {
-                $logger->debug("backlogged");
+                INFO('connector is backlogged');
                 my $backlog = Bugzilla::Extension::Push::BacklogMessage->create_from_message($message, $connector);
             }
         }