sharpetronics/QGIS
1
0
Fork 0
mirror of https://github.com/qgis/QGIS.git synced 2025-04-15 00:04:00 -04:00
Code Issues Packages Projects Releases Wiki Activity

[WFS provider] Ensure stability of QGIS FeatureId when reloading layer (fixes #20865)

Browse Source
This commit is contained in:
Even Rouault 2019-02-05 23:08:26 +01:00
parent 62e09d9467
commit e19bf11f9e
No known key found for this signature in database
GPG Key ID: 33EBBFC47B3DD87D
4 changed files with 243 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/providers/wfs/qgswfsfeatureiterator.cpp
View File

@ -32,6 +32,7 @@
#include "qgssettings.h"
#include "qgsexception.h"
#include "qgsfeedback.h"
#include "qgssqliteutils.h"
#include <algorithm>
#include <QDir>
@ -39,6 +40,8 @@
#include <QTimer>
#include <QStyle>
#include <sqlite3.h>
QgsWFSFeatureHitsAsyncRequest::QgsWFSFeatureHitsAsyncRequest( QgsWFSDataSourceURI &uri )
: QgsWfsRequest( uri )
, mNumberMatched( -1 )
@ -986,7 +989,15 @@ QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
QgsFeatureRequest requestCache;
if ( mRequest.filterType() == QgsFeatureRequest::FilterFid ||
mRequest.filterType() == QgsFeatureRequest::FilterFids )
requestCache = mRequest;
{
QgsFeatureIds qgisIds;
if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
qgisIds.insert( mRequest.filterFid() );
else
qgisIds = mRequest.filterFids();
requestCache.setFilterFids( mShared->dbIdsFromQgisIds( qgisIds ) );
}
else
{
if ( mRequest.filterType() == QgsFeatureRequest::FilterExpression )
@ -1246,6 +1257,19 @@ bool QgsWFSFeatureIterator::fetchFeature( QgsFeature &f )
copyFeature( cachedFeature, f );
geometryToDestinationCrs( f, mTransform );
// Retrieve the user-visible id from the Spatialite cache database Id
if ( mShared->mCacheIdDb.get() )
{
auto sql = QgsSqlite3Mprintf( "SELECT qgisId FROM id_cache WHERE dbId = %lld", cachedFeature.id() );
int resultCode;
auto stmt = mShared->mCacheIdDb.prepare( sql, resultCode );
if ( stmt.step() == SQLITE_ROW )
{
f.setId( stmt.columnAsInt64( 0 ) );
}
}
return true;
}

206
src/providers/wfs/qgswfsshareddata.cpp
View File

@ -62,6 +62,16 @@ QgsWFSSharedData::~QgsWFSSharedData()
QgsDebugMsgLevel( QStringLiteral( "~QgsWFSSharedData()" ), 4 );
invalidateCache();
mCacheIdDb.reset();
if ( !mCacheIdDbname.isEmpty() )
{
QFile::remove( mCacheIdDbname );
QFile::remove( mCacheIdDbname + "-wal" );
QFile::remove( mCacheIdDbname + "-shm" );
QgsWFSUtils::releaseCacheDirectory();
mCacheIdDbname.clear();
}
}
QString QgsWFSSharedData::srsName() const
@ -215,7 +225,8 @@ bool QgsWFSSharedData::createCache()
static QAtomicInt sTmpCounter = 0;
int tmpCounter = ++sTmpCounter;
mCacheDbname = QDir( QgsWFSUtils::acquireCacheDirectory() ).filePath( QStringLiteral( "wfs_cache_%1.sqlite" ).arg( tmpCounter ) );
QString cacheDirectory( QgsWFSUtils::acquireCacheDirectory() );
mCacheDbname = QDir( cacheDirectory ).filePath( QStringLiteral( "wfs_cache_%1.sqlite" ).arg( tmpCounter ) );
Q_ASSERT( !QFile::exists( mCacheDbname ) );
QgsFields cacheFields;
@ -466,6 +477,35 @@ bool QgsWFSSharedData::createCache()
return false;
}
// The id_cache should be generated once for the lifetime of QgsWFSConstants
// to ensure consistency of the ids returned to the user.
if ( mCacheIdDbname.isEmpty() )
{
mCacheIdDbname = QDir( cacheDirectory ).filePath( QStringLiteral( "wfs_id_cache_%1.sqlite" ).arg( tmpCounter ) );
Q_ASSERT( !QFile::exists( mCacheIdDbname ) );
if ( mCacheIdDb.open( mCacheIdDbname ) != SQLITE_OK )
{
QgsMessageLog::logMessage( tr( "Cannot create temporary id cache" ), tr( "WFS" ) );
return false;
}
QString errorMsg;
bool ok = mCacheIdDb.exec( QStringLiteral( "PRAGMA synchronous=OFF" ), errorMsg ) == SQLITE_OK;
// WAL is needed to avoid reader to block writers
ok &= mCacheIdDb.exec( QStringLiteral( "PRAGMA journal_mode=WAL" ), errorMsg ) == SQLITE_OK;
// gmlid is the gmlid or fid attribute coming from the GML GetFeature response
// qgisId is the feature id of the features returned to QGIS. That one should remain the same for a given gmlid even after a layer reload
// dbId is the feature id of the Spatialite feature in mCacheDataProvider. It might change for a given gmlid after a layer reload
ok &= mCacheIdDb.exec( QStringLiteral( "CREATE TABLE id_cache(gmlid TEXT, dbId INTEGER, qgisId INTEGER)" ), errorMsg ) == SQLITE_OK;
ok &= mCacheIdDb.exec( QStringLiteral( "CREATE INDEX idx_gmlid ON id_cache(gmlid)" ), errorMsg ) == SQLITE_OK;
ok &= mCacheIdDb.exec( QStringLiteral( "CREATE INDEX idx_dbId ON id_cache(dbId)" ), errorMsg ) == SQLITE_OK;
ok &= mCacheIdDb.exec( QStringLiteral( "CREATE INDEX idx_qgisId ON id_cache(qgisId)" ), errorMsg ) == SQLITE_OK;
if ( !ok )
{
QgsDebugMsg( errorMsg );
return false;
}
}
return true;
}
@ -572,6 +612,10 @@ int QgsWFSSharedData::getUpdatedCounter()
QSet<QString> QgsWFSSharedData::getExistingCachedGmlIds( const QVector<QgsWFSFeatureGmlIdPair> &featureList )
{
// We query the Spatialite cache here, not the persistent id_cache,
// since we want to know which features in this session we have already
// downloaded.
QString expr;
bool first = true;
QSet<QString> setExistingGmlIds;
@ -675,32 +719,65 @@ QSet<QString> QgsWFSSharedData::getExistingCachedMD5( const QVector<QgsWFSFeatur
// Used by WFS-T
QString QgsWFSSharedData::findGmlId( QgsFeatureId fid )
{
if ( !mCacheDataProvider )
if ( !mCacheIdDb )
return QString();
QgsFeatureRequest request;
request.setFilterFid( fid );
QgsFields dataProviderFields = mCacheDataProvider->fields();
int gmlidIdx = dataProviderFields.indexFromName( QgsWFSConstants::FIELD_GMLID );
QgsAttributeList attList;
attList.append( gmlidIdx );
request.setSubsetOfAttributes( attList );
QgsFeatureIterator iterGmlIds( mCacheDataProvider->getFeatures( request ) );
QgsFeature gmlidFeature;
while ( iterGmlIds.nextFeature( gmlidFeature ) )
auto sql = QgsSqlite3Mprintf( "SELECT gmlid FROM id_cache WHERE qgisId = %lld", fid );
int resultCode;
auto stmt = mCacheIdDb.prepare( sql, resultCode );
Q_ASSERT( resultCode == SQLITE_OK );
if ( stmt.step() == SQLITE_ROW )
{
const QVariant &v = gmlidFeature.attributes().value( gmlidIdx );
return v.toString();
return stmt.columnAsText( 0 );
}
return QString();
}
QgsFeatureIds QgsWFSSharedData::dbIdsFromQgisIds( const QgsFeatureIds &qgisIds )
{
QgsFeatureIds dbIds;
if ( !mCacheIdDb )
return dbIds;
// To avoid excessive memory consumption in expression building, do not
// query more than 1000 ids at a time.
bool first = true;
QString expr;
int i = 0;
for ( const auto &qgisId : qgisIds )
{
if ( !first )
expr += ',';
else
{
expr = QStringLiteral( "SELECT dbId FROM id_cache WHERE qgisId IN (" );
first = false;
}
expr += FID_TO_STRING( qgisId );
if ( ( i > 0 && ( i % 1000 ) == 0 ) || i + 1 == qgisIds.size() )
{
expr += ')';
int resultCode;
auto stmt = mCacheIdDb.prepare( expr.toUtf8().constData(), resultCode );
Q_ASSERT( resultCode == SQLITE_OK );
while ( stmt.step() == SQLITE_ROW )
{
dbIds.insert( stmt.columnAsInt64( 0 ) );
}
// Should we check that we got a dbId from every qgisId... ?
first = true;
}
i++;
}
return dbIds;
}
// Used by WFS-T
bool QgsWFSSharedData::deleteFeatures( const QgsFeatureIds &fidlist )
{
if ( !mCacheDataProvider )
if ( !mCacheIdDb || !mCacheDataProvider )
return false;
{
@ -708,13 +785,13 @@ bool QgsWFSSharedData::deleteFeatures( const QgsFeatureIds &fidlist )
mFeatureCount -= fidlist.size();
}
return mCacheDataProvider->deleteFeatures( fidlist );
return mCacheDataProvider->deleteFeatures( dbIdsFromQgisIds( fidlist ) );
}
// Used by WFS-T
bool QgsWFSSharedData::changeGeometryValues( const QgsGeometryMap &geometry_map )
{
if ( !mCacheDataProvider )
if ( !mCacheIdDb || !mCacheDataProvider )
return false;
// We need to replace the geometry by its bounding box and issue a attribute
@ -727,22 +804,34 @@ bool QgsWFSSharedData::changeGeometryValues( const QgsGeometryMap &geometry_map
QgsChangedAttributesMap newChangedAttrMap;
for ( QgsGeometryMap::const_iterator iter = geometry_map.constBegin(); iter != geometry_map.constEnd(); ++iter )
{
auto sql = QgsSqlite3Mprintf( "SELECT dbId FROM id_cache WHERE qgisId = %lld", iter.key() );
int resultCode;
auto stmt = mCacheIdDb.prepare( sql, resultCode );
Q_ASSERT( resultCode == SQLITE_OK );
if ( stmt.step() != SQLITE_ROW )
{
// shouldn't happen normally
QgsDebugMsg( QStringLiteral( "cannot find dbId corresponding to qgisId = %1" ).arg( iter.key() ) );
continue;
}
QgsFeatureId dbId = stmt.columnAsInt64( 0 );
QByteArray wkb = iter->asWkb();
if ( !wkb.isEmpty() )
{
QgsAttributeMap newAttrMap;
newAttrMap[idx] = QString( wkb.toHex().data() );
newChangedAttrMap[ iter.key()] = newAttrMap;
newChangedAttrMap[ dbId] = newAttrMap;
QgsGeometry polyBoundingBox = QgsGeometry::fromRect( iter.value().boundingBox() );
newGeometryMap[ iter.key()] = polyBoundingBox;
newGeometryMap[ dbId] = polyBoundingBox;
}
else
{
QgsAttributeMap newAttrMap;
newAttrMap[idx] = QString();
newChangedAttrMap[ iter.key()] = newAttrMap;
newGeometryMap[ iter.key()] = QgsGeometry();
newChangedAttrMap[ dbId] = newAttrMap;
newGeometryMap[ dbId] = QgsGeometry();
}
}
@ -753,14 +842,25 @@ bool QgsWFSSharedData::changeGeometryValues( const QgsGeometryMap &geometry_map
// Used by WFS-T
bool QgsWFSSharedData::changeAttributeValues( const QgsChangedAttributesMap &attr_map )
{
if ( !mCacheDataProvider )
if ( !mCacheIdDb || !mCacheDataProvider )
return false;
QgsFields dataProviderFields = mCacheDataProvider->fields();
QgsChangedAttributesMap newMap;
for ( QgsChangedAttributesMap::const_iterator iter = attr_map.begin(); iter != attr_map.end(); ++iter )
{
QgsFeatureId fid = iter.key();
auto sql = QgsSqlite3Mprintf( "SELECT dbId FROM id_cache WHERE qgisId = %lld", iter.key() );
int resultCode;
auto stmt = mCacheIdDb.prepare( sql, resultCode );
Q_ASSERT( resultCode == SQLITE_OK );
if ( stmt.step() != SQLITE_ROW )
{
// shouldn't happen normally
QgsDebugMsg( QStringLiteral( "cannot find dbId corresponding to qgisId = %1" ).arg( iter.key() ) );
continue;
}
QgsFeatureId dbId = stmt.columnAsInt64( 0 );
const QgsAttributeMap &attrs = iter.value();
if ( attrs.isEmpty() )
continue;
@ -774,7 +874,7 @@ bool QgsWFSSharedData::changeAttributeValues( const QgsChangedAttributesMap &att
else
newAttrMap[idx] = siter.value();
}
newMap[fid] = newAttrMap;
newMap[dbId] = newAttrMap;
}
return mCacheDataProvider->changeAttributeValues( newMap );
@ -923,10 +1023,57 @@ void QgsWFSSharedData::serializeFeatures( QVector<QgsWFSFeatureGmlIdPair> &featu
Q_ASSERT( featureListToCache.size() == updatedFeatureList.size() );
for ( int i = 0; i < updatedFeatureList.size(); i++ )
{
if ( cacheOk )
updatedFeatureList[i].first.setId( featureListToCache[i].id() );
int resultCode;
QgsFeatureId dbId( cacheOk ? featureListToCache[i].id() : mTotalFeaturesAttemptedToBeCached + i + 1 );
QgsFeatureId qgisId;
const auto &gmlId( updatedFeatureList[i].second );
if ( gmlId.isEmpty() )
{
// Degraded case. Won't work properly in reload situations, but we
// can't do better.
qgisId = dbId;
}
else
updatedFeatureList[i].first.setId( mTotalFeaturesAttemptedToBeCached + i + 1 );
{
auto sql = QgsSqlite3Mprintf( "SELECT qgisId, dbId FROM id_cache WHERE gmlid = '%q'",
gmlId.toUtf8().constData() );
auto stmt = mCacheIdDb.prepare( sql, resultCode );
Q_ASSERT( resultCode == SQLITE_OK );
if ( stmt.step() == SQLITE_ROW )
{
qgisId = stmt.columnAsInt64( 0 );
QgsFeatureId oldDbId = stmt.columnAsInt64( 1 );
if ( dbId != oldDbId )
{
sql = QgsSqlite3Mprintf( "UPDATE id_cache SET dbId = %lld WHERE gmlid = '%q'",
dbId,
gmlId.toUtf8().constData() );
//QgsDebugMsg( QStringLiteral( "%1" ).arg( sql ) );
QString errorMsg;
if ( mCacheIdDb.exec( sql, errorMsg ) != SQLITE_OK )
{
QgsMessageLog::logMessage( tr( "Problem when updating WFS id cache: %1 -> %2" ).arg( sql ).arg( errorMsg ), tr( "WFS" ) );
}
}
}
else
{
qgisId = mNextCachedIdQgisId;
mNextCachedIdQgisId ++;
sql = QgsSqlite3Mprintf( "INSERT INTO id_cache (gmlid, dbId, qgisId) VALUES ('%q', %lld, %lld)",
gmlId.toUtf8().constData(),
dbId,
qgisId );
//QgsDebugMsg( QStringLiteral( "%1" ).arg( sql ) );
QString errorMsg;
if ( mCacheIdDb.exec( sql, errorMsg ) != SQLITE_OK )
{
QgsMessageLog::logMessage( tr( "Problem when updating WFS id cache: %1 -> %2" ).arg( sql ).arg( errorMsg ), tr( "WFS" ) );
}
}
}
updatedFeatureList[i].first.setId( qgisId );
}
{
@ -1104,7 +1251,6 @@ void QgsWFSSharedData::invalidateCache()
QFile::remove( mCacheDbname );
QFile::remove( mCacheDbname + "-wal" );
QFile::remove( mCacheDbname + "-shm" );
QgsWFSUtils::releaseCacheDirectory();
mCacheDbname.clear();
}
}

13
src/providers/wfs/qgswfsshareddata.h
View File

@ -20,6 +20,7 @@
#include "qgswfsrequest.h"
#include "qgswfscapabilities.h"
#include "qgsogcutils.h"
#include "qgssqliteutils.h"
/**
* This class holds data, and logic, shared between QgsWFSProvider, QgsWFSFeatureIterator
@ -78,6 +79,9 @@ class QgsWFSSharedData : public QObject
//! Give a feature id, find the correspond fid/gml.id. Used by WFS-T
QString findGmlId( QgsFeatureId fid );
//! Retrieve the dbId from the qgisId
QgsFeatureIds dbIdsFromQgisIds( const QgsFeatureIds &qgisIds );
//! Delete from the on-disk cache the features of given fid. Used by WFS-T
bool deleteFeatures( const QgsFeatureIds &fidlist );
@ -251,6 +255,15 @@ class QgsWFSSharedData : public QObject
//! Whether we have already tried fetching one feature after realizing that the capabilities extent is wrong
bool mTryFetchingOneFeature;
//! Name of the gmlid, spatialite_id, qgis_id cache. This cache persists even after a layer reload so as to ensure feature id stability.
QString mCacheIdDbname;
//! Connection to mCacheIdDbname
sqlite3_database_unique_ptr mCacheIdDb;
//! Next value for qgisId column
QgsFeatureId mNextCachedIdQgisId = 1;
/**
* Returns the set of gmlIds that have already been downloaded and
cached, so as to avoid to cache duplicates. */

29
tests/src/python/test_provider_wfs.py
View File

@ -545,6 +545,35 @@ class TestPyQgsWFSProvider(unittest.TestCase, ProviderTestCase):
self.assertFalse(vl.dataProvider().deleteFeatures([0]))
# Reload data and check for fid stability of features whose fid/gmlid
# has already been downloaded
vl.dataProvider().reloadData()
with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=GetFeature&VERSION=1.0.0&TYPENAME=my:typename&SRSNAME=EPSG:32631'), 'wb') as f:
f.write("""
<wfs:FeatureCollection
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:gml="http://www.opengis.net/gml"
xmlns:my="http://my">
<gml:boundedBy><gml:null>unknown</gml:null></gml:boundedBy>
<gml:featureMember>
<my:typename fid="typename.10">
<my:INTFIELD>20</my:INTFIELD>
</my:typename>
</gml:featureMember>
<gml:featureMember>
<my:typename fid="typename.0">
<my:INTFIELD>30</my:INTFIELD>
</my:typename>
</gml:featureMember>
</wfs:FeatureCollection>""".encode('UTF-8'))
features = [f for f in vl.getFeatures()]
self.assertEqual(features[0].id(), 2)
self.assertEqual(features[0]['INTFIELD'], 20)
self.assertEqual(features[1].id(), 1)
self.assertEqual(features[1]['INTFIELD'], 30)
# Test with restrictToRequestBBOX=1
with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=GetFeature&VERSION=1.0.0&TYPENAME=my:typename&SRSNAME=EPSG:32631&BBOX=400000,5400000,450000,5500000'), 'wb') as f:
f.write("""