From dc18b5b36bfc10605d4c2905329e0ccd937f0828 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 11 Apr 2016 19:37:03 +0200 Subject: [PATCH] [BUGFIX / FEATURE] [OGR] Allow concurrent edition of Shapefiles and Tabfiles in QGIS & MapInfo - Closes https://hub.qgis.org/issues/14378 - Adds new virtual methods in QgsDataProvider(): enterUpdateMode() and leaveUpdateMode() and implement them in the OGR provider. Limited to shapefiles and tabfiles - Implements QgsOGRProvider:reloadData() - Robustify OGR provider methods so they don't crash if dataset re-opening fails. --- ci/travis/linux/qt5/blacklist.txt | 1 + python/core/qgsdataprovider.sip | 41 ++++ src/core/qgsdataprovider.h | 41 ++++ src/core/qgsvectorlayer.cpp | 7 + src/providers/ogr/qgsogrfeatureiterator.cpp | 16 +- src/providers/ogr/qgsogrprovider.cpp | 241 ++++++++++++++++++-- src/providers/ogr/qgsogrprovider.h | 30 ++- tests/src/python/CMakeLists.txt | 1 + tests/src/python/test_provider_ogr.py | 71 ++++++ tests/src/python/test_provider_shapefile.py | 178 ++++++++++++++- tests/src/python/test_provider_tabfile.py | 25 +- 11 files changed, 624 insertions(+), 28 deletions(-) create mode 100644 tests/src/python/test_provider_ogr.py diff --git a/ci/travis/linux/qt5/blacklist.txt b/ci/travis/linux/qt5/blacklist.txt index b208440ac0f..821b4e71246 100755 --- a/ci/travis/linux/qt5/blacklist.txt +++ b/ci/travis/linux/qt5/blacklist.txt @@ -8,6 +8,7 @@ PyQgsLocalServer PyQgsMapUnitScale PyQgsMemoryProvider PyQgsNetworkContentFetcher +PyQgsOGRProvider PyQgsPalLabelingBase PyQgsPalLabelingCanvas PyQgsPalLabelingComposer diff --git a/python/core/qgsdataprovider.sip b/python/core/qgsdataprovider.sip index 7ea28ec9d30..91936b22fb4 100644 --- a/python/core/qgsdataprovider.sip +++ b/python/core/qgsdataprovider.sip @@ -223,6 +223,47 @@ class QgsDataProvider : QObject */ virtual void invalidateConnections( const QString& connection ); + /** Enter update mode. + * + * This is aimed at providers that can open differently the connection to + * the datasource, according it to be in update mode or in read-only mode. + * A call to this method shall be balanced with a call to leaveUpdateMode(), + * if this method returns true. + * + * Most providers will have an empty implementation for that method. + * + * For backward compatibility, providers that implement enterUpdateMode() should + * still make sure to allow editing operations to work even if enterUpdateMode() + * is not explicitly called. + * + * Several successive calls to enterUpdateMode() can be done. So there is + * a concept of stack of calls that must be handled by the provider. Only the first + * call to enterUpdateMode() will really turn update mode on. + * + * @return true in case of success (or no-op implementation), false in case of failure + * + * @note added in QGIS 2.16 + */ + virtual bool enterUpdateMode(); + + /** Leave update mode. + * + * This is aimed at providers that can open differently the connection to + * the datasource, according it to be in update mode or in read-only mode. + * This method shall be balanced with a succesful call to enterUpdateMode(). + * + * Most providers will have an empty implementation for that method. + * + * Several successive calls to enterUpdateMode() can be done. So there is + * a concept of stack of calls that must be handled by the provider. Only the last + * call to leaveUpdateMode() will really turn update mode off. + * + * @return true in case of success (or no-op implementation), false in case of failure + * + * @note added in QGIS 2.16 + */ + virtual bool leaveUpdateMode(); + signals: /** diff --git a/src/core/qgsdataprovider.h b/src/core/qgsdataprovider.h index b4b3f6e6eb6..ea496bc9b02 100644 --- a/src/core/qgsdataprovider.h +++ b/src/core/qgsdataprovider.h @@ -311,6 +311,47 @@ class CORE_EXPORT QgsDataProvider : public QObject */ virtual void invalidateConnections( const QString& connection ) { Q_UNUSED( connection ); } + /** Enter update mode. + * + * This is aimed at providers that can open differently the connection to + * the datasource, according it to be in update mode or in read-only mode. + * A call to this method shall be balanced with a call to leaveUpdateMode(), + * if this method returns true. + * + * Most providers will have an empty implementation for that method. + * + * For backward compatibility, providers that implement enterUpdateMode() should + * still make sure to allow editing operations to work even if enterUpdateMode() + * is not explicitly called. + * + * Several successive calls to enterUpdateMode() can be done. So there is + * a concept of stack of calls that must be handled by the provider. Only the first + * call to enterUpdateMode() will really turn update mode on. + * + * @return true in case of success (or no-op implementation), false in case of failure. + * + * @note added in QGIS 2.16 + */ + virtual bool enterUpdateMode() { return true; } + + /** Leave update mode. + * + * This is aimed at providers that can open differently the connection to + * the datasource, according it to be in update mode or in read-only mode. + * This method shall be balanced with a succesful call to enterUpdateMode(). + * + * Most providers will have an empty implementation for that method. + * + * Several successive calls to enterUpdateMode() can be done. So there is + * a concept of stack of calls that must be handled by the provider. Only the last + * call to leaveUpdateMode() will really turn update mode off. + * + * @return true in case of success (or no-op implementation), false in case of failure. + * + * @note added in QGIS 2.16 + */ + virtual bool leaveUpdateMode() { return true; } + signals: /** diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp index 8c20f119e04..1ca099bd5b5 100644 --- a/src/core/qgsvectorlayer.cpp +++ b/src/core/qgsvectorlayer.cpp @@ -377,6 +377,7 @@ void QgsVectorLayer::reload() if ( mDataProvider ) { mDataProvider->reloadData(); + updateFields(); } } @@ -1454,6 +1455,8 @@ bool QgsVectorLayer::startEditing() emit beforeEditingStarted(); + mDataProvider->enterUpdateMode(); + if ( mDataProvider->transaction() ) { mEditBuffer = new QgsVectorLayerEditPassthrough( this ); @@ -2417,6 +2420,8 @@ bool QgsVectorLayer::commitChanges() updateFields(); mDataProvider->updateExtents(); + mDataProvider->leaveUpdateMode(); + emit repaintRequested(); return success; @@ -2467,6 +2472,8 @@ bool QgsVectorLayer::rollBack( bool deleteBuffer ) if ( rollbackExtent ) updateExtents(); + mDataProvider->leaveUpdateMode(); + emit repaintRequested(); return true; } diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index cfe7dc03f42..e9decbecb3c 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -46,6 +46,10 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool mFeatureFetched = false; mConn = QgsOgrConnPool::instance()->acquireConnection( mSource->mProvider->dataSourceUri() ); + if ( mConn->ds == nullptr ) + { + return; + } if ( mSource->mLayerName.isNull() ) { @@ -55,10 +59,18 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool { ogrLayer = OGR_DS_GetLayerByName( mConn->ds, TO8( mSource->mLayerName ) ); } + if ( ogrLayer == nullptr ) + { + return; + } if ( !mSource->mSubsetString.isEmpty() ) { ogrLayer = QgsOgrProviderUtils::setSubsetString( ogrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString ); + if ( ogrLayer == nullptr ) + { + return; + } mSubsetStringSet = true; } @@ -212,7 +224,7 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature ) { feature.setValid( false ); - if ( mClosed ) + if ( mClosed || ogrLayer == nullptr ) return false; if ( mRequest.filterType() == QgsFeatureRequest::FilterFid ) @@ -256,7 +268,7 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature ) bool QgsOgrFeatureIterator::rewind() { - if ( mClosed ) + if ( mClosed || ogrLayer == nullptr ) return false; OGR_L_ResetReading( ogrLayer ); diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp index 020628833d4..719b653b903 100644 --- a/src/providers/ogr/qgsogrprovider.cpp +++ b/src/providers/ogr/qgsogrprovider.cpp @@ -139,7 +139,7 @@ bool QgsOgrProvider::convertField( QgsField &field, const QTextCodec &encoding ) void QgsOgrProvider::repack() { - if ( ogrDriverName != "ESRI Shapefile" || !ogrOrigLayer ) + if ( !mValid || ogrDriverName != "ESRI Shapefile" || !ogrOrigLayer ) return; QByteArray layerName = OGR_FD_GetName( OGR_L_GetLayerDefn( ogrOrigLayer ) ); @@ -284,7 +284,11 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri ) , mOGRGeomType( wkbUnknown ) , mFeaturesCounted( -1 ) , mWriteAccess( false ) + , mWriteAccessPossible( false ) + , mDynamicWriteAccess( false ) , mShapefileMayBeCorrupted( false ) + , mUpdateModeStackDepth( 0 ) + , mCapabilities( 0 ) { QgsApplication::registerOgrDrivers(); @@ -355,7 +359,7 @@ QgsOgrProvider::QgsOgrProvider( QString const & uri ) } } - open(); + open( OpenModeInitial ); mNativeTypes << QgsVectorDataProvider::NativeType( tr( "Whole number (integer)" ), "integer", QVariant::Int, 1, 10 ) @@ -392,6 +396,9 @@ bool QgsOgrProvider::setSubsetString( const QString& theSQL, bool updateFeatureC { QgsCPLErrorHandler handler; + if ( ogrDataSource == nullptr ) + return false; + if ( theSQL == mSubsetString && mFeaturesCounted >= 0 ) return true; @@ -722,6 +729,8 @@ void QgsOgrProvider::loadFields() QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() ); //the attribute fields need to be read again when the encoding changes mAttributeFields.clear(); + if ( ogrLayer == nullptr ) + return; if ( mOgrGeometryTypeFilter != wkbUnknown ) { @@ -947,6 +956,8 @@ void QgsOgrProvider::updateExtents() size_t QgsOgrProvider::layerCount() const { + if ( !mValid ) + return 0; return OGR_DS_GetLayerCount( ogrDataSource ); } // QgsOgrProvider::layerCount() @@ -1168,6 +1179,9 @@ bool QgsOgrProvider::addFeature( QgsFeature& f ) bool QgsOgrProvider::addFeatures( QgsFeatureList & flist ) { + if ( !doInitialActionsForEdition() ) + return false; + setRelevantFields( ogrLayer, true, attributeIndexes() ); bool returnvalue = true; @@ -1194,6 +1208,9 @@ bool QgsOgrProvider::addFeatures( QgsFeatureList & flist ) bool QgsOgrProvider::addAttributes( const QList &attributes ) { + if ( !doInitialActionsForEdition() ) + return false; + bool returnvalue = true; for ( QList::const_iterator iter = attributes.begin(); iter != attributes.end(); ++iter ) @@ -1251,6 +1268,9 @@ bool QgsOgrProvider::addAttributes( const QList &attributes ) bool QgsOgrProvider::deleteAttributes( const QgsAttributeIds &attributes ) { + if ( !doInitialActionsForEdition() ) + return false; + #if defined(GDAL_VERSION_NUM) && GDAL_VERSION_NUM >= 1900 bool res = true; QList attrsLst = attributes.toList(); @@ -1282,6 +1302,9 @@ bool QgsOgrProvider::deleteAttributes( const QgsAttributeIds &attributes ) bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_map ) { + if ( !doInitialActionsForEdition() ) + return false; + if ( attr_map.isEmpty() ) return true; @@ -1414,6 +1437,9 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map ) { + if ( !doInitialActionsForEdition() ) + return false; + OGRFeatureH theOGRFeature = nullptr; OGRGeometryH theNewGeometry = nullptr; @@ -1481,6 +1507,9 @@ bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map ) bool QgsOgrProvider::createSpatialIndex() { + if ( !doInitialActionsForEdition() ) + return false; + if ( ogrDriverName != "ESRI Shapefile" ) return false; @@ -1501,6 +1530,9 @@ bool QgsOgrProvider::createSpatialIndex() bool QgsOgrProvider::createAttributeIndex( int field ) { + if ( !doInitialActionsForEdition() ) + return false; + QByteArray quotedLayerName = quotedIdentifier( OGR_FD_GetName( OGR_L_GetLayerDefn( ogrOrigLayer ) ) ); QByteArray dropSql = "DROP INDEX ON " + quotedLayerName; OGR_DS_ExecuteSQL( ogrDataSource, dropSql.constData(), OGR_L_GetSpatialFilter( ogrOrigLayer ), nullptr ); @@ -1515,6 +1547,9 @@ bool QgsOgrProvider::createAttributeIndex( int field ) bool QgsOgrProvider::deleteFeatures( const QgsFeatureIds & id ) { + if ( !doInitialActionsForEdition() ) + return false; + bool returnvalue = true; for ( QgsFeatureIds::const_iterator it = id.begin(); it != id.end(); ++it ) { @@ -1540,6 +1575,9 @@ bool QgsOgrProvider::deleteFeatures( const QgsFeatureIds & id ) bool QgsOgrProvider::deleteFeature( QgsFeatureId id ) { + if ( !doInitialActionsForEdition() ) + return false; + if ( FID_TO_NUMBER( id ) > std::numeric_limits::max() ) { pushError( tr( "OGR error on feature %1: id too large" ).arg( id ) ); @@ -1557,7 +1595,27 @@ bool QgsOgrProvider::deleteFeature( QgsFeatureId id ) return true; } +bool QgsOgrProvider::doInitialActionsForEdition() +{ + if ( !mValid ) + return false; + + if ( !mWriteAccess && mWriteAccessPossible && mDynamicWriteAccess ) + { + QgsDebugMsg( "Enter update mode implictly" ); + if ( !enterUpdateMode() ) + return false; + } + + return true; +} + int QgsOgrProvider::capabilities() const +{ + return mCapabilities; +} + +void QgsOgrProvider::computeCapabilities() { int ability = 0; @@ -1580,19 +1638,19 @@ int QgsOgrProvider::capabilities() const ability |= QgsVectorDataProvider::SelectAtId | QgsVectorDataProvider::SelectGeometryAtId; } - if ( mWriteAccess && OGR_L_TestCapability( ogrLayer, "SequentialWrite" ) ) + if ( mWriteAccessPossible && OGR_L_TestCapability( ogrLayer, "SequentialWrite" ) ) // true if the CreateFeature() method works for this layer. { ability |= QgsVectorDataProvider::AddFeatures; } - if ( mWriteAccess && OGR_L_TestCapability( ogrLayer, "DeleteFeature" ) ) + if ( mWriteAccessPossible && OGR_L_TestCapability( ogrLayer, "DeleteFeature" ) ) // true if this layer can delete its features { ability |= DeleteFeatures; } - if ( mWriteAccess && OGR_L_TestCapability( ogrLayer, "RandomWrite" ) ) + if ( mWriteAccessPossible && OGR_L_TestCapability( ogrLayer, "RandomWrite" ) ) // true if the SetFeature() method is operational on this layer. { // TODO According to http://shapelib.maptools.org/ (Shapefile C Library V1.2) @@ -1640,12 +1698,12 @@ int QgsOgrProvider::capabilities() const } #endif - if ( mWriteAccess && OGR_L_TestCapability( ogrLayer, "CreateField" ) ) + if ( mWriteAccessPossible && OGR_L_TestCapability( ogrLayer, "CreateField" ) ) { ability |= AddAttributes; } - if ( mWriteAccess && OGR_L_TestCapability( ogrLayer, "DeleteField" ) ) + if ( mWriteAccessPossible && OGR_L_TestCapability( ogrLayer, "DeleteField" ) ) { ability |= DeleteAttributes; } @@ -1695,7 +1753,7 @@ int QgsOgrProvider::capabilities() const #endif } - return ability; + mCapabilities = ability; } @@ -2493,6 +2551,8 @@ QgsCoordinateReferenceSystem QgsOgrProvider::crs() QgsDebugMsg( "Entering." ); QgsCoordinateReferenceSystem srs; + if ( !mValid ) + return srs; if ( ogrDriver ) { @@ -2544,7 +2604,7 @@ void QgsOgrProvider::uniqueValues( int index, QList &uniqueValues, int { uniqueValues.clear(); - if ( index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= mAttributeFields.count() ) return; const QgsField& fld = mAttributeFields.at( index ); @@ -2591,7 +2651,7 @@ void QgsOgrProvider::uniqueValues( int index, QList &uniqueValues, int QVariant QgsOgrProvider::minimumValue( int index ) { - if ( index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= mAttributeFields.count() ) { return QVariant(); } @@ -2630,7 +2690,7 @@ QVariant QgsOgrProvider::minimumValue( int index ) QVariant QgsOgrProvider::maximumValue( int index ) { - if ( index < 0 || index >= mAttributeFields.count() ) + if ( !mValid || index < 0 || index >= mAttributeFields.count() ) { return QVariant(); } @@ -2741,7 +2801,9 @@ bool QgsOgrProvider::syncToDisc() close(); QgsOgrConnPool::instance()->invalidateConnections( dataSourceUri() ); QFile::remove( sbnIndexFile ); - open(); + open( OpenModeSameAsCurrent ); + if ( !mValid ) + return false; } } @@ -2766,6 +2828,12 @@ bool QgsOgrProvider::syncToDisc() void QgsOgrProvider::recalculateFeatureCount() { + if ( ogrLayer == nullptr ) + { + mFeaturesCounted = 0; + return; + } + OGRGeometryH filter = OGR_L_GetSpatialFilter( ogrLayer ); if ( filter ) { @@ -2858,7 +2926,7 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH return OGR_DS_ExecuteSQL( ds, sql.constData(), nullptr, nullptr ); } -void QgsOgrProvider::open() +void QgsOgrProvider::open( OpenMode mode ) { bool openReadOnly = false; @@ -2889,24 +2957,31 @@ void QgsOgrProvider::open() mFilePath += ",tables=" + mLayerName; } + if ( mode == OpenModeForceReadOnly ) + openReadOnly = true; + else if ( mode == OpenModeSameAsCurrent && !mWriteAccess ) + openReadOnly = true; + // first try to open in update mode (unless specified otherwise) if ( !openReadOnly ) ogrDataSource = OGROpen( TO8F( mFilePath ), true, &ogrDriver ); + mValid = false; if ( ogrDataSource ) { mWriteAccess = true; + mWriteAccessPossible = true; } else { - QgsDebugMsg( "OGR failed to opened in update mode, trying in read-only mode" ); + mWriteAccess = false; + if ( !openReadOnly ) + { + QgsDebugMsg( "OGR failed to opened in update mode, trying in read-only mode" ); + } // try to open read-only ogrDataSource = OGROpen( TO8F( mFilePath ), false, &ogrDriver ); - - //TODO Need to set a flag or something to indicate that the layer - //TODO is in read-only mode, otherwise edit ops will fail - //TODO: capabilities() should now reflect this; need to test. } if ( ogrDataSource ) @@ -2935,6 +3010,10 @@ void QgsOgrProvider::open() mValid = setSubsetString( mSubsetString ); if ( mValid ) { + if ( mode == OpenModeInitial ) + { + computeCapabilities(); + } QgsDebugMsg( "Data source is valid" ); } else @@ -2951,6 +3030,59 @@ void QgsOgrProvider::open() { QgsMessageLog::logMessage( tr( "Data source is invalid (%1)" ).arg( QString::fromUtf8( CPLGetLastErrorMsg() ) ), tr( "OGR" ) ); } + + // For shapefiles or MapInfo .tab, so as to allow concurrent opening between + // QGIS and MapInfo, we go back to read-only mode for now. + // We limit to those drivers as re-opening is relatively cheap (other drivers + // like GeoJSON might do full content ingestion for example) + if ( mValid && mode == OpenModeInitial && mWriteAccess && + ( ogrDriverName == "ESRI Shapefile" || ogrDriverName == "MapInfo File" ) ) + { + OGR_DS_Destroy( ogrDataSource ); + ogrLayer = ogrOrigLayer = nullptr; + mValid = false; + + ogrDataSource = OGROpen( TO8F( mFilePath ), false, &ogrDriver ); + + mWriteAccess = false; + + if ( ogrDataSource ) + { + // We get the layer which was requested by the uri. The layername + // has precedence over the layerid if both are given. + if ( mLayerName.isNull() ) + { + ogrOrigLayer = OGR_DS_GetLayer( ogrDataSource, mLayerIndex ); + } + else + { + ogrOrigLayer = OGR_DS_GetLayerByName( ogrDataSource, TO8( mLayerName ) ); + } + + ogrLayer = ogrOrigLayer; + } + if ( ogrLayer != nullptr ) + { + mValid = true; + mDynamicWriteAccess = true; + + if ( !mSubsetString.isEmpty() ) + { + int featuresCountedBackup = mFeaturesCounted; + mFeaturesCounted = -1; + mValid = setSubsetString( mSubsetString, false ); + mFeaturesCounted = featuresCountedBackup; + } + } + } + + // For debug/testing purposes + if ( !mValid ) + setProperty( "_debug_open_mode", "invalid" ); + else if ( mWriteAccess ) + setProperty( "_debug_open_mode", "read-write" ); + else + setProperty( "_debug_open_mode", "read-only" ); } void QgsOgrProvider::close() @@ -2965,12 +3097,85 @@ void QgsOgrProvider::close() OGR_DS_Destroy( ogrDataSource ); } ogrDataSource = nullptr; + ogrLayer = nullptr; + ogrOrigLayer = nullptr; + mValid = false; + setProperty( "_debug_open_mode", "invalid" ); updateExtents(); QgsOgrConnPool::instance()->unref( mFilePath ); } +void QgsOgrProvider::reloadData() +{ + forceReload(); + close(); + open( OpenModeSameAsCurrent ); + if ( !mValid ) + pushError( tr( "Cannot reopen datasource %1" ).arg( dataSourceUri() ) ); +} + +bool QgsOgrProvider::enterUpdateMode() +{ + if ( !mWriteAccessPossible ) + { + return false; + } + if ( mWriteAccess ) + { + ++mUpdateModeStackDepth; + return true; + } + if ( mUpdateModeStackDepth == 0 ) + { + Q_ASSERT( mDynamicWriteAccess ); + QgsDebugMsg( QString( "Reopening %1 in update mode" ).arg( dataSourceUri() ) ); + close(); + open( OpenModeForceUpdate ); + if ( ogrDataSource == nullptr || !mWriteAccess ) + { + QgsMessageLog::logMessage( tr( "Cannot reopen datasource %1 in update mode" ).arg( dataSourceUri() ), tr( "OGR" ) ); + pushError( tr( "Cannot reopen datasource %1 in update mode" ).arg( dataSourceUri() ) ); + return false; + } + } + ++mUpdateModeStackDepth; + return true; +} + +bool QgsOgrProvider::leaveUpdateMode() +{ + if ( !mWriteAccessPossible ) + { + return false; + } + --mUpdateModeStackDepth; + if ( mUpdateModeStackDepth < 0 ) + { + QgsMessageLog::logMessage( tr( "Unbalanced call to leaveUpdateMode() w.r.t. enterUpdateMode()" ), tr( "OGR" ) ); + mUpdateModeStackDepth = 0; + return false; + } + if ( !mDynamicWriteAccess ) + { + return true; + } + if ( mUpdateModeStackDepth == 0 ) + { + QgsDebugMsg( QString( "Reopening %1 in read-only mode" ).arg( dataSourceUri() ) ); + close(); + open( OpenModeForceReadOnly ); + if ( ogrDataSource == nullptr ) + { + QgsMessageLog::logMessage( tr( "Cannot reopen datasource %1 in read-only mode" ).arg( dataSourceUri() ), tr( "OGR" ) ); + pushError( tr( "Cannot reopen datasource %1 in read-only mode" ).arg( dataSourceUri() ) ); + return false; + } + } + return true; +} + // --------------------------------------------------------------------------- QGISEXTERN QgsVectorLayerImport::ImportError createEmptyLayer( diff --git a/src/providers/ogr/qgsogrprovider.h b/src/providers/ogr/qgsogrprovider.h index c3f94a4ea54..e7910722bfa 100644 --- a/src/providers/ogr/qgsogrprovider.h +++ b/src/providers/ogr/qgsogrprovider.h @@ -174,6 +174,9 @@ class QgsOgrProvider : public QgsVectorDataProvider virtual void setEncoding( const QString& e ) override; + virtual bool enterUpdateMode() override; + + virtual bool leaveUpdateMode() override; /** Return vector file filter string * @@ -271,6 +274,9 @@ class QgsOgrProvider : public QgsVectorDataProvider */ void forceReload() override; + /** Closes and re-open the datasource */ + void reloadData() override; + protected: /** Loads fields from input file to member attributeFields */ void loadFields(); @@ -287,7 +293,15 @@ class QgsOgrProvider : public QgsVectorDataProvider /** Clean shapefile from features which are marked as deleted */ void repack(); - void open(); + enum OpenMode + { + OpenModeInitial, + OpenModeSameAsCurrent, + OpenModeForceReadOnly, + OpenModeForceUpdate, + }; + + void open( OpenMode mode ); void close(); private: @@ -356,10 +370,24 @@ class QgsOgrProvider : public QgsVectorDataProvider /** Whether the file is opened in write mode*/ bool mWriteAccess; + /** Whether the file can potentially be opened in write mode (but not necessarily currently) */ + bool mWriteAccessPossible; + + /** Whether the open mode of the datasource changes w.r.t calls to enterUpdateMode() / leaveUpdateMode() */ + bool mDynamicWriteAccess; + bool mShapefileMayBeCorrupted; /** Converts the geometry to the layer type if necessary. Takes ownership of the passed geometry */ OGRGeometryH ConvertGeometryIfNecessary( OGRGeometryH ); + + int mUpdateModeStackDepth; + + void computeCapabilities(); + + int mCapabilities; + + bool doInitialActionsForEdition(); }; diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt index 0e3d7220218..8f57dcf5aa9 100644 --- a/tests/src/python/CMakeLists.txt +++ b/tests/src/python/CMakeLists.txt @@ -62,6 +62,7 @@ ADD_PYTHON_TEST(PyQgsRulebasedRenderer test_qgsrulebasedrenderer.py) ADD_PYTHON_TEST(PyQgsSingleSymbolRenderer test_qgssinglesymbolrenderer.py) ADD_PYTHON_TEST(PyQgsShapefileProvider test_provider_shapefile.py) ADD_PYTHON_TEST(PyQgsTabfileProvider test_provider_tabfile.py) +ADD_PYTHON_TEST(PyQgsOGRProvider test_provider_ogr.py) ADD_PYTHON_TEST(PyQgsSpatialIndex test_qgsspatialindex.py) ADD_PYTHON_TEST(PyQgsSpatialiteProvider test_provider_spatialite.py) ADD_PYTHON_TEST(PyQgsSymbolLayerV2 test_qgssymbollayerv2.py) diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py new file mode 100644 index 00000000000..8b104ce58ab --- /dev/null +++ b/tests/src/python/test_provider_ogr.py @@ -0,0 +1,71 @@ +# -*- coding: utf-8 -*- +"""QGIS Unit tests for the non-shapefile, non-tabfile datasources handled by OGR provider. + +.. note:: This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +""" +__author__ = 'Even Rouault' +__date__ = '2016-04-11' +__copyright__ = 'Copyright 2016, Even Rouault' +# This will get replaced with a git SHA1 when you do a git archive +__revision__ = '$Format:%H$' + +import os +import shutil +import tempfile + +from qgis.core import QgsVectorLayer, QgsVectorDataProvider +from qgis.testing import ( + start_app, + unittest +) +from utilities import unitTestDataPath + +start_app() +TEST_DATA_DIR = unitTestDataPath() + +# Note - doesn't implement ProviderTestCase as most OGR provider is tested by the shapefile provider test + + +class PyQgsOGRProvider(unittest.TestCase): + + @classmethod + def setUpClass(cls): + """Run before all tests""" + # Create test layer + cls.basetestpath = tempfile.mkdtemp() + cls.datasource = os.path.join(cls.basetestpath, 'test.csv') + with open(cls.datasource, 'wt') as f: + f.write('id,WKT\n') + f.write('1,POINT(2 49)\n') + + cls.dirs_to_cleanup = [cls.basetestpath] + + @classmethod + def tearDownClass(cls): + """Run after all tests""" + for dirname in cls.dirs_to_cleanup: + shutil.rmtree(dirname, True) + + def testUpdateMode(self): + + vl = QgsVectorLayer(u'{}|layerid=0'.format(self.datasource), u'test', u'ogr') + self.assertTrue(vl.isValid()) + caps = vl.dataProvider().capabilities() + self.assertTrue(caps & QgsVectorDataProvider.AddFeatures) + + self.assertEqual(vl.dataProvider().property("_debug_open_mode"), "read-write") + + # No-op + self.assertTrue(vl.dataProvider().enterUpdateMode()) + self.assertEqual(vl.dataProvider().property("_debug_open_mode"), "read-write") + + # No-op + self.assertTrue(vl.dataProvider().leaveUpdateMode()) + self.assertEqual(vl.dataProvider().property("_debug_open_mode"), "read-write") + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py index 29f027c92f2..b4291787671 100644 --- a/tests/src/python/test_provider_shapefile.py +++ b/tests/src/python/test_provider_shapefile.py @@ -12,20 +12,25 @@ __copyright__ = 'Copyright 2015, The QGIS Project' # This will get replaced with a git SHA1 when you do a git archive __revision__ = '$Format:%H$' -import qgis # NOQA - import os import tempfile import shutil import glob import osgeo.gdal +import sys -from qgis.core import QgsVectorLayer, QgsFeatureRequest -from qgis.PyQt.QtCore import QSettings +from qgis.core import QgsFeature, QgsField, QgsGeometry, QgsVectorLayer, QgsFeatureRequest, QgsVectorDataProvider +from qgis.PyQt.QtCore import QSettings, QVariant from qgis.testing import start_app, unittest from utilities import unitTestDataPath from providertestbase import ProviderTestCase +try: + from osgeo import gdal + gdal_ok = True +except: + gdal_ok = False + start_app() TEST_DATA_DIR = unitTestDataPath() @@ -55,11 +60,13 @@ class TestPyQgsShapefileProvider(unittest.TestCase, ProviderTestCase): assert (cls.vl_poly.isValid()) cls.poly_provider = cls.vl_poly.dataProvider() + cls.dirs_to_cleanup = [cls.basetestpath, cls.repackfilepath] + @classmethod def tearDownClass(cls): """Run after all tests""" - shutil.rmtree(cls.basetestpath, True) - shutil.rmtree(cls.repackfilepath, True) + for dirname in cls.dirs_to_cleanup: + shutil.rmtree(dirname, True) def enableCompiler(self): QSettings().setValue(u'/qgis/compileExpressions', True) @@ -128,6 +135,165 @@ class TestPyQgsShapefileProvider(unittest.TestCase, ProviderTestCase): self.assertTrue(vl.commitChanges()) self.assertTrue(vl.selectedFeatureCount() == 0 or vl.selectedFeatures()[0]['pk'] == 1) + def testUpdateMode(self): + """ Test that on-the-fly re-opening in update/read-only mode works """ + + tmpdir = tempfile.mkdtemp() + self.dirs_to_cleanup.append(tmpdir) + srcpath = os.path.join(TEST_DATA_DIR, 'provider') + for file in glob.glob(os.path.join(srcpath, 'shapefile.*')): + shutil.copy(os.path.join(srcpath, file), tmpdir) + datasource = os.path.join(tmpdir, 'shapefile.shp') + + vl = QgsVectorLayer(u'{}|layerid=0'.format(datasource), u'test', u'ogr') + caps = vl.dataProvider().capabilities() + self.assertTrue(caps & QgsVectorDataProvider.AddFeatures) + self.assertTrue(caps & QgsVectorDataProvider.DeleteFeatures) + self.assertTrue(caps & QgsVectorDataProvider.ChangeAttributeValues) + self.assertTrue(caps & QgsVectorDataProvider.AddAttributes) + self.assertTrue(caps & QgsVectorDataProvider.DeleteAttributes) + self.assertTrue(caps & QgsVectorDataProvider.CreateSpatialIndex) + self.assertTrue(caps & QgsVectorDataProvider.SelectAtId) + self.assertTrue(caps & QgsVectorDataProvider.ChangeGeometries) + self.assertTrue(caps & QgsVectorDataProvider.SimplifyGeometries) + self.assertTrue(caps & QgsVectorDataProvider.SimplifyGeometriesWithTopologicalValidation) + #self.assertTrue(caps & QgsVectorDataProvider.ChangeFeatures) + + # We should be really opened in read-only mode even if write capabilities are declared + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-only") + + # Unbalanced call to leaveUpdateMode() + self.assertFalse(vl.dataProvider().leaveUpdateMode()) + + # Test that startEditing() / commitChanges() plays with enterUpdateMode() / leaveUpdateMode() + self.assertTrue(vl.startEditing()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + self.assertTrue(vl.dataProvider().isValid()) + + self.assertTrue(vl.commitChanges()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-only") + self.assertTrue(vl.dataProvider().isValid()) + + # Manual enterUpdateMode() / leaveUpdateMode() with 2 depths + self.assertTrue(vl.dataProvider().enterUpdateMode()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + caps = vl.dataProvider().capabilities() + self.assertTrue(caps & QgsVectorDataProvider.AddFeatures) + + f = QgsFeature() + f.setAttributes([200]) + f.setGeometry(QgsGeometry.fromWkt('Point (2 49)')) + (ret, feature_list) = vl.dataProvider().addFeatures([f]) + self.assertTrue(ret) + fid = feature_list[0].id() + + features = [f_iter for f_iter in vl.getFeatures(QgsFeatureRequest().setFilterFid(fid))] + values = [f_iter['pk'] for f_iter in features] + self.assertEquals(values, [200]) + + got_geom = [f_iter.geometry() for f_iter in features][0].geometry() + self.assertEquals((got_geom.x(), got_geom.y()), (2.0, 49.0)) + + self.assertTrue(vl.dataProvider().changeGeometryValues({fid: QgsGeometry.fromWkt('Point (3 50)')})) + self.assertTrue(vl.dataProvider().changeAttributeValues({fid: {0: 100}})) + + features = [f_iter for f_iter in vl.getFeatures(QgsFeatureRequest().setFilterFid(fid))] + values = [f_iter['pk'] for f_iter in features] + + got_geom = [f_iter.geometry() for f_iter in features][0].geometry() + self.assertEquals((got_geom.x(), got_geom.y()), (3.0, 50.0)) + + self.assertTrue(vl.dataProvider().deleteFeatures([fid])) + + # Check that it has really disappeared + if gdal_ok: + gdal.PushErrorHandler('CPLQuietErrorHandler') + features = [f_iter for f_iter in vl.getFeatures(QgsFeatureRequest().setFilterFid(fid))] + if gdal_ok: + gdal.PopErrorHandler() + self.assertEquals(features, []) + + self.assertTrue(vl.dataProvider().addAttributes([QgsField("new_field", QVariant.Int, "integer")])) + self.assertTrue(vl.dataProvider().deleteAttributes([len(vl.dataProvider().fields()) - 1])) + + self.assertTrue(vl.startEditing()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + + self.assertTrue(vl.commitChanges()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + + self.assertTrue(vl.dataProvider().enterUpdateMode()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + + self.assertTrue(vl.dataProvider().leaveUpdateMode()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + + self.assertTrue(vl.dataProvider().leaveUpdateMode()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-only") + + # Test that update mode will be implictly enabled if doing an action + # that requires update mode + (ret, _) = vl.dataProvider().addFeatures([QgsFeature()]) + self.assertTrue(ret) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + + def testUpdateModeFailedReopening(self): + ''' Test that methods on provider don't crash after a failed reopening ''' + + # Windows doesn't like removing files opened by OGR, whatever + # their open mode, so that makes it hard to test + if sys.platform == 'win32': + return + + tmpdir = tempfile.mkdtemp() + self.dirs_to_cleanup.append(tmpdir) + srcpath = os.path.join(TEST_DATA_DIR, 'provider') + for file in glob.glob(os.path.join(srcpath, 'shapefile.*')): + shutil.copy(os.path.join(srcpath, file), tmpdir) + datasource = os.path.join(tmpdir, 'shapefile.shp') + + vl = QgsVectorLayer(u'{}|layerid=0'.format(datasource), u'test', u'ogr') + + os.unlink(datasource) + + self.assertFalse(vl.dataProvider().enterUpdateMode()) + self.assertFalse(vl.dataProvider().enterUpdateMode()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "invalid") + + self.assertFalse(vl.dataProvider().isValid()) + self.assertEquals(len([f for f in vl.dataProvider().getFeatures()]), 0) + self.assertEquals(len(vl.dataProvider().subLayers()), 0) + self.assertFalse(vl.dataProvider().setSubsetString('TRUE')) + (ret, _) = vl.dataProvider().addFeatures([QgsFeature()]) + self.assertFalse(ret) + self.assertFalse(vl.dataProvider().deleteFeatures([1])) + self.assertFalse(vl.dataProvider().addAttributes([QgsField()])) + self.assertFalse(vl.dataProvider().deleteAttributes([1])) + self.assertFalse(vl.dataProvider().changeGeometryValues({0: QgsGeometry.fromWkt('Point (3 50)')})) + self.assertFalse(vl.dataProvider().changeAttributeValues({0: {0: 0}})) + self.assertFalse(vl.dataProvider().createSpatialIndex()) + self.assertFalse(vl.dataProvider().createAttributeIndex(0)) + + def testreloadData(self): + ''' Test reloadData() ''' + + tmpdir = tempfile.mkdtemp() + self.dirs_to_cleanup.append(tmpdir) + srcpath = os.path.join(TEST_DATA_DIR, 'provider') + for file in glob.glob(os.path.join(srcpath, 'shapefile.*')): + shutil.copy(os.path.join(srcpath, file), tmpdir) + datasource = os.path.join(tmpdir, 'shapefile.shp') + + vl1 = QgsVectorLayer(u'{}|layerid=0'.format(datasource), u'test', u'ogr') + vl2 = QgsVectorLayer(u'{}|layerid=0'.format(datasource), u'test', u'ogr') + self.assertTrue(vl1.startEditing()) + self.assertTrue(vl1.deleteAttributes([1])) + self.assertTrue(vl1.commitChanges()) + self.assertEquals(len(vl1.fields()) + 1, len(vl2.fields())) + # Reload + vl2.reload() + # And now check that fields are up-to-date + self.assertEquals(len(vl1.fields()), len(vl2.fields())) if __name__ == '__main__': unittest.main() diff --git a/tests/src/python/test_provider_tabfile.py b/tests/src/python/test_provider_tabfile.py index f2713eee254..dd69733cb9e 100644 --- a/tests/src/python/test_provider_tabfile.py +++ b/tests/src/python/test_provider_tabfile.py @@ -14,12 +14,13 @@ __revision__ = '$Format:%H$' import os -from qgis.core import QgsVectorLayer, QgsFeatureRequest +from qgis.core import QgsVectorLayer, QgsFeatureRequest, QgsVectorDataProvider from qgis.PyQt.QtCore import QDate, QTime, QDateTime, QVariant from qgis.testing import ( start_app, unittest ) +import osgeo.gdal from utilities import unitTestDataPath start_app() @@ -52,5 +53,27 @@ class TestPyQgsTabfileProvider(unittest.TestCase): assert isinstance(f.attributes()[datetime_idx], QDateTime) self.assertEqual(f.attributes()[datetime_idx], QDateTime(QDate(2004, 5, 3), QTime(13, 41, 00))) + # This test fails with GDAL version < 2 because tab update is new in GDAL 2.0 + @unittest.expectedFailure(int(osgeo.gdal.VersionInfo()[:1]) < 2) + def testUpdateMode(self): + """ Test that on-the-fly re-opening in update/read-only mode works """ + + basetestfile = os.path.join(TEST_DATA_DIR, 'tab_file.tab') + vl = QgsVectorLayer(u'{}|layerid=0'.format(basetestfile), u'test', u'ogr') + caps = vl.dataProvider().capabilities() + self.assertTrue(caps & QgsVectorDataProvider.AddFeatures) + + # We should be really opened in read-only mode even if write capabilities are declared + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-only") + + # Test that startEditing() / commitChanges() plays with enterUpdateMode() / leaveUpdateMode() + self.assertTrue(vl.startEditing()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-write") + self.assertTrue(vl.dataProvider().isValid()) + + self.assertTrue(vl.commitChanges()) + self.assertEquals(vl.dataProvider().property("_debug_open_mode"), "read-only") + self.assertTrue(vl.dataProvider().isValid()) + if __name__ == '__main__': unittest.main()