[Backport release-3_18] Bugfix gh41477 editbuffer passthrough

python/core/auto_generated/vector/qgsvectorlayereditbuffer.sip.in

@@ -207,6 +207,19 @@ Returns ``True`` if the specified feature ID has been deleted but not committed.
 .. versionadded:: 3.0
 %End
 
+    void updateFields( QgsFields &fields );
+%Docstring
+Updates ``fields``
+%End
+
+    virtual bool isPassthrough();
+%Docstring
+Returns ``True`` if the edit buffer makes direct changes to the data provider, such in case of transactions.
+Subclasses may override this method to indicate that they support this behavior, the default implementation returns ``False``.
+
+.. versionadded:: 3.18
+%End
+
 
   protected slots:
     void undoIndexChanged( int index );
@@ -269,8 +282,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

python/core/auto_generated/vector/qgsvectorlayereditpassthrough.sip.in

@@ -40,6 +40,8 @@ class QgsVectorLayerEditPassthrough : QgsVectorLayerEditBuffer
 
     virtual bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );
 
+    virtual bool isPassthrough();
+
 
     virtual bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues );
 

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 );
 
 
 };

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 );
+
+
 };
 
 

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 );
 }
 

src/core/vector/qgsvectorlayer.cpp

@@ -877,7 +877,7 @@ QgsRectangle QgsVectorLayer::extent() const
   }
 
   if ( !mEditBuffer ||
-       ( mEditBuffer->mDeletedFeatureIds.isEmpty() && mEditBuffer->mChangedGeometries.isEmpty() ) ||
+       ( !mEditBuffer->isPassthrough() && ( 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 && !mEditBuffer->isPassthrough() )
     {
-      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 && ! mEditBuffer->isPassthrough() ? 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 && ! mEditBuffer->isPassthrough() ? mEditBuffer->deletedFeatureIds() : QgsFeatureIds() );
+  const QgsFeatureMap addedFeatures( mEditBuffer && ! mEditBuffer->isPassthrough() ? 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 = !mEditBuffer->isPassthrough() && ( !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 && ! mEditBuffer->isPassthrough() )
       {
         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 && ! mEditBuffer->isPassthrough() )
       {
         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 && ! mEditBuffer->isPassthrough() )
       {
         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 );
       }

src/core/vector/qgsvectorlayereditbuffer.cpp

@@ -256,6 +256,10 @@ bool QgsVectorLayerEditBuffer::changeAttributeValue( QgsFeatureId fid, int field
   return true;
 }
 
+bool QgsVectorLayerEditBuffer::isPassthrough()
+{
+  return false;
+}
 
 bool QgsVectorLayerEditBuffer::addAttribute( const QgsField &field )
 {

src/core/vector/qgsvectorlayereditbuffer.h

@@ -184,6 +184,18 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
      */
     bool isFeatureDeleted( QgsFeatureId id ) const { return mDeletedFeatureIds.contains( id ); }
 
+    /**
+     * Updates \a fields
+     */
+    void updateFields( QgsFields &fields );
+
+    /**
+     * Returns TRUE if the edit buffer makes direct changes to the data provider, such in case of transactions.
+     * Subclasses may override this method to indicate that they support this behavior, the default implementation returns FALSE.
+     * \since QGIS 3.18
+     */
+    virtual bool isPassthrough();
+
     //QString dumpEditBuffer();
 
   protected slots:
@@ -236,8 +248,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 +267,6 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
 
   protected:
     QgsVectorLayer *L = nullptr;
-    friend class QgsVectorLayer;
 
     friend class QgsVectorLayerUndoCommand;
     friend class QgsVectorLayerUndoCommandAddFeature;

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;
 
@@ -66,6 +66,11 @@ bool QgsVectorLayerEditPassthrough::addFeatures( QgsFeatureList &features )
   return true;
 }
 
+bool QgsVectorLayerEditPassthrough::isPassthrough()
+{
+  return true;
+}
+
 bool QgsVectorLayerEditPassthrough::deleteFeature( QgsFeatureId fid )
 {
   return modify( new QgsVectorLayerUndoPassthroughCommandDeleteFeatures( this, QgsFeatureIds() << fid ) );

src/core/vector/qgsvectorlayereditpassthrough.h

@@ -54,6 +54,7 @@ class CORE_EXPORT QgsVectorLayerEditPassthrough : public QgsVectorLayerEditBuffe
     bool deleteFeatures( const QgsFeatureIds &fids ) override;
     bool changeGeometry( QgsFeatureId fid, const QgsGeometry &geom ) override;
     bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() ) override;
+    bool isPassthrough() override;
 
     /**
      * Changes values of attributes (but does not commit it).
@@ -87,6 +88,7 @@ class CORE_EXPORT QgsVectorLayerEditPassthrough : public QgsVectorLayerEditBuffe
     // utility function to avoid cpy/paste
     bool modify( QgsVectorLayerUndoPassthroughCommand *cmd );
 
+
 };
 
 #endif // QGSVECTORLAYEREDITPASSTHROUGH_H

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

src/core/vector/qgsvectorlayerundocommand.cpp

@@ -122,10 +122,7 @@ QgsVectorLayerUndoCommandChangeGeometry::QgsVectorLayerUndoCommandChangeGeometry
   }
 }
 
-int QgsVectorLayerUndoCommandChangeGeometry::id() const
-{
-  return 1;
-}
+
 
 bool QgsVectorLayerUndoCommandChangeGeometry::mergeWith( const QUndoCommand *other )
 {

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;