Bug 1498206 - Replace LWP::UserAgent with Mojo::UserAgent in phabbugz extension

4 files changed, 51 insertions, 41 deletions

diff --git a/Bugzilla/Test/Util.pm b/Bugzilla/Test/Util.pm
index 8124c25ee..9fbc151f7 100644
--- a/Bugzilla/Test/Util.pm
+++ b/Bugzilla/Test/Util.pm
@@ -12,10 +12,12 @@ use strict;
 use warnings;
 
 use base qw(Exporter);
-our @EXPORT = qw(create_user issue_api_key);
+our @EXPORT = qw(create_user issue_api_key mock_useragent_tx);
 
 use Bugzilla::User;
 use Bugzilla::User::APIKey;
+use Mojo::Message::Response;
+use Test2::Tools::Mock qw(mock);
 
 sub create_user {
     my ($login, $password, %extra) = @_;
@@ -47,4 +49,21 @@ sub issue_api_key {
     }
 }
 
+sub _json_content_type { $_->headers->content_type('application/json') }
+
+sub mock_useragent_tx {
+    my ($body, $modify) = @_;
+    $modify //= \&_json_content_type;
+
+    my $res = Mojo::Message::Response->new;
+    $res->code(200);
+    $res->body($body);
+    if ($modify) {
+        local $_ = $res;
+        $modify->($res);
+    }
+
+    return mock({result => $res});
+}
+
 1;
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index f2876366f..67b29f27c 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -20,14 +20,14 @@ use Bugzilla::Util qw(trim);
 use Bugzilla::Extension::PhabBugz::Constants;
 use Bugzilla::Extension::PhabBugz::Types qw(:types);
 
-use JSON::XS qw(encode_json decode_json);
 use List::Util qw(first);
-use LWP::UserAgent;
 use Taint::Util qw(untaint);
 use Try::Tiny;
 use Type::Params qw( compile );
 use Type::Utils;
 use Types::Standard qw( :types );
+use Mojo::UserAgent;
+use Mojo::JSON qw(encode_json);
 
 use base qw(Exporter);
 
@@ -159,11 +159,10 @@ sub request {
 
     my $ua = $request_cache->{phabricator_ua};
     unless ($ua) {
-        $ua = $request_cache->{phabricator_ua} = LWP::UserAgent->new(timeout => 10);
+        $ua = $request_cache->{phabricator_ua} = Mojo::UserAgent->new;
         if ($params->{proxy_url}) {
-            $ua->proxy('https', $params->{proxy_url});
+            $ua->proxy($params->{proxy_url});
         }
-        $ua->default_header('Content-Type' => 'application/x-www-form-urlencoded');
     }
 
     my $phab_api_key = $params->{phabricator_api_key};
@@ -175,25 +174,16 @@ sub request {
 
     $data->{__conduit__} = { token => $phab_api_key };
 
-    my $response = $ua->post($full_uri, { params => encode_json($data) });
-
+    my $response = $ua->post($full_uri => form => { params => encode_json($data) })->result;
     ThrowCodeError('phabricator_api_error', { reason => $response->message })
       if $response->is_error;
 
-    my $result;
-    my $result_ok = eval {
-        my $content = $response->content;
-        untaint($content);
-        $result = decode_json( $content );
-        1;
-    };
-    if (!$result_ok || $result->{error_code}) {
-        ThrowCodeError('phabricator_api_error',
-            { reason => 'JSON decode failure' }) if !$result_ok;
-        ThrowCodeError('phabricator_api_error',
-            { code   => $result->{error_code},
-              reason => $result->{error_info} }) if $result->{error_code};
-    }
+    my $result = $response->json;
+    ThrowCodeError('phabricator_api_error',
+        { reason => 'JSON decode failure' }) if !defined($result);
+    ThrowCodeError('phabricator_api_error',
+        { code   => $result->{error_code},
+          reason => $result->{error_info} }) if $result->{error_code};
 
     return $result;
 }
diff --git a/extensions/PhabBugz/t/basic.t b/extensions/PhabBugz/t/basic.t
index 9a6723ccb..af92dc64f 100644
--- a/extensions/PhabBugz/t/basic.t
+++ b/extensions/PhabBugz/t/basic.t
@@ -17,6 +17,7 @@ use Test::More;
 use Test2::Tools::Mock;
 use Data::Dumper;
 use JSON::MaybeXS;
+use Bugzilla::Test::Util qw(mock_useragent_tx);
 use Carp;
 use Try::Tiny;
 
@@ -98,13 +99,13 @@ my $feed = Bugzilla::Extension::PhabBugz::Feed->new;
 
 # Same members in both
 do {
-    my $UserAgent = mock 'LWP::UserAgent' => (
+    my $UserAgent = mock 'Mojo::UserAgent' => (
         override => [
             'post' => sub {
-                my ($self, $url, $params) = @_;
+                my ($self, $url, undef, $params) = @_;
                 my $data = decode_json($params->{params});
                 is_deeply($data->{transactions}, [], 'no-op');
-                return mock({is_error => 0, content => '{}'});
+                return mock_useragent_tx('{}');
             },
         ],
     );
@@ -119,14 +120,14 @@ do {
 
 # Project has members not in group
 do {
-    my $UserAgent = mock 'LWP::UserAgent' => (
+    my $UserAgent = mock 'Mojo::UserAgent' => (
         override => [
             'post' => sub {
-                my ($self, $url, $params) = @_;
+                my ($self, $url, undef, $params) = @_;
                 my $data = decode_json($params->{params});
                 my $expected = [ { type => 'members.remove', value => ['foo'] } ];
                 is_deeply($data->{transactions}, $expected, 'remove foo');
-                return mock({is_error => 0, content => '{}'});
+                return mock_useragent_tx('{}');
             },
         ]
     );
@@ -139,14 +140,14 @@ do {
 
 # Group has members not in project
 do {
-    my $UserAgent = mock 'LWP::UserAgent' => (
+    my $UserAgent = mock 'Mojo::UserAgent' => (
         override => [
             'post' => sub {
-                my ($self, $url, $params) = @_;
+                my ($self, $url, undef, $params) = @_;
                 my $data = decode_json($params->{params});
                 my $expected = [ { type => 'members.add', value => ['foo'] } ];
                 is_deeply($data->{transactions}, $expected, 'add foo');
-                return mock({is_error => 0, content => '{}'});
+                return mock_useragent_tx('{}');
             },
         ]
     );
@@ -164,10 +165,10 @@ do {
             'update' => sub { 1 },
         ],
     );
-    my $UserAgent = mock 'LWP::UserAgent' => (
+    my $UserAgent = mock 'Mojo::UserAgent' => (
         override => [
             'post' => sub {
-                my ($self, $url, $params) = @_;
+                my ($self, $url, undef, $params) = @_;
                 if ($url =~ /differential\.revision\.search/) {
                     my $content = <<JSON;
 {
@@ -215,10 +216,10 @@ do {
     }
 }
 JSON
-                    return mock { is_error => 0, content => $content };
+                    return mock_useragent_tx($content);
                 }
                 else {
-                    return mock { is_error => 1, message => "bad request" };
+                    return mock_useragent_tx("bad request");
                 }
             },
         ],
diff --git a/extensions/PhabBugz/t/feed-daemon-guts.t b/extensions/PhabBugz/t/feed-daemon-guts.t
index 376af18e4..0c508be98 100644
--- a/extensions/PhabBugz/t/feed-daemon-guts.t
+++ b/extensions/PhabBugz/t/feed-daemon-guts.t
@@ -12,7 +12,7 @@ use lib qw( . lib local/lib/perl5 );
 BEGIN { $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf' }
 use Bugzilla::Test::MockDB;
 use Bugzilla::Test::MockParams;
-use Bugzilla::Test::Util qw(create_user);
+use Bugzilla::Test::Util qw(create_user mock_useragent_tx);
 use Test::More;
 use Test2::Tools::Mock;
 use Try::Tiny;
@@ -31,7 +31,7 @@ Bugzilla->error_mode(ERROR_MODE_TEST);
 
 my $phab_bot = create_user(PHAB_AUTOMATION_USER, '*');
 
-my $UserAgent = mock 'LWP::UserAgent' => ();
+my $UserAgent = mock 'Mojo::UserAgent' => ();
 
 {
     SetParam('phabricator_enabled', 0);
@@ -54,9 +54,9 @@ my $UserAgent = mock 'LWP::UserAgent' => ();
 }
 
 my @bad_response = (
-    ['http error', mock({ is_error => 1, message => 'some http error' }) ],
-    ['invalid json', mock({ is_error => 0, content => '<xml>foo</xml>' })],
-    ['json containing error code', mock({ is_error => 0, content => encode_json({error_code => 1234 }) })],
+    ['http error', mock_useragent_tx("doesn't matter", sub { $_->code(500) }) ],
+    ['invalid json', mock_useragent_tx('<xml>foo</xml>') ],
+    ['json containing error code', mock_useragent_tx(encode_json({error_code => 1234 }))],
 );
 
 SetParam(phabricator_enabled => 1);
@@ -67,7 +67,7 @@ foreach my $bad_response (@bad_response) {
     my $feed = Bugzilla::Extension::PhabBugz::Feed->new;
     $UserAgent->override(
         post => sub {
-            my ( $self, $url, $params ) = @_;
+            my ( $self, $url, undef, $params ) = @_;
             return $bad_response->[1];
         }
     );