Skip to content

Commit 6cf1c50

Browse files
rouaultrouault
committedMay 22, 2018
[WFS provider] Avoid request by feature id to cause a full layer download (fixes #18740)
·
ltr-3_40final-3_2_0
1 parent b886e22 commit 6cf1c50

File tree

5 files changed

+105
-26
lines changed

5 files changed

+105
-26
lines changed
 

‎src/providers/wfs/qgswfsfeatureiterator.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ void QgsWFSThreadedFeatureDownloader::run()
809809
mWaitCond.wakeOne();
810810
}
811811
mDownloader->run( true, /* serialize features */
812-
0 /* user max features */ );
812+
mShared->requestLimit() /* user max features */ );
813813
}
814814

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

847+
// Particular case: if a single feature is requested by Fid and we already
848+
// have it in cache, no need to interrupt any download
849+
if ( mShared->mCacheDataProvider &&
850+
mRequest.filterType() == QgsFeatureRequest::FilterFid )
851+
{
852+
QgsFeatureRequest requestCache = buildRequestCache( -1 );
853+
QgsFeature f;
854+
if ( mShared->mCacheDataProvider->getFeatures( requestCache ).nextFeature( f ) )
855+
{
856+
mCacheIterator = mShared->mCacheDataProvider->getFeatures( requestCache );
857+
mDownloadFinished = true;
858+
return;
859+
}
860+
}
861+
847862
int genCounter = ( mShared->mURI.isRestrictedToRequestBBOX() && !mFilterRect.isNull() ) ?
848-
mShared->registerToCache( this, mFilterRect ) : mShared->registerToCache( this );
863+
mShared->registerToCache( this, static_cast<int>( mRequest.limit() ), mFilterRect ) :
864+
mShared->registerToCache( this, static_cast<int>( mRequest.limit() ) );
849865
mDownloadFinished = genCounter < 0;
850866
if ( !mShared->mCacheDataProvider )
851867
return;
852868

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

871+
mCacheIterator = mShared->mCacheDataProvider->getFeatures( buildRequestCache( genCounter ) );
872+
}
873+
874+
QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
875+
{
855876
QgsFeatureRequest requestCache;
856877
if ( mRequest.filterType() == QgsFeatureRequest::FilterFid ||
857878
mRequest.filterType() == QgsFeatureRequest::FilterFids )
@@ -928,7 +949,7 @@ QgsWFSFeatureIterator::QgsWFSFeatureIterator( QgsWFSFeatureSource *source,
928949
requestCache.setSubsetOfAttributes( cacheSubSet );
929950
}
930951

931-
mCacheIterator = mShared->mCacheDataProvider->getFeatures( requestCache );
952+
return requestCache;
932953
}
933954

934955
QgsWFSFeatureIterator::~QgsWFSFeatureIterator()

‎src/providers/wfs/qgswfsfeatureiterator.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,9 @@ class QgsWFSFeatureIterator : public QObject,
219219

220220
private:
221221

222+
//! Translate mRequest to a request compatible of the Spatialite cache
223+
QgsFeatureRequest buildRequestCache( int gencounter );
224+
222225
bool fetchFeature( QgsFeature &f ) override;
223226

224227
//! Copies feature attributes / geometry from srcFeature to dstFeature

‎src/providers/wfs/qgswfsshareddata.cpp

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ QgsWFSSharedData::QgsWFSSharedData( const QString &uri )
4040
, mSourceCRS( 0 )
4141
, mMaxFeatures( 0 )
4242
, mMaxFeaturesWasSetFromDefaultForPaging( false )
43+
, mRequestLimit( 0 )
4344
, mHideProgressDialog( mURI.hideDownloadProgressDialog() )
4445
, mDistinctSelect( false )
4546
, mHasWarnedAboutMissingFeatureId( false )
@@ -463,7 +464,7 @@ bool QgsWFSSharedData::createCache()
463464
return true;
464465
}
465466

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

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

919927
{
920928
QMutexLocker locker( &mMutex );
921-
if ( !mFeatureCountExact )
922-
mFeatureCount += featureListToCache.size();
923-
mTotalFeaturesAttemptedToBeCached += featureListToCache.size();
924-
if ( !localComputedExtent.isNull() && mComputedExtent.isNull() && !mTryFetchingOneFeature &&
925-
!localComputedExtent.intersects( mCapabilityExtent ) )
929+
if ( mRequestLimit != 1 )
926930
{
927-
QgsMessageLog::logMessage( tr( "Layer extent reported by the server is not correct. "
928-
"You may need to zoom again on layer while features are being downloaded" ), tr( "WFS" ) );
931+
if ( !mFeatureCountExact )
932+
mFeatureCount += featureListToCache.size();
933+
mTotalFeaturesAttemptedToBeCached += featureListToCache.size();
934+
if ( !localComputedExtent.isNull() && mComputedExtent.isNull() && !mTryFetchingOneFeature &&
935+
!localComputedExtent.intersects( mCapabilityExtent ) )
936+
{
937+
QgsMessageLog::logMessage( tr( "Layer extent reported by the server is not correct. "
938+
"You may need to zoom again on layer while features are being downloaded" ), tr( "WFS" ) );
939+
}
940+
mComputedExtent = localComputedExtent;
929941
}
930-
mComputedExtent = localComputedExtent;
931942
}
932943
}
933944

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

1013-
// In case the download was successful, we will remember this bbox
1014-
// and if the download reached the download limit or not
1015-
QgsFeature f;
1016-
f.setGeometry( QgsGeometry::fromRect( mRect ) );
1017-
QgsFeatureId id = mRegions.size();
1018-
f.setId( id );
1019-
f.initAttributes( 1 );
1020-
f.setAttribute( 0, QVariant( bDownloadLimit ) );
1021-
mRegions.push_back( f );
1022-
mCachedRegions.insertFeature( f );
1024+
if ( mRequestLimit == 0 )
1025+
{
1026+
// In case the download was successful, we will remember this bbox
1027+
// and if the download reached the download limit or not
1028+
QgsFeature f;
1029+
f.setGeometry( QgsGeometry::fromRect( mRect ) );
1030+
QgsFeatureId id = mRegions.size();
1031+
f.setId( id );
1032+
f.initAttributes( 1 );
1033+
f.setAttribute( 0, QVariant( bDownloadLimit ) );
1034+
mRegions.push_back( f );
1035+
mCachedRegions.insertFeature( f );
1036+
}
10231037
}
10241038

1025-
if ( mRect.isEmpty() && success && !bDownloadLimit && !mFeatureCountExact )
1039+
if ( mRect.isEmpty() && success && !bDownloadLimit && mRequestLimit == 0 && !mFeatureCountExact )
10261040
{
10271041
mFeatureCountExact = true;
10281042
if ( featureCount != mFeatureCount )
@@ -1066,6 +1080,7 @@ void QgsWFSSharedData::invalidateCache()
10661080
mCachedRegions = QgsSpatialIndex();
10671081
mRegions.clear();
10681082
mRect = QgsRectangle();
1083+
mRequestLimit = 0;
10691084
mGetFeatureHitsIssued = false;
10701085
mFeatureCount = 0;
10711086
mFeatureCountExact = false;

‎src/providers/wfs/qgswfsshareddata.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class QgsWFSSharedData : public QObject
5555
/**
5656
* Used by a QgsWFSFeatureIterator to start a downloader and get the
5757
generation counter. */
58-
int registerToCache( QgsWFSFeatureIterator *iterator, const QgsRectangle &rect = QgsRectangle() );
58+
int registerToCache( QgsWFSFeatureIterator *iterator, int limit, const QgsRectangle &rect = QgsRectangle() );
5959

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

114+
//! Return maximum number of features to download, or 0 if unlimited
115+
int requestLimit() const { return mRequestLimit; }
116+
114117
signals:
115118

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

164+
//! Limit of retrieved number of features for the current request
165+
int mRequestLimit;
166+
161167
//! Server capabilities
162168
QgsWfsCapabilities::Capabilities mCaps;
163169

‎tests/src/python/test_provider_wfs.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,18 @@ def testWFS10(self):
564564

565565
extent = QgsRectangle(400000.0, 5400000.0, 450000.0, 5500000.0)
566566
request = QgsFeatureRequest().setFilterRect(extent)
567-
values = [f['INTFIELD'] for f in vl.getFeatures(request)]
568-
self.assertEqual(values, [100])
567+
values = [(f.id(), f['INTFIELD']) for f in vl.getFeatures(request)]
568+
self.assertEqual(values[0][1], 100)
569+
570+
# Issue a request by id on a cached feature
571+
request = QgsFeatureRequest(values[0][0])
572+
values = [(f.id(), f['INTFIELD']) for f in vl.getFeatures(request)]
573+
self.assertEqual(values[0][1], 100)
574+
575+
# Check behavior with setLimit(1)
576+
request = QgsFeatureRequest().setLimit(1)
577+
values = [(f.id(), f['INTFIELD']) for f in vl.getFeatures(request)]
578+
self.assertEqual(values[0][1], 100)
569579

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

1284+
# Check behavior with setLimit(1)
1285+
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:
1286+
f.write("""
1287+
<wfs:FeatureCollection xmlns:wfs="http://www.opengis.net/wfs"
1288+
xmlns:gml="http://www.opengis.net/gml"
1289+
xmlns:my="http://my"
1290+
numberOfFeatures="1" timeStamp="2016-03-25T14:51:48.998Z">
1291+
<gml:featureMembers>
1292+
<my:typename gml:id="typename.12345">
1293+
<my:geometryProperty><gml:Point srsName="urn:ogc:def:crs:EPSG::4326"><gml:pos>70 -65</gml:pos></gml:Point></my:geometryProperty>
1294+
<my:ogc_fid>12345</my:ogc_fid>
1295+
</my:typename>
1296+
</gml:featureMembers>
1297+
</wfs:FeatureCollection>""".encode('UTF-8'))
1298+
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' restrictToRequestBBOX=1", 'test', 'WFS')
1299+
request = QgsFeatureRequest().setLimit(1)
1300+
values = [f['ogc_fid'] for f in vl.getFeatures(request)]
1301+
self.assertEqual(values, [12345])
1302+
1303+
# Check that the layer extent is not built from this single feature
1304+
reference = QgsGeometry.fromRect(QgsRectangle(-80, 60, -50, 80))
1305+
vl_extent = QgsGeometry.fromRect(vl.extent())
1306+
assert QgsGeometry.compare(vl_extent.asPolygon()[0], reference.asPolygon()[0], 0.00001), 'Expected {}, got {}'.format(reference.asWkt(), vl_extent.asWkt())
1307+
12741308
def testWFS20TruncatedResponse(self):
12751309
"""Test WFS 2.0 truncatedResponse"""
12761310

0 commit comments

Comments
 (0)
Please sign in to comment.