Merge pull request #143 from onaio/issue-142-the-shapefiles-issue

Fix shapefile issues

.travis.yml

@@ -13,12 +13,10 @@ matrix:
  env: TOXENV=flake8
  - python: 3.6
  env: TOXENV=pylint
- - python: 3.6
- env: TOXENV=coverage
 services:
  - postgresql
 addons:
- postgresql: 9.6
+ postgresql: "9.6"
  apt:
  packages:
  - postgresql-9.6-postgis-2.3
@@ -30,5 +28,4 @@ install:
  - pip install tox
 script: tox
 notifications:
- slack:
- - onaio:snkNXgprD498qQv4DgRREKJF
+ slack: onaio:snkNXgprD498qQv4DgRREKJF

Pipfile

@@ -17,7 +17,9 @@ model-mommy = "*"
 "pep8" = "*"
 "autopep8" = "*"
 tblib = "*"
+mock = "*"
 
 [packages]
-"e1839a8" = {path = ".", editable = true}
+e1839a8 = {editable = true,path = "."}
 future = "*"
+ona-tasking = {editable = true,path = "."}

setup.py

@@ -14,7 +14,7 @@
  url='https://github.com/onaio/tasking',
  packages=find_packages(exclude=['docs', 'tests']),
  install_requires=[
- 'Django >= 1.11, < 2.1',
+ 'Django >= 1.11.19, < 2.1',
  'python-dateutil',
  'markdown', # adds markdown support for browsable REST API
  'django-filter < 2', # for filtering in the API

tasking/__init__.py

@@ -4,5 +4,5 @@
 """
 from __future__ import unicode_literals
 
-VERSION = (0, 2, 0)
+VERSION = (0, 2, 1)
 __version__ = '.'.join(str(v) for v in VERSION)

tasking/common_tags.py

@@ -23,3 +23,4 @@
 INVALID_END_DATE = _('The end date cannnot be lesser than the start date.')
 MISSING_START_DATE = _('Cannot determine the start date. Please provide '
  'either the start date or timing rule(s)')
+INVALID_SHAPEFILE = _("Invalid shapefile")

tasking/exceptions.py

@@ -4,8 +4,8 @@
 """
 from __future__ import unicode_literals
 
-from tasking.common_tags import TARGET_DOES_NOT_EXIST, NO_SHAPEFILE
-from tasking.common_tags import MISSING_FILE, UNNECESSARY_FILE
+from tasking.common_tags import (INVALID_SHAPEFILE, MISSING_FILE, NO_SHAPEFILE,
+ TARGET_DOES_NOT_EXIST, UNNECESSARY_FILE)
 
 
 class TargetDoesNotExist(Exception):
@@ -24,6 +24,14 @@ class ShapeFileNotFound(Exception):
  message = NO_SHAPEFILE
 
 
+class InvalidShapeFile(Exception):
+ """
+ Custom Exception raised when the shapefile is not valid
+ """
+
+ message = INVALID_SHAPEFILE
+
+
 class MissingFiles(Exception):
  """
  Custom Exception raised when a file is missing for shapefile

tasking/serializers/location.py

@@ -6,6 +6,8 @@
 
 import zipfile
 from io import BytesIO
+from os import path
+import logging
 
 from django.contrib.gis.gdal import DataSource
 from django.contrib.gis.geos import MultiPolygon, Point
@@ -18,19 +20,22 @@
 from rest_framework_gis.serializers import GeometryField
 
 from tasking.common_tags import (GEODETAILS_ONLY, GEOPOINT_MISSING,
- RADIUS_MISSING)
+ RADIUS_MISSING, INVALID_SHAPEFILE)
 from tasking.exceptions import (MissingFiles, ShapeFileNotFound,
  UnnecessaryFiles)
 from tasking.models import Location
 from tasking.utils import get_shapefile
 
 
+LOGGER = logging.getLogger(__name__)
+
+
 class ShapeFileField(GeometryField):
  """
  Custom Field for Shapefile
  """
 
- def to_internal_value(self, value):
+ def to_internal_value(self, value): # pylint: disable=too-many-locals
  """
  Custom Conversion for shapefile field
  """
@@ -65,7 +70,7 @@ def to_internal_value(self, value):
  zip_file.extractall(tpath)
 
  # concatenate Shapefile path
- shp_path = "{tpath}/{shp}".format(tpath=tpath, shp=shpfile)
+ shp_path = path.join(tpath, shpfile)
 
  # Make the shapefile a DataSource
  data_source = DataSource(shp_path)
@@ -78,7 +83,12 @@ def to_internal_value(self, value):
  for polygon in polygon_data:
  polygons.append(polygon.geos)
 
- multipolygon = MultiPolygon(polygons)
+ try:
+ multipolygon = MultiPolygon(polygons)
+ except TypeError as exc:
+ # this shapefile is just not valid for some reason
+ LOGGER.exception(exc)
+ raise serializers.ValidationError(INVALID_SHAPEFILE)
 
  return multipolygon
 

tests/serializers/test_contenttype.py

@@ -32,6 +32,7 @@ def test_serializer_output(self):
 
  self.assertEqual(set(expected_fields),
  set(list(serializer_data.keys())))
- self.assertEqual(mocked_contenttype.app_label, serializer_data['app_label'])
+ self.assertEqual(
+ mocked_contenttype.app_label, serializer_data['app_label'])
  self.assertEqual(mocked_contenttype.model, serializer_data['model'])
  self.assertEqual(mocked_contenttype.id, serializer_data['id'])

tests/serializers/test_location.py

@@ -5,23 +5,25 @@
 from __future__ import unicode_literals
 
 import os
-
 from collections import OrderedDict
 
 from django.test import TestCase
 from django.utils import six
-
-from rest_framework.exceptions import ValidationError
-from rest_framework_gis.fields import GeoJsonDict
 from model_mommy import mommy
+from rest_framework.exceptions import ErrorDetail, ValidationError
+from rest_framework_gis.fields import GeoJsonDict
 
+from tasking.common_tags import (GEODETAILS_ONLY, GEOPOINT_MISSING,
+ RADIUS_MISSING)
+from tasking.exceptions import (InvalidShapeFile, MissingFiles,
+ ShapeFileNotFound, UnnecessaryFiles)
 from tasking.serializers import LocationSerializer
-from tasking.common_tags import RADIUS_MISSING, GEOPOINT_MISSING
-from tasking.common_tags import GEODETAILS_ONLY
-from tasking.exceptions import (MissingFiles,
- ShapeFileNotFound,
-  UnnecessaryFiles)
-from rest_framework.exceptions import ErrorDetail
+
+try:
+ from unittest.mock import patch
+except ImportError:
+ from mock import patch
+
 
 BASE_DIR = os.path.dirname(os.path.dirname(__file__))
 
@@ -407,3 +409,28 @@ def test_bad_shapefile_data(self):
  serializer_instance.errors,
  {"shapefile": [ErrorDetail(string=UnnecessaryFiles().message,
  code="invalid")]})
+
+ @patch('tasking.serializers.location.MultiPolygon')
+ def test_invalid_shapefile(self, mock):
+ """
+ Test invalid shapefile
+ The appropriate exception is raised when we encounter an invalid
+ shapefile 
+ """
+ mock.side_effect = TypeError
+
+ path = os.path.join(
+ BASE_DIR, 'fixtures', 'test_shapefile.zip')
+
+ with open(path, 'r+b') as shapefile:
+ data = OrderedDict(
+ name='Nairobi',
+ country='KE',
+ shapefile=shapefile
+ )
+ serializer_instance = LocationSerializer(data=data)
+ self.assertFalse(serializer_instance.is_valid())
+ self.assertEqual(
+ serializer_instance.errors,
+ {"shapefile": [ErrorDetail(string=InvalidShapeFile().message,
+ code="invalid")]})

tox.ini

@@ -5,17 +5,6 @@ envlist =
  py36
  flake8
  pylint
- coverage
-
-[testenv]
-deps =
- pipenv
-basepython =
- py35: python3.5
- py36: python3.6
-commands =
- pipenv install --dev
- python manage.py test {toxinidir}/tests -v 2 --parallel 2
 
 [testenv:py27]
 deps =
@@ -39,16 +28,18 @@ deps =
  pipenv
 basepython = python3.6
 commands =
- pipenv install --dev
+ pipenv sync --dev
  pylint --rcfile={toxinidir}/.pylintrc {toxinidir}/tasking
 
-[testenv:coverage]
+[testenv]
 deps =
- coverage
  pipenv
-basepython = python3.6
+ coverage
+basepython =
+ py35: python3.5
+ py36: python3.6
 commands =
- pipenv install --dev
+ pipenv sync --dev
  coverage erase
  coverage run --include="tasking/**.*" --omit="tests/**.*,tasking/migrations/**.*" manage.py test {toxinidir}/tests -v 2
  coverage report