From de7e3fa8a5d85ea4918b056df54cc43e068d7ac2 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 26 Jun 2019 23:18:47 +0200 Subject: [PATCH] [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: https://github.com/OSGeo/gdal/pull/1676 --- src/core/providers/gdal/qgsgdaldataitems.cpp | 6 ++ src/core/providers/ogr/qgsogrdataitems.cpp | 11 +- src/core/providers/ogr/qgsogrprovider.cpp | 75 +++++++++----- src/core/providers/ogr/qgsogrprovider.h | 5 + src/core/qgis.cpp | 13 ++- tests/src/python/test_provider_shapefile.py | 103 ++++++++++++++++++- 6 files changed, 186 insertions(+), 27 deletions(-) diff --git a/src/core/providers/gdal/qgsgdaldataitems.cpp b/src/core/providers/gdal/qgsgdaldataitems.cpp index 64b35f68b70..97f115a4c47 100644 --- 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 index 43458043077..fa7a71e39a4 100644 --- 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 index 2199bd5a803..175b37a1e0a 100644 --- 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 ) ) + int index = uri.indexOf( QLatin1String( ".shp" ), Qt::CaseInsensitive ); + if ( index > 0 ) { - 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" ) ); + 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 index be3916ee232..0f4694756fe 100644 --- 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 index 04b460f9b97..02b5dbe2c5c 100644 --- a/src/core/qgis.cpp +++ b/src/core/qgis.cpp @@ -29,6 +29,7 @@ #include "qgslogger.h" #include "qgswkbtypes.h" +#include #include // 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 index 352ef5716e6..358368fe56c 100644 --- a/tests/src/python/test_provider_shapefile.py +++ b/tests/src/python/test_provider_shapefile.py @@ -19,7 +19,7 @@ import osgeo.gdal 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 @@ class TestPyQgsShapefileProvider(unittest.TestCase, ProviderTestCase): 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()