From 871fc7dd332dadd24a7e6e1db3c7f5e8ef93b00e Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Thu, 11 Oct 2018 15:28:18 -0400 Subject: Bug 1498206 - Replace LWP::UserAgent with Mojo::UserAgent in phabbugz extension --- Bugzilla/Test/Util.pm | 21 ++++++++++++++++++++- extensions/PhabBugz/lib/Util.pm | 32 +++++++++++--------------------- extensions/PhabBugz/t/basic.t | 27 ++++++++++++++------------- extensions/PhabBugz/t/feed-daemon-guts.t | 12 ++++++------ 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 = < 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 => 'foo' })], - ['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('foo') ], + ['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]; } ); -- cgit v1.2.3-24-g4f1b