From 560fcc9fb194fef3e03fe588ee5c653a6a2a712a Mon Sep 17 00:00:00 2001 From: Francesco Bartoli Date: Fri, 10 Apr 2020 21:23:53 +0200 Subject: [PATCH] Fix issue 390 (#391) * Add decorated utility function to ignore gdal errors * Replace push and pop error handler with decorated function * Add docstrings * Add test for OGR esrijson driver * Make functions private * Fix flake8 Co-authored-by: Francesco Bartoli --- pygeoapi/provider/ogr.py | 85 +++++++++++----- tests/test_ogr_esrijson_provider.py | 146 ++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 22 deletions(-) create mode 100644 tests/test_ogr_esrijson_provider.py diff --git a/pygeoapi/provider/ogr.py b/pygeoapi/provider/ogr.py index b73e846..a5bad03 100644 --- a/pygeoapi/provider/ogr.py +++ b/pygeoapi/provider/ogr.py @@ -31,6 +31,8 @@ import importlib import logging +import functools +from typing import Any from osgeo import gdal as osgeo_gdal from osgeo import ogr as osgeo_ogr @@ -186,12 +188,30 @@ class OGRProvider(BaseProvider): LOGGER.error(msg) raise Exception(msg) if self.open_options: - self.conn = self.gdal.OpenEx( - self.data_def['source'], - self.gdal.OF_VECTOR, - open_options=self._list_open_options()) + try: + self.conn = self.gdal.OpenEx( + self.data_def['source'], + self.gdal.OF_VECTOR, + open_options=self._list_open_options()) + except Exception: + msg = 'Ignore errors during the connection for Driver \ + {source_type}'.format(source_type) + LOGGER.error(msg) + self.conn = _ignore_gdal_error( + self.gdal, 'OpenEx', self.data_def['source'], + self.gdal.OF_VECTOR, + open_options=self._list_open_options()) else: - self.conn = self.driver.Open(self.data_def['source'], 0) + try: + self.conn = self.driver.Open(self.data_def['source'], 0) + except Exception: + msg = 'Ignore errors during the connection for Driver \ + {source_type}'.format(source_type) + LOGGER.error(msg) + # ignore errors for ESRIJSON not having geometry member + # see https://github.com/OSGeo/gdal/commit/38b0feed67f80ded32be6c508323d862e1a14474 # noqa + self.conn = _ignore_gdal_error( + self.driver, 'Open', self.data_def['source'], 0) if not self.conn: msg = 'Cannot open OGR Source: %s' % self.data_def['source'] LOGGER.error(msg) @@ -360,11 +380,8 @@ class OGRProvider(BaseProvider): def _get_next_feature(self, layer): try: - # Make gdal error handler silent - self.gdal.PushErrorHandler('CPLQuietErrorHandler') - next_feature = layer.GetNextFeature() - # Restore error handler - self.gdal.PopErrorHandler() + # Ignore gdal error + next_feature = _ignore_gdal_error(layer, 'GetNextFeature') if all(val is None for val in next_feature.items().values()): self.gdal.Error( self.gdal.CE_Failure, 1, "Object properties are all null" @@ -407,11 +424,8 @@ class OGRProvider(BaseProvider): layer.ResetReading() try: - # Make gdal error handler silent - self.gdal.PushErrorHandler('CPLQuietErrorHandler') - ogr_feature = layer.GetNextFeature() - # Restore error handler - self.gdal.PopErrorHandler() + # Ignore gdal error + ogr_feature = _ignore_gdal_error(layer, 'GetNextFeature') count = 0 while ogr_feature is not None: json_feature = self._ogr_feature_to_json(ogr_feature) @@ -422,11 +436,8 @@ class OGRProvider(BaseProvider): if count == limit: break - # Make gdal error handler silent - self.gdal.PushErrorHandler('CPLQuietErrorHandler') - ogr_feature = layer.GetNextFeature() - # Restore error handler - self.gdal.PopErrorHandler() + # Ignore gdal error + ogr_feature = _ignore_gdal_error(layer, 'GetNextFeature') return feature_collection except RuntimeError as gdalerr: @@ -485,7 +496,6 @@ class SourceHelper: Default action to get a Layer object from opened OGR Driver. :return: """ - layer = self.provider.conn.GetLayerByName(self.provider.layer_name) if not layer: @@ -716,5 +726,36 @@ class GdalErrorHandler: LOGGER.error('Error Number: %s, Type: %s, Msg: %s' % ( self.err_num, level, self.err_msg)) + last_error = osgeo_gdal.GetLastErrorMsg() if self.err_level >= osgeo_gdal.CE_Failure: - raise ProviderGenericError(osgeo_gdal.GetLastErrorMsg()) + raise ProviderGenericError(last_error) + + +def _silent_gdal_error(f): + """ + Decorator function for gdal + """ + @functools.wraps(f) + def wrapper(*args, **kwargs): + osgeo_gdal.PushErrorHandler('CPLQuietErrorHandler') + v = f(*args, **kwargs) + osgeo_gdal.PopErrorHandler() + return v + + return wrapper + + +@_silent_gdal_error +def _ignore_gdal_error(inst, fn, *args, **kwargs) -> Any: + """ + Evaluate the function with the object instance. + + :param inst: Object instance + :param fn: String function name + :param args: List of positional arguments + :param kwargs: Keyword arguments + + :returns: Any function evaluation result + """ + value = getattr(inst, fn)(*args, **kwargs) + return value diff --git a/tests/test_ogr_esrijson_provider.py b/tests/test_ogr_esrijson_provider.py new file mode 100644 index 0000000..777966b --- /dev/null +++ b/tests/test_ogr_esrijson_provider.py @@ -0,0 +1,146 @@ +# ================================================================= +# +# Authors: Francesco Bartoli +# +# Copyright (c) 2020 Francesco Bartoli +# +# Permission is hereby granted, free of charge, to any person +# obtaining a copy of this software and associated documentation +# files (the "Software"), to deal in the Software without +# restriction, including without limitation the rights to use, +# copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following +# conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +# OTHER DEALINGS IN THE SOFTWARE. +# +# ================================================================= + +# Needs to be run like: python3 -m pytest + +# https://sampleserver6.arcgisonline.com/arcgis/rest/services/CommunityAddressing/FeatureServer/0 + +import logging + +import pytest +from pygeoapi.provider.ogr import OGRProvider + +LOGGER = logging.getLogger(__name__) + + +@pytest.fixture() +def config_ArcGIS_ESRIJSON(): + return { + 'name': 'OGR', + 'data': { + 'source_type': 'ESRIJSON', + 'source': 'https://sampleserver6.arcgisonline.com/arcgis/rest/services/CommunityAddressing/FeatureServer/0/query?where=objectid+%3D+objectid&outfields=*&f=json', # noqa + 'source_srs': 'EPSG:4326', + 'target_srs': 'EPSG:4326', + 'source_capabilities': { + 'paging': True + }, + 'gdal_ogr_options': { + 'EMPTY_AS_NULL': 'NO', + 'GDAL_CACHEMAX': '64', + 'CPL_DEBUG': 'NO' + }, + }, + 'id_field': 'objectid', + 'layer': 'ESRIJSON' + } + + +def test_get_fields_agol(config_ArcGIS_ESRIJSON): + """Testing field types""" + + p = OGRProvider(config_ArcGIS_ESRIJSON) + results = p.get_fields() + assert results['fulladdr'] == 'string' + assert results['municipality'] == 'string' + + +def test_get_agol(config_ArcGIS_ESRIJSON): + """Testing query for a specific object""" + + p = OGRProvider(config_ArcGIS_ESRIJSON) + result = p.get('78232831') + assert result['id'] == 78232831 + assert '2605' in result['properties']['fulladdr'] + + +def test_query_hits_agol(config_ArcGIS_ESRIJSON): + """Testing query on entire collection for hits""" + + p = OGRProvider(config_ArcGIS_ESRIJSON) + feature_collection = p.query(resulttype='hits') + assert feature_collection.get('type', None) == 'FeatureCollection' + features = feature_collection.get('features', None) + assert len(features) == 0 + hits = feature_collection.get('numberMatched', None) + assert hits is not None + assert hits > 100 + + +def test_query_bbox_hits_agol(config_ArcGIS_ESRIJSON): + """Testing query for a valid JSON object with geometry""" + + p = OGRProvider(config_ArcGIS_ESRIJSON) + feature_collection = p.query( + bbox=[-9822165.181154, 5112669.004249, + -9807305.104750, 5133712.297986], + resulttype='hits') + assert feature_collection.get('type', None) == 'FeatureCollection' + features = feature_collection.get('features', None) + assert len(features) == 0 + hits = feature_collection.get('numberMatched', None) + assert hits is not None + print('hits={}'.format(hits)) + assert hits > 1 + + +def test_query_with_limit_agol(config_ArcGIS_ESRIJSON): + """Testing query for a valid JSON object with geometry""" + + p = OGRProvider(config_ArcGIS_ESRIJSON) + feature_collection = p.query(limit=2, resulttype='results') + assert feature_collection.get('type', None) == 'FeatureCollection' + features = feature_collection.get('features', None) + assert len(features) == 2 + hits = feature_collection.get('numberMatched', None) + assert hits is None + feature = features[0] + properties = feature.get('properties', None) + assert properties is not None + geometry = feature.get('geometry', None) + assert geometry is not None + + +def test_query_with_startindex(config_ArcGIS_ESRIJSON): + """Testing query for a valid JSON object with geometry""" + + p = OGRProvider(config_ArcGIS_ESRIJSON) + feature_collection = p.query(startindex=0, limit=5, resulttype='results') + assert feature_collection.get('type', None) == 'FeatureCollection' + features = feature_collection.get('features', None) + assert len(features) == 5 + hits = feature_collection.get('numberMatched', None) + assert hits is None + feature = features[0] + properties = feature.get('properties', None) + assert properties is not None + assert feature['id'] == 78232831 + assert '2605' in properties['fulladdr'] + geometry = feature.get('geometry', None) + assert geometry is not None