Skip to content

Commit 182b1e1

Browse files
nyalldawsonnyalldawson
committedJun 10, 2019
Use * to fetch all fields from feature services, don't list them one by one
Otherwise we often exceed the maximum size of a get query
1 parent c9f8b19 commit 182b1e1

File tree

3 files changed

+39
-54
lines changed

3 files changed

+39
-54
lines changed
 

‎src/providers/arcgisrest/qgsafsshareddata.cpp

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,6 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
3535
return filterRect.isNull() || ( f.hasGeometry() && f.geometry().intersects( filterRect ) );
3636
}
3737

38-
// When fetching from server, fetch all attributes and geometry by default so that we can cache them
39-
QStringList fetchAttribNames;
40-
QList<int> fetchAttribIdx;
41-
fetchAttribIdx.reserve( mFields.size() );
42-
fetchAttribNames.reserve( mFields.size() );
43-
for ( int idx = 0, n = mFields.size(); idx < n; ++idx )
44-
{
45-
fetchAttribNames.append( mFields.at( idx ).name() );
46-
fetchAttribIdx.append( idx );
47-
}
48-
4938
// Fetch 100 features at the time
5039
int startId = ( id / 100 ) * 100;
5140
int stopId = std::min( startId + 100, mObjectIds.length() );
@@ -77,7 +66,7 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
7766

7867
const QVariantMap queryData = QgsArcGisRestUtils::getObjects(
7968
mDataSource.param( QStringLiteral( "url" ) ), authcfg, objectIds, mDataSource.param( QStringLiteral( "crs" ) ), true,
80-
fetchAttribNames, QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ),
69+
QStringList(), QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ),
8170
filterRect, errorTitle, errorMessage, headers, feedback );
8271

8372
if ( queryData.isEmpty() )
@@ -102,36 +91,33 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
10291
int featureId = startId + i;
10392

10493
// Set attributes
105-
if ( !fetchAttribIdx.isEmpty() )
94+
const QVariantMap attributesData = featureData[QStringLiteral( "attributes" )].toMap();
95+
feature.setFields( mFields );
96+
QgsAttributes attributes( mFields.size() );
97+
for ( int idx = 0; idx < mFields.size(); ++idx )
10698
{
107-
const QVariantMap attributesData = featureData[QStringLiteral( "attributes" )].toMap();
108-
feature.setFields( mFields );
109-
QgsAttributes attributes( mFields.size() );
110-
for ( int idx : qgis::as_const( fetchAttribIdx ) )
99+
QVariant attribute = attributesData[mFields.at( idx ).name()];
100+
if ( attribute.isNull() )
101+
{
102+
// ensure that null values are mapped correctly for PyQGIS
103+
attribute = QVariant( QVariant::Int );
104+
}
105+
106+
// date/datetime fields must be converted
107+
if ( mFields.at( idx ).type() == QVariant::DateTime || mFields.at( idx ).type() == QVariant::Date )
108+
attribute = QgsArcGisRestUtils::parseDateTime( attribute );
109+
110+
if ( !mFields.at( idx ).convertCompatible( attribute ) )
111+
{
112+
QgsDebugMsg( QStringLiteral( "Invalid value %1 for field %2 of type %3" ).arg( attributesData[mFields.at( idx ).name()].toString(), mFields.at( idx ).name(), mFields.at( idx ).typeName() ) );
113+
}
114+
attributes[idx] = attribute;
115+
if ( mFields.at( idx ).name() == mObjectIdFieldName )
111116
{
112-
QVariant attribute = attributesData[mFields.at( idx ).name()];
113-
if ( attribute.isNull() )
114-
{
115-
// ensure that null values are mapped correctly for PyQGIS
116-
attribute = QVariant( QVariant::Int );
117-
}
118-
119-
// date/datetime fields must be converted
120-
if ( mFields.at( idx ).type() == QVariant::DateTime || mFields.at( idx ).type() == QVariant::Date )
121-
attribute = QgsArcGisRestUtils::parseDateTime( attribute );
122-
123-
if ( !mFields.at( idx ).convertCompatible( attribute ) )
124-
{
125-
QgsDebugMsg( QStringLiteral( "Invalid value %1 for field %2 of type %3" ).arg( attributesData[mFields.at( idx ).name()].toString(), mFields.at( idx ).name(), mFields.at( idx ).typeName() ) );
126-
}
127-
attributes[idx] = attribute;
128-
if ( mFields.at( idx ).name() == mObjectIdFieldName )
129-
{
130-
featureId = startId + objectIds.indexOf( attributesData[mFields.at( idx ).name()].toInt() );
131-
}
117+
featureId = startId + objectIds.indexOf( attributesData[mFields.at( idx ).name()].toInt() );
132118
}
133-
feature.setAttributes( attributes );
134119
}
120+
feature.setAttributes( attributes );
135121

136122
// Set FID
137123
feature.setId( featureId );

‎src/providers/arcgisrest/qgsarcgisrestutils.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -439,17 +439,16 @@ QVariantMap QgsArcGisRestUtils::getObjects( const QString &layerurl, const QStri
439439
QString wkid = crs.indexOf( QLatin1String( ":" ) ) >= 0 ? crs.split( ':' )[1] : QString();
440440
queryUrl.addQueryItem( QStringLiteral( "inSR" ), wkid );
441441
queryUrl.addQueryItem( QStringLiteral( "outSR" ), wkid );
442-
QString outFields = fetchAttributes.join( QStringLiteral( "," ) );
443-
if ( fetchGeometry )
444-
{
445-
queryUrl.addQueryItem( QStringLiteral( "returnGeometry" ), QStringLiteral( "true" ) );
446-
queryUrl.addQueryItem( QStringLiteral( "outFields" ), outFields );
447-
}
442+
443+
queryUrl.addQueryItem( QStringLiteral( "returnGeometry" ), fetchGeometry ? QStringLiteral( "true" ) : QStringLiteral( "false" ) );
444+
445+
QString outFields;
446+
if ( fetchAttributes.isEmpty() )
447+
outFields = QStringLiteral( "*" );
448448
else
449-
{
450-
queryUrl.addQueryItem( QStringLiteral( "returnGeometry" ), QStringLiteral( "false" ) );
451-
queryUrl.addQueryItem( QStringLiteral( "outFields" ), outFields );
452-
}
449+
outFields = fetchAttributes.join( ',' );
450+
queryUrl.addQueryItem( QStringLiteral( "outFields" ), outFields );
451+
453452
queryUrl.addQueryItem( QStringLiteral( "returnM" ), fetchM ? QStringLiteral( "true" ) : QStringLiteral( "false" ) );
454453
queryUrl.addQueryItem( QStringLiteral( "returnZ" ), fetchZ ? QStringLiteral( "true" ) : QStringLiteral( "false" ) );
455454
if ( !filterRect.isNull() )

‎tests/src/python/test_provider_afs.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ def setUpClass(cls):
129129
cls.source = cls.vl.dataProvider()
130130

131131
with open(sanitize(endpoint,
132-
'/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false'),
132+
'/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false'),
133133
'wb') as f:
134134
f.write("""
135135
{
@@ -220,7 +220,7 @@ def setUpClass(cls):
220220
]
221221
}""".encode('UTF-8'))
222222

223-
with open(sanitize(endpoint, '/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false&geometry=-71.123000,66.330000,-65.320000,78.300000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects'), 'wb') as f:
223+
with open(sanitize(endpoint, '/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false&geometry=-71.123000,66.330000,-65.320000,78.300000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects'), 'wb') as f:
224224
f.write("""
225225
{
226226
"displayFieldName": "name",
@@ -311,7 +311,7 @@ def setUpClass(cls):
311311
}""".encode('UTF-8'))
312312

313313
with open(sanitize(endpoint,
314-
'/query?f=json&objectIds=2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false'),
314+
'/query?f=json&objectIds=2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false'),
315315
'wb') as f:
316316
f.write("""
317317
{
@@ -462,7 +462,7 @@ def testObjectIdDifferentName(self):
462462
}
463463
""".encode('UTF-8'))
464464

465-
with open(sanitize(endpoint, '/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID1,pk,cnt&returnM=false&returnZ=false'), 'wb') as f:
465+
with open(sanitize(endpoint, '/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false'), 'wb') as f:
466466
f.write("""
467467
{
468468
"displayFieldName": "LABEL",
@@ -537,7 +537,7 @@ def testDateTime(self):
537537

538538
self.assertTrue(vl.isValid())
539539
with open(sanitize(endpoint,
540-
'/query?f=json&objectIds=1,2&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID,pk,dt&returnM=false&returnZ=false'), 'wb') as f:
540+
'/query?f=json&objectIds=1,2&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false'), 'wb') as f:
541541
f.write("""
542542
{
543543
"displayFieldName": "name",
@@ -790,7 +790,7 @@ def testBadMultiPoints(self):
790790

791791
self.assertTrue(vl.isValid())
792792
with open(sanitize(endpoint,
793-
'/query?f=json&objectIds=1,2,3&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID&returnM=false&returnZ=false'), 'wb') as f:
793+
'/query?f=json&objectIds=1,2,3&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false'), 'wb') as f:
794794
f.write("""
795795
{
796796
"displayFieldName": "name",

0 commit comments

Comments
 (0)