Avoid recursion in apply on update default values

m-kuhn · m-kuhn · commit 2fbb6f09cce7 · 2017-09-29T17:52:34.000+02:00
diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip
@@ -925,13 +925,16 @@ Return the provider type for this layer
     virtual bool addFeature( QgsFeature &feature, QgsFeatureSink::Flags flags = 0 );
 
 
-    bool updateFeature( const QgsFeature &f );
+    bool updateFeature( const QgsFeature &feature, bool skipDefaultValues = false );
 %Docstring
  Updates an existing feature. This method needs to query the datasource
-on every call. Consider using changeAttributeValue() or
-changeGeometry() instead.
-\param f  Feature to update
-:return:   True in case of success and False in case of error
+ on every call. Consider using changeAttributeValue() or
+ changeGeometry() instead. The id of the feature will be used to match
+ an existing feature.
+
+  \param feature  Feature with changed geometry or attributes.
+  \param skipDefaultValues Default values will not be updated if this is true. False by default.
+  :return:   True in case of success and False in case of error
  :rtype: bool
 %End
 
diff --git a/src/app/qgsattributetypedialog.h b/src/app/qgsattributetypedialog.h
@@ -164,12 +164,10 @@ class APP_EXPORT QgsAttributeTypeDialog: public QDialog, private Ui::QgsAttribut
      */
     void setDefaultValueExpression( const QString &expression );
 
-
-
     QgsExpressionContext createExpressionContext() const override;
 
     bool applyDefaultValueOnUpdate() const;
-    void setApplyDefaultValueOnUpdate(bool applyDefaultValueOnUpdate);
+    void setApplyDefaultValueOnUpdate( bool applyDefaultValueOnUpdate );
 
   private slots:
 
diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -780,8 +780,7 @@ void QgsVectorLayer::updateDefaultValues( QgsFeatureId fid, QgsFeature feature )
         continue;
 
       feature.setAttribute( idx, defaultValue( idx, feature ) );
-      // TODO catch recursion!!
-      updateFeature( feature );
+      updateFeature( feature, true );
     }
   }
 }
@@ -961,7 +960,7 @@ bool QgsVectorLayer::addFeature( QgsFeature &feature, Flags )
   return success;
 }
 
-bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature )
+bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature, bool skipDefaultValues )
 {
   bool hasChanged = false;
   bool hasError = false;
@@ -1003,7 +1002,7 @@ bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature )
     }
   }
 
-  if ( hasChanged && !mDefaultValueOnUpdateFields.isEmpty() )
+  if ( hasChanged && !mDefaultValueOnUpdateFields.isEmpty() && !skipDefaultValues )
     updateDefaultValues( updatedFeature.id(), updatedFeature );
 
   return !hasError;
diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h
@@ -907,13 +907,17 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
 
     bool addFeature( QgsFeature &feature, QgsFeatureSink::Flags flags = 0 ) override;
 
-    /** Updates an existing feature. This method needs to query the datasource
-        on every call. Consider using changeAttributeValue() or
-        changeGeometry() instead.
-        \param f  Feature to update
-        \returns   True in case of success and False in case of error
+    /**
+     * Updates an existing feature. This method needs to query the datasource
+     * on every call. Consider using changeAttributeValue() or
+     * changeGeometry() instead. The id of the feature will be used to match
+     * an existing feature.
+     *
+     *  \param feature  Feature with changed geometry or attributes.
+     *  \param skipDefaultValues Default values will not be updated if this is true. False by default.
+     *  \returns   True in case of success and False in case of error
      */
-    bool updateFeature( const QgsFeature &f );
+    bool updateFeature( const QgsFeature &feature, bool skipDefaultValues = false );
 
     /** Insert a new vertex before the given vertex number,
      *  in the given ring, item (first number is index 0), and feature

Original file line number	Diff line number	Diff line change
`@@ -780,8 +780,7 @@ void QgsVectorLayer::updateDefaultValues( QgsFeatureId fid, QgsFeature feature )`
`780`	`780`	`continue;`
`781`	`781`
`782`	`782`	`feature.setAttribute( idx, defaultValue( idx, feature ) );`
`783`		`- // TODO catch recursion!!`
`784`		`- updateFeature( feature );`
	`783`	`+ updateFeature( feature, true );`
`785`	`784`	`}`
`786`	`785`	`}`
`787`	`786`	`}`
`@@ -961,7 +960,7 @@ bool QgsVectorLayer::addFeature( QgsFeature &feature, Flags )`
`961`	`960`	`return success;`
`962`	`961`	`}`
`963`	`962`
`964`		`-bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature )`
	`963`	`+bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature, bool skipDefaultValues )`
`965`	`964`	`{`
`966`	`965`	`bool hasChanged = false;`
`967`	`966`	`bool hasError = false;`
`@@ -1003,7 +1002,7 @@ bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature )`
`1003`	`1002`	`}`
`1004`	`1003`	`}`
`1005`	`1004`
`1006`		`- if ( hasChanged && !mDefaultValueOnUpdateFields.isEmpty() )`
	`1005`	`+ if ( hasChanged && !mDefaultValueOnUpdateFields.isEmpty() && !skipDefaultValues )`
`1007`	`1006`	`updateDefaultValues( updatedFeature.id(), updatedFeature );`
`1008`	`1007`
`1009`	`1008`	`return !hasError;`