Complete test coverage for edit buffer

Fix tests
qgis · Feb 13, 2021 · e89c163 · e89c163
1 parent 8cb15c3
commit e89c163
Show file tree

Hide file tree

Showing 8 changed files with 273 additions and 50 deletions.
diff --git a/python/core/auto_generated/vector/qgsvectorlayerundocommand.sip.in b/python/core/auto_generated/vector/qgsvectorlayerundocommand.sip.in
@@ -124,8 +124,7 @@ Constructor for QgsVectorLayerUndoCommandChangeGeometry
     virtual void redo();
 
     virtual int id() const;
-
-    virtual bool mergeWith( const QUndoCommand * );
+    virtual bool mergeWith( const QUndoCommand *other );
 
 
 };

diff --git a/python/core/auto_generated/vector/qgsvectorlayerundopassthroughcommand.sip.in b/python/core/auto_generated/vector/qgsvectorlayerundopassthroughcommand.sip.in
@@ -169,6 +169,10 @@ Constructor for QgsVectorLayerUndoPassthroughCommandChangeGeometry
     virtual void redo();
 
 
+    virtual int id() const;
+    virtual bool mergeWith( const QUndoCommand  *other );
+
+
 };
 
 

diff --git a/src/core/qgstransaction.cpp b/src/core/qgstransaction.cpp
@@ -219,7 +219,6 @@ QString QgsTransaction::createSavepoint( QString &error SIP_OUT )
 
   if ( !mLastSavePointIsDirty && !mSavepoints.isEmpty() )
   {
-    qDebug() << "Returning TOP >>>>> " << mSavepoints.top();
     return mSavepoints.top();
   }
 
@@ -263,7 +262,10 @@ bool QgsTransaction::rollbackToSavepoint( const QString &name, QString &error SI
     return false;
 
   mSavepoints.resize( idx );
-  mLastSavePointIsDirty = false;
+  // Rolling back always dirties the previous savepoint because
+  // the status of the DB has changed between the previous savepoint and the
+  // one we are rolling back to.
+  mLastSavePointIsDirty = true;
   return executeSql( QStringLiteral( "ROLLBACK TO SAVEPOINT %1" ).arg( QgsExpression::quotedColumnRef( name ) ), error );
 }
 

diff --git a/src/core/vector/qgsvectorlayerundocommand.cpp b/src/core/vector/qgsvectorlayerundocommand.cpp
@@ -122,10 +122,7 @@ QgsVectorLayerUndoCommandChangeGeometry::QgsVectorLayerUndoCommandChangeGeometry
   }
 }
 
-int QgsVectorLayerUndoCommandChangeGeometry::id() const
-{
-  return 1;
-}
+
 
 bool QgsVectorLayerUndoCommandChangeGeometry::mergeWith( const QUndoCommand *other )
 {

diff --git a/src/core/vector/qgsvectorlayerundocommand.h b/src/core/vector/qgsvectorlayerundocommand.h
@@ -131,8 +131,8 @@ class CORE_EXPORT QgsVectorLayerUndoCommandChangeGeometry : public QgsVectorLaye
 
     void undo() override;
     void redo() override;
-    int id() const override;
-    bool mergeWith( const QUndoCommand * ) override;
+    int id() const override { return 1; }
+    bool mergeWith( const QUndoCommand *other ) override;
 
   private:
     QgsFeatureId mFid;

diff --git a/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp b/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp
@@ -98,10 +98,6 @@ bool QgsVectorLayerUndoPassthroughCommand::rollBackToSavePoint()
     {
       setError();
     }
-    else
-    {
-      mBuffer->L->dataProvider()->transaction()->dirtyLastSavePoint();
-    }
   }
   return !hasError();
 }
@@ -163,9 +159,14 @@ void QgsVectorLayerUndoPassthroughCommandDeleteFeatures::undo()
 {
   if ( rollBackToSavePoint() )
   {
-    for ( const QgsFeatureId &id : mFids )
+    for ( const QgsFeatureId &fid : mFids )
     {
-      emit mBuffer->featureAdded( id );
+      mBuffer->mDeletedFeatureIds.remove( fid );
+      if ( mDeletedNewFeatures.contains( fid ) )
+      {
+        mBuffer->mAddedFeatures.insert( fid, mDeletedNewFeatures.value( fid ) );
+      }
+      emit mBuffer->featureAdded( fid );
     }
   }
 }
@@ -175,9 +176,19 @@ void QgsVectorLayerUndoPassthroughCommandDeleteFeatures::redo()
   mBuffer->L->dataProvider()->clearErrors();
   if ( setSavePoint() && mBuffer->L->dataProvider()->deleteFeatures( mFids ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
-    for ( const QgsFeatureId &id : mFids )
+    mDeletedNewFeatures.clear();
+    for ( const QgsFeatureId &fid : mFids )
     {
-      emit mBuffer->featureDeleted( id );
+      if ( mBuffer->mAddedFeatures.contains( fid ) )
+      {
+        mDeletedNewFeatures.insert( fid, mBuffer->mAddedFeatures[ fid ] );
+        mBuffer->mAddedFeatures.remove( fid );
+      }
+      else
+      {
+        mBuffer->mDeletedFeatureIds.insert( fid );
+      }
+      emit mBuffer->featureDeleted( fid );
     }
   }
   else
@@ -196,7 +207,6 @@ QgsVectorLayerUndoPassthroughCommandChangeGeometry::QgsVectorLayerUndoPassthroug
 
 void QgsVectorLayerUndoPassthroughCommandChangeGeometry::undo()
 {
-
   if ( rollBackToSavePoint() )
   {
     if ( mBuffer->mAddedFeatures.contains( mFid ) )
@@ -205,8 +215,20 @@ void QgsVectorLayerUndoPassthroughCommandChangeGeometry::undo()
     }
     else
     {
-      // TODO: unless first change?
-      mBuffer->mChangedGeometries[mFid] = mOldGeom;
+      QgsFeature tmp;
+      QgsFeatureRequest request;
+      request.setFilterFid( mFid );
+      request.setSubsetOfAttributes( {} );
+      std::unique_ptr<QgsVectorLayer> layerClone( layer()->clone() );
+      QgsFeatureIterator fi = layerClone->getFeatures( request );
+      if ( fi.nextFeature( tmp ) && tmp.geometry().equals( mOldGeom ) )
+      {
+        mBuffer->mChangedGeometries.remove( mFid );
+      }
+      else
+      {
+        mBuffer->mChangedGeometries[mFid] = mOldGeom;
+      }
     }
   }
 }
@@ -234,6 +256,26 @@ void QgsVectorLayerUndoPassthroughCommandChangeGeometry::redo()
   }
 }
 
+bool QgsVectorLayerUndoPassthroughCommandChangeGeometry::mergeWith( const QUndoCommand *other )
+{
+  if ( other->id() != id() )
+    return false;
+
+  const QgsVectorLayerUndoPassthroughCommandChangeGeometry *merge = dynamic_cast<const QgsVectorLayerUndoPassthroughCommandChangeGeometry *>( other );
+  if ( !merge )
+    return false;
+
+  if ( merge->mFid != mFid )
+    return false;
+
+  mNewGeom = merge->mNewGeom;
+  merge->mNewGeom = QgsGeometry();
+
+  return true;
+}
+
+
+
 QgsVectorLayerUndoPassthroughCommandChangeAttribute::QgsVectorLayerUndoPassthroughCommandChangeAttribute( QgsVectorLayerEditBuffer *buffer, QgsFeatureId fid, int field, const QVariant &newValue )
   : QgsVectorLayerUndoPassthroughCommand( buffer, QObject::tr( "change attribute value" ) )
   , mFid( fid )
@@ -347,9 +389,10 @@ void QgsVectorLayerUndoPassthroughCommandAddAttribute::undo()
   // note that the deleteAttribute here is only necessary to inform the provider that
   // an attribute is removed after the rollBackToSavePoint
   const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() );
-  mBuffer->L->dataProvider()->clearErrors();
-  if ( mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() )
+  if ( rollBackToSavePoint() )
   {
+    mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr );
+    mBuffer->mAddedAttributes.removeAll( mField );
     mBuffer->updateLayerFields();
     emit mBuffer->attributeDeleted( attr );
   }
@@ -366,6 +409,7 @@ void QgsVectorLayerUndoPassthroughCommandAddAttribute::redo()
   {
     mBuffer->updateLayerFields();
     const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() );
+    mBuffer->mAddedAttributes.append( mField );
     emit mBuffer->attributeAdded( attr );
   }
   else

diff --git a/src/core/vector/qgsvectorlayerundopassthroughcommand.h b/src/core/vector/qgsvectorlayerundopassthroughcommand.h
@@ -145,6 +145,8 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommandDeleteFeatures : public Qg
 
   private:
     const QgsFeatureIds mFids;
+    // Keeps track of the deleted features that belong to the added pool
+    QgsFeatureMap mDeletedNewFeatures;
 };
 
 /**
@@ -169,10 +171,13 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommandChangeGeometry : public Qg
     void undo() override;
     void redo() override;
 
+    int id() const override { return 1; }
+    bool mergeWith( const QUndoCommand  *other ) override;
+
   private:
     QgsFeatureId mFid;
-    const QgsGeometry mNewGeom;
-    const QgsGeometry mOldGeom;
+    mutable QgsGeometry mNewGeom;
+    QgsGeometry mOldGeom;
 };
 
 /**