diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | perllib/Open311.pm | 103 |
2 files changed, 42 insertions, 62 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cc3d990b..53a580b11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ - Development improvements: - Make front page cache time configurable. - Better working of /fakemapit/ under https. + - Improve Open311 error output on failing GET requests. - Backwards incompatible changes: - If you wish the default for the showname checkbox to be checked, add `sub default_show_name { 1 }` to your cobrand file. diff --git a/perllib/Open311.pm b/perllib/Open311.pm index 749e7ccad..ec3390ee7 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -8,7 +8,7 @@ use XML::Simple; use LWP::Simple; use LWP::UserAgent; use DateTime::Format::W3CDTF; -use HTTP::Request::Common qw(POST); +use HTTP::Request::Common qw(GET POST); use FixMyStreet::Cobrand; use FixMyStreet::DB; use Utils; @@ -437,80 +437,49 @@ sub _params_to_string { return $string; } -sub _get { - my $self = shift; - my $path = shift; +sub _request { + my $self = shift; + my $method = shift; + my $path = shift; my $params = shift || {}; my $uri = URI->new( $self->endpoint ); + $uri->path( $uri->path . $path ); - $params->{ jurisdiction_id } = $self->jurisdiction + $params->{jurisdiction_id} = $self->jurisdiction if $self->jurisdiction; - $uri->path( $uri->path . $path ); - my $base_uri = $uri->clone; - $uri->query_form( $params ); + $params->{api_key} = ($self->api_key || '') + if $method eq 'POST' && $self->api_key; + + my $debug_request = $method . ' ' . $uri->as_string . "\n\n"; + + my $req = do { + if ($method eq 'GET') { + $uri->query_form( $params ); + GET $uri->as_string; + } elsif ($method eq 'POST') { + POST $uri->as_string, $params; + } + }; - my $debug_request = "GET " . $base_uri->as_string . "\n\n"; $debug_request .= $self->_params_to_string($params, $debug_request); $self->debug_details( $self->debug_details . $debug_request ); - my $content; - if ( $self->test_mode ) { + if ( $self->test_mode && $req->method eq 'GET') { $self->success(1); - $content = $self->test_get_returns->{ $path }; $self->test_uri_used( $uri->as_string ); - } else { - my $ua = LWP::UserAgent->new; - - my $req = HTTP::Request->new( - GET => $uri->as_string - ); - - my $res = $ua->request( $req ); + return $self->test_get_returns->{ $path }; + } - if ( $res->is_success ) { - $content = $res->decoded_content; - $self->success(1); + my $res = do { + if ( $self->test_mode ) { + $self->test_req_used( $req ); + $self->test_get_returns->{ $path }; } else { - $self->success(0); - $self->error( sprintf( - "request failed: %s\n%s\n", - $res->status_line, - $uri->as_string - ) ); + my $ua = LWP::UserAgent->new; + $ua->request( $req ); } - } - - return $content; -} - -sub _post { - my $self = shift; - my $path = shift; - my $params = shift; - - my $uri = URI->new( $self->endpoint ); - $uri->path( $uri->path . $path ); - - $params->{jurisdiction_id} = $self->jurisdiction - if $self->jurisdiction; - $params->{api_key} = ($self->api_key || '') - if $self->api_key; - my $req = POST $uri->as_string, $params; - - my $debug_request = $req->method . ' ' . $uri->as_string . "\n\n"; - $debug_request .= $self->_params_to_string($params, $debug_request); - $self->debug_details( $self->debug_details . $debug_request ); - - my $ua = LWP::UserAgent->new(); - my $res; - - if ( $self->test_mode ) { - $res = $self->test_get_returns->{ $path }; - $self->test_req_used( $req ); - } else { - $res = $ua->request( $req ); - } + }; if ( $res->is_success ) { $self->success(1); @@ -523,10 +492,20 @@ sub _post { $self->_process_error( $res->decoded_content ), $self->debug_details ) ); - return 0; + return; } } +sub _get { + my $self = shift; + return $self->_request(GET => @_); +} + +sub _post { + my $self = shift; + return $self->_request(POST => @_); +} + sub _process_error { my $self = shift; my $error = shift; |