Fixed #15277 -- Cleaned up `ogrinspect` command, added tests and extended…

Fixed #15277 -- Cleaned up `ogrinspect` command, added tests and extended support beyond file-based OGR data sources.  Thanks, willinoed for bug report and jpaulett for initial patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16845 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/gis/management/base.py

-from django.core.management.base import BaseCommand, CommandError
-
-class ArgsCommand(BaseCommand):
-    """
-    Command class for commands that take multiple arguments.
-    """
-    args = '<arg arg ...>'
-
-    def handle(self, *args, **options):
-        if not args:
-            raise CommandError('Must provide the following arguments: %s' % self.args)
-        return self.handle_args(*args, **options)
-
-    def handle_args(self, *args, **options):
-        raise NotImplementedError()

django/contrib/gis/management/commands/ogrinspect.py

 import os
 from optparse import make_option
 from django.contrib.gis import gdal
-from django.contrib.gis.management.base import ArgsCommand, CommandError
+from django.core.management.base import LabelCommand, CommandError
 
 def layer_option(option, opt, value, parser):
     """
@@ -17,7 +17,7 @@ def layer_option(option, opt, value, parser):
 def list_option(option, opt, value, parser):
     """
     Callback for `make_option` for `ogrinspect` keywords that require
-    a string list.  If the string is 'True'/'true' then the option 
+    a string list.  If the string is 'True'/'true' then the option
     value will be a boolean instead.
     """
     if value.lower() == 'true':
@@ -25,20 +25,20 @@ def list_option(option, opt, value, parser):
     else:
         dest = [s for s in value.split(',')]
     setattr(parser.values, option.dest, dest)
-    
-class Command(ArgsCommand):
+
+class Command(LabelCommand):
     help = ('Inspects the given OGR-compatible data source (e.g., a shapefile) and outputs\n'
             'a GeoDjango model with the given model name. For example:\n'
             ' ./manage.py ogrinspect zipcode.shp Zipcode')
     args = '[data_source] [model_name]'
 
-    option_list = ArgsCommand.option_list + (
-        make_option('--blank', dest='blank', type='string', action='callback',  
+    option_list = LabelCommand.option_list + (
+        make_option('--blank', dest='blank', type='string', action='callback',
                     callback=list_option, default=False,
                     help='Use a comma separated list of OGR field names to add '
                     'the `blank=True` option to the field definition.  Set with'
                     '`true` to apply to all applicable fields.'),
-        make_option('--decimal', dest='decimal', type='string', action='callback', 
+        make_option('--decimal', dest='decimal', type='string', action='callback',
                     callback=list_option, default=False,
                     help='Use a comma separated list of OGR float fields to '
                     'generate `DecimalField` instead of the default '
@@ -46,7 +46,7 @@ class Command(ArgsCommand):
         make_option('--geom-name', dest='geom_name', type='string', default='geom',
                     help='Specifies the model name for the Geometry Field '
                     '(defaults to `geom`)'),
-        make_option('--layer', dest='layer_key', type='string', action='callback', 
+        make_option('--layer', dest='layer_key', type='string', action='callback',
                     callback=layer_option, default=0,
                     help='The key for specifying which layer in the OGR data '
                     'source to use. Defaults to 0 (the first layer). May be '
@@ -58,7 +58,7 @@ class Command(ArgsCommand):
         make_option('--no-imports', action='store_false', dest='imports', default=True,
                     help='Do not include `from django.contrib.gis.db import models` '
                     'statement.'),
-        make_option('--null', dest='null', type='string', action='callback',  
+        make_option('--null', dest='null', type='string', action='callback',
                     callback=list_option, default=False,
                     help='Use a comma separated list of OGR field names to add '
                     'the `null=True` option to the field definition.  Set with'
@@ -72,7 +72,7 @@ class Command(ArgsCommand):
 
     requires_model_validation = False
 
-    def handle_args(self, *args, **options):
+    def handle(self, *args, **options):
         try:
             data_source, model_name = args
         except ValueError:
@@ -81,10 +81,6 @@ class Command(ArgsCommand):
         if not gdal.HAS_GDAL:
             raise CommandError('GDAL is required to inspect geospatial data sources.')
 
-        # TODO: Support non file-based OGR datasources.
-        if not os.path.isfile(data_source):
-            raise CommandError('The given data source cannot be found: "%s"' % data_source)
-        
         # Removing options with `None` values.
         options = dict([(k, v) for k, v in options.items() if not v is None])
 
@@ -97,8 +93,9 @@ class Command(ArgsCommand):
         # Whether the user wants to generate the LayerMapping dictionary as well.
         show_mapping = options.pop('mapping', False)
 
-        # Popping the verbosity global option, as it's not accepted by `_ogrinspect`.
+        # Getting rid of settings that `_ogrinspect` doesn't like.
         verbosity = options.pop('verbosity', False)
+        settings = options.pop('settings', False)
 
         # Returning the output of ogrinspect with the given arguments
         # and options.
@@ -115,8 +112,8 @@ class Command(ArgsCommand):
             # This extra legwork is so that the dictionary definition comes
             # out in the same order as the fields in the model definition.
             rev_mapping = dict([(v, k) for k, v in mapping_dict.items()])
-            output.extend(['', '# Auto-generated `LayerMapping` dictionary for %s model' % model_name, 
+            output.extend(['', '# Auto-generated `LayerMapping` dictionary for %s model' % model_name,
                            '%s_mapping = {' % model_name.lower()])
             output.extend(["    '%s' : '%s'," % (rev_mapping[ogr_fld], ogr_fld) for ogr_fld in ds[options['layer_key']].fields])
             output.extend(["    '%s' : '%s'," % (options['geom_name'], mapping_dict[options['geom_name']]), '}'])
-        return '\n'.join(output)
+        return '\n'.join(output) + '\n'

django/contrib/gis/tests/__init__.py

@@ -29,15 +29,13 @@ def geo_apps(namespace=True, runtests=False):
 
     # The following GeoDjango test apps depend on GDAL support.
     if HAS_GDAL:
-        # Geographic admin requires GDAL
-        apps.append('geoadmin')
+        # Geographic admin, LayerMapping, and ogrinspect test apps
+        # all require GDAL.
+        apps.extend(['geoadmin', 'layermap', 'inspectapp'])
 
-        # 3D apps use LayerMapping, which uses GDAL.
+        # 3D apps use LayerMapping, which uses GDAL and require GEOS 3.1+.
         if connection.ops.postgis and GEOS_PREPARE:
             apps.append('geo3d')
-
-        apps.append('layermap')
-
     if runtests:
         return [('django.contrib.gis.tests', app) for app in apps]
     elif namespace:

django/contrib/gis/tests/inspectapp/models.py

+from django.contrib.gis.db import models
+
+class AllOGRFields(models.Model):
+    f_decimal = models.FloatField()
+    f_float = models.FloatField()
+    f_int = models.IntegerField()
+    f_char = models.CharField(max_length=10)
+    f_date = models.DateField()
+    f_datetime = models.DateTimeField()
+    f_time = models.TimeField()
+    geom = models.PolygonField()
+
+    objects = models.GeoManager()

django/contrib/gis/tests/inspectapp/tests.py

+import os
+from django.db import connections
+from django.test import TestCase
+from django.contrib.gis.gdal import Driver
+from django.contrib.gis.geometry.test_data import TEST_DATA
+from django.contrib.gis.utils.ogrinspect import ogrinspect
+from models import AllOGRFields
+
+
+class OGRInspectTest(TestCase):
+    def test_poly(self):
+        shp_file = os.path.join(TEST_DATA, 'test_poly', 'test_poly.shp')
+        model_def = ogrinspect(shp_file, 'MyModel')
+
+        expected = [
+            '# This is an auto-generated Django model module created by ogrinspect.',
+            'from django.contrib.gis.db import models',
+            '',
+            'class MyModel(models.Model):',
+            '    float = models.FloatField()',
+            '    int = models.FloatField()',
+            '    str = models.CharField(max_length=80)',
+            '    geom = models.PolygonField(srid=-1)',
+            '    objects = models.GeoManager()',
+        ]
+
+        self.assertEqual(model_def, '\n'.join(expected))
+
+    def test_date_field(self):
+        shp_file = os.path.join(TEST_DATA, 'cities', 'cities.shp')
+        model_def = ogrinspect(shp_file, 'City')
+
+        expected = [
+            '# This is an auto-generated Django model module created by ogrinspect.',
+            'from django.contrib.gis.db import models',
+            '',
+            'class City(models.Model):',
+            '    name = models.CharField(max_length=80)',
+            '    population = models.FloatField()',
+            '    density = models.FloatField()',
+            '    created = models.DateField()',
+            '    geom = models.PointField(srid=-1)',
+            '    objects = models.GeoManager()',
+        ]
+
+        self.assertEqual(model_def, '\n'.join(expected))
+
+    def test_time_field(self):
+        # Only possible to test this on PostGIS at the momemnt.  MySQL
+        # complains about permissions, and SpatiaLite/Oracle are
+        # insanely difficult to get support compiled in for in GDAL.
+        if not connections['default'].ops.postgis:
+            return
+
+        # Getting the database identifier used by OGR, if None returned
+        # GDAL does not have the support compiled in.
+        ogr_db = get_ogr_db_string()
+        if not ogr_db:
+            return
+
+        # writing shapefules via GDAL currently does not support writing OGRTime
+        # fields, so we need to actually use a database
+        model_def = ogrinspect(ogr_db, 'Measurement',
+                               layer_key=AllOGRFields._meta.db_table,
+                               decimal=['f_decimal'])
+
+        expected = [
+            '# This is an auto-generated Django model module created by ogrinspect.',
+            'from django.contrib.gis.db import models',
+            '',
+            'class Measurement(models.Model):',
+            '    f_decimal = models.DecimalField(max_digits=0, decimal_places=0)',
+            '    f_int = models.IntegerField()',
+            '    f_datetime = models.DateTimeField()',
+            '    f_time = models.TimeField()',
+            '    f_float = models.FloatField()',
+            '    f_char = models.CharField(max_length=10)',
+            '    f_date = models.DateField()',
+            '    geom = models.PolygonField()',
+            '    objects = models.GeoManager()',
+        ]
+
+        self.assertEqual(model_def, '\n'.join(expected))
+
+def get_ogr_db_string():
+    # Construct the DB string that GDAL will use to inspect the database.
+    # GDAL will create its own connection to the database, so we re-use the
+    # connection settings from the Django test.  This approach is a bit fragile
+    # and cannot work on any other database other than PostgreSQL at the moment.
+    db = connections.databases['default']
+
+    # Map from the django backend into the OGR driver name and database identifier
+    # http://www.gdal.org/ogr/ogr_formats.html
+    #
+    # TODO: Support Oracle (OCI), MySQL, and SpatiaLite.
+    drivers = {
+        'django.contrib.gis.db.backends.postgis': ('PostgreSQL', 'PG'),
+    }
+
+    drv_name, db_str = drivers[db['ENGINE']]
+
+    # Ensure that GDAL library has driver support for the database.
+    try:
+        Driver(drv_name)
+    except:
+        return None
+
+    # Build the params of the OGR database connection string
+    # TODO: connection strings are database-dependent, thus if
+    #       we ever test other backends, this will need to change.
+    params = ["dbname='%s'" % db['NAME']]
+    def add(key, template):
+        value = db.get(key, None)
+        # Don't add the parameter if it is not in django's settings
+        if value:
+            params.append(template % value)
+    add('HOST', "host='%s'")
+    add('PORT', "port='%s'")
+    add('USER', "user='%s'")
+    add('PASSWORD', "password='%s'")
+
+    return '%s:%s' % (db_str, ' '.join(params))

django/contrib/gis/utils/ogrinspect.py

@@ -8,7 +8,7 @@ Author: Travis Pinney, Dane Springmeyer, & Justin Bronn
 from itertools import izip
 # Requires GDAL to use.
 from django.contrib.gis.gdal import DataSource
-from django.contrib.gis.gdal.field import OFTDate, OFTDateTime, OFTInteger, OFTReal, OFTString
+from django.contrib.gis.gdal.field import OFTDate, OFTDateTime, OFTInteger, OFTReal, OFTString, OFTTime
 
 def mapping(data_source, geom_name='geom', layer_key=0, multi_geom=False):
     """
@@ -189,7 +189,7 @@ def _ogrinspect(data_source, model_name, geom_name='geom', layer_key=0, srid=Non
             yield '    %s = models.DateField(%s)' % (mfield, kwargs_str[2:])
         elif field_type is OFTDateTime:
             yield '    %s = models.DateTimeField(%s)' % (mfield, kwargs_str[2:])
-        elif field_type is OFTDate:
+        elif field_type is OFTTime:
             yield '    %s = models.TimeField(%s)' % (mfield, kwargs_str[2:])
         else:
             raise TypeError('Unknown field type %s in %s' % (field_type, mfield))