Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix NULL support in WFS server and client #8958

Merged
merged 8 commits into from
Jan 24, 2019
Merged

Fix NULL support in WFS server and client #8958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
847e7ef
GML parser set NULL values when parsing empty tags
elpaso Jan 23, 2019
71e0116
Server WFS NULL values support
elpaso Jan 23, 2019
cc9ec2d
Remove duplicated code and modernize
elpaso Jan 23, 2019
d748400
Fix WFS client NULL representation
elpaso Jan 23, 2019
83d5cb6
Fix linker error in test
elpaso Jan 23, 2019
3eb388a
Respect isNull semantics by initializing null QVariant with field type
elpaso Jan 24, 2019
33788c9
Fix typo in error message (and related tests)
elpaso Jan 24, 2019
15a81bd
Fix OWS project property issue: couldn't unset project title
elpaso Jan 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/app/qgsprojectproperties.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1115,10 +1115,7 @@ void QgsProjectProperties::apply()

QgsProject::instance()->writeEntry( QStringLiteral( "WMSServiceCapabilities" ), QStringLiteral( "/" ), grpOWSServiceCapabilities->isChecked() );
QgsProject::instance()->writeEntry( QStringLiteral( "WMSServiceTitle" ), QStringLiteral( "/" ), mWMSTitle->text() );

if ( !mWMSName->text().isEmpty() )
QgsProject::instance()->writeEntry( QStringLiteral( "WMSRootName" ), QStringLiteral( "/" ), mWMSName->text() );

QgsProject::instance()->writeEntry( QStringLiteral( "WMSRootName" ), QStringLiteral( "/" ), mWMSName->text() );
QgsProject::instance()->writeEntry( QStringLiteral( "WMSContactOrganization" ), QStringLiteral( "/" ), mWMSContactOrganization->text() );
QgsProject::instance()->writeEntry( QStringLiteral( "WMSContactPerson" ), QStringLiteral( "/" ), mWMSContactPerson->text() );
QgsProject::instance()->writeEntry( QStringLiteral( "WMSContactMail" ), QStringLiteral( "/" ), mWMSContactMail->text() );
Expand Down
11 changes: 8 additions & 3 deletions src/core/qgsgml.cpp
View file
Open in desktop
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
43 changes: 20 additions & 23 deletions src/providers/wfs/qgswfsfeatureiterator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1423,38 +1423,35 @@ void QgsWFSFeatureIterator::copyFeature( const QgsFeature &srcFeature, QgsFeatur
QgsFields &fields = mShared->mFields;
dstFeature.initAttributes( fields.size() );

auto setAttr = [ & ]( const int i )
{
int idx = srcFeature.fields().indexFromName( fields.at( i ).name() );
if ( idx >= 0 )
{
const QVariant &v = srcFeature.attributes().value( idx );
if ( v.isNull() )
dstFeature.setAttribute( i, QVariant( fields.at( i ).type() ) );
else if ( v.type() == fields.at( i ).type() )
dstFeature.setAttribute( i, v );
else if ( fields.at( i ).type() == QVariant::DateTime && !v.isNull() )
dstFeature.setAttribute( i, QVariant( QDateTime::fromMSecsSinceEpoch( v.toLongLong() ) ) );
else
dstFeature.setAttribute( i, QgsVectorDataProvider::convertValue( fields.at( i ).type(), v.toString() ) );
}
};

if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
{
Q_FOREACH ( int i, mSubSetAttributes )
for ( auto i : qgis::as_const( mSubSetAttributes ) )
{
int idx = srcFeature.fields().indexFromName( fields.at( i ).name() );
if ( idx >= 0 )
{
const QVariant &v = srcFeature.attributes().value( idx );
if ( v.type() == fields.at( i ).type() )
dstFeature.setAttribute( i, v );
else if ( fields.at( i ).type() == QVariant::DateTime && !v.isNull() )
dstFeature.setAttribute( i, QVariant( QDateTime::fromMSecsSinceEpoch( v.toLongLong() ) ) );
else
dstFeature.setAttribute( i, QgsVectorDataProvider::convertValue( fields.at( i ).type(), v.toString() ) );
}
setAttr( i );
}
}
else
{
for ( int i = 0; i < fields.size(); i++ )
{
int idx = srcFeature.fields().indexFromName( fields.at( i ).name() );
if ( idx >= 0 )
{
const QVariant &v = srcFeature.attributes().value( idx );
if ( v.type() == fields.at( i ).type() )
dstFeature.setAttribute( i, v );
else if ( fields.at( i ).type() == QVariant::DateTime && !v.isNull() )
dstFeature.setAttribute( i, QVariant( QDateTime::fromMSecsSinceEpoch( v.toLongLong() ) ) );
else
dstFeature.setAttribute( i, QgsVectorDataProvider::convertValue( fields.at( i ).type(), v.toString() ) );
}
setAttr( i );
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/server/services/wfs/qgswfsdescribefeaturetype.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ namespace QgsWfs
// test oFormat
if ( oFormat == QgsWfsParameters::Format::NONE )
throw QgsBadRequestException( QStringLiteral( "Invalid WFS Parameter" ),
"OUTPUTFORMAT " + wfsParameters.outputFormatAsString() + "is not supported" );
QStringLiteral( "OUTPUTFORMAT %1 is not supported" ).arg( wfsParameters.outputFormatAsString() ) );

QgsAccessControl *accessControl = serverIface->accessControls();

Expand Down Expand Up @@ -346,6 +346,11 @@ namespace QgsWfs
}
}

if ( !( field.constraints().constraints() & QgsFieldConstraints::Constraint::ConstraintNotNull ) )
{
attElem.setAttribute( QStringLiteral( "nillable" ), QStringLiteral( "true" ) );
}

sequenceElem.appendChild( attElem );

QString alias = field.alias();
Expand Down
10 changes: 10 additions & 0 deletions src/server/services/wfs/qgswfsgetfeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,10 @@ namespace QgsWfs

QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ).replace( cleanTagNameRegExp, QString() ) );
QDomText fieldText = doc.createTextNode( encodeValueToText( featureAttributes[idx], setup ) );
if ( featureAttributes[idx].isNull() )
{
fieldElem.setAttribute( QStringLiteral( "xsi:nil" ), QStringLiteral( "true" ) );
}
fieldElem.appendChild( fieldText );
typeNameElement.appendChild( fieldElem );
}
Expand Down Expand Up @@ -1431,12 +1435,18 @@ namespace QgsWfs
{
continue;
}

const QgsField field = fields.at( idx );
const QgsEditorWidgetSetup setup = field.editorWidgetSetup();

QString attributeName = field.name();

QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ).replace( cleanTagNameRegExp, QString() ) );
QDomText fieldText = doc.createTextNode( encodeValueToText( featureAttributes[idx], setup ) );
if ( featureAttributes[idx].isNull() )
{
fieldElem.setAttribute( QStringLiteral( "xsi:nil" ), QStringLiteral( "true" ) );
}
fieldElem.appendChild( fieldText );
typeNameElement.appendChild( fieldElem );
}
Expand Down
46 changes: 35 additions & 11 deletions src/server/services/wfs/qgswfstransaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,26 +407,50 @@ 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 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
46 changes: 35 additions & 11 deletions src/server/services/wfs/qgswfstransaction_1_0_0.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,26 +388,50 @@ 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 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
Open in desktop
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" ) ).toInt(), 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" ) ).toInt(), 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