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

12 files changed, 1350 insertions, 61 deletions

diff --git a/extensions/PhabBugz/Extension.pm b/extensions/PhabBugz/Extension.pm
index 68090aa10..b3ad44819 100644
--- a/extensions/PhabBugz/Extension.pm
+++ b/extensions/PhabBugz/Extension.pm
@@ -10,10 +10,20 @@ package Bugzilla::Extension::PhabBugz;
 use 5.10.1;
 use strict;
 use warnings;
+
 use parent qw(Bugzilla::Extension);
 
+use Bugzilla::Constants;
+use Bugzilla::Extension::PhabBugz::Feed;
+use Bugzilla::Extension::PhabBugz::Logger;
+
 our $VERSION = '0.01';
 
+BEGIN {
+    *Bugzilla::User::phab_phid = sub { return $_[0]->{phab_phid}; };
+    *Bugzilla::User::phab_review_status = sub { return $_[0]->{phab_review_status}; };
+}
+
 sub config_add_panels {
     my ($self, $args) = @_;
     my $modules = $args->{panel_modules};
@@ -40,4 +50,47 @@ sub webservice {
     $args->{dispatch}->{PhabBugz} = "Bugzilla::Extension::PhabBugz::WebService";
 }
 
+#
+# installation/config hooks
+#
+
+sub db_schema_abstract_schema {
+    my ($self, $args) = @_;
+    $args->{'schema'}->{'phabbugz'} = {
+        FIELDS => [
+            id => {
+                TYPE       => 'INTSERIAL',
+                NOTNULL    => 1,
+                PRIMARYKEY => 1,
+            },
+            name => {
+                TYPE    => 'VARCHAR(255)',
+                NOTNULL => 1,
+            },
+            value => {
+                TYPE    => 'MEDIUMTEXT',
+                NOTNULL => 1
+            }
+        ],
+        INDEXES => [
+            phabbugz_idx => {
+                FIELDS => ['name'],
+                TYPE => 'UNIQUE',
+            },
+        ],
+    };
+}
+
+sub install_filesystem {
+    my ($self, $args) = @_;
+    my $files = $args->{'files'};
+
+    my $extensionsdir = bz_locations()->{'extensionsdir'};
+    my $scriptname = $extensionsdir . "/PhabBugz/bin/phabbugzd.pl";
+
+    $files->{$scriptname} = {
+        perms => Bugzilla::Install::Filesystem::WS_EXECUTE
+    };
+}
+
 __PACKAGE__->NAME;
diff --git a/extensions/PhabBugz/bin/phabbugz_feed.pl b/extensions/PhabBugz/bin/phabbugz_feed.pl
new file mode 100755
index 000000000..9db491bd0
--- /dev/null
+++ b/extensions/PhabBugz/bin/phabbugz_feed.pl
@@ -0,0 +1,50 @@
+#!/usr/bin/perl
+
+# 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.
+
+use 5.10.1;
+use strict;
+use warnings;
+
+use lib qw(. lib local/lib/perl5);
+
+BEGIN {
+    use Bugzilla;
+    Bugzilla->extensions;
+}
+
+use Bugzilla::Extension::PhabBugz::Daemon;
+Bugzilla::Extension::PhabBugz::Daemon->start();
+
+=head1 NAME
+
+phabbugz_feed.pl - Query Phabricator for interesting changes and update bugs related to revisions.
+
+=head1 SYNOPSIS
+
+  phabbugz_feed.pl [OPTIONS] COMMAND
+
+    OPTIONS:
+      -f        Run in the foreground (don't detach)
+      -d        Output a lot of debugging information
+      -p file   Specify the file where phabbugz_feed.pl should store its current
+                process id. Defaults to F<data/phabbugz_feed.pl.pid>.
+      -n name   What should this process call itself in the system log?
+                Defaults to the full path you used to invoke the script.
+
+    COMMANDS:
+      start     Starts a new phabbugz_feed daemon if there isn't one running already
+      stop      Stops a running phabbugz_feed daemon
+      restart   Stops a running phabbugz_feed if one is running, and then
+                starts a new one.
+      check     Report the current status of the daemon.
+      install   On some *nix systems, this automatically installs and
+                configures phabbugz_feed.pl as a system service so that it will
+                start every time the machine boots.
+      uninstall Removes the system service for phabbugz_feed.pl.
+      help      Display this usage info
diff --git a/extensions/PhabBugz/bin/update_project_members.pl b/extensions/PhabBugz/bin/update_project_members.pl
index bdc054e1a..06cc55626 100755
--- a/extensions/PhabBugz/bin/update_project_members.pl
+++ b/extensions/PhabBugz/bin/update_project_members.pl
@@ -20,11 +20,9 @@ use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Group;
 
+use Bugzilla::Extension::PhabBugz::Project;
 use Bugzilla::Extension::PhabBugz::Util qw(
-    create_project
-    get_members_by_bmo_id
-    get_project_phid
-    set_project_members
+    get_phab_bmo_ids
 );
 
 Bugzilla->usage_mode(USAGE_MODE_CMDLINE);
@@ -55,23 +53,22 @@ unless ($phab_sync_groups = Bugzilla->params->{phabricator_sync_groups}) {
 my $sync_groups = Bugzilla::Group->match({ name => [ split('[,\s]+', $phab_sync_groups) ] });
 
 foreach my $group (@$sync_groups) {
-    my @users = get_group_members($group);
-
     # Create group project if one does not yet exist
     my $phab_project_name = 'bmo-' . $group->name;
-    my $project_phid = get_project_phid($phab_project_name);
-    if (!$project_phid) {
-        $project_phid = create_project($phab_project_name, 'BMO Security Group for ' . $group->name);
+    my $project = Bugzilla::Extension::PhabBugz::Project->new({
+        name => $phab_project_name
+    });
+    if (!$project->id) {
+        $project = Bugzilla::Extension::PhabBugz::Project->create({
+            name        => $phab_project_name,
+            description => 'BMO Security Group for ' . $group->name
+        });
     }
 
-    # Get the internal user ids for the bugzilla group members
-    my $phab_user_ids = [];
-    if (@users) {
-        $phab_user_ids = get_members_by_bmo_id(\@users);
-    }
+    my @group_members = get_group_members($group);
 
-    # Set the project members to the exact list
-    set_project_members($project_phid, $phab_user_ids);
+    $project->set_members(\@group_members);
+    $project->update();
 }
 
 sub get_group_members {
@@ -84,5 +81,13 @@ sub get_group_members {
             $users{$user->id} = $user;
         }
     }
-    return values %users;
-}
+
+    # Look up the phab ids for these users
+    my $phab_users = get_phab_bmo_ids({ ids => [ keys %users ] });
+    foreach my $phab_user (@{ $phab_users }) {
+        $users{$phab_user->{id}}->{phab_phid} = $phab_user->{phid};
+    }
+
+    # We only need users who have accounts in phabricator
+    return grep { $_->phab_phid } values %users;
+}
\ No newline at end of file
diff --git a/extensions/PhabBugz/lib/Constants.pm b/extensions/PhabBugz/lib/Constants.pm
index f7485e8c4..754130f0b 100644
--- a/extensions/PhabBugz/lib/Constants.pm
+++ b/extensions/PhabBugz/lib/Constants.pm
@@ -16,10 +16,12 @@ 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
new file mode 100644
index 000000000..c8b4f73af
--- /dev/null
+++ b/extensions/PhabBugz/lib/Daemon.pm
@@ -0,0 +1,100 @@
+# 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
new file mode 100644
index 000000000..d178f249b
--- /dev/null
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -0,0 +1,320 @@
+# 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
new file mode 100644
index 000000000..3127b66db
--- /dev/null
+++ b/extensions/PhabBugz/lib/Logger.pm
@@ -0,0 +1,37 @@
+# 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
new file mode 100644
index 000000000..3ad9558ff
--- /dev/null
+++ b/extensions/PhabBugz/lib/Project.pm
@@ -0,0 +1,290 @@
+# 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
new file mode 100644
index 000000000..29d665009
--- /dev/null
+++ b/extensions/PhabBugz/lib/Revision.pm
@@ -0,0 +1,372 @@
+# 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 95b2b1598..a00e20551 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -34,26 +34,38 @@ 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 => {
-            ids => $ids
-        }
+        queryKey    => 'all',
+        constraints => $constraints
     };
 
     my $result = request('differential.revision.search', $data);
@@ -61,11 +73,11 @@ sub get_revisions_by_ids {
     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 ) = @_;
+    my ( $bug, $revision_id, $revision_title, $timestamp ) = @_;
 
     my $phab_base_uri = Bugzilla->params->{phabricator_base_uri};
     ThrowUserError('invalid_phabricator_uri') unless $phab_base_uri;
@@ -80,16 +92,10 @@ 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;
-
-    my $old_user = Bugzilla->user;
-    _set_phab_user();
 
-    my $dbh = Bugzilla->dbh;
-    $dbh->bz_start_transaction;
-
-    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
+    if (!$timestamp) {
+        ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
+    }
 
     my $attachment = Bugzilla::Attachment->create(
         {
@@ -104,13 +110,9 @@ sub create_revision_attachment {
         }
     );
 
-    $bug->update($timestamp);
-    $attachment->update($timestamp);
-
-    $dbh->bz_commit_transaction;
-    Bugzilla->switch_to_shadow_db if $is_shadow_db;
-
-    Bugzilla->set_user($old_user);
+    # Insert a comment about the new attachment into the database.
+    $bug->add_comment('', { type => CMT_ATTACHMENT_CREATED,
+                            extra_data => $attachment->id });
 
     return $attachment;
 }
@@ -322,12 +324,7 @@ sub set_project_members {
 sub get_members_by_bmo_id {
     my $users = shift;
 
-    my $data = {
-        accountids => [ map { $_->id } @$users ]
-    };
-
-    my $result = request('bmoexternalaccount.search', $data);
-    return [] if (!$result->{result});
+    my $result = get_phab_bmo_ids({ ids => [ map { $_->id } @$users ] });
 
     my @phab_ids;
     foreach my $user (@{ $result->{result} }) {
@@ -338,10 +335,73 @@ 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, $include_obsolete) = @_;
+    my ($attachment) = @_;
     return ($attachment->contenttype eq PHAB_CONTENT_TYPE
-            && ($include_obsolete || !$attachment->isobsolete)
             && $attachment->attacher->login eq PHAB_AUTOMATION_USER) ? 1 : 0;
 }
 
@@ -400,10 +460,12 @@ sub request {
 
     my $result;
     my $result_ok = eval { $result = decode_json( $response->content); 1 };
-    if ( !$result_ok ) {
-        ThrowCodeError(
-            'phabricator_api_error',
-            { reason => 'JSON decode failure' } );
+    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};
     }
 
     return $result;
@@ -424,10 +486,12 @@ sub get_security_sync_groups {
     return @set_groups;
 }
 
-sub _set_phab_user {
+sub set_phab_user {
+    my $old_user = Bugzilla->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 {
@@ -446,14 +510,10 @@ sub add_security_sync_comments {
     : 'One revision was' )
     . ' made private due to unknown Bugzilla groups.';
 
-    my $old_user = Bugzilla->user;
-    _set_phab_user();
+    my $old_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 0ff2d46f9..f8c109612 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($_, 1) } @{ $bug->attachments() };
+      grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
 
     return { result => [] } if !@attachments;
 
diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm
index 4f0a57793..988403727 100644
--- a/extensions/Push/lib/Connector/Phabricator.pm
+++ b/extensions/Push/lib/Connector/Phabricator.pm
@@ -22,8 +22,8 @@ use Bugzilla::Extension::PhabBugz::Constants;
 use Bugzilla::Extension::PhabBugz::Util qw(
   add_comment_to_revision create_private_revision_policy
   edit_revision_policy get_attachment_revisions get_bug_role_phids
-  get_revisions_by_ids intersect is_attachment_phab_revision
-  make_revision_public make_revision_private set_revision_subscribers
+  get_revisions_by_ids intersect make_revision_public
+  make_revision_private set_revision_subscribers
   get_security_sync_groups add_security_sync_comments);
 use Bugzilla::Extension::Push::Constants;
 use Bugzilla::Extension::Push::Util qw(is_public);