From ddc77fc15aa8a64fb30d966474dd0965ac7be0b9 Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Sun, 19 Jan 2020 12:45:09 +0100
Subject: [PATCH 2/2] opengis: handle XML format for WFS features too (#38865)

---
 passerelle/apps/opengis/models.py | 102 +++++++++++++++++++------
 tests/test_opengis.py             | 122 ++++++++++++++++++++++++------
 2 files changed, 176 insertions(+), 48 deletions(-)

diff --git a/passerelle/apps/opengis/models.py b/passerelle/apps/opengis/models.py
index 19f0dd54..b315b351 100644
--- a/passerelle/apps/opengis/models.py
+++ b/passerelle/apps/opengis/models.py
@@ -45,6 +45,30 @@ def build_dict_from_xml(elem):
             d[attribute_name] = build_dict_from_xml(child)
     return d
 
+def parse_xml_features(element):
+    features = []
+    ns = {'gml': 'http://www.opengis.net/gml'}
+    for featureMember in element.findall(".//gml:featureMember", ns):
+        feature = {}
+        properties = {}
+        for elem in featureMember[0]:
+            if (elem.tag.endswith('geo_shape')
+                and elem[0].tag == '{%s}Point' % ns['gml']
+                and elem[0][0].tag == '{%s}pos' % ns['gml']):
+                    coordinates = [float(x) for x in elem[0][0].text.split(' ')]
+                    feature['geometry'] = {
+                        'coordinates': coordinates,
+                        'type': 'Point',
+                    }
+            elif elem.text != '':
+                index = elem.tag.find('}')
+                properties[elem.tag[index+1:]] = elem.text
+        feature['properties'] = properties
+        feature['type'] = "Feature"
+        features.append(feature)
+    return features
+
+
 PROJECTIONS = (
     ('EPSG:2154', _('EPSG:2154 (Lambert-93)')),
     ('EPSG:3857', _('EPSG:3857 (WGS 84 / Pseudo-Mercator)')),
@@ -72,35 +96,48 @@ class OpenGIS(BaseResource):
     class Meta:
         verbose_name = _('OpenGIS')
 
-    def get_service_version(self, service_type, service_url, renew=False):
+    def get_service_options(self, service_type, service_url, renew=False):
         if not service_url:
             raise APIError('no %s URL declared' % service_type)
-        cache_key = 'opengis-%s-%s-version' % (service_type, self.id)
+        cache_key = 'opengis-%s-%s-options' % (service_type, self.id)
         if not renew:
-            service_version = cache.get(cache_key)
-            if service_version:
-                return service_version
+            service_options = cache.get(cache_key)
+            if service_options:
+                return service_options
         response = self.requests.get(service_url, params={'request': 'GetCapabilities'})
         element = ET.fromstring(response.content)
         service_version = element.attrib.get('version')
-        # cache version number for an hour
-        cache.set(cache_key, service_version, 3600)
-        return service_version
+        service_outputformat = None
+        if service_type == 'wfs':
+            service_outputformat = 'xml'
+            ns = {'ows': 'http://www.opengis.net/ows/1.1'}
+            op = element.find(".//ows:Operation[@name='GetFeature']", ns)
+            if op is not None:
+                values = [x.text for x in op.iterfind(".//ows:Value", ns)]
+                if values and len([x for x in values if 'json' in x]) > 0:
+                    service_outputformat = 'json'
+        service_options = (service_version, service_outputformat)
+        # cache version number and outputFormat for an hour
+        cache.set(cache_key, service_options, 3600)
+        return service_options
 
     def get_wms_service_version(self, renew=False):
-        return self.get_service_version('wms', self.wms_service_url, renew=renew)
+        return self.get_service_options('wms', self.wms_service_url, renew=renew)[0]
 
     def get_wfs_service_version(self, renew=False):
-        return self.get_service_version('wfs', self.wfs_service_url, renew=renew)
+        return self.get_service_options('wfs', self.wfs_service_url, renew=renew)[0]
 
-    def get_typename_label(self):
-        version_str = self.get_wfs_service_version()
+    def get_wfs_typename_label(self):
+        version_str = self.get_wfs_service_version()[0]
         version_tuple = tuple(int(x) for x in version_str.split('.'))
         if version_tuple <= (1, 1, 0):
             return 'TYPENAME'
         else:
             return 'TYPENAMES'
 
+    def get_wfs_outputformat(self):
+        return self.get_service_options('wfs', self.wfs_service_url)[1]
+
     def check_status(self):
         if self.wms_service_url:
             response = self.requests.get(
@@ -148,13 +185,14 @@ class OpenGIS(BaseResource):
               })
     def features(self, request, type_names, property_name, cql_filter=None,
                  filter_property_name=None, q=None, **kwargs):
+        output_format = self.get_wfs_outputformat()
         params = {
             'VERSION': self.get_wfs_service_version(),
             'SERVICE': 'WFS',
             'REQUEST': 'GetFeature',
-            self.get_typename_label(): type_names,
+            self.get_wfs_typename_label(): type_names,
             'PROPERTYNAME': property_name,
-            'OUTPUTFORMAT': 'json',
+            'OUTPUTFORMAT': output_format,
         }
         if cql_filter:
             params.update({'CQL_FILTER': cql_filter})
@@ -165,15 +203,23 @@ class OpenGIS(BaseResource):
                     operator = 'LIKE'
                 params['CQL_FILTER'] += ' AND %s %s \'%%%s%%\'' % (filter_property_name, operator, q)
         response = self.requests.get(self.wfs_service_url, params=params)
+        self.handle_opengis_error(response)
+        # continue if handle_opengis_error did not raise an error
+        if output_format == 'json':
+            try:
+                features = response.json()['features']
+            except ValueError:
+                raise APIError(u'OpenGIS Error: unparsable error',
+                               data={'content': repr(response.content[:1024])})
+        else:
+            try:
+                element = ET.fromstring(response.content.encode(encoding='UTF-8'))
+            except ET.ParseError as exception:
+                raise APIError(u'OpenGIS Error: %s' % exception,
+                               data={'content': repr(response.content[:1024])})
+            features = parse_xml_features(element)
         data = []
-        try:
-            response.json()
-        except ValueError:
-            self.handle_opengis_error(response)
-            # if handle_opengis_error did not raise an error, we raise a generic one
-            raise APIError(u'OpenGIS Error: unparsable error',
-                           data={'content': repr(response.content[:1024])})
-        for feature in response.json()['features']:
+        for feature in features:
             feature['text'] = feature['properties'].get(property_name)
             data.append(feature)
         return {'data': data}
@@ -301,20 +347,26 @@ class OpenGIS(BaseResource):
         lon, lat = self.convert_coordinates(lon, lat)
 
         cql_filter = 'DWITHIN(the_geom,Point(%s %s),%s,meters)' % (lon, lat, self.search_radius)
+        output_format = self.get_wfs_outputformat()
         params = {
             'VERSION': self.get_wfs_service_version(),
             'SERVICE': 'WFS',
             'REQUEST': 'GetFeature',
-            self.get_typename_label(): self.query_layer,
-            'OUTPUTFORMAT': 'json',
+            self.get_wfs_typename_label(): self.query_layer,
+            'OUTPUTFORMAT': output_format,
             'CQL_FILTER': cql_filter
         }
         response = self.requests.get(self.wfs_service_url, params=params)
         if not response.ok:
             raise APIError('Webservice returned status code %s' % response.status_code)
+        if output_format == 'json':
+            features = response.json().get('features')
+        else:
+            element = ET.fromstring(response.text.encode(encoding='UTF-8'))
+            features = parse_xml_features(element)
         closest_feature = {}
         min_delta = None
-        for feature in response.json().get('features'):
+        for feature in features:
             if not feature['geometry']['type'] == 'Point':
                 continue  # skip unknown
             lon_diff = abs(float(lon) - float(feature['geometry']['coordinates'][0]))
diff --git a/tests/test_opengis.py b/tests/test_opengis.py
index 71354356..576ef6af 100644
--- a/tests/test_opengis.py
+++ b/tests/test_opengis.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 import mock
 import pytest
 
@@ -39,6 +40,19 @@ FAKE_SERVICE_CAPABILITIES = '''<?xml version="1.0" encoding="UTF-8"?>
         <ows:ServiceType>WFS</ows:ServiceType><ows:ServiceTypeVersion>2.0.0</ows:ServiceTypeVersion><ows:Fees/>
         <ows:AccessConstraints/>
     </ows:ServiceIdentification>
+    <ows:ServiceProvider/>
+    <ows:OperationsMetadata>
+      <ows:Operation name="GetFeature">
+        <ows:DCP/>
+        <ows:Parameter name="outputFormat">
+          <ows:AllowedValues>
+            <ows:Value>text/xml; subtype=gml/3.2</ows:Value>
+            <ows:Value>application/json</ows:Value>
+            <ows:Value>json</ows:Value>
+          </ows:AllowedValues>
+        </ows:Parameter>
+      </ows:Operation>
+    </ows:OperationsMetadata>
 </wfs:WFS_Capabilities>'''
 
 FAKE_SERVICE_CAPABILITIES_V1_0_0 = '''<?xml version="1.0" encoding="UTF-8"?>
@@ -69,7 +83,7 @@ FAKE_FEATURES_JSON = '''
             "properties": {
                 "nom": "Champagnier"
             },
-"type": "Feature"
+            "type": "Feature"
         },
         {
             "geometry": null,
@@ -115,6 +129,25 @@ FAKE_FEATURES_JSON = '''
     "type": "FeatureCollection"
 }'''
 
+FAKE_FEATURES_XML = u'''<?xml version="1.0" encoding="UTF-8"?>
+<wfs:FeatureCollection xmlns:gml="http://www.opengis.net/gml"
+xmlns:wfs="http://www.opengis.net/wfs" xmlns:xxx="http://www.somewhere.net/xxx">
+  <gml:boundedBy/>
+  <gml:featureMember>
+    <xxx:points_apport_volontaire_dmt gml:id="points_apport_volontaire_dmt.e097e421cc609bcc752c43cd4e4c9d992cb0ee3d">
+      <xxx:geo_shape>
+        <gml:Point srsName="urn:x-ogc:def:crs:EPSG:4326">
+          <gml:pos>1914018.64 4224644.61</gml:pos>
+        </gml:Point>
+      </xxx:geo_shape>
+      <numero>4</numero>
+      <xxx:nom_voie>place victor hugo</xxx:nom_voie>
+      <xxx:code_post>38000</xxx:code_post>
+      <xxx:nom_commune>Grenoble</xxx:nom_commune>
+    </xxx:points_apport_volontaire_dmt>
+  </gml:featureMember>
+</wfs:FeatureCollection>'''
+
 FAKE_ERROR = u'''<ows:ExceptionReport
      xmlns:xs="http://www.w3.org/2001/XMLSchema"
      xmlns:ows="http://www.opengis.net/ows/1.1"
@@ -241,19 +274,30 @@ def geoserver_responses(url, **kwargs):
 def geoserver_responses_v1_0_0(url, **kwargs):
     if kwargs['params'].get('request') == 'GetCapabilities':
         return utils.FakedResponse(status_code=200, content=FAKE_SERVICE_CAPABILITIES_V1_0_0)
-    return utils.FakedResponse(status_code=200, content=FAKE_FEATURES_JSON)
+    return utils.FakedResponse(status_code=200, content=FAKE_FEATURES_XML)
+
 
 def geoserver_responses_errors(url, **kwargs):
     if kwargs['params'].get('request') == 'GetCapabilities':
         return utils.FakedResponse(status_code=200, content=FAKE_SERVICE_CAPABILITIES)
     return utils.FakedResponse(status_code=200, content=FAKE_ERROR)
 
+def geoserver_responses_errors_v1_0_0(url, **kwargs):
+    if kwargs['params'].get('request') == 'GetCapabilities':
+        return utils.FakedResponse(status_code=200, content=FAKE_SERVICE_CAPABILITIES_V1_0_0)
+    return utils.FakedResponse(status_code=200, content=FAKE_ERROR)
+
 
 def geoserver_responses_errors_unparsable(url, **kwargs):
     if kwargs['params'].get('request') == 'GetCapabilities':
         return utils.FakedResponse(status_code=200, content=FAKE_SERVICE_CAPABILITIES)
     return utils.FakedResponse(status_code=200, content=FAKE_ERROR[:10])
 
+def geoserver_responses_errors_unparsable_v1_0_0(url, **kwargs):
+    if kwargs['params'].get('request') == 'GetCapabilities':
+        return utils.FakedResponse(status_code=200, content=FAKE_SERVICE_CAPABILITIES_V1_0_0)
+    return utils.FakedResponse(status_code=200, content=FAKE_ERROR[:10])
+
 
 @mock.patch('passerelle.utils.Request.get')
 def test_feature_info(mocked_get, app, connector):
@@ -318,19 +362,23 @@ def test_get_feature_with_no_wfs_url(mocked_get, app, connector):
     assert resp.json['err_desc'] == 'no wfs URL declared'
 
 
+@pytest.mark.parametrize("server_responses, nb_res", [
+    (geoserver_responses, 7),
+    (geoserver_responses_v1_0_0, 1),
+])
 @mock.patch('passerelle.utils.Request.get')
-def test_get_feature(mocked_get, app, connector):
+def test_get_feature(mocked_get, server_responses, nb_res, app, connector):
     endpoint = utils.generic_endpoint_url('opengis', 'features', slug=connector.slug)
     assert endpoint == '/opengis/test/features'
-    mocked_get.side_effect = geoserver_responses
+    mocked_get.side_effect = server_responses
     resp = app.get(endpoint, params={'type_names': 'ref_metro_limites_communales', 'property_name': 'nom'})
     assert mocked_get.call_args[1]['params']['REQUEST'] == 'GetFeature'
     assert mocked_get.call_args[1]['params']['PROPERTYNAME'] == 'nom'
-    assert mocked_get.call_args[1]['params']['TYPENAMES'] == 'ref_metro_limites_communales'
-    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == 'json'
+    assert mocked_get.call_args[1]['params'][connector.get_wfs_typename_label()] == 'ref_metro_limites_communales'
+    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == connector.get_wfs_outputformat()
     assert mocked_get.call_args[1]['params']['SERVICE'] == 'WFS'
     assert mocked_get.call_args[1]['params']['VERSION'] == connector.get_wfs_service_version()
-    assert len(resp.json['data']) == 7
+    assert len(resp.json['data']) == nb_res
     for item in resp.json['data']:
         assert 'text' in item
 
@@ -368,19 +416,23 @@ def test_get_filtered_by_property_feature(mocked_get, app, connector):
     assert 'CQL_FILTER' not in mocked_get.call_args[1]['params']
 
 
+@pytest.mark.parametrize("server_responses", [
+    (geoserver_responses_errors),
+    (geoserver_responses_errors_v1_0_0),
+])
 @mock.patch('passerelle.utils.Request.get')
-def test_get_feature_error(mocked_get, app, connector):
+def test_get_feature_error(mocked_get, server_responses, app, connector):
     endpoint = utils.generic_endpoint_url('opengis', 'features', slug=connector.slug)
     assert endpoint == '/opengis/test/features'
-    mocked_get.side_effect = geoserver_responses_errors
+    mocked_get.side_effect = server_responses
     resp = app.get(endpoint, params={
         'type_names': 'ref_metro_limites_communales',
         'property_name': 'nom'
     })
     assert mocked_get.call_args[1]['params']['REQUEST'] == 'GetFeature'
     assert mocked_get.call_args[1]['params']['PROPERTYNAME'] == 'nom'
-    assert mocked_get.call_args[1]['params']['TYPENAMES'] == 'ref_metro_limites_communales'
-    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == 'json'
+    assert mocked_get.call_args[1]['params'][connector.get_wfs_typename_label()] == 'ref_metro_limites_communales'
+    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == connector.get_wfs_outputformat()
     assert mocked_get.call_args[1]['params']['SERVICE'] == 'WFS'
     assert mocked_get.call_args[1]['params']['VERSION'] == connector.get_wfs_service_version()
     result = resp.json
@@ -389,43 +441,67 @@ def test_get_feature_error(mocked_get, app, connector):
     assert 'Could not parse' in result['data']['text']
 
 
+@pytest.mark.parametrize("server_responses, error_msg", [
+    (geoserver_responses_errors_unparsable, 'unparsable error'),
+    (geoserver_responses_errors_unparsable_v1_0_0, 'unclosed token: line 1, column 0'),
+])
 @mock.patch('passerelle.utils.Request.get')
-def test_get_feature_error2(mocked_get, app, connector):
+def test_get_feature_error2(mocked_get, server_responses, error_msg, app, connector):
     endpoint = utils.generic_endpoint_url('opengis', 'features', slug=connector.slug)
     assert endpoint == '/opengis/test/features'
-    mocked_get.side_effect = geoserver_responses_errors_unparsable
+    mocked_get.side_effect = server_responses
     resp = app.get(endpoint, params={
         'type_names': 'ref_metro_limites_communales',
         'property_name': 'nom'
     })
     assert mocked_get.call_args[1]['params']['REQUEST'] == 'GetFeature'
     assert mocked_get.call_args[1]['params']['PROPERTYNAME'] == 'nom'
-    assert mocked_get.call_args[1]['params']['TYPENAMES'] == 'ref_metro_limites_communales'
-    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == 'json'
+    assert mocked_get.call_args[1]['params'][connector.get_wfs_typename_label()] == 'ref_metro_limites_communales'
+    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == connector.get_wfs_outputformat()
     assert mocked_get.call_args[1]['params']['SERVICE'] == 'WFS'
     assert mocked_get.call_args[1]['params']['VERSION'] == connector.get_wfs_service_version()
     result = resp.json
     assert result['err'] == 1
-    assert result['err_desc'] == 'OpenGIS Error: unparsable error'
+    assert result['err_desc'] == 'OpenGIS Error: %s' % error_msg
     assert '<ows:' in result['data']['content']
 
 
 @pytest.mark.parametrize("server_responses, version, typename_label", [
+    (geoserver_responses, '2.0.0', 'TYPENAMES'),
     (geoserver_responses_v1_0_0, '1.0.0', 'TYPENAME'),
-    (geoserver_responses, '2.0.0', 'TYPENAMES')])
+])
 @mock.patch('passerelle.utils.Request.get')
 def test_typename_parameter_upgrade(mocked_get, server_responses, version, typename_label, app, connector):
     endpoint = utils.generic_endpoint_url('opengis', 'features', slug=connector.slug)
     assert endpoint == '/opengis/test/features'
     mocked_get.side_effect = server_responses
-    resp = app.get(endpoint, params={'type_names': '...', 'property_name': '...'})
+    resp = app.get(endpoint, params={'type_names': 'ref_metro_limites_communales', 'property_name': 'nom'})
     assert mocked_get.call_args[1]['params']['REQUEST'] == 'GetFeature'
     assert mocked_get.call_args[1]['params']['VERSION'] == version
     assert typename_label in mocked_get.call_args[1]['params'].keys()
+    assert mocked_get.call_args[1]['params'][connector.get_wfs_typename_label()] == 'ref_metro_limites_communales'
+
+
+@pytest.mark.parametrize("server_responses, service_format", [
+    (geoserver_responses, 'json'),
+    (geoserver_responses_v1_0_0, 'xml'),
+])
+@mock.patch('passerelle.utils.Request.get')
+def test_outputFormat_possible_values(mocked_get, server_responses, service_format, app, connector):
+    endpoint = utils.generic_endpoint_url('opengis', 'features', slug=connector.slug)
+    assert endpoint == '/opengis/test/features'
+    mocked_get.side_effect = server_responses
+    resp = app.get(endpoint, params={'type_names': '...', 'property_name': '...'})
+    assert mocked_get.call_args[1]['params']['REQUEST'] == 'GetFeature'
+    assert mocked_get.call_args[1]['params']['OUTPUTFORMAT'] == service_format
 
 
+@pytest.mark.parametrize("resp1, resp2, resp3, resp4", [
+    (FAKE_SERVICE_CAPABILITIES, FAKE_GEOLOCATED_FEATURE, '{"features": []}', '{}'),
+    (FAKE_SERVICE_CAPABILITIES_V1_0_0, FAKE_FEATURES_XML, '<?xml version="1.0"?><empty/>', ''),
+])
 @mock.patch('passerelle.utils.Request.get')
-def test_reverse_geocoding(mocked_get, app, connector):
+def test_reverse_geocoding(mocked_get, resp1, resp2, resp3, resp4, app, connector):
     connector.search_radius = 45
     connector.projection = 'EPSG:3945'
     connector.save()
@@ -434,10 +510,10 @@ def test_reverse_geocoding(mocked_get, app, connector):
 
     def side_effect(url, **kwargs):
         if kwargs['params'].get('request') == 'GetCapabilities':
-            return utils.FakedResponse(status_code=200, content=FAKE_SERVICE_CAPABILITIES)
+            return utils.FakedResponse(status_code=200, content=resp1)
         return mock.DEFAULT
     mocked_get.side_effect = side_effect
-    mocked_get.return_value = utils.FakedResponse(content=FAKE_GEOLOCATED_FEATURE, status_code=200)
+    mocked_get.return_value = utils.FakedResponse(content=resp2, status_code=200)
     resp = app.get(endpoint,
                    params={
                        'lat': '45.1893469606986',
@@ -455,7 +531,7 @@ def test_reverse_geocoding(mocked_get, app, connector):
     connector.projection = 'EPSG:4326'
     connector.search_radius = 10
     connector.save()
-    mocked_get.return_value = utils.FakedResponse(content='{"features": []}', status_code=200)
+    mocked_get.return_value = utils.FakedResponse(content=resp3, status_code=200)
     resp = app.get(
         endpoint,
         params={
@@ -466,7 +542,7 @@ def test_reverse_geocoding(mocked_get, app, connector):
     assert resp.json['err'] == 1
     assert resp.json['err_desc'] == 'Unable to geocode'
 
-    mocked_get.return_value = utils.FakedResponse(status_code=404, content='{}', ok=False)
+    mocked_get.return_value = utils.FakedResponse(status_code=404, content=resp4, ok=False)
     resp = app.get(endpoint,
                    params={
                        'lat': '45.183784',
-- 
2.24.1

