From 244ba5ca3ad0ab44e7ee4bc7ca554bc88a1b4f90 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Thu, 25 Oct 2018 07:08:00 +1000 Subject: [PATCH] [afs] Handle invalid responses returned from FeatureServer multipoint layers, where individual features may have point geometries Not sure if this is a bug in ArcGIS server (probably, yeah, let's go with definitely. I couldn't check the source to see.) But in general QGIS approach is to be forgiving and do our best to make up for badly behaved servers). See https://community.esri.com/thread/14037 --- .../arcgisrest/qgsarcgisrestutils.cpp | 22 +++-- tests/src/python/test_provider_afs.py | 86 +++++++++++++++++++ 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/providers/arcgisrest/qgsarcgisrestutils.cpp b/src/providers/arcgisrest/qgsarcgisrestutils.cpp index 7d1d7907aa0..a1e7d35ea27 100644 --- a/src/providers/arcgisrest/qgsarcgisrestutils.cpp +++ b/src/providers/arcgisrest/qgsarcgisrestutils.cpp @@ -202,21 +202,31 @@ static std::unique_ptr< QgsPoint > parseEsriGeometryPoint( const QVariantMap &ge static std::unique_ptr< QgsMultiPoint > parseEsriGeometryMultiPoint( const QVariantMap &geometryData, QgsWkbTypes::Type pointType ) { // {"points" : [[ , , , ] , [ , , , ], ... ]} - QVariantList coordsList = geometryData[QStringLiteral( "points" )].toList(); - if ( coordsList.isEmpty() ) - return nullptr; + const QVariantList coordsList = geometryData[QStringLiteral( "points" )].toList(); std::unique_ptr< QgsMultiPoint > multiPoint = qgis::make_unique< QgsMultiPoint >(); - Q_FOREACH ( const QVariant &coordData, coordsList ) + for ( const QVariant &coordData : coordsList ) { - QVariantList coordList = coordData.toList(); + const QVariantList coordList = coordData.toList(); std::unique_ptr< QgsPoint > p = parsePoint( coordList, pointType ); if ( !p ) { - return nullptr; + continue; } multiPoint->addGeometry( p.release() ); } + + // second chance -- sometimes layers are reported as multipoint but features have single + // point geometries. Silently handle this and upgrade to multipoint. + std::unique_ptr< QgsPoint > p = parseEsriGeometryPoint( geometryData, pointType ); + if ( p ) + multiPoint->addGeometry( p.release() ); + + if ( multiPoint->numGeometries() == 0 ) + { + // didn't find any points, so reset geometry to null + multiPoint.reset(); + } return multiPoint; } diff --git a/tests/src/python/test_provider_afs.py b/tests/src/python/test_provider_afs.py index f32d4ef4141..b0c209c2e0a 100644 --- a/tests/src/python/test_provider_afs.py +++ b/tests/src/python/test_provider_afs.py @@ -710,6 +710,92 @@ class TestPyQgsAFSProvider(unittest.TestCase, ProviderTestCase): self.assertEqual(vl.featureCount(), 2) self.assertEqual([f['pk'] for f in vl.getFeatures()], [2, 4]) + def testBadMultiPoints(self): + """ + Test invalid server response where a layer's type is multipoint but single point geometries + are returned. Thanks Jack. Thack. + """ + endpoint = self.basetestpath + '/multipoint_fake_qgis_http_endpoint' + with open(sanitize(endpoint, '?f=json'), 'wb') as f: + f.write(""" + {"currentVersion":10.22,"id":1,"name":"QGIS Test","type":"Feature Layer","description": + "QGIS Provider Test Layer.\n","geometryType":"esriGeometryMultipoint","copyrightText":"","parentLayer":{"id":0,"name":"QGIS Tests"},"subLayers":[], + "minScale":72225,"maxScale":0, + "defaultVisibility":true, + "extent":{"xmin":-71.123,"ymin":66.33,"xmax":-65.32,"ymax":78.3, + "spatialReference":{"wkid":4326,"latestWkid":4326}}, + "hasAttachments":false,"htmlPopupType":"esriServerHTMLPopupTypeAsHTMLText", + "displayField":"LABEL","typeIdField":null, + "fields":[{"name":"OBJECTID","type":"esriFieldTypeOID","alias":"OBJECTID","domain":null}], + "relationships":[],"canModifyLayer":false,"canScaleSymbols":false,"hasLabels":false, + "capabilities":"Map,Query,Data","maxRecordCount":1000,"supportsStatistics":true, + "supportsAdvancedQueries":true,"supportedQueryFormats":"JSON, AMF", + "ownershipBasedAccessControlForFeatures":{"allowOthersToQuery":true},"useStandardizedQueries":true}""".encode( + 'UTF-8')) + + with open(sanitize(endpoint, '/query?f=json_where=OBJECTID=OBJECTID_returnIdsOnly=true'), 'wb') as f: + f.write(""" + { + "objectIdFieldName": "OBJECTID", + "objectIds": [ + 1, + 2, + 3 + ] + } + """.encode('UTF-8')) + + # Create test layer + vl = QgsVectorLayer("url='http://" + endpoint + "' crs='epsg:4326'", 'test', 'arcgisfeatureserver') + + 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: + f.write(""" + { + "displayFieldName": "name", + "fieldAliases": { + "name": "name" + }, + "geometryType": "esriGeometryMultipoint", + "spatialReference": { + "wkid": 4326, + "latestWkid": 4326 + }, + "fields":[{"name":"OBJECTID","type":"esriFieldTypeOID","alias":"OBJECTID","domain":null}, + {"name":"Shape","type":"esriFieldTypeGeometry","alias":"Shape","domain":null}], + "features": [ + { + "attributes": { + "OBJECTID": 1 + }, + "geometry": { + "x": -70, + "y": 66 + } + }, + { + "attributes": { + "OBJECTID": 2 + }, + "geometry": null + }, + { + "attributes": { + "OBJECTID": 3 + }, + "geometry": + {"points" :[[-68,70], + [-22,21]] + } + } + ] + }""".encode('UTF-8')) + + features = [f for f in vl.getFeatures()] + self.assertEqual(len(features), 3) + self.assertEqual([f.geometry().asWkt() for f in features], ['MultiPoint ((-70 66))', '', 'MultiPoint ((-68 70),(-22 21))']) + if __name__ == '__main__': unittest.main()