From 296ced4bd5cac27f71d96c853924b6ceb74a1490 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Fri, 6 Oct 2023 21:10:33 +0300 Subject: [PATCH 1/3] Handle non-iterable objects gracefully Signed-off-by: Kipchirchir Sigei --- onadata/libs/serializers/geojson_serializer.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/onadata/libs/serializers/geojson_serializer.py b/onadata/libs/serializers/geojson_serializer.py index a2608225a4..7e7e4dc3f6 100644 --- a/onadata/libs/serializers/geojson_serializer.py +++ b/onadata/libs/serializers/geojson_serializer.py @@ -141,7 +141,10 @@ def to_representation(self, instance): points = instance.json.get(geo_field) if geo_field in geotrace_xpaths or geo_field in polygon_xpaths: value = get_values_matching_key(instance.json, geo_field) - points = next(value) + try: + points = next(value) + except TypeError: + points = None geometry = ( geometry_from_string(points, simple_style) if points and isinstance(points, str) From 935f0862b24b88b04a3d83cd3da03318ef335b5c Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Wed, 11 Oct 2023 09:22:57 +0300 Subject: [PATCH 2/3] Handle empty geoshapes and geotraces in data Signed-off-by: Kipchirchir Sigei --- onadata/libs/serializers/geojson_serializer.py | 3 ++- onadata/libs/utils/dict_tools.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/onadata/libs/serializers/geojson_serializer.py b/onadata/libs/serializers/geojson_serializer.py index 7e7e4dc3f6..54fa9a7370 100644 --- a/onadata/libs/serializers/geojson_serializer.py +++ b/onadata/libs/serializers/geojson_serializer.py @@ -141,9 +141,10 @@ def to_representation(self, instance): points = instance.json.get(geo_field) if geo_field in geotrace_xpaths or geo_field in polygon_xpaths: value = get_values_matching_key(instance.json, geo_field) + # handle empty geoms try: points = next(value) - except TypeError: + except StopIteration: points = None geometry = ( geometry_from_string(points, simple_style) diff --git a/onadata/libs/utils/dict_tools.py b/onadata/libs/utils/dict_tools.py index fe796c6632..9a37cc931e 100644 --- a/onadata/libs/utils/dict_tools.py +++ b/onadata/libs/utils/dict_tools.py @@ -22,8 +22,14 @@ def _get_values(doc, key): yield item elif isinstance(v, list): for i in v: - for j in _get_values(i, key): - yield j + if isinstance(i, (dict, list)): + try: + for j in _get_values(i, key): + yield j + except StopIteration: + continue + elif i == key: + yield i return _get_values(doc, key) From 0fd6c37b7bd928d5d5a21b6c135bd467df0e8863 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Thu, 12 Oct 2023 09:56:19 +0300 Subject: [PATCH 3/3] Add tests Signed-off-by: Kipchirchir Sigei --- .../api/tests/viewsets/test_data_viewset.py | 112 ++++++++++++++++++ .../fixtures/geolocation/empty_geoshapes.csv | 3 + .../fixtures/geolocation/empty_geotraces.csv | 3 + onadata/libs/utils/dict_tools.py | 1 + 4 files changed, 119 insertions(+) create mode 100644 onadata/apps/main/tests/fixtures/geolocation/empty_geoshapes.csv create mode 100644 onadata/apps/main/tests/fixtures/geolocation/empty_geotraces.csv diff --git a/onadata/apps/api/tests/viewsets/test_data_viewset.py b/onadata/apps/api/tests/viewsets/test_data_viewset.py index dcdc50a310..bef0b1a0bf 100644 --- a/onadata/apps/api/tests/viewsets/test_data_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_data_viewset.py @@ -2504,6 +2504,118 @@ def test_geoshapes_in_repeats(self): } self.assertEqual(response.data, data) + def test_empty_geotraces_in_repeats(self): + # publish sample geotrace submissions + md = """ + | survey | + | | type | name | label | required | calculation | + | | begin repeat | segment | Waterway trace | | | + | | calculate | point_position | | | position(..)| + | | geotrace | blueline | GPS Coordinates | yes | | + | | end repeat | + """ + self.xform = self._publish_markdown( + md, self.user, self.project, id_string="geotraces" + ) + # publish submissions + self._publish_submit_geoms_in_repeats("empty_geotraces") + view = DataViewSet.as_view({"get": "list"}) + request = self.factory.get("/", **self.extra) + response = view(request, pk=self.xform.pk, format="geojson") + self.assertEqual(response.status_code, 200) + # get geojson from geo_field + data_get = {"geo_field": "segment/blueline"} + request = self.factory.get("/", data=data_get, **self.extra) + response = view(request, pk=self.xform.pk, format="geojson") + instances = self.xform.instances.all().order_by("id") + self.assertEqual(response.status_code, 200) + self.assertEqual(self.xform.instances.count(), 2) + self.assertEqual(len(response.data["features"]), 2) + self.assertEqual(self.xform.geotrace_xpaths(), ["segment/blueline"]) + # test LineString geojson format + data = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": None, + "properties": {"id": instances[0].pk, "xform": self.xform.pk}, + }, + { + "type": "Feature", + "geometry": { + "type": "LineString", + "coordinates": [ + [36.809057, -1.269392], + [36.803303, -1.271966], + [36.805943, -1.268118], + [36.808822, -1.269405], + ], + }, + "properties": {"id": instances[1].pk, "xform": self.xform.pk}, + }, + ], + } + self.assertEqual(response.data, data) + + def test_empty_geoshapes_in_repeats(self): + # publish sample geoshape submissions + md = """ + | survey | + | | type | name | label | required | calculation | + | | begin repeat | segment | Waterway trace | | | + | | calculate | point_position | | | position(..)| + | | geoshape | blueline | GPS Coordinates | yes | | + | | end repeat | + """ + self.xform = self._publish_markdown( + md, self.user, self.project, id_string="geoshapes" + ) + # publish submissions + self._publish_submit_geoms_in_repeats("empty_geoshapes") + view = DataViewSet.as_view({"get": "list"}) + request = self.factory.get("/", **self.extra) + response = view(request, pk=self.xform.pk, format="geojson") + self.assertEqual(response.status_code, 200) + # get geojson from specific field + data_get = {"geo_field": "segment/blueline"} + request = self.factory.get("/", data=data_get, **self.extra) + response = view(request, pk=self.xform.pk, format="geojson") + instances = self.xform.instances.all().order_by("id") + self.assertEqual(response.status_code, 200) + self.assertEqual(self.xform.instances.count(), 2) + self.assertEqual(len(response.data["features"]), 2) + self.assertEqual(self.xform.polygon_xpaths(), ["segment/blueline"]) + # test Polygon geojson format + data = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": None, + "properties": {"id": instances[0].pk, "xform": self.xform.pk}, + }, + { + "type": "Feature", + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [36.79198, -1.29728], + [36.785793, -1.298009], + [36.789744, -1.29961], + [36.790625, -1.300146], + [36.792107, -1.300897], + [36.79198, -1.29728], + ] + ], + }, + "properties": {"id": instances[1].pk, "xform": self.xform.pk}, + }, + ], + } + self.assertEqual(response.data, data) + def test_instances_with_geopoints(self): # publish sample geo submissions self._publish_submit_geojson() diff --git a/onadata/apps/main/tests/fixtures/geolocation/empty_geoshapes.csv b/onadata/apps/main/tests/fixtures/geolocation/empty_geoshapes.csv new file mode 100644 index 0000000000..482767bedc --- /dev/null +++ b/onadata/apps/main/tests/fixtures/geolocation/empty_geoshapes.csv @@ -0,0 +1,3 @@ +today,start,end,deviceid,segment[1]/point_position,segment[1]/blueline,segment[2]/point_position,segment[2]/blueline,meta/instanceID,_id,_uuid,_submission_time,_date_modified,_tags,_notes,_version,_duration,_submitted_by,_total_media,_media_count,_media_all_received,_xform_id +2023-09-15,2023-09-15T12:53:19.451+03:00,2023-09-15T12:55:10.386+03:00,enketo.ona.io:qrggmjketRepjz8x,1,n/a,n/a,n/a,uuid:cb2f1dae-47f2-4919-9fea-5ede2fda841c,122094074,cb2f1dae-47f2-4919-9fea-5ede2fda841c,2023-09-15T09:55:11,2023-09-15T09:55:11,,,202309150951,111.0,kipsigei,0,0,True,764655 +2023-09-15,2023-09-15T12:55:10.436+03:00,2023-09-15T12:55:37.212+03:00,enketo.ona.io:qrggmjketRepjz8x,1,-1.29728 36.79198 0 0;-1.298009 36.785793 0 0;-1.29961 36.789744 0 0;-1.300146 36.790625 0 0;-1.300897 36.792107 0 0;-1.29728 36.79198 0 0,2,-1.297391 36.789659 0 0;-1.298163 36.78296 0 0;-1.299751 36.7882 0 0;-1.297391 36.789659 0 0,uuid:6ac3e33c-d064-45e0-8597-880227e36435,122094094,6ac3e33c-d064-45e0-8597-880227e36435,2023-09-15T09:55:37,2023-09-15T09:55:37,,,202309150951,27.0,kipsigei,0,0,True,764655 diff --git a/onadata/apps/main/tests/fixtures/geolocation/empty_geotraces.csv b/onadata/apps/main/tests/fixtures/geolocation/empty_geotraces.csv new file mode 100644 index 0000000000..e9345fedda --- /dev/null +++ b/onadata/apps/main/tests/fixtures/geolocation/empty_geotraces.csv @@ -0,0 +1,3 @@ +today,start,end,deviceid,segment[1]/point_position,segment[1]/blueline,segment[2]/point_position,segment[2]/blueline,meta/instanceID,_id,_uuid,_submission_time,_date_modified,_tags,_notes,_version,_duration,_submitted_by,_total_media,_media_count,_media_all_received,_xform_id +2023-09-15,2023-09-15T12:39:32.676+03:00,2023-09-15T12:39:52.828+03:00,enketo.ona.io:qrggmjketRepjz8x,1,n/a,n/a,n/a,uuid:51558e36-de1c-49c1-bf66-199060b2655b,122093541,51558e36-de1c-49c1-bf66-199060b2655b,2023-09-15T09:39:53,2023-09-15T09:39:53,,,202309150939,20.0,kipsigei,0,0,True,764649 +2023-09-15,2023-09-15T12:39:52.858+03:00,2023-09-15T12:40:31.290+03:00,enketo.ona.io:qrggmjketRepjz8x,1,-1.269392 36.809057 0 0;-1.271966 36.803303 0 0;-1.268118 36.805943 0 0;-1.269405 36.808822 0 0,2,-1.25616 36.865203 0 0;-1.251569 36.86919 0 0;-1.255495 36.870234 0 0;-1.256203 36.8654 0 0,uuid:39c3ee3f-bab3-4fca-81ce-011c6f26eb98,122093563,39c3ee3f-bab3-4fca-81ce-011c6f26eb98,2023-09-15T09:40:32,2023-09-15T09:40:32,,,202309150939,39.0,kipsigei,0,0,True,764649 diff --git a/onadata/libs/utils/dict_tools.py b/onadata/libs/utils/dict_tools.py index 9a37cc931e..e5624337a2 100644 --- a/onadata/libs/utils/dict_tools.py +++ b/onadata/libs/utils/dict_tools.py @@ -11,6 +11,7 @@ def get_values_matching_key(doc, key): """ def _get_values(doc, key): + # pylint: disable=too-many-nested-blocks if doc is not None: if key in doc: yield doc[key]