aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md1
-rw-r--r--perllib/Open311.pm103
2 files changed, 42 insertions, 62 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0b4c58073..e871a55da 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -41,6 +41,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;