Skip to content
Permalink
Browse files
[BUGFIX / FEATURE] [OGR] Allow concurrent edition of Shapefiles and T…
…abfiles 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.
  • Loading branch information
rouault committed May 4, 2016
1 parent ca74cc0 commit dc18b5b
Show file tree
Hide file tree
Showing 11 changed files with 624 additions and 28 deletions.
@@ -8,6 +8,7 @@ PyQgsLocalServer
PyQgsMapUnitScale
PyQgsMemoryProvider
PyQgsNetworkContentFetcher
PyQgsOGRProvider
PyQgsPalLabelingBase
PyQgsPalLabelingCanvas
PyQgsPalLabelingComposer
@@ -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:

/**
@@ -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:

/**
@@ -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();

This comment has been minimized.

Copy link
@m-kuhn

m-kuhn Jul 13, 2016

Member

@rouault Shouldn't that only be done if the commit was successful and the edit buffer is deleted?

This comment has been minimized.

Copy link
@rouault

rouault Jul 13, 2016

Author Contributor

Hum, probably indeed


emit repaintRequested();

return success;
@@ -2467,6 +2472,8 @@ bool QgsVectorLayer::rollBack( bool deleteBuffer )
if ( rollbackExtent )
updateExtents();

mDataProvider->leaveUpdateMode();

emit repaintRequested();
return true;
}
@@ -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 );

4 comments on commit dc18b5b

@Fromax
Copy link

@Fromax Fromax commented on dc18b5b May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello
Please see the latest comments on the "Issues" page, where it appears that the issue has been resolved, but only from QGIS release 2.15.0-69.
Can you confirm that the patch is being backported to Latest and LTR?

@jef-n
Copy link
Member

@jef-n jef-n commented on dc18b5b May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fromax nice. good ol' busy bee from TOS.

@rouault
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n / @m-kuhn Regarding backporting of this to 2.14, what is your opinion ? It indeeds fixes an important use case for some users, but it is a non-trivial amount of changes in the OGR provider, and with new API QgsDataProvider::enterUpdateMode() / QgsDataProvider::leaveUpdateMode() added

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on dc18b5b May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it fixes an important bug and you feel confident that it does not break things I think it's worth it.

Please sign in to comment.