Skip to content

Commit

Permalink
[Buckinghamshire] Fix WFS lookups to use POST.
Browse files Browse the repository at this point in the history 
The server no longer accepts GET requests with a filter.
  • Loading branch information
@dracos
dracos committed Dec 5, 2024
1 parent acb92bb commit c2e6c00
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 58 deletions.
82 changes: 29 additions & 53 deletions perllib/FixMyStreet/Cobrand/Buckinghamshire.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ with 'FixMyStreet::Roles::BoroughEmails';
use SUPER;


use LWP::Simple;
use LWP::UserAgent;
use URI;
use Try::Tiny;
use Utils;
Expand Down Expand Up @@ -736,48 +736,18 @@ around 'munge_sendreport_params' => sub {

sub car_park_wfs_query {
my ($self, $row) = @_;

my $uri = URI->new("https://maps.buckinghamshire.gov.uk/server/services/Transport/Car_Parks/MapServer/WFSServer");
$uri->query_form(
REQUEST => "GetFeature",
SERVICE => "WFS",
SRSNAME => "urn:ogc:def:crs:EPSG::27700",
TYPENAME => "BC_CAR_PARKS",
VERSION => "1.1.0",
propertyName => 'OBJECTID,Shape',
);

try {
return $self->_get($self->_wfs_uri($row, $uri));
} catch {
# Ignore WFS errors.
return {};
};
my $uri = "https://maps.buckinghamshire.gov.uk/server/services/Transport/Car_Parks/MapServer/WFSServer";
return $self->_wfs_post($uri, $row, 'BC_CAR_PARKS', ['OBJECTID', 'Shape']);
}

sub speed_limit_wfs_query {
my ($self, $row) = @_;

my $uri = URI->new("https://maps.buckinghamshire.gov.uk/server/services/Transport/OS_Highways_Speed/MapServer/WFSServer");
$uri->query_form(
REQUEST => "GetFeature",
SERVICE => "WFS",
SRSNAME => "urn:ogc:def:crs:EPSG::27700",
TYPENAME => "OS_Highways_Speed:OS_Highways_Speed",
VERSION => "1.1.0",
propertyName => 'OBJECTID,Shape,speed',
);

try {
return $self->_get($self->_wfs_uri($row, $uri));
} catch {
# Ignore WFS errors.
return {};
};
my $uri = "https://maps.buckinghamshire.gov.uk/server/services/Transport/OS_Highways_Speed/MapServer/WFSServer";
return $self->_wfs_post($uri, $row, 'OS_Highways_Speed:OS_Highways_Speed', ['OBJECTID', 'Shape', 'speed']);
}

sub _wfs_uri {
my ($self, $row, $base_uri) = @_;
sub _wfs_post {
my ($self, $uri, $row, $typename, $properties) = @_;

# This fn may be called before cobrand has been set in the
# reporting flow and local_coords needs it to be set
Expand All @@ -787,33 +757,39 @@ sub _wfs_uri {
my $buffer = 50; # metres
my ($w, $s, $e, $n) = ($x-$buffer, $y-$buffer, $x+$buffer, $y+$buffer);

my $filter = "
<ogc:Filter xmlns:ogc=\"http://www.opengis.net/ogc\">
$properties = map { "<wfs:PropertyName>$_</wfs:PropertyName>" } @$properties;
my $data = <<EOF;
<wfs:GetFeature service="WFS" version="1.1.0" xmlns:wfs="http://www.opengis.net/wfs">
<wfs:Query typeName="$typename">
$properties
<ogc:Filter xmlns:ogc="http://www.opengis.net/ogc">
<ogc:BBOX>
<ogc:PropertyName>Shape</ogc:PropertyName>
<gml:Envelope xmlns:gml='http://www.opengis.net/gml' srsName='EPSG:27700'>
<gml:Envelope xmlns:gml="http://www.opengis.net/gml" srsName="EPSG:27700">
<gml:lowerCorner>$w $s</gml:lowerCorner>
<gml:upperCorner>$e $n</gml:upperCorner>
</gml:Envelope>
</ogc:BBOX>
</ogc:Filter>";
$filter =~ s/\n\s+//g;
</ogc:Filter>
</wfs:Query>
</wfs:GetFeature>
EOF

# URI encodes ' ' as '+' but arcgis wants it to be '%20'
# Putting %20 into the filter string doesn't work because URI then escapes
# the '%' as '%25' so you get a double encoding issue.
#
# Avoid all of that and just put the filter on the end of the $base_uri
$filter = URI::Escape::uri_escape_utf8($filter);

return "$base_uri&filter=$filter";
try {
return $self->_post($uri, $data);
} catch {
# Ignore WFS errors.
return {};

Check warning on line 782 in perllib/FixMyStreet/Cobrand/Buckinghamshire.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/Buckinghamshire.pm#L782 

Added line #L782 was not covered by tests
};
}

# Wrapper around LWP::Simple::get to make mocking in tests easier.
sub _get {
my ($self, $uri) = @_;
sub _post {
my ($self, $uri, $data) = @_;

Check warning on line 788 in perllib/FixMyStreet/Cobrand/Buckinghamshire.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/Buckinghamshire.pm#L788 

Added line #L788 was not covered by tests

get($uri);
my $ua = LWP::UserAgent->new;
my $res = $ua->post($uri, Content_Type => "text/xml", Content => $data);
return $res->decoded_content;

Check warning on line 792 in perllib/FixMyStreet/Cobrand/Buckinghamshire.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/Buckinghamshire.pm#L790-L792 

Added lines #L790 - L792 were not covered by tests
}

around 'report_validation' => sub {
Expand Down
10 changes: 5 additions & 5 deletions t/cobrand/bucks.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ my $bucks = Test::MockModule->new('FixMyStreet::Cobrand::Buckinghamshire');

subtest 'Prevents car park reports being made outside a car park' => sub {
# Simulate no car parks found
$bucks->mock('_get', sub { "<wfs:FeatureCollection></wfs:FeatureCollection>" });
$bucks->mock('_post', sub { "<wfs:FeatureCollection></wfs:FeatureCollection>" });

$mech->get_ok('/report/new?latitude=51.615559&longitude=-0.556903&category=Barrier+problem');
$mech->submit_form_ok({
Expand All @@ -419,7 +419,7 @@ subtest 'Prevents car park reports being made outside a car park' => sub {

subtest 'Allows car park reports to be made in a car park' => sub {
# Now simulate a car park being found
$bucks->mock('_get', sub {
$bucks->mock('_post', sub {
"<wfs:FeatureCollection>
<gml:featureMember>
<Transport_BC_Car_Parks:BC_CAR_PARKS>
Expand Down Expand Up @@ -517,7 +517,7 @@ subtest 'sends grass cutting reports on roads 30mph or more to the council' => s
};

subtest "server side speed limit lookup for council grass cutting report" => sub {
$bucks->mock('_get', sub { "<OS_Highways_Speed:speed>60.00000000</OS_Highways_Speed:speed>" });
$bucks->mock('_post', sub { "<OS_Highways_Speed:speed>60.00000000</OS_Highways_Speed:speed>" });

$mech->get_ok('/report/new?latitude=51.615559&longitude=-0.556903&category=Grass+cutting');
$mech->submit_form_ok({
Expand All @@ -536,7 +536,7 @@ subtest "server side speed limit lookup for council grass cutting report" => sub
};

subtest "server side speed limit lookup for parish grass cutting report" => sub {
$bucks->mock('_get', sub { "<OS_Highways_Speed:speed>30.00000000</OS_Highways_Speed:speed>" });
$bucks->mock('_post', sub { "<OS_Highways_Speed:speed>30.00000000</OS_Highways_Speed:speed>" });

$mech->get_ok('/report/new?latitude=51.615559&longitude=-0.556903&category=Grass+cutting');
$mech->submit_form_ok({
Expand All @@ -554,7 +554,7 @@ subtest "server side speed limit lookup for parish grass cutting report" => sub
};

subtest "server side speed limit lookup with unknown speed limit" => sub {
$bucks->mock('_get', sub { '' });
$bucks->mock('_post', sub { '' });

$mech->get_ok('/report/new?latitude=51.615559&longitude=-0.556903&category=Grass+cutting');
$mech->submit_form_ok({
Expand Down

0 comments on commit c2e6c00

Please sign in to comment.