[Layer dependencies] Emit QgsVectorLayer::dataChanged on commitChanges

src/core/vector/qgsvectorlayer.cpp

@@ -6109,22 +6109,21 @@ void QgsVectorLayer::emitDataChanged()
   if ( mDataChangedFired )
     return;
 
+  // If we are asked to fire dataChanged from a layer we depend on,
+  // be sure that this layer is not in the process of committing its changes, because
+  // we will be asked to fire dataChanged at the end of his commit, and we don't
+  // want to fire this signal more than necessary.
+  if ( QgsVectorLayer *layerWeDependUpon = qobject_cast<QgsVectorLayer *>( sender() );
+       layerWeDependUpon && layerWeDependUpon->mCommitChangesActive )
+    return;
+
   updateExtents(); // reset cached extent to reflect data changes
 
   mDataChangedFired = true;
   emit dataChanged();
   mDataChangedFired = false;
 }
 
-void QgsVectorLayer::onAfterCommitChangesDependency()
-{
-  QGIS_PROTECT_QOBJECT_THREAD_ACCESS
-
-  mDataChangedFired = true;
-  reload();
-  mDataChangedFired = false;
-}
-
 bool QgsVectorLayer::setDependencies( const QSet<QgsMapLayerDependency> &oDeps )
 {
   QGIS_PROTECT_QOBJECT_THREAD_ACCESS
@@ -6152,7 +6151,7 @@ bool QgsVectorLayer::setDependencies( const QSet<QgsMapLayerDependency> &oDeps )
       disconnect( lyr, &QgsVectorLayer::geometryChanged, this, &QgsVectorLayer::emitDataChanged );
       disconnect( lyr, &QgsVectorLayer::dataChanged, this, &QgsVectorLayer::emitDataChanged );
       disconnect( lyr, &QgsVectorLayer::repaintRequested, this, &QgsVectorLayer::triggerRepaint );
-      disconnect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::onAfterCommitChangesDependency );
+      disconnect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::emitDataChanged );
     }
   }
 
@@ -6176,7 +6175,7 @@ bool QgsVectorLayer::setDependencies( const QSet<QgsMapLayerDependency> &oDeps )
       connect( lyr, &QgsVectorLayer::geometryChanged, this, &QgsVectorLayer::emitDataChanged );
       connect( lyr, &QgsVectorLayer::dataChanged, this, &QgsVectorLayer::emitDataChanged );
       connect( lyr, &QgsVectorLayer::repaintRequested, this, &QgsVectorLayer::triggerRepaint );
-      connect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::onAfterCommitChangesDependency );
+      connect( lyr, &QgsVectorLayer::afterCommitChanges, this, &QgsVectorLayer::emitDataChanged );
     }
   }
 

src/core/vector/qgsvectorlayer.h

@@ -2802,7 +2802,6 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
     void onSymbolsCounted();
     void onDirtyTransaction( const QString &sql, const QString &name );
     void emitDataChanged();
-    void onAfterCommitChangesDependency();
 
   private:
     void updateDefaultValues( QgsFeatureId fid, QgsFeature feature = QgsFeature(), QgsExpressionContext *context = nullptr );

tests/src/python/test_layer_dependencies.py

@@ -26,7 +26,6 @@
     QgsRectangle,
     QgsSnappingConfig,
     QgsSnappingUtils,
-    QgsTolerance,
     QgsVectorLayer,
 )
 import unittest
@@ -193,14 +192,22 @@ def test_circular_dependencies_with_2_layers(self):
         self.assertEqual(len(spy_points_data_changed), 3)
         self.assertEqual(len(spy_lines_data_changed), 2)
 
-        # added feature is deleted and added with its new defined id
-        # (it was -1 before) so it fires 2 more signal dataChanged on
-        # depending line (on featureAdded and on featureDeleted)
-        # and so 2 more signal on points because it depends on line
-        self.pointsLayer.commitChanges()
+        # commit changes fires dataChanged because external changes could happen (provider side)
+        self.pointsLayer.commitChanges(False)
+        self.assertEqual(len(spy_points_data_changed), 4)
+        self.assertEqual(len(spy_lines_data_changed), 3)
+
+        # points fire dataChanged on geometryChanged
+        # line depends on point, so fire dataChanged
+        self.pointsLayer.changeGeometry(f.id(), QgsGeometry.fromWkt("POINT(0 2)"))
         self.assertEqual(len(spy_points_data_changed), 5)
         self.assertEqual(len(spy_lines_data_changed), 4)
 
+        # commit changes fires dataChanged because external changes could happen (provider side)
+        self.assertTrue(self.pointsLayer.commitChanges())
+        self.assertEqual(len(spy_points_data_changed), 6)
+        self.assertEqual(len(spy_lines_data_changed), 5)
+
         # repaintRequested is called on commit changes on point
         # so it is on depending line
         # (ideally only one repaintRequested signal is fired, but it's harmless to fire multiple ones)
@@ -229,16 +236,22 @@ def test_circular_dependencies_with_1_layer(self):
         self.linesLayer.addFeatures([f])
         self.assertEqual(len(spy_lines_data_changed), 2)
 
-        # added feature is deleted and added with its new defined id
-        # (it was -1 before) so it fires 2 more signal dataChanged on
-        # depending line (on featureAdded and on featureDeleted)
-        self.linesLayer.commitChanges()
-        self.assertEqual(len(spy_lines_data_changed), 4)
+        # line fire dataChanged on commitChanges
+        self.linesLayer.commitChanges(False)
+        self.assertEqual(len(spy_lines_data_changed), 3)
 
         # repaintRequested is called only once on commit changes on line
         # (ideally only one repaintRequested signal is fired, but it's harmless to fire multiple ones)
         self.assertGreaterEqual(len(spy_lines_repaint_requested), 2)
 
+        # line fire dataChanged on geometryChanged
+        self.linesLayer.changeGeometry(f.id(), QgsGeometry.fromWkt("LINESTRING(0 0, 2 2)"))
+        self.assertEqual(len(spy_lines_data_changed), 4)
+
+        # commit changes fires dataChanged because external changes could happen (provider side)
+        self.linesLayer.commitChanges()
+        self.assertEqual(len(spy_lines_data_changed), 5)
+
     def test_layerDefinitionRewriteId(self):
         tmpfile = os.path.join(tempfile.tempdir, "test.qlr")