Revert "Bug 1409957 - Create polling daemon to query Phabricator for recent transcations and update bug data according to revision changes"

This reverts commit 7da8e374e0c96e10077690935e829b0c04fc82f4.

8 files changed, 41 insertions, 1222 deletions

diff --git a/extensions/PhabBugz/lib/Constants.pm b/extensions/PhabBugz/lib/Constants.pm
index 754130f0b..f7485e8c4 100644
--- a/extensions/PhabBugz/lib/Constants.pm
+++ b/extensions/PhabBugz/lib/Constants.pm
@@ -16,12 +16,10 @@ our @EXPORT = qw(
     PHAB_AUTOMATION_USER
     PHAB_ATTACHMENT_PATTERN
     PHAB_CONTENT_TYPE
-    PHAB_POLL_SECONDS
 );
 
 use constant PHAB_ATTACHMENT_PATTERN => qr/^phabricator-D(\d+)/;
 use constant PHAB_AUTOMATION_USER    => 'phab-bot@bmo.tld';
 use constant PHAB_CONTENT_TYPE       => 'text/x-phabricator-request';
-use constant PHAB_POLL_SECONDS       => 5;
 
 1;
diff --git a/extensions/PhabBugz/lib/Daemon.pm b/extensions/PhabBugz/lib/Daemon.pm
deleted file mode 100644
index c8b4f73af..000000000
--- a/extensions/PhabBugz/lib/Daemon.pm
+++ /dev/null
@@ -1,100 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Extension::PhabBugz::Daemon;
-
-use 5.10.1;
-use strict;
-use warnings;
-
-use Bugzilla::Constants;
-use Bugzilla::Extension::PhabBugz::Feed;
-use Bugzilla::Extension::PhabBugz::Logger;
-
-use Carp qw(confess);
-use Daemon::Generic;
-use File::Basename;
-use File::Spec;
-use Pod::Usage;
-
-sub start {
-    newdaemon();
-}
-
-#
-# daemon::generic config
-#
-
-sub gd_preconfig {
-    my $self = shift;
-    my $pidfile = $self->{gd_args}{pidfile};
-    if (!$pidfile) {
-        $pidfile = File::Spec->catfile(bz_locations()->{datadir}, $self->{gd_progname} . ".pid");
-    }
-    return (pidfile => $pidfile);
-}
-
-sub gd_getopt {
-    my $self = shift;
-    $self->SUPER::gd_getopt();
-    if ($self->{gd_args}{progname}) {
-        $self->{gd_progname} = $self->{gd_args}{progname};
-    } else {
-        $self->{gd_progname} = basename($0);
-    }
-    $self->{_original_zero} = $0;
-    $0 = $self->{gd_progname};
-}
-
-sub gd_postconfig {
-    my $self = shift;
-    $0 = delete $self->{_original_zero};
-}
-
-sub gd_more_opt {
-    my $self = shift;
-    return (
-        'pidfile=s' => \$self->{gd_args}{pidfile},
-        'n=s'       => \$self->{gd_args}{progname},
-    );
-}
-
-sub gd_usage {
-    pod2usage({ -verbose => 0, -exitval => 'NOEXIT' });
-    return 0;
-};
-
-sub gd_redirect_output {
-    my $self = shift;
-
-    my $filename = File::Spec->catfile(bz_locations()->{datadir}, $self->{gd_progname} . ".log");
-    open(STDERR, ">>", $filename) or (print "could not open stderr: $!" && exit(1));
-    close(STDOUT);
-    open(STDOUT, ">&", STDERR) or die "redirect STDOUT -> STDERR: $!";
-    $SIG{HUP} = sub {
-        close(STDERR);
-        open(STDERR, ">>", $filename) or (print "could not open stderr: $!" && exit(1));
-    };
-}
-
-sub gd_setup_signals {
-    my $self = shift;
-    $self->SUPER::gd_setup_signals();
-    $SIG{TERM} = sub { $self->gd_quit_event(); }
-}
-
-sub gd_run {
-    my $self = shift;
-    $::SIG{__DIE__} = \&Carp::confess if $self->{debug};
-    my $phabbugz = Bugzilla::Extension::PhabBugz::Feed->new();
-    $phabbugz->is_daemon(1);
-    $phabbugz->logger(
-        Bugzilla::Extension::PhabBugz::Logger->new(debugging => $self->{debug}));
-    $phabbugz->start();
-}
-
-1;
diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
deleted file mode 100644
index d178f249b..000000000
--- a/extensions/PhabBugz/lib/Feed.pm
+++ /dev/null
@@ -1,320 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Extension::PhabBugz::Feed;
-
-use 5.10.1;
-
-use List::Util qw(first);
-use List::MoreUtils qw(any);
-use Moo;
-
-use Bugzilla::Constants;
-use Bugzilla::Extension::PhabBugz::Constants;
-use Bugzilla::Extension::PhabBugz::Revision;
-use Bugzilla::Extension::PhabBugz::Util qw(
-    add_security_sync_comments
-    create_private_revision_policy
-    create_revision_attachment
-    edit_revision_policy
-    get_bug_role_phids
-    get_phab_bmo_ids
-    get_security_sync_groups
-    is_attachment_phab_revision
-    make_revision_public
-    request
-    set_phab_user
-);
-
-has 'is_daemon' => ( is => 'rw', default => 0 );
-has 'logger'    => ( is => 'rw' );
-
-sub start {
-    my ($self) = @_;
-    while (1) {
-        my $ok = eval {
-            if (Bugzilla->params->{phabricator_enabled}) {
-                $self->feed_query();
-                Bugzilla->_cleanup();
-            }
-            1;
-        };
-        $self->logger->error( $@ // "unknown exception" ) unless $ok;
-        sleep(PHAB_POLL_SECONDS);
-    }
-}
-
-sub feed_query {
-    my ($self) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    # Ensure Phabricator syncing is enabled
-    if (!Bugzilla->params->{phabricator_enabled}) {
-        $self->logger->info("PHABRICATOR SYNC DISABLED");
-        return;
-    }
-
-    $self->logger->info("FEED: Fetching new transactions");
-
-    my $last_id = $dbh->selectrow_array("
-        SELECT value FROM phabbugz WHERE name = 'feed_last_id'");
-    $last_id ||= 0;
-    $self->logger->debug("QUERY LAST_ID: $last_id");
-
-    # Check for new transctions (stories)
-    my $transactions = $self->feed_transactions($last_id);
-    if (!@$transactions) {
-        $self->logger->info("FEED: No new transactions");
-        return;
-    }
-
-    # Process each story
-    foreach my $story_data (@$transactions) {
-        my $skip = 0;
-        my $story_id    = $story_data->{id};
-        my $story_phid  = $story_data->{storyPHID};
-        my $author_phid = $story_data->{authorPHID};
-        my $object_phid = $story_data->{objectPHID};
-        my $story_text  = $story_data->{text};
-
-        $self->logger->debug("STORY ID: $story_id");
-        $self->logger->debug("STORY PHID: $story_phid");
-        $self->logger->debug("AUTHOR PHID: $author_phid");
-        $self->logger->debug("OBJECT PHID: $object_phid");
-        $self->logger->debug("STORY TEXT: $story_text");
-
-        # Only interested in changes to revisions for now.
-        if ($object_phid !~ /^PHID-DREV/) {
-            $self->logger->debug("SKIP: Not a revision change");
-            $skip = 1;
-        }
-
-        # Skip changes done by phab-bot user
-        my $phab_users = get_phab_bmo_ids({ phids => [$author_phid] });
-        if (!$skip && @$phab_users) {
-            my $user = Bugzilla::User->new({ id => $phab_users->[0]->{id}, cache => 1 });
-            $skip = 1 if $user->login eq PHAB_AUTOMATION_USER;
-        }
-
-        if (!$skip) {
-            my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [$object_phid] });
-            $self->process_revision_change($revision, $story_text);
-        }
-        else {
-            $self->logger->info('SKIPPING');
-        }
-
-        # Store the largest last key so we can start from there in the next session
-        $self->logger->debug("UPDATING FEED_LAST_ID: $story_id");
-        $dbh->do("REPLACE INTO phabbugz (name, value) VALUES ('feed_last_id', ?)",
-                 undef, $story_id);
-    }
-}
-
-sub process_revision_change {
-    my ($self, $revision, $story_text) = @_;
-
-    # Pre setup before making changes
-    my $old_user = set_phab_user();
-
-    my $is_shadow_db = Bugzilla->is_shadow_db;
-    Bugzilla->switch_to_main_db if $is_shadow_db;
-
-    my $dbh = Bugzilla->dbh;
-    $dbh->bz_start_transaction;
-
-    my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
-
-    my $log_message = sprintf(
-        "REVISION CHANGE FOUND: D%d: %s | bug: %d | %s",
-        $revision->id,
-        $revision->title,
-        $revision->bug_id,
-        $story_text);
-    $self->logger->info($log_message);
-
-    my $bug = Bugzilla::Bug->new({ id => $revision->bug_id, cache => 1 });
-
-    # REVISION SECURITY POLICY
-
-    # Do not set policy if a custom policy has already been set
-    # This keeps from setting new custom policy everytime a change
-    # is made.
-    unless ($revision->view_policy =~ /^PHID-PLCY/) {
-
-        # If bug is public then remove privacy policy
-        if (!@{ $bug->groups_in }) {
-            $revision->set_policy('view', 'public');
-            $revision->set_policy('edit', 'users');
-        }
-        # else bug is private
-        else {
-            my @set_groups = get_security_sync_groups($bug);
-
-            # If bug privacy groups do not have any matching synchronized groups,
-            # then leave revision private and it will have be dealt with manually.
-            if (!@set_groups) {
-                add_security_sync_comments([$revision], $bug);
-            }
-
-            my $policy_phid = create_private_revision_policy($bug, \@set_groups);
-            my $subscribers = get_bug_role_phids($bug);
-
-            $revision->set_policy('view', $policy_phid);
-            $revision->set_policy('edit', $policy_phid);
-            $revision->set_subscribers($subscribers);
-        }
-    }
-
-    my $attachment = create_revision_attachment($bug, $revision->id, $revision->title, $timestamp);
-
-    # ATTACHMENT OBSOLETES
-
-    # fixup attachments on current bug
-    my @attachments =
-      grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
-
-    foreach my $attachment (@attachments) {
-        my ($attach_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
-        next if $attach_revision_id != $revision->id;
-
-        my $make_obsolete = $revision->status eq 'abandoned' ? 1 : 0;
-        $attachment->set_is_obsolete($make_obsolete);
-
-        if ($revision->id == $attach_revision_id
-            && $revision->title ne $attachment->description) {
-            $attachment->set_description($revision->title);
-        }
-
-        $attachment->update($timestamp);
-        last;
-    }
-
-    # fixup attachments with same revision id but on different bugs
-    my $other_attachments = Bugzilla::Attachment->match({
-        mimetype => PHAB_CONTENT_TYPE,
-        filename => 'phabricator-D' . $revision->id . '-url.txt',
-        WHERE    => { 'bug_id != ? AND NOT isobsolete' => $bug->id }
-    });
-    foreach my $attachment (@$other_attachments) {
-        $attachment->set_is_obsolete(1);
-        $attachment->update($timestamp);
-    }
-
-    # REVIEWER STATUSES
-
-    my (@accepted_phids, @denied_phids, @accepted_user_ids, @denied_user_ids);
-    foreach my $reviewer (@{ $revision->reviewers }) {
-        push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted';
-        push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected';
-    }
-
-    my $phab_users = get_phab_bmo_ids({ phids => \@accepted_phids });
-    @accepted_user_ids = map { $_->{id} } @$phab_users;
-    $phab_users = get_phab_bmo_ids({ phids => \@denied_phids });
-    @denied_user_ids = map { $_->{id} } @$phab_users;
-
-    foreach my $attachment (@attachments) {
-        my ($attach_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
-        next if $revision->id != $attach_revision_id;
-
-        # Clear old 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 (any { $flag->setter->id == $_ } @denied_user_ids) {
-                push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
-            }
-            if (any { $flag->setter->id == $_ } @accepted_user_ids) {
-                $accepted_done{$flag->setter->id}++;
-            }
-            if ($flag->status eq '+'
-                && !any { $flag->setter->id == $_ } (@accepted_user_ids, @denied_user_ids)) {
-                push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
-            }
-        }
-
-        $flag_type ||= first { $_->name eq 'review' } @{ $attachment->flag_types };
-
-        # 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 });
-            push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' });
-        }
-
-        # Also add comment to for attachment update showing the user's name
-        # that changed the revision.
-        my $comment;
-        foreach my $flag_data (@new_flags) {
-            $comment .= $flag_data->{setter}->name . " has approved the revision.\n";
-        }
-        foreach my $flag_data (@denied_flags) {
-            $comment .= $flag_data->{setter}->name . " has requested changes to the revision.\n";
-        }
-        foreach my $flag_data (@removed_flags) {
-            $comment .= $flag_data->{setter}->name . " has been removed from the revision.\n";
-        }
-
-        if ($comment) {
-            $comment .= "\n" . Bugzilla->params->{phabricator_base_uri} . "D" . $revision->id;
-            # Add transaction_id as anchor if one present
-            # $comment .= "#" . $params->{transaction_id} if $params->{transaction_id};
-            $bug->add_comment($comment, {
-                isprivate  => $attachment->isprivate,
-                type       => CMT_ATTACHMENT_UPDATED,
-                extra_data => $attachment->id
-            });
-        }
-
-        $attachment->set_flags([ @denied_flags, @removed_flags ], \@new_flags);
-        $attachment->update($timestamp);
-    }
-
-    # FINISH UP
-
-    $bug->update($timestamp);
-    $revision->update();
-
-    Bugzilla::BugMail::Send($revision->bug_id, { changer => Bugzilla->user });
-
-    $dbh->bz_commit_transaction;
-    Bugzilla->switch_to_shadow_db if $is_shadow_db;
-
-    Bugzilla->set_user($old_user);
-
-    $self->logger->info("SUCCESS");
-}
-
-sub feed_transactions {
-    my ($self, $after) = @_;
-    my $data = { view => 'text' };
-    $data->{after} = $after if $after;
-    my $result = request('feed.query_id', $data);
-
-    # Stupid Conduit. If the feed results are empty it returns
-    # an empty list ([]). If there is data it returns it in a
-    # hash ({}) so we have adjust to be consistent.
-    my $stories = ref $result->{result}{data} eq 'HASH'
-                  ? $result->{result}{data}
-                  : {};
-
-    # PHP array retain key order but Perl does not. So we will
-    # loop over the data and place the stories into a list instead
-    # of a hash. We will then sort the list by id.
-    my @story_list;
-    foreach my $story_phid (keys %$stories) {
-        my $story_data = $stories->{$story_phid};
-        $story_data->{storyPHID} = $story_phid;
-        push(@story_list, $story_data);
-    }
-
-    return [ sort { $a->{id} <=> $b->{id} } @story_list ];
-}
-
-1;
diff --git a/extensions/PhabBugz/lib/Logger.pm b/extensions/PhabBugz/lib/Logger.pm
deleted file mode 100644
index 3127b66db..000000000
--- a/extensions/PhabBugz/lib/Logger.pm
+++ /dev/null
@@ -1,37 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Extension::PhabBugz::Logger;
-
-use 5.10.1;
-
-use Moo;
-
-use Bugzilla::Extension::PhabBugz::Constants;
-
-has 'debugging' => ( is => 'ro' );
-
-sub info  { shift->_log_it('INFO', @_) }
-sub error { shift->_log_it('ERROR', @_) }
-sub debug { shift->_log_it('DEBUG', @_) }
-
-sub _log_it {
-    my ($self, $method, $message) = @_;
-
-    return if $method eq 'DEBUG' && !$self->debugging;
-    chomp $message;
-    if ($ENV{MOD_PERL}) {
-        require Apache2::Log;
-        Apache2::ServerRec::warn("FEED $method: $message");
-    } elsif ($ENV{SCRIPT_FILENAME}) {
-        print STDERR "FEED $method: $message\n";
-    } else {
-        print STDERR '[' . localtime(time) ."] $method: $message\n";
-    }
-}
-
-1;
diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm
deleted file mode 100644
index 3ad9558ff..000000000
--- a/extensions/PhabBugz/lib/Project.pm
+++ /dev/null
@@ -1,290 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Extension::PhabBugz::Project;
-
-use 5.10.1;
-use strict;
-use warnings;
-
-use Bugzilla::Error;
-use Bugzilla::Util qw(trim);
-use Bugzilla::Extension::PhabBugz::Util qw(
-    request
-    get_phab_bmo_ids
-);
-
-#########################
-#    Initialization     #
-#########################
-
-sub new {
-    my ($class, $params) = @_;
-    my $self = $params ? _load($params) : {};
-    bless($self, $class);
-    return $self;
-}
-
-sub _load {
-    my ($params) = @_;
-
-    my $data = {
-        queryKey    => 'all',
-        attachments => {
-            projects    => 1,
-            reviewers   => 1,
-            subscribers => 1
-        },
-        constraints => $params
-    };
-
-    my $result = request('project.search', $data);
-    if (exists $result->{result}{data} && @{ $result->{result}{data} }) {
-        return $result->{result}->{data}->[0];
-    }
-
-    return $result;
-}
-
-# {
-#   "data": [
-#     {
-#       "id": 1,
-#       "type": "PROJ",
-#       "phid": "PHID-PROJ-pfssn7lndryddv7hbx4i",
-#       "fields": {
-#         "name": "bmo-core-security",
-#         "slug": "bmo-core-security",
-#         "milestone": null,
-#         "depth": 0,
-#         "parent": null,
-#         "icon": {
-#           "key": "group",
-#           "name": "Group",
-#           "icon": "fa-users"
-#         },
-#         "color": {
-#           "key": "red",
-#           "name": "Red"
-#         },
-#         "dateCreated": 1500403964,
-#         "dateModified": 1505248862,
-#         "policy": {
-#           "view": "admin",
-#           "edit": "admin",
-#           "join": "admin"
-#         },
-#         "description": "BMO Security Group for core-security"
-#       },
-#       "attachments": {
-#         "members": {
-#           "members": [
-#             {
-#               "phid": "PHID-USER-23ia7vewbjgcqahewncu"
-#             },
-#             {
-#               "phid": "PHID-USER-uif2miph2poiehjeqn5q"
-#             }
-#           ]
-#         },
-#         "ancestors": {
-#           "ancestors": []
-#         },
-#         "watchers": {
-#           "watchers": []
-#         }
-#       }
-#     }
-#   ],
-#   "maps": {
-#     "slugMap": {}
-#   },
-#   "query": {
-#     "queryKey": null
-#   },
-#   "cursor": {
-#     "limit": 100,
-#     "after": null,
-#     "before": null,
-#     "order": null
-#   }
-# }
-
-#########################
-#     Modification      #
-#########################
-
-sub create {
-    my ($class, $params) = @_;
-
-    my $name = trim($params->{name});
-    $name || ThrowCodeError('param_required', { param => 'name' });
-
-    my $description = $params->{description} || 'Need description';
-    my $view_policy = $params->{view_policy} || 'admin';
-    my $edit_policy = $params->{edit_policy} || 'admin';
-    my $join_policy = $params->{join_policy} || 'admin';
-
-    my $data = {
-        transactions => [
-            { type => 'name',        value => $name        },
-            { type => 'description', value => $description },
-            { type => 'edit',        value => $edit_policy },
-            { type => 'join',        value => $join_policy },
-            { type => 'view',        value => $view_policy },
-            { type => 'icon',        value => 'group'      },
-            { type => 'color',       value => 'red'        }
-        ]
-    };
-
-    my $result = request('project.edit', $data);
-
-    return $class->new({ phids => $result->{result}{object}{phid} });
-}
-
-sub update {
-    my ($self) = @_;
-
-    my $data = {
-        objectIdentifier => $self->phid,
-        transactions     => []
-    };
-
-    if ($self->{set_name})  {
-        push(@{ $data->{transactions} }, {
-            type  => 'name',
-            value => $self->{set_name}
-        });
-    }
-
-    if ($self->{set_description})  {
-        push(@{ $data->{transactions} }, {
-            type  => 'description',
-            value => $self->{set_description}
-        });
-    }
-
-    if ($self->{set_members}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'members.set',
-            value => $self->{set_members}
-        });
-    }
-    else {
-        if ($self->{add_members}) {
-            push(@{ $data->{transactions} }, {
-                type  => 'members.add',
-                value => $self->{add_members}
-            });
-        }
-
-        if ($self->{remove_members}) {
-            push(@{ $data->{transactions} }, {
-                type  => 'members.remove',
-                value => $self->{remove_members}
-            });
-        }
-    }
-
-    if ($self->{set_policy}) {
-        foreach my $name ("view", "edit") {
-            next unless $self->{set_policy}->{$name};
-            push(@{ $data->{transactions} }, {
-                type  => $name,
-                value => $self->{set_policy}->{$name}
-            });
-        }
-    }
-
-    my $result = request('project.edit', $data);
-
-    return $result;
-}
-
-#########################
-#      Accessors        #
-#########################
-
-sub id              { return $_[0]->{id};                          }
-sub phid            { return $_[0]->{phid};                        }
-sub type            { return $_[0]->{type};                        }
-sub name            { return $_[0]->{fields}->{name};              }
-sub description     { return $_[0]->{fields}->{description};       }
-sub creation_ts     { return $_[0]->{fields}->{dateCreated};       }
-sub modification_ts { return $_[0]->{fields}->{dateModified};      }
-
-sub view_policy { return $_[0]->{fields}->{policy}->{view}; }
-sub edit_policy { return $_[0]->{fields}->{policy}->{edit}; }
-sub join_policy { return $_[0]->{fields}->{policy}->{join}; }
-
-sub members_raw { return $_[0]->{attachments}->{members}->{members}; }
-
-sub members {
-    my ($self) = @_;
-    return $self->{members} if $self->{members};
-
-    my @phids;
-    foreach my $member (@{ $self->members_raw }) {
-        push(@phids, $member->{phid});
-    }
-
-    return [] if !@phids;
-
-    my $users = get_phab_bmo_ids({ phids => \@phids });
-
-    my @members;
-    foreach my $user (@$users) {
-        my $member = Bugzilla::User->new({ id => $user->{id}, cache => 1});
-        $member->{phab_phid} = $user->{phid};
-        push(@members, $member);
-    }
-
-    return \@members;
-}
-
-#########################
-#       Mutators        #
-#########################
-
-sub set_name {
-    my ($self, $name) = @_;
-    $name = trim($name);
-    $self->{set_name} = $name;
-}
-
-sub set_description {
-    my ($self, $description) = @_;
-    $description = trim($description);
-    $self->{set_description} = $description;
-}
-
-sub add_member {
-    my ($self, $member) = @_;
-    $self->{add_members} ||= [];
-    my $member_phid = blessed $member ? $member->phab_phid : $member;
-    push(@{ $self->{add_members} }, $member_phid);
-}
-
-sub remove_member {
-    my ($self, $member) = @_;
-    $self->{remove_members} ||= [];
-    my $member_phid = blessed $member ? $member->phab_phid : $member;
-    push(@{ $self->{remove_members} }, $member_phid);
-}
-
-sub set_members {
-    my ($self, $members) = @_;
-    $self->{set_members} = [ map { $_->phab_phid } @$members ];
-}
-
-sub set_policy {
-    my ($self, $name, $policy) = @_;
-    $self->{set_policy} ||= {};
-    $self->{set_policy}->{$name} = $policy;
-}
-
-1;
\ No newline at end of file
diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm
deleted file mode 100644
index 29d665009..000000000
--- a/extensions/PhabBugz/lib/Revision.pm
+++ /dev/null
@@ -1,372 +0,0 @@
-# This Source Code Form is hasject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Extension::PhabBugz::Revision;
-
-use 5.10.1;
-use strict;
-use warnings;
-
-use Bugzilla::Bug;
-use Bugzilla::Error;
-use Bugzilla::Util qw(trim);
-use Bugzilla::Extension::PhabBugz::Util qw(
-    get_phab_bmo_ids
-    request
-);
-
-use Types::Standard -all;
-
-my $SearchResult = Dict[
-    id     => Int,
-    type   => Str,
-    phid   => Str,
-    fields => Dict[
-        title             => Str,
-        authorPHID        => Str,
-        dateCreated       => Int,
-        dateModified      => Int,
-        policy            => Dict[ view => Str, edit => Str ],
-        "bugzilla.bug-id" => Int,
-    ],
-    attachments => Dict[
-        reviewers => Dict[
-            reviewers => ArrayRef[
-                Dict[
-                    reviewerPHID => Str,
-                    status       => Str,
-                    isBlocking   => Bool,
-                    actorPHID    => Maybe[Str],
-                ],
-            ],
-        ],
-        subscribers => Dict[
-            subscriberPHIDs => ArrayRef[Str],
-            subscriberCount => Int,
-            viewerIsSubscribed => Bool,
-        ],
-        projects => Dict[ projectPHIDs => ArrayRef[Str] ],
-    ],
-];
-
-my $NewParams    = Dict[ phids => ArrayRef[Str] ];
-
-#########################
-#    Initialization     #
-#########################
-
-sub new {
-    my ($class, $params) = @_;
-    $NewParams->assert_valid($params);
-    my $self = _load($params);
-    $SearchResult->assert_valid($self);
-
-    return bless($self, $class);
-}
-
-sub _load {
-    my ($params) = @_;
-
-    my $data = {
-        queryKey    => 'all',
-        attachments => {
-            projects    => 1,
-            reviewers   => 1,
-            subscribers => 1
-        },
-        constraints => $params
-    };
-
-    my $result = request('differential.revision.search', $data);
-    if (exists $result->{result}{data} && @{ $result->{result}{data} }) {
-        return $result->{result}->{data}->[0];
-    }
-
-    return $result;
-}
-
-# {
-#   "data": [
-#     {
-#       "id": 25,
-#       "type": "DREV",
-#       "phid": "PHID-DREV-uozm3ggfp7e7uoqegmc3",
-#       "fields": {
-#         "title": "Added .arcconfig",
-#         "authorPHID": "PHID-USER-4wigy3sh5fc5t74vapwm",
-#         "dateCreated": 1507666113,
-#         "dateModified": 1508514027,
-#         "policy": {
-#           "view": "public",
-#           "edit": "admin"
-#         },
-#         "bugzilla.bug-id": "1154784"
-#       },
-#       "attachments": {
-#         "reviewers": {
-#           "reviewers": [
-#             {
-#               "reviewerPHID": "PHID-USER-2gjdpu7thmpjxxnp7tjq",
-#               "status": "added",
-#               "isBlocking": false,
-#               "actorPHID": null
-#             },
-#             {
-#               "reviewerPHID": "PHID-USER-o5dnet6dp4dkxkg5b3ox",
-#               "status": "rejected",
-#               "isBlocking": false,
-#               "actorPHID": "PHID-USER-o5dnet6dp4dkxkg5b3ox"
-#             }
-#           ]
-#         },
-#         "subscribers": {
-#           "subscriberPHIDs": [],
-#           "subscriberCount": 0,
-#           "viewerIsSubscribed": true
-#         },
-#         "projects": {
-#           "projectPHIDs": []
-#         }
-#       }
-#     }
-#   ],
-#   "maps": {},
-#   "query": {
-#     "queryKey": null
-#   },
-#   "cursor": {
-#     "limit": 100,
-#     "after": null,
-#     "before": null,
-#     "order": null
-#   }
-# }
-
-#########################
-#     Modification      #
-#########################
-
-sub update {
-    my ($self) = @_;
-
-    my $data = {
-        objectIdentifier => $self->phid,
-        transactions     => []
-    };
-
-    if ($self->{added_comments}) {
-        foreach my $comment (@{ $self->{added_comments} }) {
-            push(@{ $data->{transactions} }, {
-                type  => 'comment',
-                value => $comment
-            });
-        }
-    }
-
-    if ($self->{set_subscribers}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'subscribers.set',
-            value => $self->{set_subscribers}
-        });
-    }
-
-    if ($self->{add_subscribers}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'subscribers.add',
-            value => $self->{add_subscribers}
-        });
-    }
-
-    if ($self->{remove_subscribers}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'subscribers.remove',
-            value => $self->{remove_subscribers}
-        });
-    }
-
-    if ($self->{set_reviewers}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'reviewers.set',
-            value => $self->{set_reviewers}
-        });
-    }
-
-    if ($self->{add_reviewers}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'reviewers.add',
-            value => $self->{add_reviewers}
-        });
-    }
-
-    if ($self->{remove_reviewers}) {
-        push(@{ $data->{transactions} }, {
-            type  => 'reviewers.remove',
-            value => $self->{remove_reviewers}
-        });
-    }
-
-    if ($self->{set_policy}) {
-        foreach my $name ("view", "edit") {
-            next unless $self->{set_policy}->{$name};
-            push(@{ $data->{transactions} }, {
-                type  => $name,
-                value => $self->{set_policy}->{$name}
-            });
-        }
-    }
-
-    my $result = request('differential.revision.edit', $data);
-
-    return $result;
-}
-
-#########################
-#      Accessors        #
-#########################
-
-sub id              { $_[0]->{id};                          }
-sub phid            { $_[0]->{phid};                        }
-sub title           { $_[0]->{fields}->{title};             }
-sub status          { $_[0]->{fields}->{status}->{value};   }
-sub creation_ts     { $_[0]->{fields}->{dateCreated};       }
-sub modification_ts { $_[0]->{fields}->{dateModified};      }
-sub author_phid     { $_[0]->{fields}->{authorPHID};        }
-sub bug_id          { $_[0]->{fields}->{'bugzilla.bug-id'}; }
-
-sub view_policy { $_[0]->{fields}->{policy}->{view}; }
-sub edit_policy { $_[0]->{fields}->{policy}->{edit}; }
-
-sub reviewers_raw    { $_[0]->{attachments}->{reviewers}->{reviewers};         }
-sub subscribers_raw  { $_[0]->{attachments}->{subscribers};                    }
-sub projects_raw     { $_[0]->{attachments}->{projects};                       }
-sub subscriber_count { $_[0]->{attachments}->{subscribers}->{subscriberCount}; }
-
-sub bug {
-    my ($self) = @_;
-    return $self->{bug} ||= Bugzilla::Bug->new({ id => $self->bug_id, cache => 1 });
-}
-
-sub author {
-    my ($self) = @_;
-    return $self->{author} if $self->{author};
-    my $users = get_phab_bmo_ids({ phids => [$self->author_phid] });
-    if (@$users) {
-        $self->{author} = new Bugzilla::User({ id => $users->[0]->{id}, cache => 1 });
-        $self->{author}->{phab_phid} = $self->author_phid;
-        return $self->{author};
-    }
-    return undef;
-}
-
-sub reviewers {
-    my ($self) = @_;
-    return $self->{reviewers} if $self->{reviewers};
-
-    my @phids;
-    foreach my $reviewer (@{ $self->reviewers_raw }) {
-        push(@phids, $reviewer->{reviewerPHID});
-    }
-
-    return [] if !@phids;
-
-    my $users = get_phab_bmo_ids({ phids => \@phids });
-
-    my @reviewers;
-    foreach my $user (@$users) {
-        my $reviewer = Bugzilla::User->new({ id => $user->{id}, cache => 1});
-        $reviewer->{phab_phid} = $user->{phid};
-        foreach my $reviewer_data (@{ $self->reviewers_raw }) {
-            if ($reviewer_data->{reviewerPHID} eq $user->{phid}) {
-                $reviewer->{phab_review_status} = $reviewer_data->{status};
-                last;
-            }
-        }
-        push(@reviewers, $reviewer);
-    }
-
-    return \@reviewers;
-}
-
-sub subscribers {
-    my ($self) = @_;
-    return $self->{subscribers} if $self->{subscribers};
-
-    my @phids;
-    foreach my $phid (@{ $self->subscribers_raw->{subscriberPHIDs} }) {
-        push(@phids, $phid);
-    }
-
-    my $users = get_phab_bmo_ids({ phids => \@phids });
-
-    return [] if !@phids;
-
-    my @subscribers;
-    foreach my $user (@$users) {
-        my $subscriber = Bugzilla::User->new({ id => $user->{id}, cache => 1});
-        $subscriber->{phab_phid} = $user->{phid};
-        push(@subscribers, $subscriber);
-    }
-
-    return \@subscribers;
-}
-
-#########################
-#       Mutators        #
-#########################
-
-sub add_comment {
-    my ($self, $comment) = @_;
-    $comment = trim($comment);
-    $self->{added_comments} ||= [];
-    push(@{ $self->{added_comments} }, $comment);
-}
-
-sub add_reviewer {
-    my ($self, $reviewer) = @_;
-    $self->{add_reviewers} ||= [];
-    my $reviewer_phid = blessed $reviewer ? $reviewer->phab_phid : $reviewer;
-    push(@{ $self->{add_reviewers} }, $reviewer_phid);
-}
-
-sub remove_reviewer {
-    my ($self, $reviewer) = @_;
-    $self->{remove_reviewers} ||= [];
-    my $reviewer_phid = blessed $reviewer ? $reviewer->phab_phid : $reviewer;
-    push(@{ $self->{remove_reviewers} }, $reviewer_phid);
-}
-
-sub set_reviewers {
-    my ($self, $reviewers) = @_;
-    $self->{set_reviewers} = [ map { $_->phab_phid } @$reviewers ];
-}
-
-sub add_subscriber {
-    my ($self, $subscriber) = @_;
-    $self->{add_subscribers} ||= [];
-    my $subscriber_phid = blessed $subscriber ? $subscriber->phab_phid : $subscriber;
-    push(@{ $self->{add_subscribers} }, $subscriber_phid);
-}
-
-sub remove_subscriber {
-    my ($self, $subscriber) = @_;
-    $self->{remove_subscribers} ||= [];
-    my $subscriber_phid = blessed $subscriber ? $subscriber->phab_phid : $subscriber;
-    push(@{ $self->{remove_subscribers} }, $subscriber_phid);
-}
-
-sub set_subscribers {
-    my ($self, $subscribers) = @_;
-    $self->{set_subscribers} = $subscribers;
-}
-
-sub set_policy {
-    my ($self, $name, $policy) = @_;
-    $self->{set_policy} ||= {};
-    $self->{set_policy}->{$name} = $policy;
-}
-
-1;
\ No newline at end of file
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index a00e20551..95b2b1598 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -34,38 +34,26 @@ our @EXPORT = qw(
     get_attachment_revisions
     get_bug_role_phids
     get_members_by_bmo_id
-    get_members_by_phid
-    get_phab_bmo_ids
     get_project_phid
     get_revisions_by_ids
-    get_revisions_by_phids
     get_security_sync_groups
     intersect
     is_attachment_phab_revision
     make_revision_private
     make_revision_public
     request
-    set_phab_user
     set_project_members
     set_revision_subscribers
 );
 
 sub get_revisions_by_ids {
     my ($ids) = @_;
-    return _get_revisions({ ids => $ids });
-}
-
-sub get_revisions_by_phids {
-    my ($phids) = @_;
-    return _get_revisions({ phids => $phids });
-}
-
-sub _get_revisions {
-    my ($constraints) = @_;
 
     my $data = {
-        queryKey    => 'all',
-        constraints => $constraints
+        queryKey => 'all',
+        constraints => {
+            ids => $ids
+        }
     };
 
     my $result = request('differential.revision.search', $data);
@@ -73,11 +61,11 @@ sub _get_revisions {
     ThrowUserError('invalid_phabricator_revision_id')
         unless (exists $result->{result}{data} && @{ $result->{result}{data} });
 
-    return $result->{result}{data};
+    return @{$result->{result}{data}};
 }
 
 sub create_revision_attachment {
-    my ( $bug, $revision_id, $revision_title, $timestamp ) = @_;
+    my ( $bug, $revision_id, $revision_title ) = @_;
 
     my $phab_base_uri = Bugzilla->params->{phabricator_base_uri};
     ThrowUserError('invalid_phabricator_uri') unless $phab_base_uri;
@@ -92,10 +80,16 @@ sub create_revision_attachment {
     return $review_attachment if defined $review_attachment;
 
     # No attachment is present, so we can now create new one
+    my $is_shadow_db = Bugzilla->is_shadow_db;
+    Bugzilla->switch_to_main_db if $is_shadow_db;
 
-    if (!$timestamp) {
-        ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
-    }
+    my $old_user = Bugzilla->user;
+    _set_phab_user();
+
+    my $dbh = Bugzilla->dbh;
+    $dbh->bz_start_transaction;
+
+    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
 
     my $attachment = Bugzilla::Attachment->create(
         {
@@ -110,9 +104,13 @@ sub create_revision_attachment {
         }
     );
 
-    # Insert a comment about the new attachment into the database.
-    $bug->add_comment('', { type => CMT_ATTACHMENT_CREATED,
-                            extra_data => $attachment->id });
+    $bug->update($timestamp);
+    $attachment->update($timestamp);
+
+    $dbh->bz_commit_transaction;
+    Bugzilla->switch_to_shadow_db if $is_shadow_db;
+
+    Bugzilla->set_user($old_user);
 
     return $attachment;
 }
@@ -324,7 +322,12 @@ sub set_project_members {
 sub get_members_by_bmo_id {
     my $users = shift;
 
-    my $result = get_phab_bmo_ids({ ids => [ map { $_->id } @$users ] });
+    my $data = {
+        accountids => [ map { $_->id } @$users ]
+    };
+
+    my $result = request('bmoexternalaccount.search', $data);
+    return [] if (!$result->{result});
 
     my @phab_ids;
     foreach my $user (@{ $result->{result} }) {
@@ -335,73 +338,10 @@ sub get_members_by_bmo_id {
     return \@phab_ids;
 }
 
-sub get_members_by_phid {
-    my $phids = shift;
-
-    my $result = get_phab_bmo_ids({ phids => $phids });
-
-    my @bmo_ids;
-    foreach my $user (@{ $result->{result} }) {
-        push(@bmo_ids, $user->{id})
-          if ($user->{phid} && $user->{phid} =~ /^PHID-USER/);
-    }
-
-    return \@bmo_ids;
-}
-
-sub get_phab_bmo_ids {
-    my ($params) = @_;
-    my $memcache = Bugzilla->memcached;
-
-    # Try to find the values in memcache first
-    my @results;
-    if ($params->{ids}) {
-        my @bmo_ids = @{ $params->{ids} };
-        for (my $i = 0; $i < @bmo_ids; $i++) {
-            my $phid = $memcache->get({ key => "phab_user_bmo_id_" . $bmo_ids[$i] });
-            if ($phid) {
-                push(@results, {
-                    id   => $bmo_ids[$i],
-                    phid => $phid
-                });
-                splice(@bmo_ids, $i, 1);
-            }
-        }
-        $params->{ids} = \@bmo_ids;
-    }
-
-    if ($params->{phids}) {
-        my @phids = @{ $params->{phids} };
-        for (my $i = 0; $i < @phids; $i++) {
-            my $bmo_id = $memcache->get({ key => "phab_user_phid_" . $phids[$i] });
-            if ($bmo_id) {
-                push(@results, {
-                    id   => $bmo_id,
-                    phid => $phids[$i]
-                });
-                splice(@phids, $i, 1);
-            }
-        }
-        $params->{phids} = \@phids;
-    }
-
-    my $result = request('bugzilla.account.search', $params);
-
-    # Store new values in memcache for later retrieval
-    foreach my $user (@{ $result->{result} }) {
-        $memcache->set({ key   => "phab_user_bmo_id_" . $user->{id},
-                         value => $user->{phid} });
-        $memcache->set({ key   => "phab_user_phid_" . $user->{phid},
-                         value => $user->{id} });
-        push(@results, $user);
-    }
-
-    return \@results;
-}
-
 sub is_attachment_phab_revision {
-    my ($attachment) = @_;
+    my ($attachment, $include_obsolete) = @_;
     return ($attachment->contenttype eq PHAB_CONTENT_TYPE
+            && ($include_obsolete || !$attachment->isobsolete)
             && $attachment->attacher->login eq PHAB_AUTOMATION_USER) ? 1 : 0;
 }
 
@@ -460,12 +400,10 @@ sub request {
 
     my $result;
     my $result_ok = eval { $result = decode_json( $response->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};
+    if ( !$result_ok ) {
+        ThrowCodeError(
+            'phabricator_api_error',
+            { reason => 'JSON decode failure' } );
     }
 
     return $result;
@@ -486,12 +424,10 @@ sub get_security_sync_groups {
     return @set_groups;
 }
 
-sub set_phab_user {
-    my $old_user = Bugzilla->user;
+sub _set_phab_user {
     my $user = Bugzilla::User->new( { name => PHAB_AUTOMATION_USER } );
     $user->{groups} = [ Bugzilla::Group->get_all ];
     Bugzilla->set_user($user);
-    return $old_user;
 }
 
 sub add_security_sync_comments {
@@ -510,10 +446,14 @@ sub add_security_sync_comments {
     : 'One revision was' )
     . ' made private due to unknown Bugzilla groups.';
 
-    my $old_user = set_phab_user();
+    my $old_user = Bugzilla->user;
+    _set_phab_user();
 
     $bug->add_comment( $bmo_error_message, { isprivate => 0 } );
 
+    my $bug_changes = $bug->update();
+    $bug->send_changes($bug_changes);
+
     Bugzilla->set_user($old_user);
 }
 
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index b552e5656..738077880 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -268,7 +268,7 @@ sub obsolete_attachments {
     my $bug = Bugzilla::Bug->check($bug_id);
 
     my @attachments =
-      grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
+      grep { is_attachment_phab_revision($_, 1) } @{ $bug->attachments() };
 
     return { result => [] } if !@attachments;