Skip to content

Commit

Permalink
♻ [REF] Improve exceptions management on GPX/KML handling methods
Browse files Browse the repository at this point in the history
  • Loading branch information
Chatewgne committed Dec 2, 2024
1 parent 865dda6 commit 5381f1e
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 13 deletions.
10 changes: 10 additions & 0 deletions geotrek/cirkwi/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ def next_row(self):
first += self.rows

def get_part(self, dst, src, val):
"""
The CirkwiParser class handles some XML-related specificities for the fields mapping:
- `src` values should follow an XML path syntax so the separator is '/' instead of '.'
- `src` value "locomotions/locomotion" gets the text value of the first <locomotion> XML element encountered (inside a <locomotions> element at the row's root level)
- "informations/information[@langue='fr']/titre" means we get the text value of the first <titre> element having a parent <information> with attribute `langue` equals "fr"
- the "<LANG>" marker is replaced with the `default_language` of the parser
- a '@@' sequence at the end followed by a attribute name indicates the fetched value should be the value of the attribute (for instance "locomotions/locomotion@@type" will get the value of the 'type' attribute of the <locomotion> element)
- finally the '/*' sequence at the end of a `src` value indicates a list of all XML elements matching the path should be returned (so "locomotions/locomotion/*" has the same meaning than "locomotions.*.locomotion" from the base Parser class)
"""
# Recursively extract XML attributes
if '@@' in src and src[:2] != '@@':
part, attrib = src.split('@@', 1)
Expand Down
2 changes: 1 addition & 1 deletion geotrek/cirkwi/tests/test_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def test_create_treks(self, mocked_get):

# Test update
mocked_get.side_effect = self.make_dummy_get('circuits_updated.xml')
call_command('import', 'geotrek.cirkwi.tests.test_parsers.TestCirkwiTrekParserFrUpdateOnly', verbosity=2)
call_command('import', 'geotrek.cirkwi.tests.test_parsers.TestCirkwiTrekParserFrUpdateOnly', verbosity=0)
self.assertEqual(Trek.objects.count(), 2)
t = Trek.objects.get(name_fr="Le patrimoine de Plancoët à VTT")
self.assertEqual(t.eid, '10926')
Expand Down
1 change: 0 additions & 1 deletion geotrek/common/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
from geotrek.common.utils.parsers import add_http_prefix
from geotrek.common.utils.translation import get_translated_fields


if 'modeltranslation' in settings.INSTALLED_APPS:
from modeltranslation.fields import TranslationField

Expand Down
10 changes: 5 additions & 5 deletions geotrek/common/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from ..utils.file_infos import get_encoding_file
from ..utils.import_celery import create_tmp_destination, subclasses
from ..utils.parsers import (add_http_prefix, get_geom_from_gpx,
get_geom_from_kml, maybe_fix_encoding_to_utf8)
get_geom_from_kml, maybe_fix_encoding_to_utf8, GeomValueError)


class UtilsTest(TestCase):
Expand Down Expand Up @@ -142,7 +142,7 @@ def test_gpx_with_route_points_can_be_converted(self):
def test_it_raises_an_error_on_not_continuous_segments(self):
gpx = self._get_gpx_from('geotrek/trekking/tests/data/apidae_trek_parser/trace_with_not_continuous_segments.gpx')

with self.assertRaises(ValueError):
with self.assertRaises(GeomValueError):
get_geom_from_gpx(gpx)

def test_it_handles_segment_with_single_point(self):
Expand All @@ -158,7 +158,7 @@ def test_it_handles_segment_with_single_point(self):
def test_it_raises_an_error_when_no_linestring(self):
gpx = self._get_gpx_from('geotrek/trekking/tests/data/apidae_trek_parser/trace_with_no_feature.gpx')

with self.assertRaises(ValueError):
with self.assertRaises(GeomValueError):
get_geom_from_gpx(gpx)

def test_it_handles_multiple_continuous_features(self):
Expand All @@ -185,7 +185,7 @@ def test_it_handles_multiple_continuous_features_with_one_empty(self):

def test_it_raises_error_on_multiple_not_continuous_features(self):
gpx = self._get_gpx_from('geotrek/trekking/tests/data/apidae_trek_parser/trace_with_multiple_not_continuous_features.gpx')
with self.assertRaises(ValueError):
with self.assertRaises(GeomValueError):
get_geom_from_gpx(gpx)


Expand All @@ -212,7 +212,7 @@ def test_kml_can_be_converted(self):
def test_it_raises_exception_when_no_linear_data(self):
kml = self._get_kml_from('geotrek/trekking/tests/data/apidae_trek_parser/trace_with_no_line.kml')

with self.assertRaises(ValueError):
with self.assertRaises(GeomValueError):
get_geom_from_kml(kml)


Expand Down
12 changes: 8 additions & 4 deletions geotrek/common/utils/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
from geotrek.common.utils.file_infos import get_encoding_file


class GeomValueError(Exception):
pass


def add_http_prefix(url):
if url.startswith('http'):
return url
Expand Down Expand Up @@ -64,15 +68,15 @@ def maybe_get_linestring_from_layer(layer):
if geos.geom_type == 'MultiLineString':
geos = geos.merged # If possible we merge the MultiLineString into a LineString
if geos.geom_type == 'MultiLineString':
raise ValueError(
raise GeomValueError(
_("Feature geometry cannot be converted to a single continuous LineString feature"))
geoms.append(geos)

full_geom = MultiLineString(geoms)
full_geom.srid = geoms[0].srid
full_geom = full_geom.merged # If possible we merge the MultiLineString into a LineString
if full_geom.geom_type == 'MultiLineString':
raise ValueError(
raise GeomValueError(
_("Geometries from various features cannot be converted to a single continuous LineString feature"))

return full_geom
Expand All @@ -91,7 +95,7 @@ def maybe_get_linestring_from_layer(layer):
if geos:
break
else:
raise ValueError("No LineString feature found in GPX layers tracks or routes")
raise GeomValueError("No LineString feature found in GPX layers tracks or routes")
geos.transform(settings.SRID)
return geos

Expand All @@ -113,7 +117,7 @@ def get_first_geom_with_type_in(types, geoms):
for t in types:
if g.geom_type.name.startswith(t):
return g
raise ValueError('The attached KML geometry does not have any LineString or MultiLineString data')
raise GeomValueError('The attached KML geometry does not have any LineString or MultiLineString data')

with NamedTemporaryFile(mode='w+b', dir=settings.TMP_DIR) as ntf:
ntf.write(data)
Expand Down
4 changes: 2 additions & 2 deletions geotrek/trekking/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
GeotrekParser, GlobalImportError, Parser,
RowImportError, ShapeParser, DownloadImportError,
ValueImportError)
from geotrek.common.utils.parsers import get_geom_from_gpx, get_geom_from_kml
from geotrek.common.utils.parsers import get_geom_from_gpx, get_geom_from_kml, GeomValueError
from geotrek.core.models import Path, Topology
from geotrek.trekking.models import (POI, Accessibility, DifficultyLevel,
OrderedTrekChild, Service, Trek,
Expand Down Expand Up @@ -698,7 +698,7 @@ def filter_geom(self, src, val):
elif ext == 'kmz':
kml_file = zipfile.ZipFile(io.BytesIO(geom_file)).read('doc.kml')
return get_geom_from_kml(kml_file)
except ValueError as e:
except GeomValueError as e:
raise RowImportError(str(e))

def filter_labels(self, src, val):
Expand Down

0 comments on commit 5381f1e

Please sign in to comment.