Skip to content

Commit a343570

Browse files
authoredJan 6, 2018
Merge pull request #6000 from elpaso/bugfix-17795-ogr-filtered-readonly
[bugfix][ogr] Recompute capabilities when subsetfilter is set
2 parents 7605fde + 248ad5f commit a343570

File tree

5 files changed

+159
-86
lines changed

5 files changed

+159
-86
lines changed
 

‎src/providers/ogr/qgsogrprovider.cpp

Lines changed: 107 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -528,90 +528,7 @@ QgsAbstractFeatureSource *QgsOgrProvider::featureSource() const
528528

529529
bool QgsOgrProvider::setSubsetString( const QString &theSQL, bool updateFeatureCount )
530530
{
531-
QgsCPLErrorHandler handler;
532-
533-
if ( !mOgrOrigLayer )
534-
return false;
535-
536-
if ( theSQL == mSubsetString && mFeaturesCounted != QgsVectorDataProvider::Uncounted )
537-
return true;
538-
539-
if ( !theSQL.isEmpty() )
540-
{
541-
bool origFidAdded = false;
542-
QMutex *mutex = nullptr;
543-
OGRLayerH layer = mOgrOrigLayer->getHandleAndMutex( mutex );
544-
GDALDatasetH ds = mOgrOrigLayer->getDatasetHandleAndMutex( mutex );
545-
OGRLayerH subsetLayerH;
546-
{
547-
QMutexLocker locker( mutex );
548-
subsetLayerH = QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), theSQL, origFidAdded );
549-
}
550-
if ( !subsetLayerH )
551-
{
552-
pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
553-
return false;
554-
}
555-
mOgrSqlLayer = QgsOgrProviderUtils::getSqlLayer( mOgrOrigLayer.get(), subsetLayerH, theSQL );
556-
Q_ASSERT( mOgrSqlLayer.get() );
557-
mOgrLayer = mOgrSqlLayer.get();
558-
}
559-
else
560-
{
561-
mOgrSqlLayer.reset();
562-
mOgrLayer = mOgrOrigLayer.get();
563-
}
564-
mSubsetString = theSQL;
565-
566-
QString uri = mFilePath;
567-
if ( !mLayerName.isNull() )
568-
{
569-
uri += QStringLiteral( "|layername=%1" ).arg( mLayerName );
570-
}
571-
else if ( mLayerIndex >= 0 )
572-
{
573-
uri += QStringLiteral( "|layerid=%1" ).arg( mLayerIndex );
574-
}
575-
576-
if ( !mSubsetString.isEmpty() )
577-
{
578-
uri += QStringLiteral( "|subset=%1" ).arg( mSubsetString );
579-
}
580-
581-
if ( mOgrGeometryTypeFilter != wkbUnknown )
582-
{
583-
uri += QStringLiteral( "|geometrytype=%1" ).arg( ogrWkbGeometryTypeName( mOgrGeometryTypeFilter ) );
584-
}
585-
586-
if ( uri != dataSourceUri() )
587-
{
588-
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
589-
setDataSourceUri( uri );
590-
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
591-
}
592-
593-
mOgrLayer->ResetReading();
594-
595-
// getting the total number of features in the layer
596-
// TODO: This can be expensive, do we really need it!
597-
if ( updateFeatureCount )
598-
{
599-
recalculateFeatureCount();
600-
}
601-
602-
// check the validity of the layer
603-
QgsDebugMsgLevel( "checking validity", 4 );
604-
loadFields();
605-
QgsDebugMsgLevel( "Done checking validity", 4 );
606-
607-
invalidateCachedExtent( false );
608-
609-
// Changing the filter may change capabilities
610-
computeCapabilities();
611-
612-
emit dataChanged();
613-
614-
return true;
531+
return _setSubsetString( theSQL, updateFeatureCount, true );
615532
}
616533

617534
QString QgsOgrProvider::subsetString() const
@@ -1763,6 +1680,96 @@ bool QgsOgrProvider::commitTransaction()
17631680
return true;
17641681
}
17651682

1683+
bool QgsOgrProvider::_setSubsetString( const QString &theSQL, bool updateFeatureCount, bool updateCapabilities )
1684+
{
1685+
QgsCPLErrorHandler handler;
1686+
1687+
if ( !mOgrOrigLayer )
1688+
return false;
1689+
1690+
if ( theSQL == mSubsetString && mFeaturesCounted != QgsVectorDataProvider::Uncounted )
1691+
return true;
1692+
1693+
if ( !theSQL.isEmpty() )
1694+
{
1695+
bool origFidAdded = false;
1696+
QMutex *mutex = nullptr;
1697+
OGRLayerH layer = mOgrOrigLayer->getHandleAndMutex( mutex );
1698+
GDALDatasetH ds = mOgrOrigLayer->getDatasetHandleAndMutex( mutex );
1699+
OGRLayerH subsetLayerH;
1700+
{
1701+
QMutexLocker locker( mutex );
1702+
subsetLayerH = QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), theSQL, origFidAdded );
1703+
}
1704+
if ( !subsetLayerH )
1705+
{
1706+
pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
1707+
return false;
1708+
}
1709+
mOgrSqlLayer = QgsOgrProviderUtils::getSqlLayer( mOgrOrigLayer.get(), subsetLayerH, theSQL );
1710+
Q_ASSERT( mOgrSqlLayer.get() );
1711+
mOgrLayer = mOgrSqlLayer.get();
1712+
}
1713+
else
1714+
{
1715+
mOgrSqlLayer.reset();
1716+
mOgrLayer = mOgrOrigLayer.get();
1717+
}
1718+
mSubsetString = theSQL;
1719+
1720+
QString uri = mFilePath;
1721+
if ( !mLayerName.isNull() )
1722+
{
1723+
uri += QStringLiteral( "|layername=%1" ).arg( mLayerName );
1724+
}
1725+
else if ( mLayerIndex >= 0 )
1726+
{
1727+
uri += QStringLiteral( "|layerid=%1" ).arg( mLayerIndex );
1728+
}
1729+
1730+
if ( !mSubsetString.isEmpty() )
1731+
{
1732+
uri += QStringLiteral( "|subset=%1" ).arg( mSubsetString );
1733+
}
1734+
1735+
if ( mOgrGeometryTypeFilter != wkbUnknown )
1736+
{
1737+
uri += QStringLiteral( "|geometrytype=%1" ).arg( ogrWkbGeometryTypeName( mOgrGeometryTypeFilter ) );
1738+
}
1739+
1740+
if ( uri != dataSourceUri() )
1741+
{
1742+
QgsOgrConnPool::instance()->unref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
1743+
setDataSourceUri( uri );
1744+
QgsOgrConnPool::instance()->ref( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ) ) );
1745+
}
1746+
1747+
mOgrLayer->ResetReading();
1748+
1749+
// getting the total number of features in the layer
1750+
// TODO: This can be expensive, do we really need it!
1751+
if ( updateFeatureCount )
1752+
{
1753+
recalculateFeatureCount();
1754+
}
1755+
1756+
// check the validity of the layer
1757+
QgsDebugMsgLevel( "checking validity", 4 );
1758+
loadFields();
1759+
QgsDebugMsgLevel( "Done checking validity", 4 );
1760+
1761+
invalidateCachedExtent( false );
1762+
1763+
// Changing the filter may change capabilities
1764+
if ( updateCapabilities )
1765+
computeCapabilities();
1766+
1767+
emit dataChanged();
1768+
1769+
return true;
1770+
1771+
}
1772+
17661773

17671774
bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_map )
17681775
{
@@ -2186,10 +2193,19 @@ QgsVectorDataProvider::Capabilities QgsOgrProvider::capabilities() const
21862193
void QgsOgrProvider::computeCapabilities()
21872194
{
21882195
QgsVectorDataProvider::Capabilities ability = nullptr;
2196+
bool updateModeActivated = false;
21892197

21902198
// collect abilities reported by OGR
21912199
if ( mOgrLayer )
21922200
{
2201+
2202+
// We want the layer in rw mode or capabilities will be wrong
2203+
// If mUpdateModeStackDepth > 0, it means that an updateMode is already active and that we have write access
2204+
if ( mUpdateModeStackDepth == 0 )
2205+
{
2206+
updateModeActivated = _enterUpdateMode( true );
2207+
}
2208+
21932209
// Whilst the OGR documentation (e.g. at
21942210
// http://www.gdal.org/ogr/classOGRLayer.html#a17) states "The capability
21952211
// codes that can be tested are represented as strings, but #defined
@@ -2318,6 +2334,9 @@ void QgsOgrProvider::computeCapabilities()
23182334
}
23192335
}
23202336

2337+
if ( updateModeActivated )
2338+
leaveUpdateMode();
2339+
23212340
mCapabilities = ability;
23222341
}
23232342

@@ -3969,7 +3988,8 @@ void QgsOgrProvider::open( OpenMode mode )
39693988
mSubsetString.clear();
39703989
// Block signals to avoid endless recusion reloadData -> emit dataChanged -> reloadData
39713990
blockSignals( true );
3972-
mValid = setSubsetString( origSubsetString );
3991+
// Do not update capabilities: it will be done later
3992+
mValid = _setSubsetString( origSubsetString, true, false );
39733993
blockSignals( false );
39743994
if ( mValid )
39753995
{
@@ -4032,7 +4052,8 @@ void QgsOgrProvider::open( OpenMode mode )
40324052
{
40334053
int featuresCountedBackup = mFeaturesCounted;
40344054
mFeaturesCounted = -1;
4035-
mValid = setSubsetString( mSubsetString, false );
4055+
// Do not update capabilities here
4056+
mValid = _setSubsetString( mSubsetString, false, false );
40364057
mFeaturesCounted = featuresCountedBackup;
40374058
}
40384059
}

‎src/providers/ogr/qgsogrprovider.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,9 @@ class QgsOgrProvider : public QgsVectorDataProvider
210210
//! Commits a transaction
211211
bool commitTransaction();
212212

213+
//! Does the real job of settings the subset string and adds an argument to disable update capabilities
214+
bool _setSubsetString( const QString &theSQL, bool updateFeatureCount = true, bool updateCapabilities = true );
215+
213216
void addSubLayerDetailsToSubLayerList( int i, QgsOgrLayer *layer ) const;
214217

215218
QgsFields mAttributeFields;

‎tests/src/python/test_provider_ogr_gpkg.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@
3939
from qgis.PyQt.QtCore import QCoreApplication, QVariant
4040
from qgis.testing import start_app, unittest
4141
from qgis.utils import spatialite_connect
42+
from utilities import unitTestDataPath
43+
44+
TEST_DATA_DIR = unitTestDataPath()
4245

4346

4447
def GDAL_COMPUTE_VERSION(maj, min, rev):
@@ -834,6 +837,29 @@ def testCreateSpatialIndex(self):
834837
self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateSpatialIndex)
835838
self.assertTrue(vl.dataProvider().createSpatialIndex())
836839

840+
def testSubSetStringEditable_bug17795(self):
841+
"""Test that a layer is not editable after setting a subset and it's reverted to editable after the filter is removed"""
842+
843+
isEditable = QgsVectorDataProvider.ChangeAttributeValues
844+
testPath = TEST_DATA_DIR + '/' + 'provider/bug_17795.gpkg|layername=bug_17795'
845+
846+
vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
847+
self.assertTrue(vl.isValid())
848+
self.assertTrue(vl.dataProvider().capabilities() & isEditable)
849+
850+
vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
851+
vl.setSubsetString('')
852+
self.assertTrue(vl.isValid())
853+
self.assertTrue(vl.dataProvider().capabilities() & isEditable)
854+
855+
vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
856+
vl.setSubsetString('"category" = \'one\'')
857+
self.assertTrue(vl.isValid())
858+
self.assertFalse(vl.dataProvider().capabilities() & isEditable)
859+
860+
vl.setSubsetString('')
861+
self.assertTrue(vl.dataProvider().capabilities() & isEditable)
862+
837863

838864
if __name__ == '__main__':
839865
unittest.main()

‎tests/src/python/test_provider_shapefile.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,29 @@ def testCreateSpatialIndex(self):
612612
self.assertTrue(vl.dataProvider().capabilities() & QgsVectorDataProvider.CreateSpatialIndex)
613613
self.assertTrue(vl.dataProvider().createSpatialIndex())
614614

615+
def testSubSetStringEditable_bug17795(self):
616+
"""Test that a layer is not editable after setting a subset and it's reverted to editable after the filter is removed"""
617+
618+
testPath = TEST_DATA_DIR + '/' + 'lines.shp'
619+
isEditable = QgsVectorDataProvider.ChangeAttributeValues
620+
621+
vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
622+
self.assertTrue(vl.isValid())
623+
self.assertTrue(vl.dataProvider().capabilities() & isEditable)
624+
625+
vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
626+
vl.setSubsetString('')
627+
self.assertTrue(vl.isValid())
628+
self.assertTrue(vl.dataProvider().capabilities() & isEditable)
629+
630+
vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
631+
vl.setSubsetString('"Name" = \'Arterial\'')
632+
self.assertTrue(vl.isValid())
633+
self.assertFalse(vl.dataProvider().capabilities() & isEditable)
634+
635+
vl.setSubsetString('')
636+
self.assertTrue(vl.dataProvider().capabilities() & isEditable)
637+
615638

616639
if __name__ == '__main__':
617640
unittest.main()
116 KB
Binary file not shown.

0 commit comments

Comments
 (0)
Please sign in to comment.