Fix outdated geometry is returned for feature when a feature has

both unsaved attribute and geometry changes and an expression based request is used
qgis · Dec 13, 2021 · 61517fb · 61517fb
1 parent 8ad98bd
commit 61517fb
Show file tree

Hide file tree

Showing 2 changed files with 131 additions and 1 deletion.
diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp
@@ -617,13 +617,23 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedAttributeFeature( QgsFeature
   while ( mChangedFeaturesIterator.nextFeature( f ) )
   {
     if ( mFetchConsidered.contains( f.id() ) )
-      // skip deleted features and those already handled by the geometry
       continue;
 
     mFetchConsidered << f.id();
 
     updateChangedAttributes( f );
 
+    // also update geometry if needed
+    const auto changedGeometryIt = mSource->mChangedGeometries.constFind( f.id() );
+    if ( changedGeometryIt != mSource->mChangedGeometries.constEnd() )
+    {
+      if ( !mFilterRect.isNull() && !changedGeometryIt->intersects( mFilterRect ) )
+        // skip changed geometries not in rectangle and don't check again
+        continue;
+
+      f.setGeometry( *changedGeometryIt );
+    }
+
     if ( mHasVirtualAttributes )
       addVirtualAttributes( f );
 

diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -3279,6 +3279,126 @@ def testMaximumValue(self):
         pass
 
 
+class TestQgsVectorLayerSourceChangedGeometriesAndAttributesInBuffer(unittest.TestCase, FeatureSourceTestCase):
+
+    @classmethod
+    def getSource(cls):
+        vl = QgsVectorLayer(
+            'Point?crs=epsg:4326&field=pk:integer&field=cnt:integer&field=name:string(0)&field=name2:string(0)&field=num_char:string&field=dt:datetime&field=date:date&field=time:time&key=pk',
+            'test', 'memory')
+        assert (vl.isValid())
+
+        f1 = QgsFeature()
+        f1.setAttributes([5, 200, 'a', 'b', 'c', QDateTime(2020, 4, 5, 1, 2, 3), QDate(2020, 4, 5), QTime(1, 2, 3)])
+
+        f2 = QgsFeature()
+        f2.setAttributes([3, -200, 'd', 'e', 'f', QDateTime(2020, 4, 5, 1, 2, 3), QDate(2020, 4, 5), QTime(1, 2, 3)])
+        f2.setGeometry(QgsGeometry.fromWkt('Point (-70.5 65.2)'))
+
+        f3 = QgsFeature()
+        f3.setAttributes([1, -100, 'g', 'h', 'i', QDateTime(2020, 4, 5, 1, 2, 3), QDate(2020, 4, 5), QTime(1, 2, 3)])
+
+        f4 = QgsFeature()
+        f4.setAttributes([2, -200, 'j', 'k', 'l', QDateTime(2020, 4, 5, 1, 2, 3), QDate(2020, 4, 5), QTime(1, 2, 3)])
+
+        f5 = QgsFeature()
+        f5.setAttributes([4, 400, 'm', 'n', 'o', QDateTime(2020, 4, 5, 1, 2, 3), QDate(2020, 4, 5), QTime(1, 2, 3)])
+
+        vl.dataProvider().addFeatures([f1, f2, f3, f4, f5])
+
+        ids = {f['pk']: f.id() for f in vl.getFeatures()}
+
+        # modify geometries in buffer
+        vl.startEditing()
+        vl.changeGeometry(ids[5], QgsGeometry.fromWkt('Point (-71.123 78.23)'))
+        vl.changeGeometry(ids[3], QgsGeometry())
+        vl.changeGeometry(ids[1], QgsGeometry.fromWkt('Point (-70.332 66.33)'))
+        vl.changeGeometry(ids[2], QgsGeometry.fromWkt('Point (-68.2 70.8)'))
+        vl.changeGeometry(ids[4], QgsGeometry.fromWkt('Point (-65.32 78.3)'))
+
+        # modify attributes in buffer
+        vl.changeAttributeValue(ids[5], 1, -200)
+        vl.changeAttributeValue(ids[5], 2, NULL)
+        vl.changeAttributeValue(ids[5], 3, 'NuLl')
+        vl.changeAttributeValue(ids[5], 4, '5')
+        vl.changeAttributeValue(ids[5], 5, QDateTime(QDate(2020, 5, 4), QTime(12, 13, 14)))
+        vl.changeAttributeValue(ids[5], 6, QDate(2020, 5, 2))
+        vl.changeAttributeValue(ids[5], 7, QTime(12, 13, 1))
+
+        vl.changeAttributeValue(ids[3], 1, 300)
+        vl.changeAttributeValue(ids[3], 2, 'Pear')
+        vl.changeAttributeValue(ids[3], 3, 'PEaR')
+        vl.changeAttributeValue(ids[3], 4, '3')
+        vl.changeAttributeValue(ids[3], 5, NULL)
+        vl.changeAttributeValue(ids[3], 6, NULL)
+        vl.changeAttributeValue(ids[3], 7, NULL)
+
+        vl.changeAttributeValue(ids[1], 1, 100)
+        vl.changeAttributeValue(ids[1], 2, 'Orange')
+        vl.changeAttributeValue(ids[1], 3, 'oranGe')
+        vl.changeAttributeValue(ids[1], 4, '1')
+        vl.changeAttributeValue(ids[1], 5, QDateTime(QDate(2020, 5, 3), QTime(12, 13, 14)))
+        vl.changeAttributeValue(ids[1], 6, QDate(2020, 5, 3))
+        vl.changeAttributeValue(ids[1], 7, QTime(12, 13, 14))
+
+        vl.changeAttributeValue(ids[2], 1, 200)
+        vl.changeAttributeValue(ids[2], 2, 'Apple')
+        vl.changeAttributeValue(ids[2], 3, 'Apple')
+        vl.changeAttributeValue(ids[2], 4, '2')
+        vl.changeAttributeValue(ids[2], 5, QDateTime(QDate(2020, 5, 4), QTime(12, 14, 14)))
+        vl.changeAttributeValue(ids[2], 6, QDate(2020, 5, 4))
+        vl.changeAttributeValue(ids[2], 7, QTime(12, 14, 14))
+
+        vl.changeAttributeValue(ids[4], 1, 400)
+        vl.changeAttributeValue(ids[4], 2, 'Honey')
+        vl.changeAttributeValue(ids[4], 3, 'Honey')
+        vl.changeAttributeValue(ids[4], 4, '4')
+        vl.changeAttributeValue(ids[4], 5, QDateTime(QDate(2021, 5, 4), QTime(13, 13, 14)))
+        vl.changeAttributeValue(ids[4], 6, QDate(2021, 5, 4))
+        vl.changeAttributeValue(ids[4], 7, QTime(13, 13, 14))
+
+        return vl
+
+    @classmethod
+    def setUpClass(cls):
+        """Run before all tests"""
+        # Create test layer for FeatureSourceTestCase
+        cls.source = cls.getSource()
+
+    def testGetFeaturesSubsetAttributes2(self):
+        """ Override and skip this QgsFeatureSource test. We are using a memory provider, and it's actually more efficient for the memory provider to return
+        its features as direct copies (due to implicit sharing of QgsFeature)
+        """
+        pass
+
+    def testGetFeaturesNoGeometry(self):
+        """ Override and skip this QgsFeatureSource test. We are using a memory provider, and it's actually more efficient for the memory provider to return
+        its features as direct copies (due to implicit sharing of QgsFeature)
+        """
+        pass
+
+    def testOrderBy(self):
+        """ Skip order by tests - edited features are not sorted in iterators.
+        (Maybe they should be??)
+        """
+        pass
+
+    def testUniqueValues(self):
+        """ Skip unique values test - as noted in the docs this is unreliable when features are in the buffer
+        """
+        pass
+
+    def testMinimumValue(self):
+        """ Skip min values test - as noted in the docs this is unreliable when features are in the buffer
+        """
+        pass
+
+    def testMaximumValue(self):
+        """ Skip max values test - as noted in the docs this is unreliable when features are in the buffer
+        """
+        pass
+
+
 class TestQgsVectorLayerSourceDeletedFeaturesInBuffer(unittest.TestCase, FeatureSourceTestCase):
 
     @classmethod