Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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

(cherry picked from commit 182b1e1)
  • Loading branch information
nyalldawson committed Jun 10, 2019
1 parent e3b4182 commit 7557703
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 54 deletions.
62 changes: 24 additions & 38 deletions src/providers/arcgisrest/qgsafsshareddata.cpp
Expand Up @@ -35,17 +35,6 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
return filterRect.isNull() || ( f.hasGeometry() && f.geometry().intersects( filterRect ) );
}

// When fetching from server, fetch all attributes and geometry by default so that we can cache them
QStringList fetchAttribNames;
QList<int> fetchAttribIdx;
fetchAttribIdx.reserve( mFields.size() );
fetchAttribNames.reserve( mFields.size() );
for ( int idx = 0, n = mFields.size(); idx < n; ++idx )
{
fetchAttribNames.append( mFields.at( idx ).name() );
fetchAttribIdx.append( idx );
}

// Fetch 100 features at the time
int startId = ( id / 100 ) * 100;
int stopId = std::min( startId + 100, mObjectIds.length() );
Expand All @@ -72,7 +61,7 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
const QString authcfg = mDataSource.authConfigId();
const QVariantMap queryData = QgsArcGisRestUtils::getObjects(
mDataSource.param( QStringLiteral( "url" ) ), authcfg, objectIds, mDataSource.param( QStringLiteral( "crs" ) ), true,
fetchAttribNames, QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ),
QStringList(), QgsWkbTypes::hasM( mGeometryType ), QgsWkbTypes::hasZ( mGeometryType ),
filterRect, errorTitle, errorMessage, feedback );

if ( queryData.isEmpty() )
Expand All @@ -97,36 +86,33 @@ bool QgsAfsSharedData::getFeature( QgsFeatureId id, QgsFeature &f, const QgsRect
int featureId = startId + i;

// Set attributes
if ( !fetchAttribIdx.isEmpty() )
const QVariantMap attributesData = featureData[QStringLiteral( "attributes" )].toMap();
feature.setFields( mFields );
QgsAttributes attributes( mFields.size() );
for ( int idx = 0; idx < mFields.size(); ++idx )
{
const QVariantMap attributesData = featureData[QStringLiteral( "attributes" )].toMap();
feature.setFields( mFields );
QgsAttributes attributes( mFields.size() );
for ( int idx : qgis::as_const( fetchAttribIdx ) )
QVariant attribute = attributesData[mFields.at( idx ).name()];
if ( attribute.isNull() )
{
// ensure that null values are mapped correctly for PyQGIS
attribute = QVariant( QVariant::Int );
}

// date/datetime fields must be converted
if ( mFields.at( idx ).type() == QVariant::DateTime || mFields.at( idx ).type() == QVariant::Date )
attribute = QgsArcGisRestUtils::parseDateTime( attribute );

if ( !mFields.at( idx ).convertCompatible( attribute ) )
{
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() ) );
}
attributes[idx] = attribute;
if ( mFields.at( idx ).name() == mObjectIdFieldName )
{
QVariant attribute = attributesData[mFields.at( idx ).name()];
if ( attribute.isNull() )
{
// ensure that null values are mapped correctly for PyQGIS
attribute = QVariant( QVariant::Int );
}

// date/datetime fields must be converted
if ( mFields.at( idx ).type() == QVariant::DateTime || mFields.at( idx ).type() == QVariant::Date )
attribute = QgsArcGisRestUtils::parseDateTime( attribute );

if ( !mFields.at( idx ).convertCompatible( attribute ) )
{
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() ) );
}
attributes[idx] = attribute;
if ( mFields.at( idx ).name() == mObjectIdFieldName )
{
featureId = startId + objectIds.indexOf( attributesData[mFields.at( idx ).name()].toInt() );
}
featureId = startId + objectIds.indexOf( attributesData[mFields.at( idx ).name()].toInt() );
}
feature.setAttributes( attributes );
}
feature.setAttributes( attributes );

// Set FID
feature.setId( featureId );
Expand Down
19 changes: 9 additions & 10 deletions src/providers/arcgisrest/qgsarcgisrestutils.cpp
Expand Up @@ -433,17 +433,16 @@ QVariantMap QgsArcGisRestUtils::getObjects( const QString &layerurl, const QStri
QString wkid = crs.indexOf( QLatin1String( ":" ) ) >= 0 ? crs.split( ':' )[1] : QString();
queryUrl.addQueryItem( QStringLiteral( "inSR" ), wkid );
queryUrl.addQueryItem( QStringLiteral( "outSR" ), wkid );
QString outFields = fetchAttributes.join( QStringLiteral( "," ) );
if ( fetchGeometry )
{
queryUrl.addQueryItem( QStringLiteral( "returnGeometry" ), QStringLiteral( "true" ) );
queryUrl.addQueryItem( QStringLiteral( "outFields" ), outFields );
}

queryUrl.addQueryItem( QStringLiteral( "returnGeometry" ), fetchGeometry ? QStringLiteral( "true" ) : QStringLiteral( "false" ) );

QString outFields;
if ( fetchAttributes.isEmpty() )
outFields = QStringLiteral( "*" );
else
{
queryUrl.addQueryItem( QStringLiteral( "returnGeometry" ), QStringLiteral( "false" ) );
queryUrl.addQueryItem( QStringLiteral( "outFields" ), outFields );
}
outFields = fetchAttributes.join( ',' );
queryUrl.addQueryItem( QStringLiteral( "outFields" ), outFields );

queryUrl.addQueryItem( QStringLiteral( "returnM" ), fetchM ? QStringLiteral( "true" ) : QStringLiteral( "false" ) );
queryUrl.addQueryItem( QStringLiteral( "returnZ" ), fetchZ ? QStringLiteral( "true" ) : QStringLiteral( "false" ) );
if ( !filterRect.isNull() )
Expand Down
12 changes: 6 additions & 6 deletions tests/src/python/test_provider_afs.py
Expand Up @@ -129,7 +129,7 @@ def setUpClass(cls):
cls.source = cls.vl.dataProvider()

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'),
'/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=*&returnM=false&returnZ=false'),
'wb') as f:
f.write("""
{
Expand Down Expand Up @@ -220,7 +220,7 @@ def setUpClass(cls):
]
}""".encode('UTF-8'))

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:
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:
f.write("""
{
"displayFieldName": "name",
Expand Down Expand Up @@ -311,7 +311,7 @@ def setUpClass(cls):
}""".encode('UTF-8'))

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

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:
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:
f.write("""
{
"displayFieldName": "LABEL",
Expand Down Expand Up @@ -529,7 +529,7 @@ def testDateTime(self):

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

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

0 comments on commit 7557703

Please sign in to comment.