diff --git a/mapit/management/commands/mapit_import.py b/mapit/management/commands/mapit_import.py index 7779fb7f..83b03794 100644 --- a/mapit/management/commands/mapit_import.py +++ b/mapit/management/commands/mapit_import.py @@ -215,16 +215,15 @@ def verbose(*args): name = override_name else: try: - name = feat[name_field].value - except: - choices = ', '.join(layer.fields) - raise CommandError( - "Could not find name using name field '%s' - should it be something else? " - "It will be one of these: %s. Specify which with --name_field" % (name_field, choices)) - try: + name = feat.get(name_field) + if name is None: + choices = ', '.join(layer.fields) + raise CommandError( + "Could not find name using name field '%s' - should it be something else? " + "It will be one of these: %s. Specify which with --name_field" % (name_field, choices)) if not isinstance(name, six.text_type): name = name.decode(encoding) - except: + except UnicodeDecodeError: raise CommandError( "Could not decode name using encoding '%s' - is it in another encoding? " "Specify one with --encoding" % encoding) diff --git a/mapit/tests/fixtures/bad_encoding.dbf b/mapit/tests/fixtures/bad_encoding.dbf new file mode 100755 index 00000000..4e961e2e Binary files /dev/null and b/mapit/tests/fixtures/bad_encoding.dbf differ diff --git a/mapit/tests/fixtures/bad_encoding.prj b/mapit/tests/fixtures/bad_encoding.prj new file mode 100755 index 00000000..b1641ae3 --- /dev/null +++ b/mapit/tests/fixtures/bad_encoding.prj @@ -0,0 +1 @@ +PROJCS["British_National_Grid",GEOGCS["GCS_OSGB_1936",DATUM["D_OSGB_1936",SPHEROID["Airy_1830",6377563.396,299.3249646]],PRIMEM["Greenwich",0.0],UNIT["Degree",0.0174532925199433]],PROJECTION["Transverse_Mercator"],PARAMETER["False_Easting",400000.0],PARAMETER["False_Northing",-100000.0],PARAMETER["Central_Meridian",-2.0],PARAMETER["Scale_Factor",0.9996012717],PARAMETER["Latitude_Of_Origin",49.0],UNIT["Meter",1.0],AUTHORITY["EPSG",27700]] \ No newline at end of file diff --git a/mapit/tests/fixtures/bad_encoding.shp b/mapit/tests/fixtures/bad_encoding.shp new file mode 100755 index 00000000..6b3e2f48 Binary files /dev/null and b/mapit/tests/fixtures/bad_encoding.shp differ diff --git a/mapit/tests/fixtures/bad_encoding.shx b/mapit/tests/fixtures/bad_encoding.shx new file mode 100755 index 00000000..7d7ffd58 Binary files /dev/null and b/mapit/tests/fixtures/bad_encoding.shx differ diff --git a/mapit/tests/test_import_commands.py b/mapit/tests/test_import_commands.py index b4d3e9e2..22caa3f2 100644 --- a/mapit/tests/test_import_commands.py +++ b/mapit/tests/test_import_commands.py @@ -2,6 +2,7 @@ from django.test import TestCase from django.core.management import call_command +from django.core.management.base import CommandError from django.conf import settings from django.utils.six import StringIO @@ -11,35 +12,38 @@ class MapitImportTest(TestCase): """Test the mapit_import management command""" - def test_loads_kml_files(self): - # Assert no areas in db before - self.assertEqual(Area.objects.count(), 0) - + def setUp(self): # Create a country, generation, area type and a name type for the # areas we'll load in from the KML file - england = Country.objects.create(code="E", name="England") - generation = Generation.objects.create( + self.england = Country.objects.create(code="E", name="England") + self.generation = Generation.objects.create( active=True, description="Test generation", ) - big_type = Type.objects.create( + self.big_type = Type.objects.create( code="BIG", description="A large test area", ) - os_name_type = NameType.objects.create( + self.os_name_type = NameType.objects.create( code='O', description="Ordnance Survey name", ) + def test_loads_kml_files(self): + # Assert no areas in db before + self.assertEqual(Area.objects.count(), 0) + + + # Load in the KML file fixtures_dir = os.path.join(settings.BASE_DIR, 'mapit', 'tests', 'fixtures') call_command( 'mapit_import', os.path.join(fixtures_dir, 'example.kml'), - country_code=england.code, - generation_id=generation.id, - area_type_code=big_type.code, - name_type_code=os_name_type.code, + country_code=self.england.code, + generation_id=self.generation.id, + area_type_code=self.big_type.code, + name_type_code=self.os_name_type.code, commit=True, # These keep the commands quiet, comment out if you're debugging stderr=StringIO(), @@ -52,7 +56,29 @@ def test_loads_kml_files(self): # Check it loaded the data properly area = Area.objects.all()[0] self.assertEqual(area.name, "London") - self.assertEqual(area.country.name, england.name) - self.assertEqual(area.generation_low.id, generation.id) - self.assertEqual(area.generation_high.id, generation.id) + self.assertEqual(area.country.name, self.england.name) + self.assertEqual(area.generation_low.id, self.generation.id) + self.assertEqual(area.generation_high.id, self.generation.id) self.assertEqual(area.type.code, "BIG") + + def test_reports_encoding_errors(self): + # Load in a badly encoded file (it contains Windows charset encoded + # apostrophe's, but reports itself as being UTF-8) + fixtures_dir = os.path.join(settings.BASE_DIR, 'mapit', 'tests', 'fixtures') + with self.assertRaises(CommandError) as context: + call_command( + 'mapit_import', + os.path.join(fixtures_dir, 'bad_encoding.shp'), + country_code=self.england.code, + generation_id=self.generation.id, + area_type_code=self.big_type.code, + name_type_code=self.os_name_type.code, + commit=True, + # These keep the commands quiet, comment out if you're debugging + stderr=StringIO(), + stdout=StringIO(), + ) + # Previously, this would have been a message about not being able + # to find the name field. + expected_message = "Could not decode name using encoding 'utf-8' - is it in another encoding? \nSpecify one with --encoding" + self.assertEqual(context.exception.message, expected_message)