Bug 1482145 - add a scope_guard option to set_user()

diff --git a/Bugzilla.pm b/Bugzilla.pm
index f26819d93f2b1d09775b03771512ae3911c9e444..4d5e559d9c96439be6db0b115017fbabb9caa3aa 100644 (file)
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -46,6 +46,7 @@ use File::Basename;
 use File::Spec::Functions;
 use Safe;
 use JSON::XS qw(decode_json);
+use Scope::Guard;
 
 use parent qw(Bugzilla::CPAN);
 
@@ -275,8 +276,20 @@ sub user {
 }
 
 sub set_user {
-    my (undef, $user) = @_;
-    request_cache->{user} = $user;
+    my (undef, $new_user, %option) = @_;
+
+    if ($option{scope_guard}) {
+        my $old_user = request_cache->{user};
+        request_cache->{user} = $new_user;
+        return Scope::Guard->new(
+            sub {
+                request_cache->{user} = $old_user;
+            }
+        )
+    }
+    else {
+        request_cache->{user} = $new_user;
+    }
 }
 
 sub sudoer {

diff --git a/Makefile.PL b/Makefile.PL
index 4cf8755c6e56b7bcd9ac5c9bf7e698945738b135..3bf6926a55f1175f534a9b8471749ab94c06ceba 100755 (executable)
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -74,6 +74,7 @@ my %requires = (
     'Mozilla::CA'              => '20160104',
     'Parse::CPAN::Meta'        => '1.44',
     'Role::Tiny'               => '2.000003',
+    'Scope::Guard'             => '0.21',
     'Sereal'                   => '4.004',
     'Taint::Util'              => '0.08',
     'Template'                 => '2.24',

diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index 4799bd0a3ef33e2bede340d46c3ef1088eb58b97..4d2732f94ffaa69d4371f269359e90502bc9fdce 100644 (file)
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -385,8 +385,8 @@ sub process_revision_change {
         $story_text);
     INFO($log_message);
 
-    # Pre setup before making changes
-    my $old_user = set_phab_user();
+    # change to the phabricator user, which returns a guard that restores the previous user.
+    my $restore_prev_user = set_phab_user();
     my $bug = $revision->bug;
 
     # Check to make sure bug id is valid and author can see it
@@ -619,8 +619,6 @@ sub process_revision_change {
         Bugzilla::BugMail::Send($bug_id, { changer => $rev_attachment->attacher });
     }
 
-    Bugzilla->set_user($old_user);
-
     INFO('SUCCESS: Revision D' . $revision->id . ' processed');
 }
 
@@ -639,7 +637,7 @@ sub process_new_user {
     my $bug_user  = $phab_user->bugzilla_user;
 
     # Pre setup before querying DB
-    my $old_user = set_phab_user();
+    my $restore_prev_user = set_phab_user();
 
     # CHECK AND WARN FOR POSSIBLE USERNAME SQUATTING
     INFO("Checking for username squatters");
@@ -758,8 +756,6 @@ sub process_new_user {
         }
     }
 
-    Bugzilla->set_user($old_user);
-
     INFO('SUCCESS: User ' . $phab_user->id . ' processed');
 }
 
@@ -868,11 +864,8 @@ sub add_flag_comment {
     my ( $bug, $attachment, $comment, $user, $old_flags, $new_flags, $timestamp )
         = @$params{qw(bug attachment comment user old_flags new_flags timestamp)};
 
-    my $old_user;
-    if ($user) {
-        $old_user = Bugzilla->user;
-        Bugzilla->set_user($user);
-    }
+    # when this function returns, Bugzilla->user will return to its previous value.
+    my $restore_prev_user = Bugzilla->set_user($user, scope_guard => 1);
 
     INFO("Flag comment: $comment");
     $bug->add_comment(
@@ -886,8 +879,6 @@ sub add_flag_comment {
 
     $attachment->set_flags( $old_flags, $new_flags );
     $attachment->update($timestamp);
-
-    Bugzilla->set_user($old_user) if $old_user;
 }
 
 1;

diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index 4e846badce3d923c647f60d097fa362529725ccd..9c79c18555f37706f3e1e03cad15ebab7244de11 100644 (file)
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -65,37 +65,26 @@ sub create_revision_attachment {
     }
 
     # If submitter, then switch to that user when creating attachment
-    my ($old_user, $attachment);
-    try {
-        if ($submitter) {
-            $old_user = Bugzilla->user;
-            $submitter->{groups} = [ Bugzilla::Group->get_all ]; # We need to always be able to add attachment
-            Bugzilla->set_user($submitter);
+    local $submitter->{groups} = [ Bugzilla::Group->get_all ]; # We need to always be able to add attachment
+    my $restore_prev_user = Bugzilla->set_user($submitter, scope_guard => 1);
+
+    my $attachment = Bugzilla::Attachment->create(
+        {
+            bug         => $bug,
+            creation_ts => $timestamp,
+            data        => $revision_uri,
+            description => $revision->title,
+            filename    => 'phabricator-D' . $revision->id . '-url.txt',
+            ispatch     => 0,
+            isprivate   => 0,
+            mimetype    => PHAB_CONTENT_TYPE,
         }
+    );
 
-        $attachment = Bugzilla::Attachment->create(
-            {
-                bug         => $bug,
-                creation_ts => $timestamp,
-                data        => $revision_uri,
-                description => $revision->title,
-                filename    => 'phabricator-D' . $revision->id . '-url.txt',
-                ispatch     => 0,
-                isprivate   => 0,
-                mimetype    => PHAB_CONTENT_TYPE,
-            }
-        );
+    # Insert a comment about the new attachment into the database.
+    $bug->add_comment($revision->summary, { type       => CMT_ATTACHMENT_CREATED,
+                                            extra_data => $attachment->id });
 
-        # Insert a comment about the new attachment into the database.
-        $bug->add_comment($revision->summary, { type       => CMT_ATTACHMENT_CREATED,
-                                                extra_data => $attachment->id });
-    }
-    catch {
-        die $_;
-    }
-    finally {
-        Bugzilla->set_user($old_user) if $old_user;
-    };
 
     return $attachment;
 }
@@ -210,11 +199,10 @@ sub request {
 }
 
 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;
+
+    return Bugzilla->set_user($user, scope_guard => 1);
 }
 
 sub get_needs_review {