Skip to content
Permalink
Browse files

[WFS provider] Avoid request by feature id to cause a full layer down…

…load (fixes #18740)
  • Loading branch information
rouault committed May 22, 2018
1 parent b886e22 commit 6cf1c502d1e057aa63ba7b48e2a532324a98194d
@@ -809,7 +809,7 @@ void QgsWFSThreadedFeatureDownloader::run()
mWaitCond.wakeOne();
}
mDownloader->run( true, /* serialize features */
0 /* user max features */ );
mShared->requestLimit() /* user max features */ );
}

// -------------------------
@@ -844,14 +844,35 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource *source,
if ( !threshold.isEmpty() )
mWriteTransferThreshold = threshold.toInt();

// Particular case: if a single feature is requested by Fid and we already
// have it in cache, no need to interrupt any download
if ( mShared->mCacheDataProvider &&
mRequest.filterType() == QgsFeatureRequest::FilterFid )
{
QgsFeatureRequest requestCache = buildRequestCache( -1 );
QgsFeature f;
if ( mShared->mCacheDataProvider->getFeatures( requestCache ).nextFeature( f ) )
{
mCacheIterator = mShared->mCacheDataProvider->getFeatures( requestCache );
mDownloadFinished = true;
return;
}
}

int genCounter = ( mShared->mURI.isRestrictedToRequestBBOX() && !mFilterRect.isNull() ) ?
mShared->registerToCache( this, mFilterRect ) : mShared->registerToCache( this );
mShared->registerToCache( this, static_cast<int>( mRequest.limit() ), mFilterRect ) :
mShared->registerToCache( this, static_cast<int>( mRequest.limit() ) );
mDownloadFinished = genCounter < 0;
if ( !mShared->mCacheDataProvider )
return;

QgsDebugMsg( QString( "QgsWFSFeatureIterator::constructor(): genCounter=%1 " ).arg( genCounter ) );

mCacheIterator = mShared->mCacheDataProvider->getFeatures( buildRequestCache( genCounter ) );
}

QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
{
QgsFeatureRequest requestCache;
if ( mRequest.filterType() == QgsFeatureRequest::FilterFid ||
mRequest.filterType() == QgsFeatureRequest::FilterFids )
@@ -928,7 +949,7 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource *source,
requestCache.setSubsetOfAttributes( cacheSubSet );
}

mCacheIterator = mShared->mCacheDataProvider->getFeatures( requestCache );
return requestCache;
}

QgsWFSFeatureIterator::~QgsWFSFeatureIterator()
@@ -219,6 +219,9 @@ class QgsWFSFeatureIterator : public QObject,

private:

//! Translate mRequest to a request compatible of the Spatialite cache
QgsFeatureRequest buildRequestCache( int gencounter );

bool fetchFeature( QgsFeature &f ) override;

//! Copies feature attributes / geometry from srcFeature to dstFeature
@@ -40,6 +40,7 @@ QgsWFSSharedData::QgsWFSSharedData( const QString &uri )
, mSourceCRS( 0 )
, mMaxFeatures( 0 )
, mMaxFeaturesWasSetFromDefaultForPaging( false )
, mRequestLimit( 0 )
, mHideProgressDialog( mURI.hideDownloadProgressDialog() )
, mDistinctSelect( false )
, mHasWarnedAboutMissingFeatureId( false )
@@ -463,7 +464,7 @@ bool QgsWFSSharedData::createCache()
return true;
}

int QgsWFSSharedData::registerToCache( QgsWFSFeatureIterator *iterator, const QgsRectangle &rect )
int QgsWFSSharedData::registerToCache( QgsWFSFeatureIterator *iterator, int limit, const QgsRectangle &rect )
{
// This locks prevents 2 readers to register at the same time (and particularly
// destroy the current mDownloader at the same time)
@@ -527,10 +528,17 @@ int QgsWFSSharedData::registerToCache( QgsWFSFeatureIterator *iterator, const Qg
{
newDownloadNeeded = true;
}
// If there's a ongoing download with a limitation, and the new download is
// unlimited, we need a new download.
else if ( !( mWFSVersion.startsWith( QLatin1String( "1.0" ) ) ) && limit <= 0 && mRequestLimit > 0 )
{
newDownloadNeeded = true;
}

if ( newDownloadNeeded || !mDownloader )
{
mRect = rect;
mRequestLimit = ( limit > 0 && !( mWFSVersion.startsWith( QLatin1String( "1.0" ) ) ) ) ? limit : 0;
// to prevent deadlock when waiting the end of the downloader thread that will try to take the mutex in serializeFeatures()
mMutex.unlock();
delete mDownloader;
@@ -918,16 +926,19 @@ void QgsWFSSharedData::serializeFeatures( QVector<QgsWFSFeatureGmlIdPair> &featu

{
QMutexLocker locker( &mMutex );
if ( !mFeatureCountExact )
mFeatureCount += featureListToCache.size();
mTotalFeaturesAttemptedToBeCached += featureListToCache.size();
if ( !localComputedExtent.isNull() && mComputedExtent.isNull() && !mTryFetchingOneFeature &&
!localComputedExtent.intersects( mCapabilityExtent ) )
if ( mRequestLimit != 1 )
{
QgsMessageLog::logMessage( tr( "Layer extent reported by the server is not correct. "
"You may need to zoom again on layer while features are being downloaded" ), tr( "WFS" ) );
if ( !mFeatureCountExact )
mFeatureCount += featureListToCache.size();
mTotalFeaturesAttemptedToBeCached += featureListToCache.size();
if ( !localComputedExtent.isNull() && mComputedExtent.isNull() && !mTryFetchingOneFeature &&
!localComputedExtent.intersects( mCapabilityExtent ) )
{
QgsMessageLog::logMessage( tr( "Layer extent reported by the server is not correct. "
"You may need to zoom again on layer while features are being downloaded" ), tr( "WFS" ) );
}
mComputedExtent = localComputedExtent;
}
mComputedExtent = localComputedExtent;
}
}

@@ -1010,19 +1021,22 @@ void QgsWFSSharedData::endOfDownload( bool success, int featureCount,
mCachedRegions = QgsSpatialIndex();
}

// In case the download was successful, we will remember this bbox
// and if the download reached the download limit or not
QgsFeature f;
f.setGeometry( QgsGeometry::fromRect( mRect ) );
QgsFeatureId id = mRegions.size();
f.setId( id );
f.initAttributes( 1 );
f.setAttribute( 0, QVariant( bDownloadLimit ) );
mRegions.push_back( f );
mCachedRegions.insertFeature( f );
if ( mRequestLimit == 0 )
{
// In case the download was successful, we will remember this bbox
// and if the download reached the download limit or not
QgsFeature f;
f.setGeometry( QgsGeometry::fromRect( mRect ) );
QgsFeatureId id = mRegions.size();
f.setId( id );
f.initAttributes( 1 );
f.setAttribute( 0, QVariant( bDownloadLimit ) );
mRegions.push_back( f );
mCachedRegions.insertFeature( f );
}
}

if ( mRect.isEmpty() && success && !bDownloadLimit && !mFeatureCountExact )
if ( mRect.isEmpty() && success && !bDownloadLimit && mRequestLimit == 0 && !mFeatureCountExact )
{
mFeatureCountExact = true;
if ( featureCount != mFeatureCount )
@@ -1066,6 +1080,7 @@ void QgsWFSSharedData::invalidateCache()
mCachedRegions = QgsSpatialIndex();
mRegions.clear();
mRect = QgsRectangle();
mRequestLimit = 0;
mGetFeatureHitsIssued = false;
mFeatureCount = 0;
mFeatureCountExact = false;
@@ -55,7 +55,7 @@ class QgsWFSSharedData : public QObject
/**
* Used by a QgsWFSFeatureIterator to start a downloader and get the
generation counter. */
int registerToCache( QgsWFSFeatureIterator *iterator, const QgsRectangle &rect = QgsRectangle() );
int registerToCache( QgsWFSFeatureIterator *iterator, int limit, const QgsRectangle &rect = QgsRectangle() );

/**
* Used by the rewind() method of an iterator so as to get the up-to-date
@@ -111,6 +111,9 @@ class QgsWFSSharedData : public QObject
//! Return whether the feature download is finished
bool downloadFinished() const { return mDownloadFinished; }

//! Return maximum number of features to download, or 0 if unlimited
int requestLimit() const { return mRequestLimit; }

signals:

//! Raise error
@@ -158,6 +161,9 @@ class QgsWFSSharedData : public QObject
//! Whether mMaxFeatures was set to a non 0 value for the purpose of paging
bool mMaxFeaturesWasSetFromDefaultForPaging;

//! Limit of retrieved number of features for the current request
int mRequestLimit;

//! Server capabilities
QgsWfsCapabilities::Capabilities mCaps;

@@ -564,8 +564,18 @@ def testWFS10(self):

extent = QgsRectangle(400000.0, 5400000.0, 450000.0, 5500000.0)
request = QgsFeatureRequest().setFilterRect(extent)
values = [f['INTFIELD'] for f in vl.getFeatures(request)]
self.assertEqual(values, [100])
values = [(f.id(), f['INTFIELD']) for f in vl.getFeatures(request)]
self.assertEqual(values[0][1], 100)

# Issue a request by id on a cached feature
request = QgsFeatureRequest(values[0][0])
values = [(f.id(), f['INTFIELD']) for f in vl.getFeatures(request)]
self.assertEqual(values[0][1], 100)

# Check behavior with setLimit(1)
request = QgsFeatureRequest().setLimit(1)
values = [(f.id(), f['INTFIELD']) for f in vl.getFeatures(request)]
self.assertEqual(values[0][1], 100)

def testWFS10_outputformat_GML3(self):
"""Test WFS 1.0 with OUTPUTFORMAT=GML3"""
@@ -1271,6 +1281,30 @@ def testWFSGetOnlyFeaturesInViewExtent(self):
values = [f['ogc_fid'] for f in vl.getFeatures(request)]
self.assertEqual(values, [101])

# Check behavior with setLimit(1)
with open(sanitize(endpoint, "?SERVICE=WFS&REQUEST=GetFeature&VERSION=1.1.0&TYPENAME=my:typename&MAXFEATURES=1&SRSNAME=urn:ogc:def:crs:EPSG::4326"), '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"
numberOfFeatures="1" timeStamp="2016-03-25T14:51:48.998Z">
<gml:featureMembers>
<my:typename gml:id="typename.12345">
<my:geometryProperty><gml:Point srsName="urn:ogc:def:crs:EPSG::4326"><gml:pos>70 -65</gml:pos></gml:Point></my:geometryProperty>
<my:ogc_fid>12345</my:ogc_fid>
</my:typename>
</gml:featureMembers>
</wfs:FeatureCollection>""".encode('UTF-8'))
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' restrictToRequestBBOX=1", 'test', 'WFS')
request = QgsFeatureRequest().setLimit(1)
values = [f['ogc_fid'] for f in vl.getFeatures(request)]
self.assertEqual(values, [12345])

# Check that the layer extent is not built from this single feature
reference = QgsGeometry.fromRect(QgsRectangle(-80, 60, -50, 80))
vl_extent = QgsGeometry.fromRect(vl.extent())
assert QgsGeometry.compare(vl_extent.asPolygon()[0], reference.asPolygon()[0], 0.00001), 'Expected {}, got {}'.format(reference.asWkt(), vl_extent.asWkt())

def testWFS20TruncatedResponse(self):
"""Test WFS 2.0 truncatedResponse"""

0 comments on commit 6cf1c50

Please sign in to comment.
You can’t perform that action at this time.