Skip to content

Commit

Permalink
GML parser set NULL values when parsing empty tags
Browse files Browse the repository at this point in the history 
This is part of a bigger fix to support NULL
values in WFS client and server components.
  • Loading branch information
@elpaso
elpaso committed Jan 23, 2019
1 parent 34ec183 commit 847e7ef
Show file tree
Hide file tree
Showing 11 changed files with 788 additions and 581 deletions.
11 changes: 8 additions & 3 deletions src/core/qgsgml.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1167,19 +1167,20 @@ void QgsGmlStreamingParser::setAttribute( const QString &name, const QString &va
{
//find index with attribute name
QMap<QString, QPair<int, QgsField> >::const_iterator att_it = mThematicAttributes.constFind( name );
bool conversionOk = true;
if ( att_it != mThematicAttributes.constEnd() )
{
QVariant var;
switch ( att_it.value().second.type() )
{
case QVariant::Double:
var = QVariant( value.toDouble() );
var = QVariant( value.toDouble( &conversionOk ) );
break;
case QVariant::Int:
var = QVariant( value.toInt() );
var = QVariant( value.toInt( &conversionOk ) );
break;
case QVariant::LongLong:
var = QVariant( value.toLongLong() );
var = QVariant( value.toLongLong( &conversionOk ) );
break;
case QVariant::DateTime:
var = QVariant( QDateTime::fromString( value, Qt::ISODate ) );
Expand All @@ -1188,6 +1189,10 @@ void QgsGmlStreamingParser::setAttribute( const QString &name, const QString &va
var = QVariant( value );
break;
}
if ( ! conversionOk ) // Assume is NULL
{
var = QVariant();
}
Q_ASSERT( mCurrentFeature );
mCurrentFeature->setAttribute( att_it.value().first, var );
}
Expand Down
2 changes: 2 additions & 0 deletions src/providers/wfs/qgswfsrequest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ bool QgsWfsRequest::sendPOST( const QUrl &url, const QString &contentTypeHeader,
mForceRefresh = true;
mResponse.clear();

qDebug() << QString::fromUtf8( data );

if ( url.toEncoded().contains( "fake_qgis_http_endpoint" ) )
{
// Hack for testing purposes
Expand Down
8 changes: 8 additions & 0 deletions src/server/services/wfs/qgswfsgetfeature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,10 @@ namespace QgsWfs
{
continue;
}
if ( featureAttributes[idx].isNull() )
{
continue;
}
const QgsField field = fields.at( idx );
const QgsEditorWidgetSetup setup = field.editorWidgetSetup();
QString attributeName = field.name();
Expand Down Expand Up @@ -1431,6 +1435,10 @@ namespace QgsWfs
{
continue;
}
if ( featureAttributes[idx].isNull() )
{
continue;
}
const QgsField field = fields.at( idx );
const QgsEditorWidgetSetup setup = field.editorWidgetSetup();
QString attributeName = field.name();
Expand Down
47 changes: 35 additions & 12 deletions src/server/services/wfs/qgswfstransaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,26 +407,49 @@ namespace QgsWfs
}
QgsField field = fields.at( fieldMapIt.value() );
QVariant value = it.value();
if ( field.type() == 2 )
if ( value.isNull() )
{
value = it.value().toInt( &conversionSuccess );
if ( !conversionSuccess )
if ( field.constraints().constraints() & QgsFieldConstraints::Constraint::ConstraintNotNull )
{
action.error = true;
action.errorMsg = QStringLiteral( "Property conversion error on layer '%1'" ).arg( typeName );
action.errorMsg = QStringLiteral( "NOT NULL constraint error error on layer '%1', field '%2'" ).arg( typeName, field.name() );
vlayer->rollBack();
break;
}
}
else if ( field.type() == 6 )
else // Not NULL
{
value = it.value().toDouble( &conversionSuccess );
if ( !conversionSuccess )
if ( field.type() == QVariant::Type::Int )
{
action.error = true;
action.errorMsg = QStringLiteral( "Property conversion error on layer '%1'" ).arg( typeName );
vlayer->rollBack();
break;
value = it.value().toInt( &conversionSuccess );
if ( !conversionSuccess )
{
action.error = true;
action.errorMsg = QStringLiteral( "Property conversion error on layer '%1'" ).arg( typeName );
vlayer->rollBack();
break;
}
}
else if ( field.type() == QVariant::Type::Double )
{
value = it.value().toDouble( &conversionSuccess );
if ( !conversionSuccess )
{
action.error = true;
action.errorMsg = QStringLiteral( "Property conversion error on layer '%1'" ).arg( typeName );
vlayer->rollBack();
break;
}
}
else if ( field.type() == QVariant::Type::LongLong )
{
value = it.value().toLongLong( &conversionSuccess );
if ( !conversionSuccess )
{
action.error = true;
action.errorMsg = QStringLiteral( "Property conversion error on layer '%1'" ).arg( typeName );
vlayer->rollBack();
break;
}
}
}
vlayer->changeAttributeValue( feature.id(), fieldMapIt.value(), value );
Expand Down
22 changes: 16 additions & 6 deletions tests/src/core/testqgsgml.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,13 @@ class TestQgsGML : public QObject

const QString data1( "<myns:FeatureCollection "
"xmlns:myns='http://myns' "
"xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' "
"xmlns:gml='http://www.opengis.net/gml'>"
"<gml:boundedBy><gml:null>unknown</gml:null></gml:boundedBy>"
"<gml:featureMember>"
"<myns:mytypename fid='mytypename.1'>"
"<myns:intfield>1</myns:intfield>"
"<myns:nillablefield xsi:nil='true'/>"
"<myns:longfield>1234567890123</myns:longfield>"
"<myns:doublefield>1.23</myns:doublefield>"
"<myns:strfield>foo</myns:strfield>"
Expand All @@ -105,6 +107,7 @@ void TestQgsGML::testFromURL()
{
QgsFields fields;
fields.append( QgsField( QStringLiteral( "intfield" ), QVariant::Int, QStringLiteral( "int" ) ) );
fields.append( QgsField( QStringLiteral( "nillablefield" ), QVariant::Int, QStringLiteral( "nillablefield" ) ) );
QgsGml gmlParser( QStringLiteral( "mytypename" ), QStringLiteral( "mygeom" ), fields );
QgsWkbTypes::Type wkbType;
QTemporaryFile tmpFile;
Expand All @@ -117,30 +120,36 @@ void TestQgsGML::testFromURL()
QCOMPARE( featureMaps.size(), 1 );
QCOMPARE( gmlParser.idsMap().size(), 1 );
QCOMPARE( gmlParser.crs().authid(), QString( "EPSG:27700" ) );
QCOMPARE( featureMaps[0]->attribute( QStringLiteral( "intfield" ) ), 1 );
QVERIFY( featureMaps[0]->attribute( QStringLiteral( "nillablefield" ) ).isNull( ) );
delete featureMaps[ 0 ];
}

void TestQgsGML::testFromByteArray()
{
QgsFields fields;
fields.append( QgsField( QStringLiteral( "intfield" ), QVariant::Int, QStringLiteral( "int" ) ) );
fields.append( QgsField( QStringLiteral( "nillablefield" ), QVariant::Int, QStringLiteral( "nillablefield" ) ) );
QgsGml gmlParser( QStringLiteral( "mytypename" ), QStringLiteral( "mygeom" ), fields );
QgsWkbTypes::Type wkbType;
QCOMPARE( gmlParser.getFeatures( data1.toAscii(), &wkbType ), 0 );
QMap<QgsFeatureId, QgsFeature * > featureMaps = gmlParser.featuresMap();
QCOMPARE( featureMaps.size(), 1 );
QVERIFY( featureMaps.constFind( 0 ) != featureMaps.constEnd() );
QCOMPARE( featureMaps[ 0 ]->attributes().size(), 1 );
QCOMPARE( featureMaps[ 0 ]->attributes().size(), 2 );
QMap<QgsFeatureId, QString > idsMap = gmlParser.idsMap();
QVERIFY( idsMap.constFind( 0 ) != idsMap.constEnd() );
QCOMPARE( idsMap[ 0 ], QString( "mytypename.1" ) );
QCOMPARE( featureMaps[0]->attribute( QStringLiteral( "intfield" ) ), 1 );
QVERIFY( featureMaps[0]->attribute( QStringLiteral( "nillablefield" ) ).isNull( ) );
delete featureMaps[ 0 ];
}

void TestQgsGML::testStreamingParser()
{
QgsFields fields;
fields.append( QgsField( QStringLiteral( "intfield" ), QVariant::Int, QStringLiteral( "int" ) ) );
fields.append( QgsField( QStringLiteral( "nillablefield" ), QVariant::Int, QStringLiteral( "nillablefield" ) ) );
fields.append( QgsField( QStringLiteral( "longfield" ), QVariant::LongLong, QStringLiteral( "longlong" ) ) );
fields.append( QgsField( QStringLiteral( "doublefield" ), QVariant::Double, QStringLiteral( "double" ) ) );
fields.append( QgsField( QStringLiteral( "strfield" ), QVariant::String, QStringLiteral( "string" ) ) );
Expand All @@ -152,12 +161,13 @@ void TestQgsGML::testStreamingParser()
QCOMPARE( gmlParser.isException(), false );
QVector<QgsGmlStreamingParser::QgsGmlFeaturePtrGmlIdPair> features = gmlParser.getAndStealReadyFeatures();
QCOMPARE( features.size(), 1 );
QCOMPARE( features[0].first->attributes().size(), 5 );
QCOMPARE( features[0].first->attributes().size(), 6 );
QCOMPARE( features[0].first->attributes().at( 0 ), QVariant( 1 ) );
QCOMPARE( features[0].first->attributes().at( 1 ), QVariant( Q_INT64_C( 1234567890123 ) ) );
QCOMPARE( features[0].first->attributes().at( 2 ), QVariant( 1.23 ) );
QCOMPARE( features[0].first->attributes().at( 3 ), QVariant( "foo" ) );
QCOMPARE( features[0].first->attributes().at( 4 ), QVariant( QDateTime( QDate( 2016, 4, 10 ), QTime( 12, 34, 56, 789 ), Qt::UTC ) ) );
QCOMPARE( features[0].first->attributes().at( 1 ), QVariant( ) );
QCOMPARE( features[0].first->attributes().at( 2 ), QVariant( Q_INT64_C( 1234567890123 ) ) );
QCOMPARE( features[0].first->attributes().at( 3 ), QVariant( 1.23 ) );
QCOMPARE( features[0].first->attributes().at( 4 ), QVariant( "foo" ) );
QCOMPARE( features[0].first->attributes().at( 5 ), QVariant( QDateTime( QDate( 2016, 4, 10 ), QTime( 12, 34, 56, 789 ), Qt::UTC ) ) );
QVERIFY( features[0].first->hasGeometry() );
QCOMPARE( features[0].first->geometry().wkbType(), QgsWkbTypes::Point );
QCOMPARE( features[0].first->geometry().asPoint(), QgsPointXY( 10, 20 ) );
Expand Down
8 changes: 7 additions & 1 deletion tests/src/python/test_qgsserver.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import urllib.parse
import urllib.error
import email
import difflib

from io import StringIO
from qgis.server import QgsServer, QgsServerRequest, QgsBufferServerRequest, QgsBufferServerResponse
Expand Down Expand Up @@ -70,7 +71,12 @@ def assertXMLEqual(self, response, expected, msg=''):
response_lines = response.splitlines()
expected_lines = expected.splitlines()
line_no = 1
self.assertEqual(len(expected_lines), len(response_lines), "Expected and response have different number of lines!\n{}".format(msg))

diffs = []
for diff in difflib.unified_diff([l.decode('utf8') for l in expected_lines], [l.decode('utf8') for l in response_lines]):
diffs.append(diff)

self.assertEqual(len(expected_lines), len(response_lines), "Expected and response have different number of lines!\n{}\n{}".format(msg, '\n'.join(diffs)))
for expected_line in expected_lines:
expected_line = expected_line.strip()
response_line = response_lines[line_no - 1].strip()
Expand Down
Loading

0 comments on commit 847e7ef

Please sign in to comment.