Merge pull request #5376 from pblottiere/dirty

[FEATURE] Add a flag to dirty edit buffer when using executeSql in transactions

python/core/qgstransaction.sip

@@ -97,10 +97,16 @@ class QgsTransaction : QObject /Abstract/
  :rtype: bool
 %End
 
-    virtual bool executeSql( const QString &sql, QString &error /Out/ ) = 0;
+    virtual bool executeSql( const QString &sql, QString &error /Out/, bool isDirty = false ) = 0;
 %Docstring
  Execute the ``sql`` string. The result must not be a tuple, so running a
  ``SELECT`` query will return an error.
+
+ \param sql The sql query to execute
+ \param error The error message
+ \param isDirty Flag to indicate if the underlying data will be modified
+
+ :return: true if everything is OK, false otherwise
  :rtype: bool
 %End
 
@@ -161,6 +167,11 @@ class QgsTransaction : QObject /Abstract/
  Emitted after a rollback
 %End
 
+    void dirtied( const QString &sql );
+%Docstring
+ Emitted if a sql query is executed and the underlying data is modified
+%End
+
   protected:
 
 

python/core/qgsvectorlayereditpassthrough.sip

@@ -41,6 +41,19 @@ class QgsVectorLayerEditPassthrough : QgsVectorLayerEditBuffer
     virtual void rollBack();
 
 
+    bool update( QgsTransaction *transaction, const QString &sql );
+%Docstring
+ Update underlying data with a SQL query embedded in a transaction.
+
+ \param transaction Transaction in which the sql query has been run
+ \param sql The SQL query updating data
+
+ :return: true if the undo/redo command is well added to the stack, false otherwise
+
+.. versionadded:: 3.0
+ :rtype: bool
+%End
+
 };
 
 /************************************************************************

python/core/qgsvectorlayerundopassthroughcommand.sip

@@ -23,11 +23,12 @@ class QgsVectorLayerUndoPassthroughCommand : QgsVectorLayerUndoCommand
 %End
   public:
 
-    QgsVectorLayerUndoPassthroughCommand( QgsVectorLayerEditBuffer *buffer, const QString &text );
+    QgsVectorLayerUndoPassthroughCommand( QgsVectorLayerEditBuffer *buffer, const QString &text, bool autocreate = true );
 %Docstring
  Constructor for QgsVectorLayerUndoPassthroughCommand
  \param buffer associated edit buffer
  \param text text associated with command
+ \param autocreate flag allowing to automatically create a savepoint if necessary
 %End
 
     bool hasError() const;
@@ -46,10 +47,12 @@ class QgsVectorLayerUndoPassthroughCommand : QgsVectorLayerUndoCommand
  :rtype: bool
 %End
 
-    bool setSavePoint();
+    bool setSavePoint( const QString &savePointId = QString() );
 %Docstring
- Set the command savepoint or set error status
- error satus should be false prior to call
+ Set the command savepoint or set error status.
+ Error satus should be false prior to call. If the savepoint given in
+ parameter is empty, then a new one is created if none is currently
+ available in the transaction.
  :rtype: bool
 %End
 
@@ -58,6 +61,21 @@ class QgsVectorLayerUndoPassthroughCommand : QgsVectorLayerUndoCommand
  Set error flag and append "failed" to text
 %End
 
+    void setErrorMessage( const QString &errorMessage );
+%Docstring
+ Sets the error message.
+
+.. versionadded:: 3.0
+%End
+
+    QString errorMessage() const;
+%Docstring
+ Returns the error message or an empty string if there's none.
+
+.. versionadded:: 3.0
+ :rtype: str
+%End
+
 };
 
 
@@ -246,6 +264,32 @@ class QgsVectorLayerUndoPassthroughCommandRenameAttribute : QgsVectorLayerUndoPa
 
 };
 
+
+class QgsVectorLayerUndoPassthroughCommandUpdate : QgsVectorLayerUndoPassthroughCommand
+{
+%Docstring
+ Undo command for running a specific sql query in transaction group.
+.. versionadded:: 3.0
+%End
+
+%TypeHeaderCode
+#include "qgsvectorlayerundopassthroughcommand.h"
+%End
+  public:
+
+    QgsVectorLayerUndoPassthroughCommandUpdate( QgsVectorLayerEditBuffer *buffer /Transfer/, QgsTransaction *transaction, const QString &sql );
+%Docstring
+ Constructor for QgsVectorLayerUndoCommandUpdate
+ \param buffer associated edit buffer
+ \param transaction transaction running the sql query
+ \param sql the query
+%End
+
+    virtual void undo();
+    virtual void redo();
+
+};
+
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

src/core/qgstransaction.cpp

@@ -201,7 +201,7 @@ QString QgsTransaction::createSavepoint( QString &error SIP_OUT )
   if ( !mTransactionActive )
     return QString();
 
-  if ( !mLastSavePointIsDirty )
+  if ( !mLastSavePointIsDirty && !mSavepoints.isEmpty() )
     return mSavepoints.top();
 
   const QString name( QUuid::createUuid().toString() );

src/core/qgstransaction.h

@@ -111,8 +111,14 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT
     /**
      * Execute the \a sql string. The result must not be a tuple, so running a
      * ``SELECT`` query will return an error.
+     *
+     * \param sql The sql query to execute
+     * \param error The error message
+     * \param isDirty Flag to indicate if the underlying data will be modified
+     *
+     * \returns true if everything is OK, false otherwise
      */
-    virtual bool executeSql( const QString &sql, QString &error SIP_OUT ) = 0;
+    virtual bool executeSql( const QString &sql, QString &error SIP_OUT, bool isDirty = false ) = 0;
 
     /**
      * Checks if a the provider of a given \a layer supports transactions.
@@ -165,6 +171,11 @@ class CORE_EXPORT QgsTransaction : public QObject SIP_ABSTRACT
      */
     void afterRollback();
 
+    /**
+     * Emitted if a sql query is executed and the underlying data is modified
+     */
+    void dirtied( const QString &sql );
+
   protected:
     QgsTransaction( const QString &connString ) SIP_SKIP;
 

src/core/qgsvectorlayer.cpp

@@ -1345,6 +1345,8 @@ bool QgsVectorLayer::startEditing()
   if ( mDataProvider->transaction() )
   {
     mEditBuffer = new QgsVectorLayerEditPassthrough( this );
+
+    connect( mDataProvider->transaction(), &QgsTransaction::dirtied, this, &QgsVectorLayer::onDirtyTransaction, Qt::UniqueConnection );
   }
   else
   {
@@ -4575,3 +4577,11 @@ bool QgsVectorLayer::readExtentFromXml() const
   return mReadExtentFromXml;
 }
 
+void QgsVectorLayer::onDirtyTransaction( const QString &sql )
+{
+  QgsTransaction *tr = dataProvider()->transaction();
+  if ( tr && mEditBuffer )
+  {
+    qobject_cast<QgsVectorLayerEditPassthrough *>( mEditBuffer )->update( tr, sql );
+  }
+}

src/core/qgsvectorlayer.h

@@ -2066,6 +2066,7 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte
     void onFeatureDeleted( QgsFeatureId fid );
     void onRelationsLoaded();
     void onSymbolsCounted();
+    void onDirtyTransaction( const QString &sql );
 
   protected:
     //! Set the extent

src/core/qgsvectorlayereditbuffer.h

@@ -267,6 +267,7 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
     friend class QgsVectorLayerUndoPassthroughCommandAddAttribute;
     friend class QgsVectorLayerUndoPassthroughCommandDeleteAttribute;
     friend class QgsVectorLayerUndoPassthroughCommandRenameAttribute;
+    friend class QgsVectorLayerUndoPassthroughCommandUpdate;
 
     /**
      * Deleted feature IDs which are not committed.  Note a feature can be added and then deleted

src/core/qgsvectorlayereditpassthrough.cpp

@@ -17,6 +17,7 @@
 #include "qgsvectorlayer.h"
 #include "qgsvectordataprovider.h"
 #include "qgsvectorlayerundopassthroughcommand.h"
+#include "qgstransaction.h"
 
 QgsVectorLayerEditPassthrough::QgsVectorLayerEditPassthrough( QgsVectorLayer *layer )
   : mModified( false )
@@ -35,7 +36,12 @@ bool QgsVectorLayerEditPassthrough::modify( QgsVectorLayerUndoPassthroughCommand
   if ( cmd->hasError() )
     return false;
 
-  mModified = true;
+  if ( !mModified )
+  {
+    mModified = true;
+    emit layerModified();
+  }
+
   return true;
 }
 
@@ -105,3 +111,8 @@ void QgsVectorLayerEditPassthrough::rollBack()
 {
   mModified = false;
 }
+
+bool QgsVectorLayerEditPassthrough::update( QgsTransaction *tr, const QString &sql )
+{
+  return modify( new QgsVectorLayerUndoPassthroughCommandUpdate( this, tr, sql ) );
+}

src/core/qgsvectorlayereditpassthrough.h

@@ -20,6 +20,7 @@
 
 class QgsVectorLayer;
 class QgsVectorLayerUndoPassthroughCommand;
+class QgsTransaction;
 
 /**
  * \ingroup core
@@ -43,6 +44,18 @@ class CORE_EXPORT QgsVectorLayerEditPassthrough : public QgsVectorLayerEditBuffe
     bool commitChanges( QStringList &commitErrors ) override;
     void rollBack() override;
 
+    /**
+     * Update underlying data with a SQL query embedded in a transaction.
+     *
+     * \param transaction Transaction in which the sql query has been run
+     * \param sql The SQL query updating data
+     *
+     * \returns true if the undo/redo command is well added to the stack, false otherwise
+     *
+     * \since QGIS 3.0
+     */
+    bool update( QgsTransaction *transaction, const QString &sql );
+
   private:
     bool mModified;
 

src/core/qgsvectorlayerundopassthroughcommand.cpp

@@ -29,9 +29,9 @@
 //@todo use setObsolete instead of mHasError when upgrading qt version, this will allow auto removal of the command
 // for the moment a errored command is left on the stack
 
-QgsVectorLayerUndoPassthroughCommand::QgsVectorLayerUndoPassthroughCommand( QgsVectorLayerEditBuffer *buffer, const QString &text )
+QgsVectorLayerUndoPassthroughCommand::QgsVectorLayerUndoPassthroughCommand( QgsVectorLayerEditBuffer *buffer, const QString &text, bool autocreate )
   : QgsVectorLayerUndoCommand( buffer )
-  , mSavePointId( mBuffer->L->isEditCommandActive()
+  , mSavePointId( mBuffer->L->isEditCommandActive() || !autocreate
                   ? mBuffer->L->dataProvider()->transaction()->savePoints().last()
                   : mBuffer->L->dataProvider()->transaction()->createSavepoint( mError ) )
   , mHasError( !mError.isEmpty() )
@@ -54,19 +54,36 @@ void QgsVectorLayerUndoPassthroughCommand::setError()
   }
 }
 
-bool QgsVectorLayerUndoPassthroughCommand::setSavePoint()
+void QgsVectorLayerUndoPassthroughCommand::setErrorMessage( const QString &errorMessage )
+{
+  mError = errorMessage;
+}
+
+QString QgsVectorLayerUndoPassthroughCommand::errorMessage() const
+{
+  return mError;
+}
+
+bool QgsVectorLayerUndoPassthroughCommand::setSavePoint( const QString &savePointId )
 {
   if ( !hasError() )
   {
-    // re-create savepoint only if mRecreateSavePoint and rollBackToSavePoint as occurred
-    if ( mRecreateSavePoint && mBuffer->L->dataProvider()->transaction()->savePoints().indexOf( mSavePointId ) == -1 )
+    if ( savePointId.isEmpty() )
     {
-      mSavePointId = mBuffer->L->dataProvider()->transaction()->createSavepoint( mSavePointId, mError );
-      if ( mSavePointId.isEmpty() )
+      // re-create savepoint only if mRecreateSavePoint and rollBackToSavePoint as occurred
+      if ( mRecreateSavePoint && mBuffer->L->dataProvider()->transaction()->savePoints().indexOf( mSavePointId ) == -1 )
       {
-        setError();
+        mSavePointId = mBuffer->L->dataProvider()->transaction()->createSavepoint( mSavePointId, mError );
+        if ( mSavePointId.isEmpty() )
+        {
+          setError();
+        }
       }
     }
+    else
+    {
+      mSavePointId = savePointId;
+    }
   }
   return !hasError();
 }
@@ -333,3 +350,55 @@ void QgsVectorLayerUndoPassthroughCommandRenameAttribute::redo()
     setError();
   }
 }
+
+QgsVectorLayerUndoPassthroughCommandUpdate::QgsVectorLayerUndoPassthroughCommandUpdate( QgsVectorLayerEditBuffer *buffer, QgsTransaction *transaction, const QString &sql )
+  : QgsVectorLayerUndoPassthroughCommand( buffer, QObject::tr( "custom transaction" ), false )
+  , mTransaction( transaction )
+  , mSql( sql )
+{
+}
+
+void QgsVectorLayerUndoPassthroughCommandUpdate::undo()
+{
+  if ( rollBackToSavePoint() )
+  {
+    mUndone = true;
+    emit mBuffer->L->layerModified();
+  }
+  else
+  {
+    setError();
+  }
+}
+
+void QgsVectorLayerUndoPassthroughCommandUpdate::redo()
+{
+  // the first time that the sql query is execute is within QgsTransaction
+  // itself. So the redo has to be executed only after an undo action.
+  if ( mUndone )
+  {
+    QString errorMessage;
+
+    QString savePointId = mTransaction->createSavepoint( errorMessage );
+
+    if ( errorMessage.isEmpty() )
+    {
+      setSavePoint( savePointId );
+
+      if ( mTransaction->executeSql( mSql, errorMessage ) )
+      {
+        mUndone = false;
+      }
+      else
+      {
+        setErrorMessage( errorMessage );
+        setError();
+      }
+    }
+    else
+    {
+      setErrorMessage( errorMessage );
+      setError();
+    }
+  }
+}

src/core/qgsvectorlayerundopassthroughcommand.h

@@ -36,20 +36,15 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommand : public QgsVectorLayerUn
      * Constructor for QgsVectorLayerUndoPassthroughCommand
      * \param buffer associated edit buffer
      * \param text text associated with command
+     * \param autocreate flag allowing to automatically create a savepoint if necessary
      */
-    QgsVectorLayerUndoPassthroughCommand( QgsVectorLayerEditBuffer *buffer, const QString &text );
+    QgsVectorLayerUndoPassthroughCommand( QgsVectorLayerEditBuffer *buffer, const QString &text, bool autocreate = true );
 
     /**
      * Returns error status
      */
     bool hasError() const { return mHasError; }
 
-  private:
-    QString mError;
-    QString mSavePointId;
-    bool mHasError;
-    bool mRecreateSavePoint;
-
   protected:
 
     /**
@@ -60,16 +55,37 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommand : public QgsVectorLayerUn
     bool rollBackToSavePoint();
 
     /**
-     * Set the command savepoint or set error status
-     * error satus should be false prior to call
+     * Set the command savepoint or set error status.
+     * Error satus should be false prior to call. If the savepoint given in
+     * parameter is empty, then a new one is created if none is currently
+     * available in the transaction.
      */
-    bool setSavePoint();
+    bool setSavePoint( const QString &savePointId = QString() );
 
     /**
      * Set error flag and append "failed" to text
      */
     void setError();
 
+    /**
+     * Sets the error message.
+     *
+     * \since QGIS 3.0
+     */
+    void setErrorMessage( const QString &errorMessage );
+
+    /**
+     * Returns the error message or an empty string if there's none.
+     *
+     * \since QGIS 3.0
+     */
+    QString errorMessage() const;
+
+  private:
+    QString mSavePointId;
+    QString mError;
+    bool mHasError;
+    bool mRecreateSavePoint;
 };
 
 /**
@@ -265,4 +281,32 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommandRenameAttribute : public Q
     const QString mOldName;
 };
 
+/**
+ * \ingroup core
+ * \class QgsVectorLayerUndoPassthroughCommandUpdate
+ * \brief Undo command for running a specific sql query in transaction group.
+ * \since QGIS 3.0
+ */
+
+class CORE_EXPORT QgsVectorLayerUndoPassthroughCommandUpdate : public QgsVectorLayerUndoPassthroughCommand
+{
+  public:
+
+    /**
+     * Constructor for QgsVectorLayerUndoCommandUpdate
+     * \param buffer associated edit buffer
+     * \param transaction transaction running the sql query
+     * \param sql the query
+     */
+    QgsVectorLayerUndoPassthroughCommandUpdate( QgsVectorLayerEditBuffer *buffer SIP_TRANSFER, QgsTransaction *transaction, const QString &sql );
+
+    virtual void undo() override;
+    virtual void redo() override;
+
+  private:
+    QgsTransaction *mTransaction = nullptr;
+    QString mSql;
+    bool mUndone = false;
+};
+
 #endif

src/providers/postgres/qgspostgrestransaction.cpp

@@ -57,13 +57,19 @@ bool QgsPostgresTransaction::rollbackTransaction( QString &error )
   return false;
 }
 
-bool QgsPostgresTransaction::executeSql( const QString &sql, QString &errorMsg )
+bool QgsPostgresTransaction::executeSql( const QString &sql, QString &errorMsg, bool isDirty )
 {
   if ( !mConn )
   {
     return false;
   }
 
+  QString err;
+  if ( isDirty )
+  {
+    createSavepoint( err );
+  }
+
   QgsDebugMsg( QString( "Transaction sql: %1" ).arg( sql ) );
   mConn->lock();
   QgsPostgresResult r( mConn->PQexec( sql, true ) );
@@ -72,8 +78,21 @@ bool QgsPostgresTransaction::executeSql( const QString &sql, QString &errorMsg )
   {
     errorMsg = QStringLiteral( "Status %1 (%2)" ).arg( r.PQresultStatus() ).arg( r.PQresultErrorMessage() );
     QgsDebugMsg( errorMsg );
+
+    if ( isDirty )
+    {
+      rollbackToSavepoint( savePoints().last(), err );
+    }
+
     return false;
   }
+
+  if ( isDirty )
+  {
+    dirtyLastSavePoint();
+    emit dirtied( sql );
+  }
+
   QgsDebugMsg( QString( "Status %1 (OK)" ).arg( r.PQresultStatus() ) );
   return true;
 }

src/providers/postgres/qgspostgrestransaction.h

@@ -29,7 +29,7 @@ class QgsPostgresTransaction : public QgsTransaction
 
   public:
     explicit QgsPostgresTransaction( const QString &connString );
-    bool executeSql( const QString &sql, QString &error ) override;
+    bool executeSql( const QString &sql, QString &error, bool isDirty = false ) override;
     QgsPostgresConn *connection() const { return mConn; }
 
 

tests/src/python/test_provider_postgres.py

@@ -33,7 +33,8 @@ from qgis.core import (
     QgsReadWriteContext,
     QgsRectangle,
     QgsDefaultValue,
-    QgsDataSourceUri
+    QgsDataSourceUri,
+    QgsProject
 )
 from qgis.gui import QgsGui
 from qgis.PyQt.QtCore import QDate, QTime, QDateTime, QVariant, QDir, QObject
@@ -312,6 +313,54 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase):
         self.vl.addFeature(f)  # Should not deadlock during an active iteration
         f = next(it)
 
+    def testTransactionNotDirty(self):
+        # create a vector ayer based on postgres
+        vl = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=POLYGON table="qgis_test"."some_poly_data" (geom) sql=', 'test', 'postgres')
+        self.assertTrue(vl.isValid())
+
+        # prepare a project with transactions enabled
+        p = QgsProject()
+        p.setAutoTransaction(True)
+        p.addMapLayers([vl])
+        vl.startEditing()
+
+        # check that the feature used for testing is ok
+        ft0 = vl.getFeatures('pk=1')
+        f = QgsFeature()
+        self.assertTrue(ft0.nextFeature(f))
+
+        # update the data within the transaction
+        tr = vl.dataProvider().transaction()
+        sql = "update qgis_test.some_poly_data set pk=33 where pk=1"
+        self.assertTrue(tr.executeSql(sql, True)[0])
+
+        # check that the pk of the feature has been changed
+        ft = vl.getFeatures('pk=1')
+        self.assertFalse(ft.nextFeature(f))
+
+        ft = vl.getFeatures('pk=33')
+        self.assertTrue(ft.nextFeature(f))
+
+        # underlying data has been modified but the layer is not tagged as
+        # modified
+        self.assertTrue(vl.isModified())
+
+        # undo sql query
+        vl.undoStack().undo()
+
+        # check that the original feature with pk is back
+        ft0 = vl.getFeatures('pk=1')
+        self.assertTrue(ft0.nextFeature(f))
+
+        # redo
+        vl.undoStack().redo()
+
+        # check that the pk of the feature has been changed
+        ft1 = vl.getFeatures('pk=1')
+        self.assertFalse(ft1.nextFeature(f))
+
+        p.setAutoTransaction(False)
+
     def testDomainTypes(self):
         """Test that domain types are correctly mapped"""