Skip to content

Commit

Permalink
Merge pull request #8185 from rouault/fix_19571
Browse files Browse the repository at this point in the history
[WFS client] Try to handle layers of type GeometryCollection if the first GeometryCollection is made of geometries of the same type (fixes #19571)
  • Loading branch information
rouault authored Oct 15, 2018
2 parents 4cc4bab + 9014285 commit c89a542
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 10 deletions.
63 changes: 60 additions & 3 deletions src/providers/wfs/qgswfsfeatureiterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#include "qgsmessagelog.h"
#include "qgsgeometry.h"
#include "qgsgml.h"
#include "qgsgeometrycollection.h"
#include "qgsmultipoint.h"
#include "qgsmultilinestring.h"
#include "qgsmultipolygon.h"
#include "qgsogcutils.h"
#include "qgswfsconstants.h"
#include "qgswfsfeatureiterator.h"
Expand Down Expand Up @@ -329,10 +333,20 @@ QUrl QgsWFSFeatureDownloader::buildURL( qint64 startIndex, int maxFeatures, bool
else if ( !mShared->mRect.isNull() )
{
bool invertAxis = false;
if ( !mShared->mWFSVersion.startsWith( QLatin1String( "1.0" ) ) && !mShared->mURI.ignoreAxisOrientation() &&
mShared->mSourceCRS.hasAxisInverted() )
if ( !mShared->mWFSVersion.startsWith( QLatin1String( "1.0" ) ) &&
!mShared->mURI.ignoreAxisOrientation() )
{
invertAxis = true;
// This is a bit nasty, but if the server reports OGC::CRS84
// mSourceCRS will report hasAxisInverted() == false, but srsName()
// will be urn:ogc:def:crs:EPSG::4326, so axis inversion is needed...
if ( mShared->srsName() == QLatin1String( "urn:ogc:def:crs:EPSG::4326" ) )
{
invertAxis = true;
}
else if ( mShared->mSourceCRS.hasAxisInverted() )
{
invertAxis = true;
}
}
if ( mShared->mURI.invertAxisOrientation() )
{
Expand Down Expand Up @@ -712,6 +726,49 @@ void QgsWFSFeatureDownloader::run( bool serializeFeatures, int maxFeatures )
f.setGeometry( g );
}

// If receiving a geometry collection, but expecting a multipoint/...,
// then try to convert it
if ( f.hasGeometry() &&
f.geometry().wkbType() == QgsWkbTypes::GeometryCollection &&
( mShared->mWKBType == QgsWkbTypes::MultiPoint ||
mShared->mWKBType == QgsWkbTypes::MultiLineString ||
mShared->mWKBType == QgsWkbTypes::MultiPolygon ) )
{
QgsWkbTypes::Type singleType = QgsWkbTypes::singleType( mShared->mWKBType );
auto g = f.geometry().constGet();
auto gc = qgsgeometry_cast<QgsGeometryCollection *>( g );
bool allExpectedType = true;
for ( int i = 0; i < gc->numGeometries(); ++i )
{
if ( gc->geometryN( i )->wkbType() != singleType )
{
allExpectedType = false;
break;
}
}
if ( allExpectedType )
{
QgsGeometryCollection *newGC;
if ( mShared->mWKBType == QgsWkbTypes::MultiPoint )
{
newGC = new QgsMultiPoint();
}
else if ( mShared->mWKBType == QgsWkbTypes::MultiLineString )
{
newGC = new QgsMultiLineString();
}
else
{
newGC = new QgsMultiPolygon();
}
for ( int i = 0; i < gc->numGeometries(); ++i )
{
newGC->addGeometry( gc->geometryN( i )->clone() );
}
f.setGeometry( QgsGeometry( newGC ) );
}
}

featureList.push_back( QgsWFSFeatureGmlIdPair( f, gmlId ) );
delete featPair.first;
if ( ( i > 0 && ( i % 1000 ) == 0 ) || i + 1 == featurePtrList.size() )
Expand Down
56 changes: 51 additions & 5 deletions src/providers/wfs/qgswfsprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ QgsWFSProvider::QgsWFSProvider( const QString &uri, const ProviderOptions &optio
//fetch attributes of layer and type of its geometry attribute
//WBC 111221: extracting geometry type here instead of getFeature allows successful
//layer creation even when no features are retrieved (due to, e.g., BBOX or FILTER)
if ( !describeFeatureType( mShared->mGeometryAttribute, mShared->mFields, mWKBType ) )
if ( !describeFeatureType( mShared->mGeometryAttribute, mShared->mFields, mShared->mWKBType ) )
{
mValid = false;
return;
Expand All @@ -120,7 +120,7 @@ QgsWFSProvider::QgsWFSProvider( const QString &uri, const ProviderOptions &optio
}

//Failed to detect feature type from describeFeatureType -> get first feature from layer to detect type
if ( mWKBType == QgsWkbTypes::Unknown )
if ( mShared->mWKBType == QgsWkbTypes::Unknown )
{
QgsWFSFeatureDownloader downloader( mShared.get() );
connect( &downloader, static_cast<void ( QgsWFSFeatureDownloader::* )( QVector<QgsWFSFeatureGmlIdPair> )>( &QgsWFSFeatureDownloader::featureReceived ),
Expand Down Expand Up @@ -478,7 +478,7 @@ bool QgsWFSProvider::processSQL( const QString &sqlString, QString &errorMsg, QS
if ( typeName == mShared->mURI.typeName() )
{
mShared->mGeometryAttribute = geometryAttribute;
mWKBType = geomType;
mShared->mWKBType = geomType;
mThisTypenameFields = fields;
}

Expand Down Expand Up @@ -665,7 +665,53 @@ void QgsWFSProvider::featureReceivedAnalyzeOneFeature( QVector<QgsWFSFeatureGmlI
QgsGeometry geometry = feat.geometry();
if ( !geometry.isNull() )
{
mWKBType = geometry.wkbType();
mShared->mWKBType = geometry.wkbType();

// Fragile heuristics that helps for
// https://sampleservices.luciad.com/ogc/wfs/sampleswfs?REQUEST=GetFeature&SERVICE=WFS&TYPENAME=rivers&VERSION=1.1.0
// In case the geometry is a geometry collection, analyze its members to
// see if they are of the same type. If then, assume that all features
// will be similar, and report the proper MultiPoint/MultiLineString/
// MultiPolygon type instead.
if ( mShared->mWKBType == QgsWkbTypes::GeometryCollection )
{
auto geomColl = geometry.asGeometryCollection();
mShared->mWKBType = QgsWkbTypes::Unknown;
for ( const auto &geom : geomColl )
{
if ( mShared->mWKBType == QgsWkbTypes::Unknown )
{
mShared->mWKBType = geom.wkbType();
}
else if ( mShared->mWKBType != geom.wkbType() )
{
mShared->mWKBType = QgsWkbTypes::Unknown;
break;
}
}
if ( mShared->mWKBType != QgsWkbTypes::Unknown )
{
if ( mShared->mWKBType == QgsWkbTypes::Point )
{
QgsDebugMsg( QStringLiteral( "Layer of unknown type. First element is a GeometryCollection of Point. Advertizing optimistically as MultiPoint" ) );
mShared->mWKBType = QgsWkbTypes::MultiPoint;
}
else if ( mShared->mWKBType == QgsWkbTypes::LineString )
{
QgsDebugMsg( QStringLiteral( "Layer of unknown type. First element is a GeometryCollection of LineString. Advertizing optimistically as MultiLineString" ) );
mShared->mWKBType = QgsWkbTypes::MultiLineString;
}
else if ( mShared->mWKBType == QgsWkbTypes::Polygon )
{
QgsDebugMsg( QStringLiteral( "Layer of unknown type. First element is a GeometryCollection of Polygon. Advertizing optimistically as MultiPolygon" ) );
mShared->mWKBType = QgsWkbTypes::MultiPolygon;
}
else
{
mShared->mWKBType = QgsWkbTypes::Unknown;
}
}
}
}
}
}
Expand Down Expand Up @@ -744,7 +790,7 @@ void QgsWFSProvider::reloadData()

QgsWkbTypes::Type QgsWFSProvider::wkbType() const
{
return mWKBType;
return mShared->mWKBType;
}

long QgsWFSProvider::featureCount() const
Expand Down
2 changes: 0 additions & 2 deletions src/providers/wfs/qgswfsprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@ class QgsWFSProvider : public QgsVectorDataProvider
//! String used to define a subset of the layer
QString mSubsetString;

//! Geometry type of the features in this layer
mutable QgsWkbTypes::Type mWKBType = QgsWkbTypes::Unknown;
//! Flag if provider is valid
bool mValid = true;
//! Namespace URL of the server (comes from DescribeFeatureDocument)
Expand Down
3 changes: 3 additions & 0 deletions src/providers/wfs/qgswfsshareddata.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ class QgsWFSSharedData : public QObject
EPSG:XXXX srsName and not EPSG urns */
bool mGetFeatureEPSGDotHonoursEPSGOrder;

//! Geometry type of the features in this layer
QgsWkbTypes::Type mWKBType = QgsWkbTypes::Unknown;

private:

//! Main mutex to protect most data members that can be modified concurrently
Expand Down
96 changes: 96 additions & 0 deletions tests/src/python/test_provider_wfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3485,6 +3485,102 @@ def testDescribeFeatureTypeWithSingleInclude(self):
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename'", 'test', 'WFS')
self.assertTrue(vl.isValid())

def testGeometryCollectionAsMultiLineString(self):
"""Test https://issues.qgis.org/issues/19571 """

endpoint = self.__class__.basetestpath + '/fake_qgis_http_endpoint_gc_as_mls'

with open(sanitize(endpoint, '?SERVICE=WFS?REQUEST=GetCapabilities?VERSION=1.1.0'), 'wb') as f:
f.write("""
<wfs:WFS_Capabilities version="1.1.0" xmlns="http://www.opengis.net/wfs" xmlns:wfs="http://www.opengis.net/wfs" xmlns:ogc="http://www.opengis.net/ogc" xmlns:ows="http://www.opengis.net/ows" xmlns:gml="http://schemas.opengis.net/gml">
<FeatureTypeList>
<FeatureType>
<Name>my:typename</Name>
<Title>Title</Title>
<Abstract>Abstract</Abstract>
<DefaultCRS>urn:ogc:def:crs:OGC:1.3:CRS84</DefaultCRS>
<WGS84BoundingBox>
<LowerCorner>2 49</LowerCorner>
<UpperCorner>3 50</UpperCorner>
</WGS84BoundingBox>
</FeatureType>
</FeatureTypeList>
</wfs:WFS_Capabilities>""".encode('UTF-8'))

with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=DescribeFeatureType&VERSION=1.1.0&TYPENAME=my:typename'), 'wb') as f:
f.write("""
<schema
targetNamespace="http://my"
xmlns:my="http://my"
xmlns:ogc="http://www.opengis.net/ogc"
xmlns:xsd="http://www.w3.org/2001/XMLSchema"
xmlns="http://www.w3.org/2001/XMLSchema"
xmlns:gml="http://www.opengis.net/gml"
elementFormDefault="qualified" version="0.1" >
<import namespace="http://www.opengis.net/gml"
schemaLocation="http://schemas.opengis.net/gml/3.1.1/base/gml.xsd" />
<element name="typename"
type="my:typenameType"
substitutionGroup="gml:_Feature" />
<complexType name="typenameType">
<complexContent>
<extension base="gml:AbstractFeatureType">
<sequence>
<element name="geometryProperty" type="gml:GeometryPropertyType" minOccurs="0" maxOccurs="1"/>
</sequence>
</extension>
</complexContent>
</complexType>
</schema>
""".encode('UTF-8'))

get_features = """
<wfs:FeatureCollection
xmlns:my="http://my"
xmlns:gml="http://www.opengis.net/gml"
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:ogc="http://www.opengis.net/ogc">
<gml:boundedBy>
<gml:Envelope srsName="urn:ogc:def:crs:OGC:1.3:CRS84">
<gml:lowerCorner>2 49</gml:lowerCorner>
<gml:upperCorner>3 50</gml:upperCorner>
</gml:Envelope>
</gml:boundedBy>
<gml:featureMember>
<my:typename gml:id="typename.1">
<my:geometryProperty>
<gml:MultiGeometry srsName="urn:ogc:def:crs:OGC:1.3:CRS84">
<gml:geometryMember>
<gml:LineString>
<gml:coordinates>2,49 3,50</gml:coordinates>
</gml:LineString>
</gml:geometryMember>
</gml:MultiGeometry>
</my:geometryProperty>
</my:typename>
</gml:featureMember>
</wfs:FeatureCollection>
"""

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:
f.write(get_features.encode('UTF-8'))

with open(sanitize(endpoint, """?SERVICE=WFS&REQUEST=GetFeature&VERSION=1.1.0&TYPENAME=my:typename&SRSNAME=urn:ogc:def:crs:EPSG::4326"""), 'wb') as f:
f.write(get_features.encode('UTF-8'))

vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.1.0'", 'test', 'WFS')
self.assertTrue(vl.isValid())

got_f = [f for f in vl.getFeatures()]
geom = got_f[0].geometry().constGet()
geom_string = geom.asWkt()
geom_string = re.sub(r'\.\d+', '', geom_string)
self.assertEqual(geom_string, 'MultiLineString ((2 49, 3 50))')

reference = QgsGeometry.fromRect(QgsRectangle(2, 49, 3, 50))
vl_extent = QgsGeometry.fromRect(vl.extent())
assert QgsGeometry.compare(vl_extent.asPolygon()[0], reference.asPolygon()[0], 0.00001), 'Expected {}, got {}'.format(reference.asWkt(), vl_extent.asWkt())


if __name__ == '__main__':
unittest.main()

0 comments on commit c89a542

Please sign in to comment.