Fix renaming a virtual field leads to crash (fix #15669)

nyalldawson · nyalldawson · commit d55807722080 · 2016-10-12T20:54:39.000+10:00
(cherry-picked from 3a76708)
diff --git a/python/core/qgsexpressionfieldbuffer.sip b/python/core/qgsexpressionfieldbuffer.sip
@@ -27,6 +27,16 @@ class QgsExpressionFieldBuffer
      */
     void removeExpression( int index );
 
+    /**
+     * Renames an expression field at a given index
+     *
+     * @param index The index of the expression to change
+     * @param name   New name for field
+     *
+     * @note added in 3.0
+     */
+    void renameExpression( int index, const QString& name );
+
     /**
      * Changes the expression at a given index
      *
diff --git a/src/core/qgsexpressionfieldbuffer.cpp b/src/core/qgsexpressionfieldbuffer.cpp
@@ -33,6 +33,11 @@ void QgsExpressionFieldBuffer::removeExpression( int index )
   mExpressions.removeAt( index );
 }
 
+void QgsExpressionFieldBuffer::renameExpression( int index, const QString& name )
+{
+  mExpressions[index].field.setName( name );
+}
+
 void QgsExpressionFieldBuffer::updateExpression( int index, const QString& exp )
 {
   mExpressions[index].expression = exp;
diff --git a/src/core/qgsexpressionfieldbuffer.h b/src/core/qgsexpressionfieldbuffer.h
@@ -67,6 +67,16 @@ class CORE_EXPORT QgsExpressionFieldBuffer
      */
     void removeExpression( int index );
 
+    /**
+     * Renames an expression field at a given index
+     *
+     * @param index The index of the expression to change
+     * @param name   New name for field
+     *
+     * @note added in 3.0
+     */
+    void renameExpression( int index, const QString& name );
+
     /**
      * Changes the expression at a given index
      *
diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -2405,12 +2405,41 @@ void QgsVectorLayer::remAttributeAlias( int attIndex )
   }
 }
 
-bool QgsVectorLayer::renameAttribute( int attIndex, const QString& newName )
+bool QgsVectorLayer::renameAttribute( int index, const QString& newName )
 {
-  if ( !mEditBuffer || !mDataProvider )
+  if ( index < 0 || index >= fields().count() )
     return false;
 
-  return mEditBuffer->renameAttribute( attIndex, newName );
+  switch ( mUpdatedFields.fieldOrigin( index ) )
+  {
+    case QgsFields::OriginExpression:
+    {
+      if ( mExpressionFieldBuffer )
+      {
+        int oi = mUpdatedFields.fieldOriginIndex( index );
+        mExpressionFieldBuffer->renameExpression( oi, newName );
+        updateFields();
+        return true;
+      }
+      else
+      {
+        return false;
+      }
+    }
+
+    case QgsFields::OriginProvider:
+    case QgsFields::OriginEdit:
+
+      if ( !mEditBuffer || !mDataProvider )
+        return false;
+
+      return mEditBuffer->renameAttribute( index, newName );
+
+    case QgsFields::OriginJoin:
+    case QgsFields::OriginUnknown:
+      return false;
+
+  }
 }
 
 void QgsVectorLayer::addAttributeAlias( int attIndex, const QString& aliasString )
diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -1033,6 +1033,16 @@ def checkFieldNames(names):
         #layer.undoStack().redo()
         #checkFieldNames(['fldint'])
 
+    def test_RenameExpressionField(self):
+        layer = createLayerWithOnePoint()
+        exp_field_idx = layer.addExpressionField('1+1', QgsField('math_is_hard', QVariant.Int))
+
+        #rename and check
+        self.assertTrue(layer.renameAttribute(exp_field_idx, 'renamed'))
+        self.assertEqual(layer.fields()[exp_field_idx].name(), 'renamed')
+        f = next(layer.getFeatures())
+        self.assertEqual(f.fields()[exp_field_idx].name(), 'renamed')
+
     def test_fields(self):
         layer = createLayerWithOnePoint()
 

-Original file line number
+Diff line change
      */
     void removeExpression( int index );
 +    /**
 +     * Renames an expression field at a given index
 +     *
 +     * @param index The index of the expression to change
 +     * @param name   New name for field
 +     *Collapse commentComment on line R35
DelazJ commented on Oct 12, 2016 
DelazJ
on Oct 12, 2016
Contributor
@nyalldawson shouldn't the release number be updated?
Code has comments. Press enter to view.
 +     * @note added in 3.0
 +     */
 +    void renameExpression( int index, const QString& name );
++
     /**
      * Changes the expression at a given index
+     *
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,11 @@ void QgsExpressionFieldBuffer::removeExpression( int index )`
`33`	`33`	`mExpressions.removeAt( index );`
`34`	`34`	`}`
`35`	`35`
	`36`	`+void QgsExpressionFieldBuffer::renameExpression( int index, const QString& name )`
	`37`	`+{`
	`38`	`+ mExpressions[index].field.setName( name );`
	`39`	`+}`
	`40`	`+`
`36`	`41`	`void QgsExpressionFieldBuffer::updateExpression( int index, const QString& exp )`
`37`	`42`	`{`
`38`	`43`	`mExpressions[index].expression = exp;`