Skip to content
Permalink
Browse files

Fix QgsFeature default ctor set fid to 0

The QgsFeature default constructors initialized
feature id to 0, which is a valid feature id
instead of initializing it fo FID_NULL.

This was just wrong and broke the validator for
UNIQUE constraints in case a feature with fid 0
existed in the data provider.

Fixes #36962
  • Loading branch information
elpaso committed Jun 22, 2020
1 parent e4718d7 commit 0a4214499dda54315b4e999bd098cb30d035edbe
@@ -136,14 +136,14 @@ geometry and a list of field/values attributes.
sipCpp->deleteAttribute( fieldIdx );
%End

QgsFeature( qint64 id = 0 );
QgsFeature( qint64 id = FID_NULL );
%Docstring
Constructor for QgsFeature

:param id: feature id
%End

QgsFeature( const QgsFields &fields, qint64 id = 0 );
QgsFeature( const QgsFields &fields, qint64 id = FID_NULL );
%Docstring
Constructor for QgsFeature

@@ -176,9 +176,9 @@ class CORE_EXPORT QgsFeature
* \param id feature id
*/
#ifndef SIP_RUN
QgsFeature( QgsFeatureId id = QgsFeatureId() );
QgsFeature( QgsFeatureId id = FID_NULL );
#else
QgsFeature( qint64 id = 0 );
QgsFeature( qint64 id = FID_NULL );
#endif

/**
@@ -187,9 +187,9 @@ class CORE_EXPORT QgsFeature
* \param id feature id
*/
#ifndef SIP_RUN
QgsFeature( const QgsFields &fields, QgsFeatureId id = QgsFeatureId() );
QgsFeature( const QgsFields &fields, QgsFeatureId id = FID_NULL );
#else
QgsFeature( const QgsFields &fields, qint64 id = 0 );
QgsFeature( const QgsFields &fields, qint64 id = FID_NULL );
#endif

/**
@@ -178,7 +178,7 @@ void QgsMapToolDigitizeFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
//grass provider has its own mechanism of feature addition
if ( provider->capabilities() & QgsVectorDataProvider::AddFeatures )
{
QgsFeature f( vlayer->fields(), 0 );
QgsFeature f( vlayer->fields() );

QgsGeometry g;
if ( layerWKBType == QgsWkbTypes::Point )
@@ -32,6 +32,7 @@ class TestQgsFeature: public QObject
void init();// will be called before each testfunction is executed.
void cleanup();// will be called after every testfunction.
void attributesTest(); //test QgsAttributes
void constructorTest(); //test default constructors
void attributesToMap();
void create();//test creating a feature
void copy();// test cpy destruction (double delete)
@@ -120,6 +121,18 @@ void TestQgsFeature::attributesTest()
QCOMPARE( attr7.size(), 5 );
}

void TestQgsFeature::constructorTest()
{
QgsFeature f;
QVERIFY( FID_IS_NULL( f.id() ) );
QgsFeature f2 { QgsFields() };
QVERIFY( FID_IS_NULL( f2.id() ) );
QgsFeature f3 { 1234 };
QVERIFY( ! FID_IS_NULL( f3.id() ) );
QgsFeature f4 { QgsFields(), 1234 };
QVERIFY( ! FID_IS_NULL( f4.id() ) );
}

void TestQgsFeature::attributesToMap()
{
QgsAttributes attr1;
@@ -30,7 +30,7 @@
class TestQgsFeature(unittest.TestCase):

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

def test_FeatureDefaultConstructor(self):
"""Test for FID_IS_NULL default constructors See: https://github.com/qgis/QGIS/issues/36962"""
feat = QgsFeature()
# it should be FID_NULL std::numeric_limits<QgsFeatureId>::min(),
# not sure if I can test the exact value in python
self.assertNotEqual(feat.id(), 0)
self.assertTrue(feat.id() < 0)

feat = QgsFeature(QgsFields())
self.assertNotEqual(feat.id(), 0)
self.assertTrue(feat.id() < 0)

feat = QgsFeature(1234)
self.assertEqual(feat.id(), 1234)

feat = QgsFeature(QgsFields(), 1234)
self.assertEqual(feat.id(), 1234)

def test_ValidFeature(self):
myPath = os.path.join(unitTestDataPath(), 'points.shp')
myLayer = QgsVectorLayer(myPath, 'Points', 'ogr')

0 comments on commit 0a42144

Please sign in to comment.
You can’t perform that action at this time.