Skip to content

Commit

Permalink
[BUGFIX / FEATURE] [OGR] Allow concurrent edition of Shapefiles and T…
Browse files Browse the repository at this point in the history
…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.
1 change: 1 addition & 0 deletions ci/travis/linux/qt5/blacklist.txt
Expand Up @@ -8,6 +8,7 @@ PyQgsLocalServer
PyQgsMapUnitScale
PyQgsMemoryProvider
PyQgsNetworkContentFetcher
PyQgsOGRProvider
PyQgsPalLabelingBase
PyQgsPalLabelingCanvas
PyQgsPalLabelingComposer
Expand Down
41 changes: 41 additions & 0 deletions python/core/qgsdataprovider.sip
Expand Up @@ -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:

/**
Expand Down
41 changes: 41 additions & 0 deletions src/core/qgsdataprovider.h
Expand Up @@ -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:

/**
Expand Down
7 changes: 7 additions & 0 deletions src/core/qgsvectorlayer.cpp
Expand Up @@ -377,6 +377,7 @@ void QgsVectorLayer::reload()
if ( mDataProvider )
{
mDataProvider->reloadData();
updateFields();
}
}

Expand Down Expand Up @@ -1454,6 +1455,8 @@ bool QgsVectorLayer::startEditing()

emit beforeEditingStarted();

mDataProvider->enterUpdateMode();

if ( mDataProvider->transaction() )
{
mEditBuffer = new QgsVectorLayerEditPassthrough( this );
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2467,6 +2472,8 @@ bool QgsVectorLayer::rollBack( bool deleteBuffer )
if ( rollbackExtent )
updateExtents();

mDataProvider->leaveUpdateMode();

emit repaintRequested();
return true;
}
Expand Down
16 changes: 14 additions & 2 deletions src/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -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() )
{
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 )
Expand Down Expand Up @@ -256,7 +268,7 @@ bool QgsOgrFeatureIterator::fetchFeature( QgsFeature& feature )

bool QgsOgrFeatureIterator::rewind()
{
if ( mClosed )
if ( mClosed || ogrLayer == nullptr )
return false;

OGR_L_ResetReading( ogrLayer );
Expand Down

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.