From 8cb15c3260080b29888912d83dc37ae7b9422697 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Fri, 12 Feb 2021 19:00:57 +0100 Subject: [PATCH] Editing buffer passthrough --- .../vector/qgsvectorlayereditbuffer.sip.in | 3 +- src/core/qgstransaction.cpp | 3 + src/core/vector/qgsvectorlayer.cpp | 50 +++---- src/core/vector/qgsvectorlayereditbuffer.h | 10 +- .../vector/qgsvectorlayereditpassthrough.cpp | 7 +- .../vector/qgsvectorlayerfeatureiterator.cpp | 16 ++- .../qgsvectorlayerundopassthroughcommand.cpp | 135 +++++++++++++++--- .../qgsvectorlayerundopassthroughcommand.h | 5 +- .../python/test_qgsvectorlayereditbuffer.py | 59 +++++++- 9 files changed, 231 insertions(+), 57 deletions(-) diff --git a/python/core/auto_generated/vector/qgsvectorlayereditbuffer.sip.in b/python/core/auto_generated/vector/qgsvectorlayereditbuffer.sip.in index b1f5bb9458a..0c63f859b52 100644 --- 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/src/core/qgstransaction.cpp b/src/core/qgstransaction.cpp index 797acb84ec6..e8d69d77cff 100644 --- a/src/core/qgstransaction.cpp +++ b/src/core/qgstransaction.cpp @@ -218,7 +218,10 @@ QString QgsTransaction::createSavepoint( QString &error SIP_OUT ) return QString(); if ( !mLastSavePointIsDirty && !mSavepoints.isEmpty() ) + { + qDebug() << "Returning TOP >>>>> " << mSavepoints.top(); return mSavepoints.top(); + } const QString name( QStringLiteral( "qgis" ) + ( QUuid::createUuid().toString().mid( 1, 24 ).replace( '-', QString() ) ) ); diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp index 66052eb0486..a8267139d08 100644 --- a/src/core/vector/qgsvectorlayer.cpp +++ b/src/core/vector/qgsvectorlayer.cpp @@ -874,7 +874,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(); @@ -887,9 +887,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() ) { @@ -3363,13 +3364,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() ) { @@ -3453,9 +3454,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(); @@ -4040,7 +4041,7 @@ QSet QgsVectorLayer::uniqueValues( int index, int limit ) const { uniqueValues = mDataProvider->uniqueValues( index, limit ); - if ( mEditBuffer ) + if ( mEditBuffer && ! mDataProvider->transaction() ) { QSet vals; const auto constUniqueValues = uniqueValues; @@ -4088,10 +4089,11 @@ QSet 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; @@ -4147,7 +4149,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 ); @@ -4186,10 +4188,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 ); } @@ -4257,7 +4259,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 ); @@ -4290,10 +4292,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 index 69f3e9a2c6d..a094fb9f3bf 100644 --- 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 index 461f7bdb1cb..0a66150488c 100644 --- 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; @@ -109,6 +109,11 @@ bool QgsVectorLayerEditPassthrough::renameAttribute( int attr, const QString &ne bool QgsVectorLayerEditPassthrough::commitChanges( QStringList & /*commitErrors*/ ) { mModified = false; + for ( int i = 0; i < L->undoStack()->count(); ++i ) + { + const QgsVectorLayerUndoPassthroughCommand *cmdPrt = static_cast( L->undoStack()->command( i ) ); + qDebug() << cmdPrt->text(); + } return true; } diff --git a/src/core/vector/qgsvectorlayerfeatureiterator.cpp b/src/core/vector/qgsvectorlayerfeatureiterator.cpp index c29ec744c2c..b627d614002 100644 --- a/src/core/vector/qgsvectorlayerfeatureiterator.cpp +++ b/src/core/vector/qgsvectorlayerfeatureiterator.cpp @@ -71,12 +71,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( 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( layer->editBuffer()->addedAttributes() ); + mDeletedAttributeIds = QgsAttributeList( layer->editBuffer()->deletedAttributeIds() ); + } #if 0 } #endif diff --git a/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp b/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp index bb0254efe9c..2886104c712 100644 --- a/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp +++ b/src/core/vector/qgsvectorlayerundopassthroughcommand.cpp @@ -98,6 +98,10 @@ bool QgsVectorLayerUndoPassthroughCommand::rollBackToSavePoint() { setError(); } + else + { + mBuffer->L->dataProvider()->transaction()->dirtyLastSavePoint(); + } } return !hasError(); } @@ -115,6 +119,7 @@ QgsVectorLayerUndoPassthroughCommandAddFeatures::QgsVectorLayerUndoPassthroughCo mInitialFeatures.last().setId( sAddedIdLowWaterMark ); } mFeatures = mInitialFeatures; + } void QgsVectorLayerUndoPassthroughCommandAddFeatures::undo() @@ -123,6 +128,7 @@ void QgsVectorLayerUndoPassthroughCommandAddFeatures::undo() { for ( const QgsFeature &f : qgis::as_const( mFeatures ) ) { + mBuffer->mAddedFeatures.remove( f.id() ); emit mBuffer->featureDeleted( f.id() ); } mFeatures = mInitialFeatures; @@ -132,10 +138,12 @@ void QgsVectorLayerUndoPassthroughCommandAddFeatures::undo() void QgsVectorLayerUndoPassthroughCommandAddFeatures::redo() { mFeatures = mInitialFeatures; - if ( setSavePoint() && mBuffer->L->dataProvider()->addFeatures( mFeatures ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->addFeatures( mFeatures ) && ! mBuffer->L->dataProvider()->hasErrors() ) { for ( const QgsFeature &f : qgis::as_const( mFeatures ) ) { + mBuffer->mAddedFeatures.insert( f.id(), f ); emit mBuffer->featureAdded( f.id() ); } } @@ -164,7 +172,8 @@ void QgsVectorLayerUndoPassthroughCommandDeleteFeatures::undo() void QgsVectorLayerUndoPassthroughCommandDeleteFeatures::redo() { - if ( setSavePoint() && mBuffer->L->dataProvider()->deleteFeatures( mFids ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->deleteFeatures( mFids ) && ! mBuffer->L->dataProvider()->hasErrors() ) { for ( const QgsFeatureId &id : mFids ) { @@ -187,9 +196,18 @@ QgsVectorLayerUndoPassthroughCommandChangeGeometry::QgsVectorLayerUndoPassthroug void QgsVectorLayerUndoPassthroughCommandChangeGeometry::undo() { + if ( rollBackToSavePoint() ) { - emit mBuffer->geometryChanged( mFid, mOldGeom ); + if ( mBuffer->mAddedFeatures.contains( mFid ) ) + { + mBuffer->mAddedFeatures[ mFid ].setGeometry( mOldGeom ); + } + else + { + // TODO: unless first change? + mBuffer->mChangedGeometries[mFid] = mOldGeom; + } } } @@ -197,8 +215,17 @@ void QgsVectorLayerUndoPassthroughCommandChangeGeometry::redo() { QgsGeometryMap geomMap; geomMap.insert( mFid, mNewGeom ); - if ( setSavePoint() && mBuffer->L->dataProvider()->changeGeometryValues( geomMap ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->changeGeometryValues( geomMap ) && ! mBuffer->L->dataProvider()->hasErrors() ) { + if ( mBuffer->mAddedFeatures.contains( mFid ) ) + { + mBuffer->mAddedFeatures[ mFid ].setGeometry( mNewGeom ); + } + else + { + mBuffer->mChangedGeometries[ mFid ] = mNewGeom; + } emit mBuffer->geometryChanged( mFid, mNewGeom ); } else @@ -210,29 +237,98 @@ void QgsVectorLayerUndoPassthroughCommandChangeGeometry::redo() QgsVectorLayerUndoPassthroughCommandChangeAttribute::QgsVectorLayerUndoPassthroughCommandChangeAttribute( QgsVectorLayerEditBuffer *buffer, QgsFeatureId fid, int field, const QVariant &newValue ) : QgsVectorLayerUndoPassthroughCommand( buffer, QObject::tr( "change attribute value" ) ) , mFid( fid ) - , mField( field ) + , mFieldIndex( field ) , mNewValue( newValue ) , mOldValue( mBuffer->L->getFeature( mFid ).attribute( field ) ) + , mFirstChange( true ) { + + if ( mBuffer->mAddedFeatures.contains( mFid ) ) + { + // work with added feature + QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.constFind( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.constEnd() ); + if ( it.value().attribute( mFieldIndex ).isValid() ) + { + mOldValue = it.value().attribute( mFieldIndex ); + mFirstChange = false; + } + } + else if ( mBuffer->mChangedAttributeValues.contains( mFid ) && mBuffer->mChangedAttributeValues[mFid].contains( mFieldIndex ) ) + { + mOldValue = mBuffer->mChangedAttributeValues[mFid][mFieldIndex]; + mFirstChange = false; + } } void QgsVectorLayerUndoPassthroughCommandChangeAttribute::undo() { if ( rollBackToSavePoint() ) { - emit mBuffer->attributeValueChanged( mFid, mField, mOldValue ); + QVariant original = mOldValue; + + if ( mBuffer->mAddedFeatures.contains( mFid ) ) + { + // added feature + QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + it.value().setAttribute( mFieldIndex, mOldValue ); + } + else if ( mFirstChange ) + { + // existing feature + mBuffer->mChangedAttributeValues[mFid].remove( mFieldIndex ); + if ( mBuffer->mChangedAttributeValues[mFid].isEmpty() ) + mBuffer->mChangedAttributeValues.remove( mFid ); + + if ( !mOldValue.isValid() ) + { + // get old value from provider + QgsFeature tmp; + QgsFeatureRequest request; + request.setFilterFid( mFid ); + request.setFlags( QgsFeatureRequest::NoGeometry ); + request.setSubsetOfAttributes( QgsAttributeList() << mFieldIndex ); + std::unique_ptr layerClone( layer()->clone() ); + QgsFeatureIterator fi = layerClone->getFeatures( request ); + if ( fi.nextFeature( tmp ) ) + original = tmp.attribute( mFieldIndex ); + } + } + else + { + mBuffer->mChangedAttributeValues[mFid][mFieldIndex] = mOldValue; + } + emit mBuffer->attributeValueChanged( mFid, mFieldIndex, original ); } } void QgsVectorLayerUndoPassthroughCommandChangeAttribute::redo() { QgsAttributeMap map; - map.insert( mField, mNewValue ); + map.insert( mFieldIndex, mNewValue ); QgsChangedAttributesMap attribMap; attribMap.insert( mFid, map ); - if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) && ! mBuffer->L->dataProvider()->hasErrors() ) { - emit mBuffer->attributeValueChanged( mFid, mField, mNewValue ); + // Update existing feature + QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid ); + if ( it != mBuffer->mAddedFeatures.end() ) + { + it.value().setAttribute( mFieldIndex, mNewValue ); + } + else + { + // changed attribute of existing feature + if ( !mBuffer->mChangedAttributeValues.contains( mFid ) ) + { + mBuffer->mChangedAttributeValues.insert( mFid, QgsAttributeMap() ); + } + + mBuffer->mChangedAttributeValues[mFid].insert( mFieldIndex, mNewValue ); + } + emit mBuffer->attributeValueChanged( mFid, mFieldIndex, mNewValue ); } else { @@ -251,7 +347,8 @@ 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() ); - if ( mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && rollBackToSavePoint() ) + mBuffer->L->dataProvider()->clearErrors(); + if ( mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() ) { mBuffer->updateLayerFields(); emit mBuffer->attributeDeleted( attr ); @@ -264,7 +361,8 @@ void QgsVectorLayerUndoPassthroughCommandAddAttribute::undo() void QgsVectorLayerUndoPassthroughCommandAddAttribute::redo() { - if ( setSavePoint() && mBuffer->L->dataProvider()->addAttributes( QList() << mField ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->addAttributes( QList() << mField ) && ! mBuffer->L->dataProvider()->hasErrors() ) { mBuffer->updateLayerFields(); const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() ); @@ -286,7 +384,8 @@ void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::undo() { // note that the addAttributes here is only necessary to inform the provider that // an attribute is added back after the rollBackToSavePoint - if ( mBuffer->L->dataProvider()->addAttributes( QList() << mField ) && rollBackToSavePoint() ) + mBuffer->L->dataProvider()->clearErrors(); + if ( mBuffer->L->dataProvider()->addAttributes( QList() << mField ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() ) { mBuffer->updateLayerFields(); const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() ); @@ -301,7 +400,8 @@ void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::undo() void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::redo() { const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() ); - if ( setSavePoint() && mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && ! mBuffer->L->dataProvider()->hasErrors() ) { mBuffer->updateLayerFields(); emit mBuffer->attributeDeleted( attr ); @@ -326,7 +426,8 @@ void QgsVectorLayerUndoPassthroughCommandRenameAttribute::undo() // an attribute is renamed after the rollBackToSavePoint QgsFieldNameMap map; map[ mAttr ] = mOldName; - if ( mBuffer->L->dataProvider()->renameAttributes( map ) && rollBackToSavePoint() ) + mBuffer->L->dataProvider()->clearErrors(); + if ( mBuffer->L->dataProvider()->renameAttributes( map ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() ) { mBuffer->updateLayerFields(); emit mBuffer->attributeRenamed( mAttr, mOldName ); @@ -341,7 +442,8 @@ void QgsVectorLayerUndoPassthroughCommandRenameAttribute::redo() { QgsFieldNameMap map; map[ mAttr ] = mNewName; - if ( setSavePoint() && mBuffer->L->dataProvider()->renameAttributes( map ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->renameAttributes( map ) && ! mBuffer->L->dataProvider()->hasErrors() ) { mBuffer->updateLayerFields(); emit mBuffer->attributeRenamed( mAttr, mNewName ); @@ -427,7 +529,8 @@ void QgsVectorLayerUndoPassthroughCommandChangeAttributes::redo() { QgsChangedAttributesMap attribMap; attribMap.insert( mFid, mNewValues ); - if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) ) + mBuffer->L->dataProvider()->clearErrors(); + if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) && ! mBuffer->L->dataProvider()->hasErrors() ) { for ( auto it = mNewValues.constBegin(); it != mNewValues.constEnd(); ++it ) { diff --git a/src/core/vector/qgsvectorlayerundopassthroughcommand.h b/src/core/vector/qgsvectorlayerundopassthroughcommand.h index e5a845fb41e..e18003d12cc 100644 --- a/src/core/vector/qgsvectorlayerundopassthroughcommand.h +++ b/src/core/vector/qgsvectorlayerundopassthroughcommand.h @@ -200,9 +200,10 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommandChangeAttribute: public Qg private: QgsFeatureId mFid; - const int mField; + const int mFieldIndex; const QVariant mNewValue; - const QVariant mOldValue; + QVariant mOldValue; + bool mFirstChange; }; /** diff --git a/tests/src/python/test_qgsvectorlayereditbuffer.py b/tests/src/python/test_qgsvectorlayereditbuffer.py index c12209c244b..937f5aa78e4 100644 --- a/tests/src/python/test_qgsvectorlayereditbuffer.py +++ b/tests/src/python/test_qgsvectorlayereditbuffer.py @@ -10,15 +10,18 @@ __author__ = 'Nyall Dawson' __date__ = '15/07/2016' __copyright__ = 'Copyright 2016, The QGIS Project' +import os import qgis # NOQA - -from qgis.PyQt.QtCore import QVariant +from qgis.PyQt.QtCore import QVariant, QTemporaryDir from qgis.core import (QgsVectorLayer, QgsFeature, + QgsProject, QgsGeometry, QgsPointXY, - QgsField) + QgsField, + QgsVectorFileWriter, + QgsCoordinateTransformContext) from qgis.testing import start_app, unittest start_app() @@ -391,6 +394,56 @@ class TestQgsVectorLayerEditBuffer(unittest.TestCase): self.assertEqual(layer.editBuffer().addedAttributes()[0].name(), 'new1') self.assertEqual(layer.editBuffer().addedAttributes()[1].name(), 'new2') + def testTransactionGroup(self): + """Test that the buffer works as expected when in transaction""" + + ml = QgsVectorLayer('Point?field=int:integer&crs=epsg:4326', 'test', 'memory') + self.assertTrue(ml.isValid()) + + d = QTemporaryDir() + options = QgsVectorFileWriter.SaveVectorOptions() + options.driverName = 'GPKG' + options.layerName = 'layer_a' + err, _ = QgsVectorFileWriter.writeAsVectorFormatV2(ml, os.path.join(d.path(), 'transaction_test.gpkg'), QgsCoordinateTransformContext(), options) + + self.assertEqual(err, QgsVectorFileWriter.NoError) + self.assertTrue(os.path.isfile(os.path.join(d.path(), 'transaction_test.gpkg'))) + + options.layerName = 'layer_b' + options.actionOnExistingFile = QgsVectorFileWriter.CreateOrOverwriteLayer + err, _ = QgsVectorFileWriter.writeAsVectorFormatV2(ml, os.path.join(d.path(), 'transaction_test.gpkg'), QgsCoordinateTransformContext(), options) + + def _test(autoTransaction): + + layer_a = QgsVectorLayer(os.path.join(d.path(), 'transaction_test.gpkg|layername=layer_a')) + layer_b = QgsVectorLayer(os.path.join(d.path(), 'transaction_test.gpkg|layername=layer_b')) + + self.assertTrue(layer_a.isValid()) + self.assertTrue(layer_b.isValid()) + + project = QgsProject() + project.setAutoTransaction(autoTransaction) + project.addMapLayers([layer_a, layer_b]) + + self.assertFalse(layer_b.isEditable()) + self.assertTrue(layer_a.startEditing()) + + if not autoTransaction: + self.assertTrue(layer_b.startEditing()) + + self.assertTrue(layer_b.isEditable()) + + f = QgsFeature(layer_a.fields()) + f.setGeometry(QgsGeometry.fromWkt('point(7 45)')) + + self.assertTrue(layer_a.addFeatures([f])) + + buffer = layer_a.editBuffer() + self.assertTrue(len(buffer.addedFeatures()) > 0) + + _test(False) + _test(True) + if __name__ == '__main__': unittest.main()