[FEATURE] [OGR provider] Handle read-write support for .shz and .shp.…

…zip with GDAL 3.1 GDAL 3.1 will bring read-write supprot for single-layer ZIP compressed shape files (.shz), or multi-layer ones (.shp.zip). Do the few tweaking in QGIS so that it is handled properly. Related GDAL PR: OSGeo/gdal#1676
qgis · Jun 27, 2019 · de7e3fa · de7e3fa
1 parent f1bd39d
commit de7e3fa
Show file tree

Hide file tree

Showing 6 changed files with 186 additions and 27 deletions.
diff --git a/src/core/providers/gdal/qgsgdaldataitems.cpp b/src/core/providers/gdal/qgsgdaldataitems.cpp
@@ -158,6 +158,12 @@ QgsDataItem *QgsGdalDataItemProvider::createDataItem( const QString &pathIn, Qgs
     scanExtSetting = true;
   }
 
+  if ( path.endsWith( QLatin1String( ".shp.zip" ), Qt::CaseInsensitive ) )
+  {
+    // .shp.zip are vector datasets
+    return nullptr;
+  }
+
   // get suffix, removing .gz if present
   QString tmpPath = path; //path used for testing, not for layer creation
   if ( is_vsigzip )

diff --git a/src/core/providers/ogr/qgsogrdataitems.cpp b/src/core/providers/ogr/qgsogrdataitems.cpp
@@ -443,6 +443,14 @@ QgsDataItem *QgsOgrDataItemProvider::createDataItem( const QString &pathIn, QgsD
     tmpPath.chop( 3 );
   QFileInfo info( tmpPath );
   QString suffix = info.suffix().toLower();
+
+// GDAL 3.1 Shapefile driver directly handles .shp.zip files
+  if ( path.endsWith( QLatin1String( ".shp.zip" ), Qt::CaseInsensitive ) &&
+       GDALIdentifyDriver( path.toUtf8().constData(), nullptr ) )
+  {
+    suffix = QStringLiteral( "shp.zip" );
+  }
+
   // extract basename with extension
   info.setFile( path );
   QString name = info.fileName();
@@ -551,7 +559,8 @@ QgsDataItem *QgsOgrDataItemProvider::createDataItem( const QString &pathIn, QgsD
   static QStringList sSkipFastTrackExtensions { QStringLiteral( "xlsx" ),
       QStringLiteral( "ods" ),
       QStringLiteral( "csv" ),
-      QStringLiteral( "nc" ) };
+      QStringLiteral( "nc" ),
+      QStringLiteral( "shp.zip" ) };
 
   // Fast track: return item without testing if:
   // scanExtSetting or zipfile and scan zip == "Basic scan"

diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp
@@ -2384,6 +2384,9 @@ bool QgsOgrProvider::createSpatialIndex()
     QgsDebugMsg( QStringLiteral( "SQL: %1" ).arg( QString::fromUtf8( sql ) ) );
     mOgrOrigLayer->ExecuteSQLNoReturn( sql );
 
+    if ( !mFilePath.endsWith( QLatin1String( ".shp" ), Qt::CaseInsensitive ) )
+      return true;
+
     QFileInfo fi( mFilePath );     // to get the base name
     //find out, if the .qix file is there
     return QFileInfo::exists( fi.path().append( '/' ).append( fi.completeBaseName() ).append( ".qix" ) );
@@ -2876,8 +2879,11 @@ QString createFilters( const QString &type )
       }
       else if ( driverName.startsWith( QLatin1String( "ESRI" ) ) )
       {
-        sFileFilters += createFileFilter_( QObject::tr( "ESRI Shapefiles" ), QStringLiteral( "*.shp" ) );
+        QString exts = GDALGetMetadataItem( driver, GDAL_DMD_EXTENSIONS, "" );
+        sFileFilters += createFileFilter_( QObject::tr( "ESRI Shapefiles" ), exts.contains( "shz" ) ? QStringLiteral( "*.shp *.shz *.shp.zip" ) : QStringLiteral( "*.shp" ) );
         sExtensions << QStringLiteral( "shp" ) << QStringLiteral( "dbf" );
+        if ( exts.contains( "shz" ) )
+          sExtensions << QStringLiteral( "shz" ) << QStringLiteral( "shp.zip" );
       }
       else if ( driverName.startsWith( QObject::tr( "FMEObjects Gateway" ) ) )
       {
@@ -3390,7 +3396,8 @@ bool QgsOgrProviderUtils::createEmptyDataSource( const QString &uri,
 
   if ( driverName == QLatin1String( "ESRI Shapefile" ) )
   {
-    if ( !uri.endsWith( QLatin1String( ".shp" ), Qt::CaseInsensitive ) )
+    if ( !uri.endsWith( QLatin1String( ".shp" ), Qt::CaseInsensitive ) &&
+         !uri.endsWith( QLatin1String( ".shz" ), Qt::CaseInsensitive ) )
     {
       errorMessage = QObject::tr( "URI %1 doesn't end with .shp" ).arg( uri );
       QgsDebugMsg( errorMessage );
@@ -3572,17 +3579,21 @@ bool QgsOgrProviderUtils::createEmptyDataSource( const QString &uri,
 
   if ( driverName == QLatin1String( "ESRI Shapefile" ) )
   {
-    QString layerName = uri.left( uri.indexOf( QLatin1String( ".shp" ), Qt::CaseInsensitive ) );
-    QFile prjFile( layerName + ".qpj" );
-    if ( prjFile.open( QIODevice::WriteOnly | QIODevice::Truncate ) )
-    {
-      QTextStream prjStream( &prjFile );
-      prjStream << myWkt.toLocal8Bit().data() << endl;
-      prjFile.close();
-    }
-    else
+    int index = uri.indexOf( QLatin1String( ".shp" ), Qt::CaseInsensitive );
+    if ( index > 0 )
     {
-      QgsMessageLog::logMessage( QObject::tr( "Couldn't create file %1.qpj" ).arg( layerName ), QObject::tr( "OGR" ) );
+      QString layerName = uri.left( index );
+      QFile prjFile( layerName + ".qpj" );
+      if ( prjFile.open( QIODevice::WriteOnly | QIODevice::Truncate ) )
+      {
+        QTextStream prjStream( &prjFile );
+        prjStream << myWkt.toLocal8Bit().data() << endl;
+        prjFile.close();
+      }
+      else
+      {
+        QgsMessageLog::logMessage( QObject::tr( "Couldn't create file %1.qpj" ).arg( layerName ), QObject::tr( "OGR" ) );
+      }
     }
   }
 
@@ -3610,17 +3621,21 @@ QgsCoordinateReferenceSystem QgsOgrProvider::crs() const
 
   if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) )
   {
-    QString layerName = mFilePath.left( mFilePath.indexOf( QLatin1String( ".shp" ), Qt::CaseInsensitive ) );
-    QFile prjFile( layerName + ".qpj" );
-    if ( prjFile.open( QIODevice::ReadOnly ) )
+    int index = mFilePath.indexOf( QLatin1String( ".shp" ), Qt::CaseInsensitive );
+    if ( index > 0 )
     {
-      QTextStream prjStream( &prjFile );
-      QString myWktString = prjStream.readLine();
-      prjFile.close();
+      QString layerName = mFilePath.left( index );
+      QFile prjFile( layerName + ".qpj" );
+      if ( prjFile.open( QIODevice::ReadOnly ) )
+      {
+        QTextStream prjStream( &prjFile );
+        QString myWktString = prjStream.readLine();
+        prjFile.close();
 
-      srs = QgsCoordinateReferenceSystem::fromWkt( myWktString.toUtf8().constData() );
-      if ( srs.isValid() )
-        return srs;
+        srs = QgsCoordinateReferenceSystem::fromWkt( myWktString.toUtf8().constData() );
+        if ( srs.isValid() )
+          return srs;
+      }
     }
   }
 
@@ -4805,7 +4820,7 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
 
   GDALDriverH driver = GDALGetDatasetDriver( hDS );
   QString driverName = GDALGetDriverShortName( driver );
-  ds->canBeShared = canDriverShareSameDatasetAmongLayers( driverName );
+  ds->canBeShared = canDriverShareSameDatasetAmongLayers( driverName, updateMode, dsName );
 
   QgsOgrLayerUniquePtr layer = QgsOgrLayer::CreateForLayer(
                                  ident, layerName, ds, hLayer );
@@ -5232,7 +5247,7 @@ QgsOgrProviderUtils::DatasetWithLayers *QgsOgrProviderUtils::createDatasetWithLa
 
   GDALDriverH driver = GDALGetDatasetDriver( hDS );
   QString driverName = GDALGetDriverShortName( driver );
-  ds->canBeShared = canDriverShareSameDatasetAmongLayers( driverName );
+  ds->canBeShared = canDriverShareSameDatasetAmongLayers( driverName, updateMode, dsName );
 
   layer = QgsOgrLayer::CreateForLayer(
             ident, layerName, ds, hLayer );
@@ -5424,6 +5439,20 @@ bool QgsOgrProviderUtils::canDriverShareSameDatasetAmongLayers( const QString &d
   return driverName != QStringLiteral( "OSM" );
 }
 
+bool QgsOgrProviderUtils::canDriverShareSameDatasetAmongLayers( const QString &driverName,
+    bool updateMode,
+    const QString &dsName )
+{
+  // For .shp.zip with multiple layers, in update mode, we want that each
+  // layer has its own dataset, so that when its gets closed and reopened,
+  // the .shp.zip is updated. Otherwise if we share the same dataset, the .shp.zip
+  // would only be updated when all layers are unloaded, and thus readers will see
+  // an outdated version of the .shp.zip. This works only if editing operations are
+  // done separately on layers (which is how it works from the GUI)
+  return canDriverShareSameDatasetAmongLayers( driverName ) &&
+         !( updateMode && dsName.endsWith( QLatin1String( ".shp.zip" ), Qt::CaseInsensitive ) );
+}
+
 
 QgsOgrDatasetSharedPtr QgsOgrDataset::create( const QgsOgrProviderUtils::DatasetIdentification &ident,
     QgsOgrProviderUtils::DatasetWithLayers *ds )

diff --git a/src/core/providers/ogr/qgsogrprovider.h b/src/core/providers/ogr/qgsogrprovider.h
@@ -522,6 +522,11 @@ class CORE_EXPORT QgsOgrProviderUtils
 
     //! Whether a driver can share the same dataset handle among different layers
     static bool canDriverShareSameDatasetAmongLayers( const QString &driverName );
+
+    //! Whether a driver can share the same dataset handle among different layers
+    static bool canDriverShareSameDatasetAmongLayers( const QString &driverName,
+        bool updateMode,
+        const QString &dsName );
 };
 
 

diff --git a/src/core/qgis.cpp b/src/core/qgis.cpp
@@ -29,6 +29,7 @@
 #include "qgslogger.h"
 #include "qgswkbtypes.h"
 
+#include <gdal.h>
 #include <ogr_api.h>
 
 // Version constants
@@ -225,8 +226,16 @@ bool qgsVariantGreaterThan( const QVariant &lhs, const QVariant &rhs )
 
 QString qgsVsiPrefix( const QString &path )
 {
-  if ( path.startsWith( QLatin1String( "/vsizip/" ), Qt::CaseInsensitive ) ||
-       path.endsWith( QLatin1String( ".zip" ), Qt::CaseInsensitive ) )
+  if ( path.startsWith( QLatin1String( "/vsizip/" ), Qt::CaseInsensitive ) )
+    return QStringLiteral( "/vsizip/" );
+  else if ( path.endsWith( QLatin1String( ".shp.zip" ), Qt::CaseInsensitive ) )
+  {
+    // GDAL 3.1 Shapefile driver directly handles .shp.zip files
+    if ( GDALIdentifyDriver( path.toUtf8().constData(), nullptr ) )
+      return QString();
+    return QStringLiteral( "/vsizip/" );
+  }
+  else if ( path.endsWith( QLatin1String( ".zip" ), Qt::CaseInsensitive ) )
     return QStringLiteral( "/vsizip/" );
   else if ( path.startsWith( QLatin1String( "/vsitar/" ), Qt::CaseInsensitive ) ||
             path.endsWith( QLatin1String( ".tar" ), Qt::CaseInsensitive ) ||

diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
@@ -19,7 +19,7 @@
 import osgeo.ogr
 import sys
 
-from qgis.core import QgsSettings, QgsFeature, QgsField, QgsGeometry, QgsVectorLayer, QgsFeatureRequest, QgsVectorDataProvider, QgsWkbTypes
+from qgis.core import QgsApplication, QgsSettings, QgsFeature, QgsField, QgsGeometry, QgsVectorLayer, QgsFeatureRequest, QgsVectorDataProvider, QgsWkbTypes
 from qgis.PyQt.QtCore import QVariant
 from qgis.testing import start_app, unittest
 from utilities import unitTestDataPath
@@ -713,6 +713,107 @@ def testMultipatch(self):
         self.assertEqual(f.geometry().constGet().asWkt(),
                          'MultiPolygonZ (((0 0 0, 0 1 0, 1 1 0, 0 0 0)),((0 0 0, 1 1 0, 1 0 0, 0 0 0)),((0 0 0, 0 -1 0, 1 -1 0, 0 0 0)),((0 0 0, 1 -1 0, 1 0 0, 0 0 0)))')
 
+    def testShzSupport(self):
+        ''' Test support for single layer compressed shapefiles (.shz) '''
+
+        if int(osgeo.gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(3, 1, 0):
+            return
+
+        tmpfile = os.path.join(self.basetestpath, 'testShzSupport.shz')
+        ds = osgeo.ogr.GetDriverByName('ESRI Shapefile').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('testShzSupport', geom_type=osgeo.ogr.wkbPoint)
+        lyr.CreateField(osgeo.ogr.FieldDefn('attr', osgeo.ogr.OFTInteger))
+        f = osgeo.ogr.Feature(lyr.GetLayerDefn())
+        f.SetField('attr', 1)
+        f.SetGeometry(osgeo.ogr.CreateGeometryFromWkt('POINT(0 0)'))
+        lyr.CreateFeature(f)
+        f = None
+        ds = None
+
+        vl = QgsVectorLayer(tmpfile, 'test', 'ogr')
+        self.assertTrue(vl.isValid())
+        self.assertEqual(vl.wkbType(), QgsWkbTypes.Point)
+        f = next(vl.getFeatures())
+        assert f['attr'] == 1
+        self.assertEqual(f.geometry().constGet().asWkt(), 'Point (0 0)')
+
+        self.assertTrue(vl.startEditing())
+        self.assertTrue(vl.changeAttributeValue(f.id(), 0, -1))
+        self.assertTrue(vl.commitChanges())
+
+        f = next(vl.getFeatures())
+        assert f['attr'] == -1
+
+        # Check DataItem
+        registry = QgsApplication.dataItemProviderRegistry()
+        ogrprovider = next(provider for provider in registry.providers() if provider.name() == 'OGR')
+        item = ogrprovider.createDataItem(tmpfile, None)
+        self.assertTrue(item.uri().endswith('testShzSupport.shz'))
+
+    def testShpZipSupport(self):
+        ''' Test support for multi layer compressed shapefiles (.shp.zip) '''
+
+        if int(osgeo.gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(3, 1, 0):
+            return
+
+        tmpfile = os.path.join(self.basetestpath, 'testShpZipSupport.shp.zip')
+        ds = osgeo.ogr.GetDriverByName('ESRI Shapefile').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('layer1', geom_type=osgeo.ogr.wkbPoint)
+        lyr.CreateField(osgeo.ogr.FieldDefn('attr', osgeo.ogr.OFTInteger))
+        f = osgeo.ogr.Feature(lyr.GetLayerDefn())
+        f.SetField('attr', 1)
+        f.SetGeometry(osgeo.ogr.CreateGeometryFromWkt('POINT(0 0)'))
+        lyr.CreateFeature(f)
+        f = None
+        lyr = ds.CreateLayer('layer2', geom_type=osgeo.ogr.wkbMultiLineString)
+        lyr.CreateField(osgeo.ogr.FieldDefn('attr', osgeo.ogr.OFTInteger))
+        f = osgeo.ogr.Feature(lyr.GetLayerDefn())
+        f.SetField('attr', 2)
+        f.SetGeometry(osgeo.ogr.CreateGeometryFromWkt('LINESTRING(0 0,1 1)'))
+        lyr.CreateFeature(f)
+        f = None
+        ds = None
+
+        vl1 = QgsVectorLayer(tmpfile + '|layername=layer1', 'test', 'ogr')
+        vl2 = QgsVectorLayer(tmpfile + '|layername=layer2', 'test', 'ogr')
+        self.assertTrue(vl1.isValid())
+        self.assertTrue(vl2.isValid())
+        self.assertEqual(vl1.wkbType(), QgsWkbTypes.Point)
+        self.assertEqual(vl2.wkbType(), QgsWkbTypes.MultiLineString)
+        f1 = next(vl1.getFeatures())
+        f2 = next(vl2.getFeatures())
+        assert f1['attr'] == 1
+        self.assertEqual(f1.geometry().constGet().asWkt(), 'Point (0 0)')
+        assert f2['attr'] == 2
+        self.assertEqual(f2.geometry().constGet().asWkt(), 'MultiLineString ((0 0, 1 1))')
+
+        self.assertTrue(vl1.startEditing())
+        self.assertTrue(vl2.startEditing())
+        self.assertTrue(vl1.changeAttributeValue(f1.id(), 0, -1))
+        self.assertTrue(vl2.changeAttributeValue(f2.id(), 0, -2))
+        self.assertTrue(vl1.commitChanges())
+        self.assertTrue(vl2.commitChanges())
+
+        f = next(vl1.getFeatures())
+        assert f['attr'] == -1
+
+        f = next(vl2.getFeatures())
+        assert f['attr'] == -2
+
+        # Check DataItem
+        registry = QgsApplication.dataItemProviderRegistry()
+        ogrprovider = next(provider for provider in registry.providers() if provider.name() == 'OGR')
+        item = ogrprovider.createDataItem(tmpfile, None)
+        children = item.createChildren()
+        self.assertEqual(len(children), 2)
+        uris = sorted([children[i].uri() for i in range(2)])
+        self.assertIn('testShpZipSupport.shp.zip|layername=layer1', uris[0])
+        self.assertIn('testShpZipSupport.shp.zip|layername=layer2', uris[1])
+
+        gdalprovider = next(provider for provider in registry.providers() if provider.name() == 'GDAL')
+        item = gdalprovider.createDataItem(tmpfile, None)
+        assert not item
+
 
 if __name__ == '__main__':
     unittest.main()