Skip to content

Commit 06ab26a

Browse files
authoredMay 27, 2019
Merge pull request #29949 from rouault/fix_github_29858
[WFS provider] Support layers with GML field names only differing by cases (github fixes #29858)
2 parents 3dddfe4 + 5eb5139 commit 06ab26a

File tree

5 files changed

+127
-10
lines changed

5 files changed

+127
-10
lines changed
 

‎src/providers/wfs/qgswfsfeatureiterator.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,7 +1020,7 @@ QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
10201020
const auto subsetOfAttributes = mRequest.subsetOfAttributes();
10211021
for ( int i : subsetOfAttributes )
10221022
{
1023-
int idx = dataProviderFields.indexFromName( mShared->mFields.at( i ).name() );
1023+
int idx = dataProviderFields.indexFromName( mShared->mMapGMLFieldNameToSQLiteColumnName[mShared->mFields.at( i ).name()] );
10241024
if ( idx >= 0 )
10251025
cacheSubSet.append( idx );
10261026
idx = mShared->mFields.indexFromName( mShared->mFields.at( i ).name() );
@@ -1034,7 +1034,7 @@ QgsFeatureRequest QgsWFSFeatureIterator::buildRequestCache( int genCounter )
10341034
const auto referencedColumns = mRequest.filterExpression()->referencedColumns();
10351035
for ( const QString &field : referencedColumns )
10361036
{
1037-
int idx = dataProviderFields.indexFromName( field );
1037+
int idx = dataProviderFields.indexFromName( mShared->mMapGMLFieldNameToSQLiteColumnName[field] );
10381038
if ( idx >= 0 && !cacheSubSet.contains( idx ) )
10391039
cacheSubSet.append( idx );
10401040
idx = mShared->mFields.indexFromName( field );
@@ -1248,7 +1248,7 @@ bool QgsWFSFeatureIterator::fetchFeature( QgsFeature &f )
12481248
continue;
12491249
}
12501250

1251-
copyFeature( cachedFeature, f );
1251+
copyFeature( cachedFeature, f, true );
12521252
geometryToDestinationCrs( f, mTransform );
12531253

12541254
// Retrieve the user-visible id from the Spatialite cache database Id
@@ -1342,7 +1342,7 @@ bool QgsWFSFeatureIterator::fetchFeature( QgsFeature &f )
13421342
continue;
13431343
}
13441344

1345-
copyFeature( feat, f );
1345+
copyFeature( feat, f, false );
13461346
return true;
13471347
}
13481348

@@ -1429,7 +1429,7 @@ bool QgsWFSFeatureIterator::close()
14291429
}
14301430

14311431

1432-
void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeature &dstFeature )
1432+
void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeature &dstFeature, bool srcIsCache )
14331433
{
14341434
//copy the geometry
14351435
QgsGeometry geometry = srcFeature.geometry();
@@ -1450,7 +1450,7 @@ void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeatur
14501450

14511451
auto setAttr = [ & ]( const int i )
14521452
{
1453-
int idx = srcFeature.fields().indexFromName( fields.at( i ).name() );
1453+
int idx = srcFeature.fields().indexFromName( srcIsCache ? mShared->mMapGMLFieldNameToSQLiteColumnName[fields.at( i ).name()] : fields.at( i ).name() );
14541454
if ( idx >= 0 )
14551455
{
14561456
const QVariant &v = srcFeature.attributes().value( idx );

‎src/providers/wfs/qgswfsfeatureiterator.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ class QgsWFSFeatureIterator : public QObject,
227227
bool fetchFeature( QgsFeature &f ) override;
228228

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

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

‎src/providers/wfs/qgswfsshareddata.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
#include <cpl_conv.h>
3434
#include <ogr_api.h>
3535

36+
#include <set>
37+
3638
#include <sqlite3.h>
3739

3840
QgsWFSSharedData::QgsWFSSharedData( const QString &uri )
@@ -231,6 +233,7 @@ bool QgsWFSSharedData::createCache()
231233
Q_ASSERT( !QFile::exists( mCacheDbname ) );
232234

233235
QgsFields cacheFields;
236+
std::set<QString> setSQLiteColumnNameUpperCase;
234237
for ( const QgsField &field : qgis::as_const( mFields ) )
235238
{
236239
QVariant::Type type = field.type();
@@ -241,7 +244,19 @@ bool QgsWFSSharedData::createCache()
241244
// it to a String
242245
type = QVariant::LongLong;
243246
}
244-
cacheFields.append( QgsField( field.name(), type, field.typeName() ) );
247+
248+
// Make sure we don't have several field names that only differ by their case
249+
QString sqliteFieldName( field.name() );
250+
int counter = 2;
251+
while ( setSQLiteColumnNameUpperCase.find( sqliteFieldName.toUpper() ) != setSQLiteColumnNameUpperCase.end() )
252+
{
253+
sqliteFieldName = field.name() + QStringLiteral( "%1" ).arg( counter );
254+
counter++;
255+
}
256+
setSQLiteColumnNameUpperCase.insert( sqliteFieldName.toUpper() );
257+
mMapGMLFieldNameToSQLiteColumnName[field.name()] = sqliteFieldName;
258+
259+
cacheFields.append( QgsField( sqliteFieldName, type, field.typeName() ) );
245260
}
246261
// Add some field for our internal use
247262
cacheFields.append( QgsField( QgsWFSConstants::FIELD_GEN_COUNTER, QVariant::Int, QStringLiteral( "int" ) ) );
@@ -385,6 +400,7 @@ bool QgsWFSSharedData::createCache()
385400
{
386401
mCacheTablename = QStringLiteral( "features" );
387402
sql = QStringLiteral( "CREATE TABLE %1 (%2 INTEGER PRIMARY KEY" ).arg( mCacheTablename, fidName );
403+
388404
for ( const QgsField &field : qgis::as_const( cacheFields ) )
389405
{
390406
QString type( QStringLiteral( "VARCHAR" ) );
@@ -394,6 +410,7 @@ bool QgsWFSSharedData::createCache()
394410
type = QStringLiteral( "BIGINT" );
395411
else if ( field.type() == QVariant::Double )
396412
type = QStringLiteral( "REAL" );
413+
397414
sql += QStringLiteral( ", %1 %2" ).arg( quotedIdentifier( field.name() ), type );
398415
}
399416
sql += QLatin1String( ")" );
@@ -869,7 +886,7 @@ bool QgsWFSSharedData::changeAttributeValues( const QgsChangedAttributesMap &att
869886
QgsAttributeMap newAttrMap;
870887
for ( QgsAttributeMap::const_iterator siter = attrs.begin(); siter != attrs.end(); ++siter )
871888
{
872-
int idx = dataProviderFields.indexFromName( mFields.at( siter.key() ).name() );
889+
int idx = dataProviderFields.indexFromName( mMapGMLFieldNameToSQLiteColumnName[mFields.at( siter.key() ).name()] );
873890
Q_ASSERT( idx >= 0 );
874891
if ( siter.value().type() == QVariant::DateTime && !siter.value().isNull() )
875892
newAttrMap[idx] = QVariant( siter.value().toDateTime().toMSecsSinceEpoch() );
@@ -988,7 +1005,7 @@ void QgsWFSSharedData::serializeFeatures( QVector<QgsWFSFeatureGmlIdPair> &featu
9881005
//and the attributes
9891006
for ( int i = 0; i < mFields.size(); i++ )
9901007
{
991-
int idx = dataProviderFields.indexFromName( mFields.at( i ).name() );
1008+
int idx = dataProviderFields.indexFromName( mMapGMLFieldNameToSQLiteColumnName[mFields.at( i ).name()] );
9921009
if ( idx >= 0 )
9931010
{
9941011
const QVariant &v = gmlFeature.attributes().value( i );

‎src/providers/wfs/qgswfsshareddata.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include "qgsogcutils.h"
2323
#include "qgssqliteutils.h"
2424

25+
#include <map>
26+
2527
/**
2628
* This class holds data, and logic, shared between QgsWFSProvider, QgsWFSFeatureIterator
2729
* and QgsWFSFeatureDownloader. It manages the on-disk cache, as a SpatiaLite
@@ -240,6 +242,10 @@ class QgsWFSSharedData : public QObject
240242
//! Tablename of the on-disk cache
241243
QString mCacheTablename;
242244

245+
//! Map each GML field name to the column name in the spatialite DB cache
246+
// This is useful when there are GML fields with same name, but different case
247+
std::map<QString, QString> mMapGMLFieldNameToSQLiteColumnName;
248+
243249
//! Spatial index of requested cached regions
244250
QgsSpatialIndex mCachedRegions;
245251

‎tests/src/python/test_provider_wfs.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3966,6 +3966,100 @@ def testFilteredFeatureRequests(self):
39663966
req.setExpressionContext(ctx)
39673967
qgis_feat = next(vl.getFeatures(req))
39683968

3969+
def testWFSFieldWithSameNameButDifferentCase(self):
3970+
"""Test a layer with field foo and FOO """
3971+
3972+
endpoint = self.__class__.basetestpath + '/fake_qgis_http_endpoint_FieldWithSameNameButDifferentCase'
3973+
3974+
with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=GetCapabilities&VERSION=1.0.0'), 'wb') as f:
3975+
f.write("""
3976+
<WFS_Capabilities version="1.0.0" xmlns="http://www.opengis.net/wfs" xmlns:ogc="http://www.opengis.net/ogc">
3977+
<FeatureTypeList>
3978+
<FeatureType>
3979+
<Name>my:typename</Name>
3980+
<Title>Title</Title>
3981+
<Abstract>Abstract</Abstract>
3982+
<SRS>EPSG:32631</SRS>
3983+
<!-- in WFS 1.0, LatLongBoundingBox is in SRS units, not necessarily lat/long... -->
3984+
<LatLongBoundingBox minx="400000" miny="5400000" maxx="450000" maxy="5500000"/>
3985+
</FeatureType>
3986+
</FeatureTypeList>
3987+
</WFS_Capabilities>""".encode('UTF-8'))
3988+
3989+
with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=DescribeFeatureType&VERSION=1.0.0&TYPENAME=my:typename'), 'wb') as f:
3990+
f.write("""
3991+
<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">
3992+
<xsd:import namespace="http://www.opengis.net/gml"/>
3993+
<xsd:complexType name="typenameType">
3994+
<xsd:complexContent>
3995+
<xsd:extension base="gml:AbstractFeatureType">
3996+
<xsd:sequence>
3997+
<xsd:element maxOccurs="1" minOccurs="0" name="foo" nillable="true" type="xsd:int"/>
3998+
<xsd:element maxOccurs="1" minOccurs="0" name="FOO" nillable="true" type="xsd:int"/>
3999+
<xsd:element maxOccurs="1" minOccurs="0" name="FOO2" nillable="true" type="xsd:int"/>
4000+
<xsd:element maxOccurs="1" minOccurs="0" name="geometry" nillable="true" type="gml:PointPropertyType"/>
4001+
</xsd:sequence>
4002+
</xsd:extension>
4003+
</xsd:complexContent>
4004+
</xsd:complexType>
4005+
<xsd:element name="typename" substitutionGroup="gml:_Feature" type="my:typenameType"/>
4006+
</xsd:schema>
4007+
""".encode('UTF-8'))
4008+
4009+
with open(sanitize(endpoint, '?SERVICE=WFS&REQUEST=GetFeature&VERSION=1.0.0&TYPENAME=my:typename&SRSNAME=EPSG:32631'), 'wb') as f:
4010+
f.write("""
4011+
<wfs:FeatureCollection
4012+
xmlns:wfs="http://www.opengis.net/wfs"
4013+
xmlns:gml="http://www.opengis.net/gml"
4014+
xmlns:my="http://my">
4015+
<gml:boundedBy><gml:null>unknown</gml:null></gml:boundedBy>
4016+
<gml:featureMember>
4017+
<my:typename fid="typename.0">
4018+
<my:foo>1</my:foo>
4019+
<my:FOO>2</my:FOO>
4020+
<my:FOO2>3</my:FOO2>
4021+
</my:typename>
4022+
</gml:featureMember>
4023+
</wfs:FeatureCollection>""".encode('UTF-8'))
4024+
4025+
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
4026+
self.assertTrue(vl.isValid())
4027+
self.assertEqual(len(vl.fields()), 3)
4028+
4029+
values = [f['foo'] for f in vl.getFeatures()]
4030+
self.assertEqual(values, [1])
4031+
4032+
values = [f['FOO'] for f in vl.getFeatures()]
4033+
self.assertEqual(values, [2])
4034+
4035+
values = [f['FOO2'] for f in vl.getFeatures()]
4036+
self.assertEqual(values, [3])
4037+
4038+
# Also test that on file iterator works
4039+
os.environ['QGIS_WFS_ITERATOR_TRANSFER_THRESHOLD'] = '0'
4040+
4041+
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
4042+
values = [f['foo'] for f in vl.getFeatures()]
4043+
self.assertEqual(values, [1])
4044+
4045+
values = [f['FOO'] for f in vl.getFeatures()]
4046+
self.assertEqual(values, [2])
4047+
4048+
values = [f['FOO2'] for f in vl.getFeatures()]
4049+
self.assertEqual(values, [3])
4050+
4051+
del os.environ['QGIS_WFS_ITERATOR_TRANSFER_THRESHOLD']
4052+
4053+
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
4054+
request = QgsFeatureRequest().setFilterExpression('FOO = 2')
4055+
values = [f['FOO'] for f in vl.getFeatures(request)]
4056+
self.assertEqual(values, [2])
4057+
4058+
vl = QgsVectorLayer("url='http://" + endpoint + "' typename='my:typename' version='1.0.0'", 'test', 'WFS')
4059+
request = QgsFeatureRequest().setSubsetOfAttributes(['FOO'], vl.fields())
4060+
values = [f['FOO'] for f in vl.getFeatures(request)]
4061+
self.assertEqual(values, [2])
4062+
39694063

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

0 commit comments

Comments
 (0)
Please sign in to comment.