Merge pull request #41539 from elpaso/bugfix-gh41477-editbuffer-passt…

…hrough Bugfix gh41477 editbuffer passthrough
qgis · Apr 6, 2021 · bf4b0e5ed0a969e10301ab473eb10908d353dee9 · bf4b0e5
1 parent 523510c
commit bf4b0e5ed0a969e10301ab473eb10908d353dee9
diff --git a/python/core/auto_generated/vector/qgsvectorlayereditbuffer.sip.in b/python/core/auto_generated/vector/qgsvectorlayereditbuffer.sip.in
@@ -208,6 +208,7 @@ Returns ``True`` if the specified feature ID has been deleted but not committed.
 %End
 
 
+
   protected slots:
     void undoIndexChanged( int index );
 
@@ -269,8 +270,6 @@ Emitted after committing an attribute rename
 Constructor for QgsVectorLayerEditBuffer
 %End
 
-    void updateFields( QgsFields &fields );
-
     void updateFeatureGeometry( QgsFeature &f );
 %Docstring
 Update feature with uncommitted geometry updates

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
@@ -218,7 +218,9 @@ QString QgsTransaction::createSavepoint( QString &error SIP_OUT )
     return QString();
 
   if ( !mLastSavePointIsDirty && !mSavepoints.isEmpty() )
+  {
     return mSavepoints.top();
+  }
 
   const QString name( QStringLiteral( "qgis" ) + ( QUuid::createUuid().toString().mid( 1, 24 ).replace( '-', QString() ) ) );
 
@@ -260,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/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp
@@ -877,7 +877,7 @@ QgsRectangle QgsVectorLayer::extent() const
   }
 
   if ( !mEditBuffer ||
-       ( mEditBuffer->mDeletedFeatureIds.isEmpty() && mEditBuffer->mChangedGeometries.isEmpty() ) ||
+       ( !mDataProvider->transaction() && ( mEditBuffer->deletedFeatureIds().isEmpty() && mEditBuffer->changedGeometries().isEmpty() ) ) ||
        QgsDataSourceUri( mDataProvider->dataSourceUri() ).useEstimatedMetadata() )
   {
     mDataProvider->updateExtents();
@@ -890,9 +890,10 @@ QgsRectangle QgsVectorLayer::extent() const
       rect.combineExtentWith( r );
     }
 
-    if ( mEditBuffer )
+    if ( mEditBuffer && !mDataProvider->transaction() )
     {
-      for ( QgsFeatureMap::const_iterator it = mEditBuffer->mAddedFeatures.constBegin(); it != mEditBuffer->mAddedFeatures.constEnd(); ++it )
+      const auto addedFeatures = mEditBuffer->addedFeatures();
+      for ( QgsFeatureMap::const_iterator it = addedFeatures.constBegin(); it != addedFeatures.constEnd(); ++it )
       {
         if ( it->hasGeometry() )
         {
@@ -3366,13 +3367,13 @@ long QgsVectorLayer::featureCount() const
   if ( ! mDataProvider )
     return -1;
   return mDataProvider->featureCount() +
-         ( mEditBuffer ? mEditBuffer->mAddedFeatures.size() - mEditBuffer->mDeletedFeatureIds.size() : 0 );
+         ( mEditBuffer && ! mDataProvider->transaction() ? mEditBuffer->addedFeatures().size() - mEditBuffer->deletedFeatureIds().size() : 0 );
 }
 
 QgsFeatureSource::FeatureAvailability QgsVectorLayer::hasFeatures() const
 {
-  const QgsFeatureIds deletedFeatures( mEditBuffer ? mEditBuffer->deletedFeatureIds() : QgsFeatureIds() );
-  const QgsFeatureMap addedFeatures( mEditBuffer ? mEditBuffer->addedFeatures() : QgsFeatureMap() );
+  const QgsFeatureIds deletedFeatures( mEditBuffer && ! mDataProvider->transaction() ? mEditBuffer->deletedFeatureIds() : QgsFeatureIds() );
+  const QgsFeatureMap addedFeatures( mEditBuffer && ! mDataProvider->transaction() ? mEditBuffer->addedFeatures() : QgsFeatureMap() );
 
   if ( mEditBuffer && !deletedFeatures.empty() )
   {
@@ -3456,9 +3457,9 @@ bool QgsVectorLayer::rollBack( bool deleteBuffer )
     return false;
   }
 
-  bool rollbackExtent = !mEditBuffer->mDeletedFeatureIds.isEmpty() ||
-                        !mEditBuffer->mAddedFeatures.isEmpty() ||
-                        !mEditBuffer->mChangedGeometries.isEmpty();
+  bool rollbackExtent = !mDataProvider->transaction() && ( !mEditBuffer->deletedFeatureIds().isEmpty() ||
+                        !mEditBuffer->addedFeatures().isEmpty() ||
+                        !mEditBuffer->changedGeometries().isEmpty() );
 
   emit beforeRollBack();
 
@@ -4043,7 +4044,7 @@ QSet<QVariant> QgsVectorLayer::uniqueValues( int index, int limit ) const
     {
       uniqueValues = mDataProvider->uniqueValues( index, limit );
 
-      if ( mEditBuffer )
+      if ( mEditBuffer && ! mDataProvider->transaction() )
       {
         QSet<QString> vals;
         const auto constUniqueValues = uniqueValues;
@@ -4091,10 +4092,11 @@ QSet<QVariant> QgsVectorLayer::uniqueValues( int index, int limit ) const
 
     case QgsFields::OriginEdit:
       // the layer is editable, but in certain cases it can still be avoided going through all features
-      if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
-           mEditBuffer->mAddedFeatures.isEmpty() &&
-           !mEditBuffer->mDeletedAttributeIds.contains( index ) &&
-           mEditBuffer->mChangedAttributeValues.isEmpty() )
+      if ( mDataProvider->transaction() || (
+             mEditBuffer->deletedFeatureIds().isEmpty() &&
+             mEditBuffer->addedFeatures().isEmpty() &&
+             !mEditBuffer->deletedAttributeIds().contains( index ) &&
+             mEditBuffer->changedAttributeValues().isEmpty() ) )
       {
         uniqueValues = mDataProvider->uniqueValues( index, limit );
         return uniqueValues;
@@ -4150,7 +4152,7 @@ QStringList QgsVectorLayer::uniqueStringsMatching( int index, const QString &sub
     {
       results = mDataProvider->uniqueStringsMatching( index, substring, limit, feedback );
 
-      if ( mEditBuffer )
+      if ( mEditBuffer && ! mDataProvider->transaction() )
       {
         QgsFeatureMap added = mEditBuffer->addedFeatures();
         QMapIterator< QgsFeatureId, QgsFeature > addedIt( added );
@@ -4189,10 +4191,10 @@ QStringList QgsVectorLayer::uniqueStringsMatching( int index, const QString &sub
 
     case QgsFields::OriginEdit:
       // the layer is editable, but in certain cases it can still be avoided going through all features
-      if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
-           mEditBuffer->mAddedFeatures.isEmpty() &&
-           !mEditBuffer->mDeletedAttributeIds.contains( index ) &&
-           mEditBuffer->mChangedAttributeValues.isEmpty() )
+      if ( mDataProvider->transaction() || ( mEditBuffer->deletedFeatureIds().isEmpty() &&
+                                             mEditBuffer->addedFeatures().isEmpty() &&
+                                             !mEditBuffer->deletedAttributeIds().contains( index ) &&
+                                             mEditBuffer->changedAttributeValues().isEmpty() ) )
       {
         return mDataProvider->uniqueStringsMatching( index, substring, limit, feedback );
       }
@@ -4260,7 +4262,7 @@ QVariant QgsVectorLayer::minimumOrMaximumValue( int index, bool minimum ) const
     case QgsFields::OriginProvider: //a provider field
     {
       QVariant val = minimum ? mDataProvider->minimumValue( index ) : mDataProvider->maximumValue( index );
-      if ( mEditBuffer )
+      if ( mEditBuffer && ! mDataProvider->transaction() )
       {
         QgsFeatureMap added = mEditBuffer->addedFeatures();
         QMapIterator< QgsFeatureId, QgsFeature > addedIt( added );
@@ -4293,10 +4295,10 @@ QVariant QgsVectorLayer::minimumOrMaximumValue( int index, bool minimum ) const
     case QgsFields::OriginEdit:
     {
       // the layer is editable, but in certain cases it can still be avoided going through all features
-      if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
-           mEditBuffer->mAddedFeatures.isEmpty() &&
-           !mEditBuffer->mDeletedAttributeIds.contains( index ) &&
-           mEditBuffer->mChangedAttributeValues.isEmpty() )
+      if ( mDataProvider->transaction() || ( mEditBuffer->deletedFeatureIds().isEmpty() &&
+                                             mEditBuffer->addedFeatures().isEmpty() &&
+                                             !mEditBuffer->deletedAttributeIds().contains( index ) &&
+                                             mEditBuffer->changedAttributeValues().isEmpty() ) )
       {
         return minimum ? mDataProvider->minimumValue( index ) : mDataProvider->maximumValue( index );
       }

diff --git a/src/core/vector/qgsvectorlayereditbuffer.h b/src/core/vector/qgsvectorlayereditbuffer.h
@@ -184,6 +184,13 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
      */
     bool isFeatureDeleted( QgsFeatureId id ) const { return mDeletedFeatureIds.contains( id ); }
 
+    /**
+     * Updates \a fields
+     * \note Not available in Python bindings
+     * \since QGIS 3.18
+     */
+    void updateFields( QgsFields &fields ) SIP_SKIP;
+
     //QString dumpEditBuffer();
 
   protected slots:
@@ -236,8 +243,6 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
     //! Constructor for QgsVectorLayerEditBuffer
     QgsVectorLayerEditBuffer() = default;
 
-    void updateFields( QgsFields &fields );
-
     //! Update feature with uncommitted geometry updates
     void updateFeatureGeometry( QgsFeature &f );
 
@@ -257,7 +262,6 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
 
   protected:
     QgsVectorLayer *L = nullptr;
-    friend class QgsVectorLayer;
 
     friend class QgsVectorLayerUndoCommand;
     friend class QgsVectorLayerUndoCommandAddFeature;

diff --git a/src/core/vector/qgsvectorlayereditpassthrough.cpp b/src/core/vector/qgsvectorlayereditpassthrough.cpp
@@ -32,7 +32,7 @@ bool QgsVectorLayerEditPassthrough::isModified() const
 
 bool QgsVectorLayerEditPassthrough::modify( QgsVectorLayerUndoPassthroughCommand *cmd )
 {
-  L->undoStack()->push( cmd ); // push takes owneship -> no need for cmd to be a smart ptr
+  L->undoStack()->push( cmd ); // push takes ownership -> no need for cmd to be a smart ptr
   if ( cmd->hasError() )
     return false;
 

diff --git a/src/core/vector/qgsvectorlayerfeatureiterator.cpp b/src/core/vector/qgsvectorlayerfeatureiterator.cpp
@@ -74,12 +74,16 @@ QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( const QgsVectorLayer *
     else
     {
 #endif
-      mAddedFeatures = QgsFeatureMap( layer->editBuffer()->addedFeatures() );
-      mChangedGeometries = QgsGeometryMap( layer->editBuffer()->changedGeometries() );
-      mDeletedFeatureIds = QgsFeatureIds( layer->editBuffer()->deletedFeatureIds() );
-      mChangedAttributeValues = QgsChangedAttributesMap( layer->editBuffer()->changedAttributeValues() );
-      mAddedAttributes = QList<QgsField>( layer->editBuffer()->addedAttributes() );
-      mDeletedAttributeIds = QgsAttributeList( layer->editBuffer()->deletedAttributeIds() );
+      // If we are inside a transaction the iterator "sees" the current status
+      if ( layer->dataProvider() && ! layer->dataProvider()->transaction() )
+      {
+        mAddedFeatures = QgsFeatureMap( layer->editBuffer()->addedFeatures() );
+        mChangedGeometries = QgsGeometryMap( layer->editBuffer()->changedGeometries() );
+        mDeletedFeatureIds = QgsFeatureIds( layer->editBuffer()->deletedFeatureIds() );
+        mChangedAttributeValues = QgsChangedAttributesMap( layer->editBuffer()->changedAttributeValues() );
+        mAddedAttributes = QList<QgsField>( layer->editBuffer()->addedAttributes() );
+        mDeletedAttributeIds = QgsAttributeList( layer->editBuffer()->deletedAttributeIds() );
+      }
 #if 0
     }
 #endif

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;