diff --git a/python/core/qgstransaction.sip b/python/core/qgstransaction.sip index a0d4ade640e3..5ba6dc25f299 100644 --- a/python/core/qgstransaction.sip +++ b/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: diff --git a/python/core/qgsvectorlayereditpassthrough.sip b/python/core/qgsvectorlayereditpassthrough.sip index aea3472a154d..496d30d18438 100644 --- a/python/core/qgsvectorlayereditpassthrough.sip +++ b/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 + }; /************************************************************************ diff --git a/python/core/qgsvectorlayerundopassthroughcommand.sip b/python/core/qgsvectorlayerundopassthroughcommand.sip index 59855ad4849f..33ce8cb5fc00 100644 --- a/python/core/qgsvectorlayerundopassthroughcommand.sip +++ b/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 * * * diff --git a/src/core/qgstransaction.cpp b/src/core/qgstransaction.cpp index 5f906991fd0e..27964cf2020b 100644 --- a/src/core/qgstransaction.cpp +++ b/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() ); diff --git a/src/core/qgstransaction.h b/src/core/qgstransaction.h index ca4bb54902de..4c7d8a4ce3c5 100644 --- a/src/core/qgstransaction.h +++ b/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; diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp index 397acd19b7e0..980d748059eb 100644 --- a/src/core/qgsvectorlayer.cpp +++ b/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( mEditBuffer )->update( tr, sql ); + } +} diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h index f1ca934c9477..45641d382c4a 100644 --- a/src/core/qgsvectorlayer.h +++ b/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 diff --git a/src/core/qgsvectorlayereditbuffer.h b/src/core/qgsvectorlayereditbuffer.h index 23221e201286..cb8a48c72211 100644 --- a/src/core/qgsvectorlayereditbuffer.h +++ b/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 diff --git a/src/core/qgsvectorlayereditpassthrough.cpp b/src/core/qgsvectorlayereditpassthrough.cpp index ac5bdacd9dee..8a9dffadb696 100644 --- a/src/core/qgsvectorlayereditpassthrough.cpp +++ b/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 ) ); +} diff --git a/src/core/qgsvectorlayereditpassthrough.h b/src/core/qgsvectorlayereditpassthrough.h index ef097c56d79a..3d3032d246a6 100644 --- a/src/core/qgsvectorlayereditpassthrough.h +++ b/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; diff --git a/src/core/qgsvectorlayerundopassthroughcommand.cpp b/src/core/qgsvectorlayerundopassthroughcommand.cpp index 34b8fc7dc3e1..7bcffe85427b 100644 --- a/src/core/qgsvectorlayerundopassthroughcommand.cpp +++ b/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(); + } + } +} diff --git a/src/core/qgsvectorlayerundopassthroughcommand.h b/src/core/qgsvectorlayerundopassthroughcommand.h index 0a4c03f63ad5..fc06c65895e3 100644 --- a/src/core/qgsvectorlayerundopassthroughcommand.h +++ b/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 diff --git a/src/providers/postgres/qgspostgrestransaction.cpp b/src/providers/postgres/qgspostgrestransaction.cpp index e20198ef9927..23b4da6485e3 100644 --- a/src/providers/postgres/qgspostgrestransaction.cpp +++ b/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; } diff --git a/src/providers/postgres/qgspostgrestransaction.h b/src/providers/postgres/qgspostgrestransaction.h index c107ae889d88..dc638c001c87 100644 --- a/src/providers/postgres/qgspostgrestransaction.h +++ b/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; } diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py index 4711a73c41e4..892e8fa28cfb 100644 --- a/tests/src/python/test_provider_postgres.py +++ b/tests/src/python/test_provider_postgres.py @@ -33,7 +33,8 @@ 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 @@ def testNestedInsert(self): 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"""