Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[WFS provider] Ensure stability of QGIS FeatureId when reloading layer (
fixes #20865)
  • Loading branch information
rouault committed Feb 6, 2019
1 parent 62e09d9 commit e19bf11
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 31 deletions.
26 changes: 25 additions & 1 deletion src/providers/wfs/qgswfsfeatureiterator.cpp
Expand Up @@ -32,13 +32,16 @@
#include "qgssettings.h"
#include "qgsexception.h"
#include "qgsfeedback.h"
#include "qgssqliteutils.h"

#include <algorithm>
#include <QDir>
#include <QProgressDialog>
#include <QTimer>
#include <QStyle>

#include <sqlite3.h>

QgsWFSFeatureHitsAsyncRequest::QgsWFSFeatureHitsAsyncRequest( QgsWFSDataSourceURI &uri )
: QgsWfsRequest( uri )
, mNumberMatched( -1 )
Expand Down Expand Up @@ -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 )
Expand Down Expand Up @@ -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;
}

Expand Down
206 changes: 176 additions & 30 deletions src/providers/wfs/qgswfsshareddata.cpp
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -675,46 +719,79 @@ 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;

{
QMutexLocker locker( &mMutex );
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
Expand All @@ -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();
}
}

Expand All @@ -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;
Expand All @@ -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 );
Expand Down Expand Up @@ -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 );
}

{
Expand Down Expand Up @@ -1104,7 +1251,6 @@ void QgsWFSSharedData::invalidateCache()
QFile::remove( mCacheDbname );
QFile::remove( mCacheDbname + "-wal" );
QFile::remove( mCacheDbname + "-shm" );
QgsWFSUtils::releaseCacheDirectory();
mCacheDbname.clear();
}
}
Expand Down

0 comments on commit e19bf11

Please sign in to comment.