diff --git a/pygeoapi/provider/base.py b/pygeoapi/provider/base.py index 42cb15b..4d8cc65 100644 --- a/pygeoapi/provider/base.py +++ b/pygeoapi/provider/base.py @@ -65,5 +65,28 @@ class BaseProvider(object): raise NotImplementedError() + def create(self, new_feature): + """Create a new feature + """ + + raise NotImplementedError() + + def update(self, identifier, new_feature): + """Updates an existing feature id with new_feature + + :param identifier: feature id + :param new_feature: new GeoJSON feature dictionary + """ + + raise NotImplementedError() + + def delete(self, identifier): + """Updates an existing feature id with new_feature + + :param identifier: feature id + """ + + raise NotImplementedError() + def __repr__(self): return ' {}'.format(self.type) diff --git a/pygeoapi/provider/geojson.py b/pygeoapi/provider/geojson.py index b928c6b..3594193 100644 --- a/pygeoapi/provider/geojson.py +++ b/pygeoapi/provider/geojson.py @@ -29,6 +29,7 @@ import os import json +import uuid from pygeoapi.provider.base import BaseProvider @@ -37,56 +38,71 @@ class GeoJSONProvider(BaseProvider): """Provider class backed by local GeoJSON files This is meant to be simple - (no external services, no schema) + (no external services, no dependencies, no schema) at the expense of performance (no indexing, full serialization roundtrip on each request) + + Not thread safe, a single server process is assumed + + This implementation uses the feature 'id' heavily + and will override any 'id' provided in the original data. + The feature 'properties' will be preserved. + + TODO + - query method should take bbox + - instead of methods returning FeatureCollections, + we should be yielding Features and aggregating in the view + - there are strict id semantics; all features in the input GeoJSON file + must be present and be unique strings. Otherwise it will break. + - How to raise errors in the provider implementation such that + appropriate HTTP responses will be raised """ def __init__(self, definition): """initializer""" - BaseProvider.__init__(self, definition) + self.path = self.url.replace("file://", '') - # url is a file path, TODO use urlparse or support local paths - self._validate_or_create(self.url) + def _load(self): + """Load and validate the source GeoJSON file + at self.path - @classmethod - def _validate_or_create(self, path): - """Validate that the path exists and that - it is a geojson feature collection + Yes loading from disk, deserializing and validation + happens on every request. This is not efficient. """ - if os.path.exists(path): - with open(path) as src: + if os.path.exists(self.path): + with open(self.path) as src: data = json.loads(src.read()) - assert data['type'] == 'FeatureCollection' else: - with open(path, 'w') as dst: - empty = { - 'type': 'FeatureCollection', - 'features': []} - dst.write(json.dumps(empty)) + data = { + 'type': 'FeatureCollection', + 'features': []} - def _load(self, startindex=0, count=10, resulttype='results'): - with open(self.url) as src: - data = json.loads(src.read()) + # Must be a FeatureCollection + assert data['type'] == 'FeatureCollection' + # All features must have ids, TODO must be unique strings + assert all(f.get('id') for f in data['features']) - if resulttype == 'hits': - data['numberMatched'] = len(data['features']) - data['features'] = [] - else: - data['features'] = data['features'][startindex:startindex + count] return data def query(self, startindex=0, count=10, resulttype='results'): """ query the provider + :param bbox: Bounding Box in [W, S, E, N] order :returns: FeatureCollection dict of 0..n GeoJSON features - - TODO yield GeoJSON features? """ - return self._load(startindex, count, resulttype) + # TODO filter by bbox without resorting to third-party libs + data = self._load() + + if resulttype == 'hits': + data['numberMatched'] = len(data['features']) + data['features'] = [] + else: + data['features'] = data['features'][startindex:startindex + count] + + return data def get(self, identifier): """ @@ -95,30 +111,61 @@ class GeoJSONProvider(BaseProvider): :param identifier: feature id :returns: dict of single GeoJSON feature """ - collection = { - 'type': 'FeatureCollection', - 'features': []} + all_data = self._load() + for feature in all_data['features']: + if feature['id'] == identifier: + return { + 'type': 'FeatureCollection', + 'features': [feature]} + # default, no match + raise RuntimeError("Should be a 404 error") + + def create(self, new_feature): + """Create a new feature + + :param new_feature: new GeoJSON feature dictionary + """ all_data = self._load() - if self.id_field: - # Use id field - for feature in all_data['features']: - if feature[self.id_field] == identifier: - collection['features'].append(feature) - else: - # Use enumeration, zero-indexed - for i, feature in enumerate(all_data['features']): - # TODO assumes identifier is always a string - if str(i) == identifier: - collection['features'].append(feature) + # Hijack the feature id and make sure it's unique + new_feature['id'] = str(uuid.uuid4()) - # assert that one and only one feature returned - n_features = len(collection['features']) - if n_features != 1: - raise RuntimeError('Expected 1 feature, got {}'.format(n_features)) + all_data['features'].append(new_feature) - return collection + with open(self.path, 'w') as dst: + dst.write(json.dumps(all_data)) + + def update(self, identifier, new_feature): + """Updates an existing feature id with new_feature + + :param identifier: feature id + :param new_feature: new GeoJSON feature dictionary + """ + all_data = self._load() + for i, feature in enumerate(all_data['features']): + if feature['id'] == identifier: + # ensure new_feature retains id + new_feature['id'] = identifier + all_data['features'][i] = new_feature + break + + with open(self.path, 'w') as dst: + dst.write(json.dumps(all_data)) + + def delete(self, identifier): + """Updates an existing feature id with new_feature + + :param identifier: feature id + """ + all_data = self._load() + for i, feature in enumerate(all_data['features']): + if feature['id'] == identifier: + all_data['features'].pop(i) + break + + with open(self.path, 'w') as dst: + dst.write(json.dumps(all_data)) def __repr__(self): return ' {}'.format(self.url) diff --git a/tests/test_geojson_provider.py b/tests/test_geojson_provider.py new file mode 100644 index 0000000..42b66cc --- /dev/null +++ b/tests/test_geojson_provider.py @@ -0,0 +1,128 @@ +import json +import pytest + +from pygeoapi.provider.geojson import GeoJSONProvider + + +path = '/tmp/test.geojson' + + +@pytest.fixture() +def fixture(): + data = { + 'type': 'FeatureCollection', + 'features': [{ + 'type': 'Feature', + 'id': '123-456', + 'geometry': { + 'type': 'Point', + 'coordinates': [125.6, 10.1]}, + 'properties': { + 'name': 'Dinagat Islands'}}]} + + with open(path, 'w') as fh: + fh.write(json.dumps(data)) + return path + + +@pytest.fixture() +def config(): + return { + 'type': 'GeoJSON', + 'url': 'file://' + path, + 'id_field': None} + + +def test_query(fixture, config): + p = GeoJSONProvider(config) + results = p.query() + assert len(results['features']) == 1 + assert results['features'][0]['id'] == '123-456' + + +def test_get(fixture, config): + p = GeoJSONProvider(config) + results = p.get('123-456') + assert len(results['features']) == 1 + assert 'Dinagat' in results['features'][0]['properties']['name'] + + +def test_delete(fixture, config): + p = GeoJSONProvider(config) + p.delete('123-456') + + results = p.query() + assert len(results['features']) == 0 + + +def test_create(fixture, config): + p = GeoJSONProvider(config) + new_feature = { + 'type': 'Feature', + 'id': '123-456', + 'geometry': { + 'type': 'Point', + 'coordinates': [0.0, 0.0]}, + 'properties': { + 'name': 'Null Island'}} + + p.create(new_feature) + + results = p._load() + assert len(results['features']) == 2 + assert 'Dinagat' in results['features'][0]['properties']['name'] + assert 'Null' in results['features'][1]['properties']['name'] + + +def test_update(fixture, config): + p = GeoJSONProvider(config) + new_feature = { + 'type': 'Feature', + 'id': '123-456', + 'geometry': { + 'type': 'Point', + 'coordinates': [0.0, 0.0]}, + 'properties': { + 'name': 'Null Island'}} + + p.update('123-456', new_feature) + + # Should be changed + results = p.get('123-456') + assert 'Null' in results['features'][0]['properties']['name'] + + +def test_update_safe_id(fixture, config): + p = GeoJSONProvider(config) + new_feature = { + 'type': 'Feature', + 'id': 'SOMETHING DIFFERENT', + 'geometry': { + 'type': 'Point', + 'coordinates': [0.0, 0.0]}, + 'properties': { + 'name': 'Null Island'}} + + p.update('123-456', new_feature) + + # Don't let the id change, should not exist + with pytest.raises(Exception): + p.get('SOMETHING DIFFERENT') + + # Should still be at the old id + results = p.get('123-456') + assert 'Null' in results['features'][0]['properties']['name'] + + + +""" + def __init__(self, definition): + BaseProvider.__init__(self, definition) + def _load(self): + def query(self): + def get(self, identifier): + def create(self, new_feature): + def update(self, identifier, new_feature): + def delete(self, identifier): + def __repr__(self): +"""