Skip to content

Commit

Permalink
[WFS provider] Support layers with GML field names only differing by …
Browse files Browse the repository at this point in the history 
…cases (github fixes #29858)
  • Loading branch information
@rouault @nyalldawson
rouault authored and nyalldawson committed May 27, 2019
1 parent 54cf26f commit 4651892
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 10 deletions.
12 changes: 6 additions & 6 deletions src/providers/wfs/qgswfsfeatureiterator.cpp
View file
Expand Up @@ -1018,7 +1018,7 @@ QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
QgsAttributeList cacheSubSet;
Q_FOREACH ( int i, mRequest.subsetOfAttributes() )
{
int idx = dataProviderFields.indexFromName( mShared->mFields.at( i ).name() );
int idx = dataProviderFields.indexFromName( mShared->mMapGMLFieldNameToSQLiteColumnName[mShared->mFields.at( i ).name()] );
if ( idx >= 0 )
cacheSubSet.append( idx );
idx = mShared->mFields.indexFromName( mShared->mFields.at( i ).name() );
Expand All @@ -1031,7 +1031,7 @@ QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
{
Q_FOREACH ( const QString &field, mRequest.filterExpression()->referencedColumns() )
{
int idx = dataProviderFields.indexFromName( field );
int idx = dataProviderFields.indexFromName( mShared->mMapGMLFieldNameToSQLiteColumnName[field] );
if ( idx >= 0 && !cacheSubSet.contains( idx ) )
cacheSubSet.append( idx );
idx = mShared->mFields.indexFromName( field );
Expand Down Expand Up @@ -1241,7 +1241,7 @@ bool QgsWFSFeatureIterator::fetchFeature( QgsFeature &f )
continue;
}

copyFeature( cachedFeature, f );
copyFeature( cachedFeature, f, true );
geometryToDestinationCrs( f, mTransform );

// Retrieve the user-visible id from the Spatialite cache database Id
Expand Down Expand Up @@ -1335,7 +1335,7 @@ bool QgsWFSFeatureIterator::fetchFeature( QgsFeature &f )
continue;
}

copyFeature( feat, f );
copyFeature( feat, f, false );
return true;
}

Expand Down Expand Up @@ -1422,7 +1422,7 @@ bool QgsWFSFeatureIterator::close()
}


void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeature &dstFeature )
void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeature &dstFeature, bool srcIsCache )
{
//copy the geometry
QgsGeometry geometry = srcFeature.geometry();
Expand All @@ -1443,7 +1443,7 @@ void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeatur

auto setAttr = [ & ]( const int i )
{
int idx = srcFeature.fields().indexFromName( fields.at( i ).name() );
int idx = srcFeature.fields().indexFromName( srcIsCache ? mShared->mMapGMLFieldNameToSQLiteColumnName[fields.at( i ).name()] : fields.at( i ).name() );
if ( idx >= 0 )
{
const QVariant &v = srcFeature.attributes().value( idx );
Expand Down
2 changes: 1 addition & 1 deletion src/providers/wfs/qgswfsfeatureiterator.h
View file
Expand Up @@ -227,7 +227,7 @@ class QgsWFSFeatureIterator : public QObject,
bool fetchFeature( QgsFeature &f ) override;

//! Copies feature attributes / geometry from srcFeature to dstFeature
void copyFeature( const QgsFeature &srcFeature, QgsFeature &dstFeature );
void copyFeature( const QgsFeature &srcFeature, QgsFeature &dstFeature, bool srcIsCache );

std::shared_ptr<QgsWFSSharedData> mShared; //!< Mutable data shared between provider and feature sources

Expand Down
23 changes: 20 additions & 3 deletions src/providers/wfs/qgswfsshareddata.cpp
View file
Expand Up @@ -33,6 +33,8 @@
#include <cpl_conv.h>
#include <ogr_api.h>

#include <set>

#include <sqlite3.h>

QgsWFSSharedData::QgsWFSSharedData( const QString &uri )
Expand Down Expand Up @@ -230,6 +232,7 @@ bool QgsWFSSharedData::createCache()
Q_ASSERT( !QFile::exists( mCacheDbname ) );

QgsFields cacheFields;
std::set<QString> setSQLiteColumnNameUpperCase;
for ( const QgsField &field : qgis::as_const( mFields ) )
{
QVariant::Type type = field.type();
Expand All @@ -240,7 +243,19 @@ bool QgsWFSSharedData::createCache()
// it to a String
type = QVariant::LongLong;
}
cacheFields.append( QgsField( field.name(), type, field.typeName() ) );

// Make sure we don't have several field names that only differ by their case
QString sqliteFieldName( field.name() );
int counter = 2;
while ( setSQLiteColumnNameUpperCase.find( sqliteFieldName.toUpper() ) != setSQLiteColumnNameUpperCase.end() )
{
sqliteFieldName = field.name() + QStringLiteral( "%1" ).arg( counter );
counter++;
}
setSQLiteColumnNameUpperCase.insert( sqliteFieldName.toUpper() );
mMapGMLFieldNameToSQLiteColumnName[field.name()] = sqliteFieldName;

cacheFields.append( QgsField( sqliteFieldName, type, field.typeName() ) );
}
// Add some field for our internal use
cacheFields.append( QgsField( QgsWFSConstants::FIELD_GEN_COUNTER, QVariant::Int, QStringLiteral( "int" ) ) );
Expand Down Expand Up @@ -384,6 +399,7 @@ bool QgsWFSSharedData::createCache()
{
mCacheTablename = QStringLiteral( "features" );
sql = QStringLiteral( "CREATE TABLE %1 (%2 INTEGER PRIMARY KEY" ).arg( mCacheTablename, fidName );

for ( const QgsField &field : qgis::as_const( cacheFields ) )
{
QString type( QStringLiteral( "VARCHAR" ) );
Expand All @@ -393,6 +409,7 @@ bool QgsWFSSharedData::createCache()
type = QStringLiteral( "BIGINT" );
else if ( field.type() == QVariant::Double )
type = QStringLiteral( "REAL" );

sql += QStringLiteral( ", %1 %2" ).arg( quotedIdentifier( field.name() ), type );
}
sql += QLatin1String( ")" );
Expand Down Expand Up @@ -867,7 +884,7 @@ bool QgsWFSSharedData::changeAttributeValues( const QgsChangedAttributesMap &att
QgsAttributeMap newAttrMap;
for ( QgsAttributeMap::const_iterator siter = attrs.begin(); siter != attrs.end(); ++siter )
{
int idx = dataProviderFields.indexFromName( mFields.at( siter.key() ).name() );
int idx = dataProviderFields.indexFromName( mMapGMLFieldNameToSQLiteColumnName[mFields.at( siter.key() ).name()] );
Q_ASSERT( idx >= 0 );
if ( siter.value().type() == QVariant::DateTime && !siter.value().isNull() )
newAttrMap[idx] = QVariant( siter.value().toDateTime().toMSecsSinceEpoch() );
Expand Down Expand Up @@ -985,7 +1002,7 @@ void QgsWFSSharedData::serializeFeatures( QVector<QgsWFSFeatureGmlIdPair> &featu
//and the attributes
for ( int i = 0; i < mFields.size(); i++ )
{
int idx = dataProviderFields.indexFromName( mFields.at( i ).name() );
int idx = dataProviderFields.indexFromName( mMapGMLFieldNameToSQLiteColumnName[mFields.at( i ).name()] );
if ( idx >= 0 )
{
const QVariant &v = gmlFeature.attributes().value( i );
Expand Down
6 changes: 6 additions & 0 deletions src/providers/wfs/qgswfsshareddata.h
View file
Expand Up @@ -22,6 +22,8 @@
#include "qgsogcutils.h"
#include "qgssqliteutils.h"

#include <map>

/**
* This class holds data, and logic, shared between QgsWFSProvider, QgsWFSFeatureIterator
* and QgsWFSFeatureDownloader. It manages the on-disk cache, as a SpatiaLite
Expand Down Expand Up @@ -240,6 +242,10 @@ class QgsWFSSharedData : public QObject
//! Tablename of the on-disk cache
QString mCacheTablename;

//! Map each GML field name to the column name in the spatialite DB cache
// This is useful when there are GML fields with same name, but different case
std::map<QString, QString> mMapGMLFieldNameToSQLiteColumnName;

//! Spatial index of requested cached regions
QgsSpatialIndex mCachedRegions;

Expand Down
94 changes: 94 additions & 0 deletions tests/src/python/test_provider_wfs.py
View file
Expand Up @@ -3968,6 +3968,100 @@ def testFilteredFeatureRequests(self):
req.setExpressionContext(ctx)
qgis_feat = next(vl.getFeatures(req))

def testWFSFieldWithSameNameButDifferentCase(self):
"""Test a layer with field foo and FOO """

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

with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=GetCapabilities&VERSION=1.0.0'), 'wb') as f:
f.write("""
<WFS_Capabilities version="1.0.0" xmlns="http://www.opengis.net/wfs" xmlns:ogc="http://www.opengis.net/ogc">
<FeatureTypeList>
<FeatureType>
<Name>my:typename</Name>
<Title>Title</Title>
<Abstract>Abstract</Abstract>
<SRS>EPSG:32631</SRS>
<!-- in WFS 1.0, LatLongBoundingBox is in SRS units, not necessarily lat/long... -->
<LatLongBoundingBox minx="400000" miny="5400000" maxx="450000" maxy="5500000"/>
</FeatureType>
</FeatureTypeList>
</WFS_Capabilities>""".encode('UTF-8'))

with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=DescribeFeatureType&VERSION=1.0.0&TYPENAME=my:typename'), 'wb') as f:
f.write("""
<xsd:schema xmlns:my="http://my" xmlns:gml="http://www.opengis.net/gml" xmlns:xsd="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified" targetNamespace="http://my">
<xsd:import namespace="http://www.opengis.net/gml"/>
<xsd:complexType name="typenameType">
<xsd:complexContent>
<xsd:extension base="gml:AbstractFeatureType">
<xsd:sequence>
<xsd:element maxOccurs="1" minOccurs="0" name="foo" nillable="true" type="xsd:int"/>
<xsd:element maxOccurs="1" minOccurs="0" name="FOO" nillable="true" type="xsd:int"/>
<xsd:element maxOccurs="1" minOccurs="0" name="FOO2" nillable="true" type="xsd:int"/>
<xsd:element maxOccurs="1" minOccurs="0" name="geometry" nillable="true" type="gml:PointPropertyType"/>
</xsd:sequence>
</xsd:extension>
</xsd:complexContent>
</xsd:complexType>
<xsd:element name="typename" substitutionGroup="gml:_Feature" type="my:typenameType"/>
</xsd:schema>
""".encode('UTF-8'))

with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=GetFeature&VERSION=1.0.0&TYPENAME=my:typename&SRSNAME=EPSG:32631'), 'wb') as f:
f.write("""
<wfs:FeatureCollection
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:gml="http://www.opengis.net/gml"
xmlns:my="http://my">
<gml:boundedBy><gml:null>unknown</gml:null></gml:boundedBy>
<gml:featureMember>
<my:typename fid="typename.0">
<my:foo>1</my:foo>
<my:FOO>2</my:FOO>
<my:FOO2>3</my:FOO2>
</my:typename>
</gml:featureMember>
</wfs:FeatureCollection>""".encode('UTF-8'))

vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
self.assertTrue(vl.isValid())
self.assertEqual(len(vl.fields()), 3)

values = [f['foo'] for f in vl.getFeatures()]
self.assertEqual(values, [1])

values = [f['FOO'] for f in vl.getFeatures()]
self.assertEqual(values, [2])

values = [f['FOO2'] for f in vl.getFeatures()]
self.assertEqual(values, [3])

# Also test that on file iterator works
os.environ['QGIS_WFS_ITERATOR_TRANSFER_THRESHOLD'] = '0'

vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
values = [f['foo'] for f in vl.getFeatures()]
self.assertEqual(values, [1])

values = [f['FOO'] for f in vl.getFeatures()]
self.assertEqual(values, [2])

values = [f['FOO2'] for f in vl.getFeatures()]
self.assertEqual(values, [3])

del os.environ['QGIS_WFS_ITERATOR_TRANSFER_THRESHOLD']

vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
request = QgsFeatureRequest().setFilterExpression('FOO = 2')
values = [f['FOO'] for f in vl.getFeatures(request)]
self.assertEqual(values, [2])

vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
request = QgsFeatureRequest().setSubsetOfAttributes(['FOO'], vl.fields())
values = [f['FOO'] for f in vl.getFeatures(request)]
self.assertEqual(values, [2])


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

0 comments on commit 4651892

Please sign in to comment.