Skip to content

Commit 992b1f2

Browse files
authoredJul 6, 2020
Merge pull request #36967 from elpaso/bugfix-gh36962-fid-is-null
QgsFeature default constructor set fid to invalid
2 parents 01b345c + a8d78b7 commit 992b1f2

File tree

12 files changed

+66
-25
lines changed

12 files changed

+66
-25
lines changed
 

‎python/core/auto_generated/qgsfeature.sip.in

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,14 @@ geometry and a list of field/values attributes.
136136
sipCpp->deleteAttribute( fieldIdx );
137137
%End
138138

139-
QgsFeature( qint64 id = 0 );
139+
QgsFeature( qint64 id = FID_NULL );
140140
%Docstring
141141
Constructor for QgsFeature
142142

143143
:param id: feature id
144144
%End
145145

146-
QgsFeature( const QgsFields &fields, qint64 id = 0 );
146+
QgsFeature( const QgsFields &fields, qint64 id = FID_NULL );
147147
%Docstring
148148
Constructor for QgsFeature
149149

‎src/core/expression/qgsexpressioncontextutils.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "qgslayoutpagecollection.h"
3030
#include "qgslayoutatlas.h"
3131
#include "qgslayoutmultiframe.h"
32+
#include "qgsfeatureid.h"
3233

3334
QgsExpressionContextScope *QgsExpressionContextUtils::globalScope()
3435
{
@@ -525,7 +526,7 @@ QgsExpressionContextScope *QgsExpressionContextUtils::layoutScope( const QgsLayo
525526
QgsFeature atlasFeature = layout->reportContext().feature();
526527
scope->setFeature( atlasFeature );
527528
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_feature" ), QVariant::fromValue( atlasFeature ), true ) );
528-
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_featureid" ), atlasFeature.id(), true ) );
529+
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_featureid" ), FID_IS_NULL( atlasFeature.id() ) ? QVariant() : atlasFeature.id(), true ) );
529530
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_geometry" ), QVariant::fromValue( atlasFeature.geometry() ), true ) );
530531
}
531532

@@ -576,7 +577,7 @@ QgsExpressionContextScope *QgsExpressionContextUtils::atlasScope( const QgsLayou
576577
//users that these variables are available even if they have no current value
577578
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_pagename" ), QString(), true ) );
578579
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_feature" ), QVariant::fromValue( QgsFeature() ), true ) );
579-
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_featureid" ), 0, true ) );
580+
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_featureid" ), QVariant(), true ) );
580581
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_geometry" ), QVariant::fromValue( QgsGeometry() ), true ) );
581582
return scope;
582583
}
@@ -599,7 +600,7 @@ QgsExpressionContextScope *QgsExpressionContextUtils::atlasScope( const QgsLayou
599600
QgsFeature atlasFeature = atlas->layout()->reportContext().feature();
600601
scope->setFeature( atlasFeature );
601602
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_feature" ), QVariant::fromValue( atlasFeature ), true ) );
602-
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_featureid" ), atlasFeature.id(), true ) );
603+
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_featureid" ), FID_IS_NULL( atlasFeature.id() ) ? QVariant() : atlasFeature.id(), true ) );
603604
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "atlas_geometry" ), QVariant::fromValue( atlasFeature.geometry() ), true ) );
604605
}
605606

‎src/core/qgsfeature.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,9 @@ class CORE_EXPORT QgsFeature
176176
* \param id feature id
177177
*/
178178
#ifndef SIP_RUN
179-
QgsFeature( QgsFeatureId id = QgsFeatureId() );
179+
QgsFeature( QgsFeatureId id = FID_NULL );
180180
#else
181-
QgsFeature( qint64 id = 0 );
181+
QgsFeature( qint64 id = FID_NULL );
182182
#endif
183183

184184
/**
@@ -187,9 +187,9 @@ class CORE_EXPORT QgsFeature
187187
* \param id feature id
188188
*/
189189
#ifndef SIP_RUN
190-
QgsFeature( const QgsFields &fields, QgsFeatureId id = QgsFeatureId() );
190+
QgsFeature( const QgsFields &fields, QgsFeatureId id = FID_NULL );
191191
#else
192-
QgsFeature( const QgsFields &fields, qint64 id = 0 );
192+
QgsFeature( const QgsFields &fields, qint64 id = FID_NULL );
193193
#endif
194194

195195
/**

‎src/core/qgsjsonutils.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "qgsfieldformatterregistry.h"
2727
#include "qgsfieldformatter.h"
2828
#include "qgsapplication.h"
29+
#include "qgsfeatureid.h"
2930

3031
#include <QJsonDocument>
3132
#include <QJsonArray>
@@ -94,6 +95,10 @@ json QgsJsonExporter::exportFeatureToJsonObject( const QgsFeature &feature, cons
9495
featureJson["id"] = id.toString().toStdString();
9596
}
9697
}
98+
else if ( FID_IS_NULL( feature.id() ) )
99+
{
100+
featureJson["id"] = nullptr;
101+
}
97102
else
98103
{
99104
featureJson["id"] = feature.id();

‎src/gui/qgsmaptooldigitizefeature.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ void QgsMapToolDigitizeFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
178178
//grass provider has its own mechanism of feature addition
179179
if ( provider->capabilities() & QgsVectorDataProvider::AddFeatures )
180180
{
181-
QgsFeature f( vlayer->fields(), 0 );
181+
QgsFeature f( vlayer->fields() );
182182

183183
QgsGeometry g;
184184
if ( layerWKBType == QgsWkbTypes::Point )

‎tests/src/core/testqgscompositionconverter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,7 @@ void TestQgsCompositionConverter::importComposerAtlas()
699699
QCOMPARE( layout->name(), QStringLiteral( "composer atlas" ) );
700700

701701
QVERIFY( layout->atlas()->enabled() );
702+
QVERIFY( layout->atlas()->updateFeatures() > 0 );
702703

703704
checkRenderedImage( layout.get(), QTest::currentTestFunction(), 0 );
704705

‎tests/src/core/testqgsfeature.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class TestQgsFeature: public QObject
3232
void init();// will be called before each testfunction is executed.
3333
void cleanup();// will be called after every testfunction.
3434
void attributesTest(); //test QgsAttributes
35+
void constructorTest(); //test default constructors
3536
void attributesToMap();
3637
void create();//test creating a feature
3738
void copy();// test cpy destruction (double delete)
@@ -120,6 +121,18 @@ void TestQgsFeature::attributesTest()
120121
QCOMPARE( attr7.size(), 5 );
121122
}
122123

124+
void TestQgsFeature::constructorTest()
125+
{
126+
QgsFeature f;
127+
QVERIFY( FID_IS_NULL( f.id() ) );
128+
QgsFeature f2 { QgsFields() };
129+
QVERIFY( FID_IS_NULL( f2.id() ) );
130+
QgsFeature f3 { 1234 };
131+
QVERIFY( ! FID_IS_NULL( f3.id() ) );
132+
QgsFeature f4 { QgsFields(), 1234 };
133+
QVERIFY( ! FID_IS_NULL( f4.id() ) );
134+
}
135+
123136
void TestQgsFeature::attributesToMap()
124137
{
125138
QgsAttributes attr1;

‎tests/src/core/testqgsjsonutils.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ void TestQgsJsonUtils::testExportFeatureJson()
223223
const auto expectedJson { QStringLiteral( "{\"bbox\":[1.12,1.12,5.45,5.33],\"geometry\":{\"coordinates\":"
224224
"[[[1.12,1.34],[5.45,1.12],[5.34,5.33],[1.56,5.2],[1.12,1.34]],"
225225
"[[2.0,2.0],[3.0,2.0],[3.0,3.0],[2.0,3.0],[2.0,2.0]]],\"type\":\"Polygon\"}"
226-
",\"id\":0,\"properties\":{\"flddbl\":2.0,\"fldint\":1,\"fldtxt\":\"a value\"}"
226+
",\"id\":null,\"properties\":{\"flddbl\":2.0,\"fldint\":1,\"fldtxt\":\"a value\"}"
227227
",\"type\":\"Feature\"}" ) };
228228

229229
const auto j( exporter.exportFeatureToJsonObject( feature ) );
@@ -233,12 +233,14 @@ void TestQgsJsonUtils::testExportFeatureJson()
233233

234234
QgsJsonExporter exporterPrecision { &vl, 1 };
235235

236+
236237
const auto expectedJsonPrecision { QStringLiteral( "{\"bbox\":[1.1,1.1,5.5,5.3],\"geometry\":{\"coordinates\":"
237238
"[[[1.1,1.3],[5.5,1.1],[5.3,5.3],[1.6,5.2],[1.1,1.3]],"
238239
"[[2.0,2.0],[3.0,2.0],[3.0,3.0],[2.0,3.0],[2.0,2.0]]],\"type\":\"Polygon\"}"
239-
",\"id\":0,\"properties\":{\"flddbl\":2.0,\"fldint\":1,\"fldtxt\":\"a value\"}"
240+
",\"id\":123,\"properties\":{\"flddbl\":2.0,\"fldint\":1,\"fldtxt\":\"a value\"}"
240241
",\"type\":\"Feature\"}" ) };
241242

243+
feature.setId( 123 );
242244
const auto jPrecision( exporterPrecision.exportFeatureToJsonObject( feature ) );
243245
QCOMPARE( QString::fromStdString( jPrecision.dump() ), expectedJsonPrecision );
244246
const auto jsonPrecision { exporterPrecision.exportFeature( feature ) };

‎tests/src/python/test_qgsfeature.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
class TestQgsFeature(unittest.TestCase):
3131

3232
def test_CreateFeature(self):
33-
feat = QgsFeature()
33+
feat = QgsFeature(0)
3434
feat.initAttributes(1)
3535
feat.setAttribute(0, "text")
3636
feat.setGeometry(QgsGeometry.fromPointXY(QgsPointXY(123, 456)))
@@ -39,6 +39,24 @@ def test_CreateFeature(self):
3939
myMessage = '\nExpected: %s\nGot: %s' % (myExpectedId, myId)
4040
assert myId == myExpectedId, myMessage
4141

42+
def test_FeatureDefaultConstructor(self):
43+
"""Test for FID_IS_NULL default constructors See: https://github.com/qgis/QGIS/issues/36962"""
44+
feat = QgsFeature()
45+
# it should be FID_NULL std::numeric_limits<QgsFeatureId>::min(),
46+
# not sure if I can test the exact value in python
47+
self.assertNotEqual(feat.id(), 0)
48+
self.assertTrue(feat.id() < 0)
49+
50+
feat = QgsFeature(QgsFields())
51+
self.assertNotEqual(feat.id(), 0)
52+
self.assertTrue(feat.id() < 0)
53+
54+
feat = QgsFeature(1234)
55+
self.assertEqual(feat.id(), 1234)
56+
57+
feat = QgsFeature(QgsFields(), 1234)
58+
self.assertEqual(feat.id(), 1234)
59+
4260
def test_ValidFeature(self):
4361
myPath = os.path.join(unitTestDataPath(), 'points.shp')
4462
myLayer = QgsVectorLayer(myPath, 'Points', 'ogr')

‎tests/src/python/test_qgsjsonutils.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ def testExportFeatureFieldFormatter(self):
502502
source = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer",
503503
"parent", "memory")
504504
pr = source.dataProvider()
505-
pf1 = QgsFeature()
505+
pf1 = QgsFeature(123)
506506
pf1.setFields(source.fields())
507507
pf1.setAttributes(["test1", 1])
508508
pf2 = QgsFeature()
@@ -518,7 +518,7 @@ def testExportFeatureFieldFormatter(self):
518518

519519
expected = """{
520520
"geometry": null,
521-
"id": 0,
521+
"id": 123,
522522
"properties": {
523523
"fldint": "one",
524524
"fldtxt": "test1"
@@ -578,10 +578,10 @@ def testExportFeatureRelations(self):
578578
parent = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer&field=foreignkey:integer",
579579
"parent", "memory")
580580
pr = parent.dataProvider()
581-
pf1 = QgsFeature()
581+
pf1 = QgsFeature(432)
582582
pf1.setFields(parent.fields())
583583
pf1.setAttributes(["test1", 67, 123])
584-
pf2 = QgsFeature()
584+
pf2 = QgsFeature(876)
585585
pf2.setFields(parent.fields())
586586
pf2.setAttributes(["test2", 68, 124])
587587
assert pr.addFeatures([pf1, pf2])
@@ -622,7 +622,7 @@ def testExportFeatureRelations(self):
622622

623623
expected = """{
624624
"geometry": null,
625-
"id": 0,
625+
"id": 432,
626626
"properties": {
627627
"fldint": 67,
628628
"fldtxt": "test1",
@@ -646,7 +646,7 @@ def testExportFeatureRelations(self):
646646

647647
expected = """{
648648
"geometry": null,
649-
"id": 0,
649+
"id": 876,
650650
"properties": {
651651
"fldint": 68,
652652
"fldtxt": "test2",
@@ -669,7 +669,7 @@ def testExportFeatureRelations(self):
669669
child.setEditorWidgetSetup(1, setup)
670670
expected = """{
671671
"geometry": null,
672-
"id": 0,
672+
"id": 432,
673673
"properties": {
674674
"fldint": 67,
675675
"fldtxt": "test1",
@@ -697,7 +697,7 @@ def testExportFeatureRelations(self):
697697

698698
expected = """{
699699
"geometry": null,
700-
"id": 0,
700+
"id": 876,
701701
"properties": {
702702
"fldint": 68,
703703
"fldtxt": "test2",
@@ -713,7 +713,7 @@ def testExportFeatureRelations(self):
713713

714714
expected = """{
715715
"geometry": null,
716-
"id": 0,
716+
"id": 876,
717717
"properties": {
718718
"fldint": 68,
719719
"fldtxt": "test2",
@@ -875,7 +875,8 @@ def testExportFieldAlias(self):
875875
pf2 = QgsFeature()
876876
pf2.setFields(source.fields())
877877
pf2.setAttributes(["test2", 2])
878-
assert pr.addFeatures([pf1, pf2])
878+
result, features = pr.addFeatures([pf1, pf2])
879+
self.assertTrue(result)
879880

880881
source.setFieldAlias(0, "alias_fldtxt")
881882
source.setFieldAlias(1, "alias_fldint")
@@ -886,14 +887,14 @@ def testExportFieldAlias(self):
886887

887888
expected = """{
888889
"geometry": null,
889-
"id": 0,
890+
"id": 1,
890891
"properties": {
891892
"alias_fldint": 1,
892893
"alias_fldtxt": "test1"
893894
},
894895
"type": "Feature"
895896
}"""
896-
self.assertEqual(exporter.exportFeature(pf1, indent=2), expected)
897+
self.assertEqual(exporter.exportFeature(features[0], indent=2), expected)
897898

898899

899900
if __name__ == "__main__":

Error rendering embedded code

Invalid image source.

Error rendering embedded code

Invalid image source.

0 commit comments

Comments
 (0)
Please sign in to comment.