Skip to content

Commit 9e023bd

Browse files
committed
[afs] Fix/optimise handling of filter rect feature requests
Before a filter rect request would usually force fetching every single feature from the server before the request could be complete. Instead, if a filter rect is passed we first obtain a list from the server of matching features within this rect, and then iterate only over those. Fixes broken (multi-minute hang) identify tool use on AFS layers.
1 parent 45ded37 commit 9e023bd

File tree

7 files changed

+131
-24
lines changed

7 files changed

+131
-24
lines changed

src/providers/arcgisrest/qgsafsfeatureiterator.cpp

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "qgsmessagelog.h"
1919
#include "geometry/qgsgeometry.h"
2020
#include "qgsexception.h"
21+
#include "qgsarcgisrestutils.h"
2122

2223
QgsAfsFeatureSource::QgsAfsFeatureSource( const std::shared_ptr<QgsAfsSharedData> &sharedData )
2324
: mSharedData( sharedData )
@@ -59,17 +60,39 @@ QgsAfsFeatureIterator::QgsAfsFeatureIterator( QgsAfsFeatureSource *source, bool
5960
return;
6061
}
6162

63+
QgsFeatureIds requestIds;
6264
if ( mRequest.filterType() == QgsFeatureRequest::FilterFids )
6365
{
64-
mUsingFeatureIdList = true;
65-
mFeatureIdList = mRequest.filterFids();
66-
mRemainingFeatureIds = mFeatureIdList;
66+
requestIds = mRequest.filterFids();
6767
}
6868
else if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
6969
{
70-
mFeatureIdList.insert( mRequest.filterFid() );
71-
mRemainingFeatureIds = mFeatureIdList;
70+
requestIds.insert( mRequest.filterFid() );
7271
}
72+
73+
if ( !mFilterRect.isNull() )
74+
{
75+
QgsFeatureIds featuresInRect = mSource->sharedData()->getFeatureIdsInExtent( mFilterRect );
76+
if ( !requestIds.isEmpty() )
77+
{
78+
requestIds.intersect( featuresInRect );
79+
}
80+
else
81+
{
82+
requestIds = featuresInRect;
83+
}
84+
if ( requestIds.empty() )
85+
{
86+
close();
87+
return;
88+
}
89+
}
90+
91+
mFeatureIdList = requestIds.toList();
92+
std::sort( mFeatureIdList.begin(), mFeatureIdList.end() );
93+
mRemainingFeatureIds = mFeatureIdList;
94+
if ( !mRemainingFeatureIds.empty() )
95+
mFeatureIterator = mRemainingFeatureIds.at( 0 );
7396
}
7497

7598
QgsAfsFeatureIterator::~QgsAfsFeatureIterator()
@@ -87,6 +110,9 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
87110
if ( mFeatureIterator >= mSource->sharedData()->featureCount() )
88111
return false;
89112

113+
if ( !mFeatureIdList.empty() && mRemainingFeatureIds.empty() )
114+
return false;
115+
90116
switch ( mRequest.filterType() )
91117
{
92118
case QgsFeatureRequest::FilterFid:
@@ -97,33 +123,32 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
97123
bool result = mSource->sharedData()->getFeature( mRequest.filterFid(), f );
98124
geometryToDestinationCrs( f, mTransform );
99125
f.setValid( result );
100-
mRemainingFeatureIds.remove( f.id() );
126+
mRemainingFeatureIds.removeAll( f.id() );
101127
return result;
102128
}
103129

104130
case QgsFeatureRequest::FilterFids:
105131
case QgsFeatureRequest::FilterExpression:
106132
case QgsFeatureRequest::FilterNone:
107133
{
108-
QgsRectangle filterRect;
109-
if ( !mRequest.filterRect().isNull() )
110-
{
111-
filterRect = mSource->sharedData()->extent();
112-
filterRect = filterRect.intersect( &mFilterRect );
113-
}
114134
while ( mFeatureIterator < mSource->sharedData()->featureCount() )
115135
{
116-
bool success = mSource->sharedData()->getFeature( mFeatureIterator, f, filterRect );
117-
++mFeatureIterator;
118-
if ( !success )
119-
continue;
120-
if ( mUsingFeatureIdList )
136+
if ( !mFeatureIdList.empty() && mRemainingFeatureIds.empty() )
137+
return false;
138+
139+
bool success = mSource->sharedData()->getFeature( mFeatureIterator, f );
140+
if ( !mFeatureIdList.empty() )
141+
{
142+
mRemainingFeatureIds.removeAll( mFeatureIterator );
143+
if ( !mRemainingFeatureIds.empty() )
144+
mFeatureIterator = mRemainingFeatureIds.at( 0 );
145+
}
146+
else
121147
{
122-
if ( !mRemainingFeatureIds.contains( f.id() ) )
123-
continue;
124-
else
125-
mRemainingFeatureIds.remove( f.id() );
148+
++mFeatureIterator;
126149
}
150+
if ( !success )
151+
continue;
127152
geometryToDestinationCrs( f, mTransform );
128153
f.setValid( true );
129154
return true;
@@ -140,6 +165,8 @@ bool QgsAfsFeatureIterator::rewind()
140165
return false;
141166
mFeatureIterator = 0;
142167
mRemainingFeatureIds = mFeatureIdList;
168+
if ( !mRemainingFeatureIds.empty() )
169+
mFeatureIterator = mRemainingFeatureIds.at( 0 );
143170
return true;
144171
}
145172

src/providers/arcgisrest/qgsafsfeatureiterator.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,8 @@ class QgsAfsFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsAfs
5151
private:
5252
QgsFeatureId mFeatureIterator = 0;
5353

54-
bool mUsingFeatureIdList = false;
55-
QgsFeatureIds mFeatureIdList;
56-
QgsFeatureIds mRemainingFeatureIds;
54+
QList< QgsFeatureId > mFeatureIdList;
55+
QList< QgsFeatureId > mRemainingFeatureIds;
5756

5857
QgsCoordinateTransform mTransform;
5958
QgsRectangle mFilterRect;

src/providers/arcgisrest/qgsafsshareddata.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,22 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
140140

141141
return false;
142142
}
143+
144+
QgsFeatureIds QgsAfsSharedData::getFeatureIdsInExtent( const QgsRectangle &extent )
145+
{
146+
QString errorTitle;
147+
QString errorText;
148+
149+
150+
const QList<quint32> featuresInRect = QgsArcGisRestUtils::getObjectIdsByExtent( mDataSource.param( QStringLiteral( "url" ) ),
151+
extent, errorTitle, errorText );
152+
153+
QgsFeatureIds ids;
154+
for ( quint32 id : featuresInRect )
155+
{
156+
int featureId = mObjectIds.indexOf( id );
157+
if ( featureId >= 0 )
158+
ids.insert( featureId );
159+
}
160+
return ids;
161+
}

src/providers/arcgisrest/qgsafsshareddata.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class QgsAfsSharedData : public QObject
3737
void clearCache();
3838

3939
bool getFeature( QgsFeatureId id, QgsFeature &f, const QgsRectangle &filterRect = QgsRectangle() );
40+
QgsFeatureIds getFeatureIdsInExtent( const QgsRectangle &extent );
4041

4142
private:
4243
friend class QgsAfsProvider;

src/providers/arcgisrest/qgsarcgisrestutils.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,32 @@ QVariantMap QgsArcGisRestUtils::getObjects( const QString &layerurl, const QList
407407
return queryServiceJSON( queryUrl, errorTitle, errorText );
408408
}
409409

410+
QList<quint32> QgsArcGisRestUtils::getObjectIdsByExtent( const QString &layerurl, const QgsRectangle &filterRect, QString &errorTitle, QString &errorText )
411+
{
412+
QUrl queryUrl( layerurl + "/query" );
413+
queryUrl.addQueryItem( QStringLiteral( "f" ), QStringLiteral( "json" ) );
414+
queryUrl.addQueryItem( QStringLiteral( "where" ), QStringLiteral( "objectid=objectid" ) );
415+
queryUrl.addQueryItem( QStringLiteral( "returnIdsOnly" ), QStringLiteral( "true" ) );
416+
queryUrl.addQueryItem( QStringLiteral( "geometry" ), QStringLiteral( "%1,%2,%3,%4" )
417+
.arg( filterRect.xMinimum(), 0, 'f', -1 ).arg( filterRect.yMinimum(), 0, 'f', -1 )
418+
.arg( filterRect.xMaximum(), 0, 'f', -1 ).arg( filterRect.yMaximum(), 0, 'f', -1 ) );
419+
queryUrl.addQueryItem( QStringLiteral( "geometryType" ), QStringLiteral( "esriGeometryEnvelope" ) );
420+
queryUrl.addQueryItem( QStringLiteral( "spatialRel" ), QStringLiteral( "esriSpatialRelEnvelopeIntersects" ) );
421+
const QVariantMap objectIdData = queryServiceJSON( queryUrl, errorTitle, errorText );
422+
423+
if ( objectIdData.isEmpty() )
424+
{
425+
return QList<quint32>();
426+
}
427+
428+
QList<quint32> ids;
429+
foreach ( const QVariant &objectId, objectIdData["objectIds"].toList() )
430+
{
431+
ids << objectId.toInt();
432+
}
433+
return ids;
434+
}
435+
410436
QByteArray QgsArcGisRestUtils::queryService( const QUrl &u, QString &errorTitle, QString &errorText )
411437
{
412438
QEventLoop loop;

src/providers/arcgisrest/qgsarcgisrestutils.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <QStringList>
1919
#include <QVariant>
2020
#include "geometry/qgswkbtypes.h"
21+
#include "qgsfeature.h"
2122

2223
class QNetworkReply;
2324
class QgsNetworkAccessManager;
@@ -40,6 +41,7 @@ class QgsArcGisRestUtils
4041
static QVariantMap getObjects( const QString &layerurl, const QList<quint32> &objectIds, const QString &crs,
4142
bool fetchGeometry, const QStringList &fetchAttributes, bool fetchM, bool fetchZ,
4243
const QgsRectangle &filterRect, QString &errorTitle, QString &errorText );
44+
static QList<quint32> getObjectIdsByExtent( const QString &layerurl, const QgsRectangle &filterRect, QString &errorTitle, QString &errorText );
4345
static QByteArray queryService( const QUrl &url, QString &errorTitle, QString &errorText );
4446
static QVariantMap queryServiceJSON( const QUrl &url, QString &errorTitle, QString &errorText );
4547

tests/src/python/test_provider_afs.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,39 @@ def setUpClass(cls):
312312
]
313313
}""".encode('UTF-8'))
314314

315+
with open(sanitize(endpoint, '/query?f=json&where=objectid=objectid&returnIdsOnly=true&geometry=-70.000000,67.000000,-60.000000,80.000000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects'), 'wb') as f:
316+
f.write("""
317+
{
318+
"objectIdFieldName": "OBJECTID",
319+
"objectIds": [
320+
2,
321+
4
322+
]
323+
}
324+
""".encode('UTF-8'))
325+
326+
with open(sanitize(endpoint, '/query?f=json&where=objectid=objectid&returnIdsOnly=true&geometry=-73.000000,70.000000,-63.000000,80.000000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects'), 'wb') as f:
327+
f.write("""
328+
{
329+
"objectIdFieldName": "OBJECTID",
330+
"objectIds": [
331+
2,
332+
4
333+
]
334+
}
335+
""".encode('UTF-8'))
336+
337+
with open(sanitize(endpoint, '/query?f=json&where=objectid=objectid&returnIdsOnly=true&geometry=-68.721119,68.177676,-64.678700,79.123755&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects'), 'wb') as f:
338+
f.write("""
339+
{
340+
"objectIdFieldName": "OBJECTID",
341+
"objectIds": [
342+
2,
343+
4
344+
]
345+
}
346+
""".encode('UTF-8'))
347+
315348
@classmethod
316349
def tearDownClass(cls):
317350
"""Run after all tests"""

0 commit comments

Comments
 (0)