Fix crash caused by circular connections

qgis · Nov 23, 2021 · 99e6ff7 · 99e6ff7
1 parent c179587
commit 99e6ff7
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 145 deletions.
diff --git a/src/gui/editorwidgets/qgsrelationwidgetwrapper.cpp b/src/gui/editorwidgets/qgsrelationwidgetwrapper.cpp
@@ -115,7 +115,6 @@ void QgsRelationWidgetWrapper::widgetValueChanged( const QString &attribute, con
 {
   if ( mWidget && attributeChanged )
   {
-    // Maybe from here the other bug? jump to first feature?
     QgsFeature feature { mWidget->feature() };
     if ( feature.attribute( attribute ) != newValue )
     {

diff --git a/src/gui/qgsattributeform.cpp b/src/gui/qgsattributeform.cpp
@@ -482,90 +482,8 @@ bool QgsAttributeForm::saveEdits( QString *error )
   return success;
 }
 
-void QgsAttributeForm::updateValuesDependencies( const int originIdx )
-{
-  updateFieldDependencies();
-
-  updateValuesDependenciesDefaultValues( originIdx );
-  updateValuesDependenciesVirtualFields( originIdx );
-}
-
-void QgsAttributeForm::updateValuesDependenciesDefaultValues( const int originIdx )
-{
-  if ( !mDefaultValueDependencies.contains( originIdx ) )
-    return;
-
-  // create updated Feature
-  QgsFeature updatedFeature = QgsFeature( mFeature );
-  if ( mFeature.isValid() || mMode == QgsAttributeEditorContext::AddFeatureMode )
-  {
-    QgsAttributes dst = mFeature.attributes();
-    for ( QgsWidgetWrapper *ww : std::as_const( mWidgets ) )
-    {
-      QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
-      if ( eww )
-      {
-        QVariantList dstVars = QVariantList() << dst.at( eww->fieldIdx() );
-        QVariantList srcVars = QVariantList() << eww->value();
-        QList<int> fieldIndexes = QList<int>() << eww->fieldIdx();
-
-        // append additional fields
-        const QStringList additionalFields = eww->additionalFields();
-        for ( const QString &fieldName : additionalFields )
-        {
-          int idx = eww->layer()->fields().lookupField( fieldName );
-          fieldIndexes << idx;
-          dstVars << dst.at( idx );
-        }
-        srcVars.append( eww->additionalFieldValues() );
-
-        Q_ASSERT( dstVars.count() == srcVars.count() );
-
-        for ( int i = 0; i < dstVars.count(); i++ )
-        {
-
-          if ( !qgsVariantEqual( dstVars[i], srcVars[i] ) && srcVars[i].isValid() && fieldIsEditable( fieldIndexes[i] ) )
-          {
-            dst[fieldIndexes[i]] = srcVars[i];
-          }
-        }
-      }
-    }
-    updatedFeature.setAttributes( dst );
-
-    // go through depending fields and update the fields with defaultexpression
-    QList<QgsWidgetWrapper *> relevantWidgets = mDefaultValueDependencies.values( originIdx );
-    for ( QgsWidgetWrapper *ww : std::as_const( relevantWidgets ) )
-    {
-      QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
-      if ( eww )
-      {
-        //do not update when when mMode is not AddFeatureMode and it's not applyOnUpdate
-        if ( mMode != QgsAttributeEditorContext::AddFeatureMode && !eww->field().defaultValueDefinition().applyOnUpdate() )
-        {
-          continue;
-        }
-
-        //do not update when this widget is already updating (avoid recursions)
-        if ( mAlreadyUpdatedFields.contains( eww->fieldIdx() ) )
-          continue;
-
-        QgsExpressionContext context = createExpressionContext( updatedFeature );
-        QString value = mLayer->defaultValue( eww->fieldIdx(), updatedFeature, &context ).toString();
-        eww->setValue( value );
-      }
-    }
-  }
-}
-
-void QgsAttributeForm::updateValuesDependenciesVirtualFields( const int originIdx )
+QgsFeature QgsAttributeForm::getUpdatedFeature() const
 {
-  if ( !mVirtualFieldsDependencies.contains( originIdx ) )
-    return;
-
-  if ( !mFeature.isValid() )
-    return;
-
   // create updated Feature
   QgsFeature updatedFeature = QgsFeature( mFeature );
 
@@ -600,9 +518,67 @@ void QgsAttributeForm::updateValuesDependenciesVirtualFields( const int originId
   }
   updatedFeature.setAttributes( featureAttributes );
 
-  // go through depending fields and update the virtual field with its expression
-  QList<QgsWidgetWrapper *> relevantWidgets = mVirtualFieldsDependencies.values( originIdx );
+  return updatedFeature;
+}
+
+void QgsAttributeForm::updateValuesDependencies( const int originIdx )
+{
+  updateFieldDependencies();
+
+  updateValuesDependenciesDefaultValues( originIdx );
+  updateValuesDependenciesVirtualFields( originIdx );
+}
+
+void QgsAttributeForm::updateValuesDependenciesDefaultValues( const int originIdx )
+{
+  if ( !mDefaultValueDependencies.contains( originIdx ) )
+    return;
+
+  if ( !mFeature.isValid()
+       && mMode != QgsAttributeEditorContext::AddFeatureMode )
+    return;
+
+  // create updated Feature
+  QgsFeature updatedFeature = getUpdatedFeature();
+
+  // go through depending fields and update the fields with defaultexpression
+  QList<QgsWidgetWrapper *> relevantWidgets = mDefaultValueDependencies.values( originIdx );
   for ( QgsWidgetWrapper *ww : std::as_const( relevantWidgets ) )
+  {
+    QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
+    if ( eww )
+    {
+      //do not update when when mMode is not AddFeatureMode and it's not applyOnUpdate
+      if ( mMode != QgsAttributeEditorContext::AddFeatureMode && !eww->field().defaultValueDefinition().applyOnUpdate() )
+      {
+        continue;
+      }
+
+      //do not update when this widget is already updating (avoid recursions)
+      if ( mAlreadyUpdatedFields.contains( eww->fieldIdx() ) )
+        continue;
+
+      QgsExpressionContext context = createExpressionContext( updatedFeature );
+      QString value = mLayer->defaultValue( eww->fieldIdx(), updatedFeature, &context ).toString();
+      eww->setValue( value );
+    }
+  }
+}
+
+void QgsAttributeForm::updateValuesDependenciesVirtualFields( const int originIdx )
+{
+  if ( !mVirtualFieldsDependencies.contains( originIdx ) )
+    return;
+
+  if ( !mFeature.isValid() )
+    return;
+
+  // create updated Feature
+  QgsFeature updatedFeature = getUpdatedFeature();
+
+  // go through depending fields and update the virtual field with its expression
+  const QList<QgsWidgetWrapper *> relevantWidgets = mVirtualFieldsDependencies.values( originIdx );
+  for ( QgsWidgetWrapper *ww : relevantWidgets )
   {
     QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
     if ( !eww )
@@ -624,70 +600,29 @@ void QgsAttributeForm::updateValuesDependenciesVirtualFields( const int originId
 void QgsAttributeForm::updateRelatedLayerFields()
 {
   // Synchronize dependencies
-  for ( QgsWidgetWrapper *ww : std::as_const( mWidgets ) )
-  {
-    QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
-    if ( ! eww )
-      continue;
-
-    updateRelatedLayerFieldsDependencies( eww );
-  }
+  updateRelatedLayerFieldsDependencies();
 
   if ( mRelatedLayerFieldsDependencies.isEmpty() )
     return;
 
   if ( !mFeature.isValid() )
     return;
 
-  QgsAttributes featureAttributes = mFeature.attributes();
-  for ( QgsWidgetWrapper *ww : std::as_const( mWidgets ) )
-  {
-    QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
-    if ( !eww )
-      continue;
-
-    QVariantList dstVars = QVariantList() << featureAttributes.at( eww->fieldIdx() );
-    QVariantList srcVars = QVariantList() << eww->value();
-    QList<int> fieldIndexes = QList<int>() << eww->fieldIdx();
-
-    // append additional fields
-    const QStringList additionalFields = eww->additionalFields();
-    for ( const QString &fieldName : additionalFields )
-    {
-      int idx = eww->layer()->fields().lookupField( fieldName );
-      fieldIndexes << idx;
-      dstVars << featureAttributes.at( idx );
-    }
-    srcVars.append( eww->additionalFieldValues() );
-
-    Q_ASSERT( dstVars.count() == srcVars.count() );
-
-    for ( int i = 0; i < dstVars.count(); i++ )
-    {
-      if ( !qgsVariantEqual( dstVars[i], srcVars[i] ) && srcVars[i].isValid() && fieldIsEditable( fieldIndexes[i] ) )
-        featureAttributes[fieldIndexes[i]] = srcVars[i];
-    }
-  }
+  // create updated Feature
+  QgsFeature updatedFeature = getUpdatedFeature();
 
   // go through depending fields and update the fields with virtual field
-  for ( QgsWidgetWrapper *ww : std::as_const( mRelatedLayerFieldsDependencies ) )
+  const QSet<QgsEditorWidgetWrapper *> relevantWidgets = mRelatedLayerFieldsDependencies;
+  for ( QgsEditorWidgetWrapper *eww : relevantWidgets )
   {
-    QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( ww );
-    if ( !eww )
-      continue;
-
     //do not update when this widget is already updating (avoid recursions)
     if ( mAlreadyUpdatedFields.contains( eww->fieldIdx() ) )
       continue;
 
-    // create updated Feature
-    QgsFeature updatedFeature = QgsFeature( mFeature );
-    updatedFeature.setAttributes( featureAttributes );
+    // Update value
     QgsExpressionContext context = createExpressionContext( updatedFeature );
-
     QgsExpression exp( mLayer->expressionField( eww->fieldIdx() ) );
     QVariant value = exp.evaluate( &context );
-
     eww->setValue( value );
   }
 }
@@ -2509,7 +2444,7 @@ void QgsAttributeForm::afterWidgetInit()
       QgsRelationWidgetWrapper *relationWidgetWrapper = qobject_cast<QgsRelationWidgetWrapper *>( ww );
       if ( relationWidgetWrapper )
       {
-        connect( relationWidgetWrapper, &QgsRelationWidgetWrapper::relatedFeaturesChanged, this, &QgsAttributeForm::onRelatedFeaturesChanged );
+        connect( relationWidgetWrapper, &QgsRelationWidgetWrapper::relatedFeaturesChanged, this, &QgsAttributeForm::onRelatedFeaturesChanged, Qt::QueuedConnection );
       }
     }
   }
@@ -2808,9 +2743,7 @@ void QgsAttributeForm::updateFieldDependencies()
       continue;
 
     updateFieldDependenciesDefaultValue( eww );
-
     updateFieldDependenciesVirtualFields( eww );
-
     updateRelatedLayerFieldsDependencies( eww );
   }
 }
@@ -2862,19 +2795,28 @@ void QgsAttributeForm::updateFieldDependenciesVirtualFields( QgsEditorWidgetWrap
   }
 }
 
-void QgsAttributeForm::updateRelatedLayerFieldsDependencies(QgsEditorWidgetWrapper *eww)
+void QgsAttributeForm::updateRelatedLayerFieldsDependencies( QgsEditorWidgetWrapper *eww )
 {
-  for ( QgsAttributeFormWidget *formWidget : mFormWidgets )
+  if ( eww )
   {
-    QgsAttributeFormRelationEditorWidget *relationEditorWidget = dynamic_cast<QgsAttributeFormRelationEditorWidget *>( formWidget );
-    if ( !relationEditorWidget )
-      continue;
-
     QString expressionField = eww->layer()->expressionField( eww->fieldIdx() );
     if ( expressionField.contains( QStringLiteral( "relation_aggregate" ) )
          || expressionField.contains( QStringLiteral( "get_features" ) ) )
       mRelatedLayerFieldsDependencies.insert( eww );
   }
+  else
+  {
+    mRelatedLayerFieldsDependencies.clear();
+    //create defaultValueDependencies
+    for ( QgsWidgetWrapper *ww : std::as_const( mWidgets ) )
+    {
+      QgsEditorWidgetWrapper *editorWidgetWrapper = qobject_cast<QgsEditorWidgetWrapper *>( ww );
+      if ( ! editorWidgetWrapper )
+        continue;
+
+      updateRelatedLayerFieldsDependencies( editorWidgetWrapper );
+    }
+  }
 }
 
 void QgsAttributeForm::setMultiEditFeatureIdsRelations( const QgsFeatureIds &fids )

diff --git a/src/gui/qgsattributeform.h b/src/gui/qgsattributeform.h
@@ -381,7 +381,7 @@ class GUI_EXPORT QgsAttributeForm : public QWidget
     void updateFieldDependencies();
     void updateFieldDependenciesDefaultValue( QgsEditorWidgetWrapper *eww );
     void updateFieldDependenciesVirtualFields( QgsEditorWidgetWrapper *eww );
-    void updateRelatedLayerFieldsDependencies( QgsEditorWidgetWrapper *eww );
+    void updateRelatedLayerFieldsDependencies( QgsEditorWidgetWrapper *eww = nullptr );
 
     void setMultiEditFeatureIdsRelations( const QgsFeatureIds &fids );
 
@@ -413,10 +413,10 @@ class GUI_EXPORT QgsAttributeForm : public QWidget
     bool saveEdits( QString *error );
 
     //! update the default values and virtual fields in the fields after a referenced field changed
+    QgsFeature getUpdatedFeature() const;
     void updateValuesDependencies( const int originIdx );
     void updateValuesDependenciesDefaultValues( const int originIdx );
     void updateValuesDependenciesVirtualFields( const int originIdx );
-
     void updateRelatedLayerFields();
 
     void clearMultiEditMessages();
@@ -534,7 +534,7 @@ class GUI_EXPORT QgsAttributeForm : public QWidget
     /**
      * Dependency list for values depending on related layers.
      */
-    QSet<QgsWidgetWrapper *> mRelatedLayerFieldsDependencies;
+    QSet<QgsEditorWidgetWrapper *> mRelatedLayerFieldsDependencies;
 
     //! List of updated fields to avoid recursion on the setting of defaultValues
     QList<int> mAlreadyUpdatedFields;