Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #6719 from pblottiere/bugfix_transaction_constrain…
…ts_218

[backport][bugfix] Update all attributes in a single transaction
  • Loading branch information
pblottiere committed Apr 9, 2018
2 parents b1c6a87 + 032e082 commit 3c3c00a
Show file tree
Hide file tree
Showing 12 changed files with 237 additions and 5 deletions.
33 changes: 33 additions & 0 deletions python/core/qgsvectorlayer.sip
Expand Up @@ -954,6 +954,39 @@ class QgsVectorLayer : QgsMapLayer
*/
bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );

/**
* Changes attributes' values for a feature (but does not immediately
* commit the changes).
* The \a fid argument specifies the ID of the feature to be changed.
*
* The new values to be assigned to the fields are given by \a newValues.
*
* If a valid QVariant is specified for a field in \a oldValues, it will be
* used as the field value in the case of an undo operation corresponding
* to this attribute value change. If an invalid QVariant is used (the
* default behavior), then the feature's current value will be
* automatically retrieved and used. Note that this involves a feature
* request to the underlying data provider, so it is more efficient to
* explicitly pass an oldValue if it is already available.
*
* Returns true if feature's attributes was successfully changed.
*
* @note Calls to changeAttributeValues() are only valid for layers in
* which edits have been enabled by a call to startEditing(). Changes made
* to features using this method are not committed to the underlying data
* provider until a commitChanges() call is made. Any uncommitted changes
* can be discarded by calling rollBack().
*
* @see startEditing()
* @see commitChanges()
* @see changeGeometry()
* @see updateFeature()
* @see changeAttributeValue()
*
* @note added in QGIS 2.18
*/
bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues );

/** Add an attribute field (but does not commit it)
* returns true if the field was added
*/
Expand Down
7 changes: 6 additions & 1 deletion python/core/qgsvectorlayereditbuffer.sip
Expand Up @@ -70,7 +70,12 @@ class QgsVectorLayerEditBuffer : QObject
/** Stop editing and discard the edits */
virtual void rollBack();


/**
* Changes values of attributes (but does not commit it).
* @return true if attributes are well updated, false otherwise
* @note added in QGIS 2.18
*/
virtual bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues );

/** New features which are not commited. */
const QgsFeatureMap& addedFeatures();
Expand Down
1 change: 1 addition & 0 deletions python/core/qgsvectorlayereditpassthrough.sip
Expand Up @@ -12,6 +12,7 @@ class QgsVectorLayerEditPassthrough : QgsVectorLayerEditBuffer
bool deleteFeatures( const QgsFeatureIds& fids );
bool changeGeometry( QgsFeatureId fid, QgsGeometry* geom );
bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );
bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues );
bool addAttribute( const QgsField &field );
bool deleteAttribute( int attr );
bool renameAttribute( int attr, const QString& newName );
Expand Down
42 changes: 42 additions & 0 deletions src/core/qgsvectorlayer.cpp
Expand Up @@ -2384,6 +2384,48 @@ bool QgsVectorLayer::changeAttributeValue( QgsFeatureId fid, int field, const QV
return mEditBuffer->changeAttributeValue( fid, field, newValue, oldValue );
}

bool QgsVectorLayer::changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues )
{
bool result = true;

QgsAttributeMap newValidValues;
QgsAttributeMap oldValidValues;

QgsAttributeMap::const_iterator it;
for ( it = newValues.constBegin(); it != newValues.constEnd(); ++it )
{
const int field = it.key();
const QVariant newValue = it.value();
QVariant oldValue;

if ( oldValues.contains( field ) )
oldValue = oldValues[field];

switch ( fields().fieldOrigin( field ) )
{
case QgsFields::OriginProvider:
case QgsFields::OriginEdit:
case QgsFields::OriginExpression:
{
newValidValues[field] = newValue;
oldValidValues[field] = oldValue;
break;
}

case QgsFields::OriginJoin:
case QgsFields::OriginUnknown:
break;
}
}

if ( ! newValidValues.isEmpty() && mEditBuffer && mDataProvider )
{
result &= mEditBuffer->changeAttributeValues( fid, newValidValues, oldValidValues );
}

return result;
}

bool QgsVectorLayer::addAttribute( const QgsField &field )
{
if ( !mEditBuffer || !mDataProvider )
Expand Down
33 changes: 33 additions & 0 deletions src/core/qgsvectorlayer.h
Expand Up @@ -1347,6 +1347,39 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer
*/
bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() );

/**
* Changes attributes' values for a feature (but does not immediately
* commit the changes).
* The \a fid argument specifies the ID of the feature to be changed.
*
* The new values to be assigned to the fields are given by \a newValues.
*
* If a valid QVariant is specified for a field in \a oldValues, it will be
* used as the field value in the case of an undo operation corresponding
* to this attribute value change. If an invalid QVariant is used (the
* default behavior), then the feature's current value will be
* automatically retrieved and used. Note that this involves a feature
* request to the underlying data provider, so it is more efficient to
* explicitly pass an oldValue if it is already available.
*
* Returns true if feature's attributes was successfully changed.
*
* @note Calls to changeAttributeValues() are only valid for layers in
* which edits have been enabled by a call to startEditing(). Changes made
* to features using this method are not committed to the underlying data
* provider until a commitChanges() call is made. Any uncommitted changes
* can be discarded by calling rollBack().
*
* @see startEditing()
* @see commitChanges()
* @see changeGeometry()
* @see updateFeature()
* @see changeAttributeValue()
*
* @note added in QGIS 2.18
*/
bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues );

/** Add an attribute field (but does not commit it)
* returns true if the field was added
*/
Expand Down
19 changes: 19 additions & 0 deletions src/core/qgsvectorlayereditbuffer.cpp
Expand Up @@ -776,3 +776,22 @@ void QgsVectorLayerEditBuffer::updateLayerFields()
{
L->updateFields();
}

bool QgsVectorLayerEditBuffer::changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues )
{
bool success = true;
QgsAttributeMap::const_iterator it;
for ( it = newValues.constBegin() ; it != newValues.constEnd(); ++it )
{
const int field = it.key();
const QVariant newValue = it.value();
QVariant oldValue;

if ( oldValues.contains( field ) )
oldValue = oldValues[field];

success &= changeAttributeValue( fid, field, newValue, oldValue );
}

return success;
}
7 changes: 6 additions & 1 deletion src/core/qgsvectorlayereditbuffer.h
Expand Up @@ -99,7 +99,12 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
/** Stop editing and discard the edits */
virtual void rollBack();


/**
* Changes values of attributes (but does not commit it).
* @return true if attributes are well updated, false otherwise
* @note added in QGIS 2.18
*/
virtual bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues );

/** New features which are not commited. */
inline const QgsFeatureMap& addedFeatures() { return mAddedFeatures; }
Expand Down
21 changes: 21 additions & 0 deletions src/core/qgsvectorlayereditpassthrough.cpp
Expand Up @@ -109,6 +109,27 @@ bool QgsVectorLayerEditPassthrough::changeAttributeValue( QgsFeatureId fid, int
return false;
}

bool QgsVectorLayerEditPassthrough::changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues )
{
Q_UNUSED( oldValues );
bool result = false;

QgsChangedAttributesMap attribMap;
attribMap.insert( fid, newValues );

if ( L->dataProvider()->changeAttributeValues( attribMap ) )
{
result = true;
QgsAttributeMap::const_iterator it;
for ( it = newValues.constBegin(); it != newValues.constEnd(); ++it )
{
emit attributeValueChanged( fid, it.key(), it.value() );
}
}

return result;
}

bool QgsVectorLayerEditPassthrough::addAttribute( const QgsField &field )
{
if ( L->dataProvider()->addAttributes( QList<QgsField>() << field ) )
Expand Down
1 change: 1 addition & 0 deletions src/core/qgsvectorlayereditpassthrough.h
Expand Up @@ -34,6 +34,7 @@ class CORE_EXPORT QgsVectorLayerEditPassthrough : public QgsVectorLayerEditBuffe
bool deleteFeatures( const QgsFeatureIds& fids ) override;
bool changeGeometry( QgsFeatureId fid, QgsGeometry* geom ) override;
bool changeAttributeValue( QgsFeatureId fid, int field, const QVariant &newValue, const QVariant &oldValue = QVariant() ) override;
bool changeAttributeValues( QgsFeatureId fid, const QgsAttributeMap &newValues, const QgsAttributeMap &oldValues ) override;
bool addAttribute( const QgsField &field ) override;
bool deleteAttribute( int attr ) override;
bool renameAttribute( int attr, const QString& newName ) override;
Expand Down
9 changes: 8 additions & 1 deletion src/gui/qgsattributeform.cpp
Expand Up @@ -337,6 +337,9 @@ bool QgsAttributeForm::saveEdits()
{
mLayer->beginEditCommand( mEditCommandMessage );

QgsAttributeMap newValues;
QgsAttributeMap oldValues;

int n = 0;
for ( int i = 0; i < dst.count(); ++i )
{
Expand All @@ -353,10 +356,14 @@ bool QgsAttributeForm::saveEdits()
QgsDebugMsg( QString( "src:'%1' (type:%2, isNull:%3, isValid:%4)" )
.arg( src.at( i ).toString(), src.at( i ).typeName() ).arg( src.at( i ).isNull() ).arg( src.at( i ).isValid() ) );

success &= mLayer->changeAttributeValue( mFeature.id(), i, dst.at( i ), src.at( i ) );
newValues[i] = dst.at( i );
oldValues[i] = src.at( i );

n++;
}

success = mLayer->changeAttributeValues( mFeature.id(), newValues, oldValues );

if ( success && n > 0 )
{
mLayer->endEditCommand();
Expand Down
55 changes: 55 additions & 0 deletions tests/src/python/test_provider_postgres.py
Expand Up @@ -20,15 +20,26 @@

from qgis.core import (
QgsGeometry,
QgsProject,
QgsPoint,
QgsVectorLayer,
QgsVectorLayerImport,
QgsFeatureRequest,
QgsMapLayerRegistry,
QgsFeature,
QgsTransactionGroup,
NULL
)

from qgis.gui import (
QgsEditorWidgetRegistry,
QgsAttributeForm
)

from qgis.PyQt.QtCore import QSettings, QDate, QTime, QDateTime, QVariant

from qgis.PyQt.QtWidgets import QLabel

from qgis.testing import start_app, unittest
from utilities import unitTestDataPath
from providertestbase import ProviderTestCase
Expand All @@ -54,6 +65,8 @@ def setUpClass(cls):
cls.poly_provider = cls.poly_vl.dataProvider()
cls.con = psycopg2.connect(cls.dbconn)

QgsEditorWidgetRegistry.initEditors()

@classmethod
def tearDownClass(cls):
"""Run after all tests"""
Expand Down Expand Up @@ -407,6 +420,48 @@ def testStyleDatabaseWithService(self):
ids = styles[1]
self.assertEqual(len(ids), 1)

def testTransactionConstrains(self):
# create a vector layer based on postgres
vl = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'id\' table="qgis_test"."check_constraints" sql=', 'test', 'postgres')
self.assertTrue(vl.isValid())

# prepare a project with transactions enabled
p = QgsProject.instance()
p.setAutoTransaction(True)

QgsMapLayerRegistry.instance().addMapLayers([vl])

# get feature
f = next(vl.getFeatures(QgsFeatureRequest(1))) # fid=1
self.assertEqual(f.attributes(), [1, 4, 3])

# start edition
vl.startEditing()

# update attribute form with a failing constraints
# coming from the database if attributes are updated
# one at a time.
# Current feature: a = 4 / b = 3
# Update feature: a = 1 / b = 0
# If updated one at a time, '(a = 1) < (b = 3)' => FAIL!
form = QgsAttributeForm(vl)
form.setFeature(f)
self.assertTrue(form.editable())
for w in form.findChildren(QLabel):
if w.buddy():
spinBox = w.buddy()
if w.text() == 'a':
spinBox.setValue('1')
if w.text() == 'b':
spinBox.setValue('0')

# save
form.save()

# check new values
f = next(vl.getFeatures(QgsFeatureRequest(1))) # fid=1
self.assertEqual(f.attributes(), [1, 1, 0])


if __name__ == '__main__':
unittest.main()
14 changes: 12 additions & 2 deletions tests/testdata/provider/testdata_pg.sql
Expand Up @@ -30,7 +30,7 @@ SET default_with_oids = false;

--
-- TOC entry 171 (class 1259 OID 377761)
-- Name: someData; Type: TABLE; Schema: qgis_test; Owner: postgres; Tablespace:
-- Name: someData; Type: TABLE; Schema: qgis_test; Owner: postgres; Tablespace:
--

CREATE TABLE qgis_test."someData" (
Expand Down Expand Up @@ -70,7 +70,7 @@ INSERT INTO qgis_test."some_poly_data" (pk, geom) VALUES

--
-- TOC entry 3953 (class 2606 OID 377768)
-- Name: someData_pkey; Type: CONSTRAINT; Schema: qgis_test; Owner: postgres; Tablespace:
-- Name: someData_pkey; Type: CONSTRAINT; Schema: qgis_test; Owner: postgres; Tablespace:
--

ALTER TABLE ONLY qgis_test."someData"
Expand Down Expand Up @@ -399,6 +399,16 @@ CREATE TABLE qgis_test.domains
fld_numeric_domain qgis_test.numeric_domain
);

CREATE TABLE qgis_test.check_constraints (
id integer PRIMARY KEY,
a integer,
b integer, CHECK (a > b)
);
INSERT INTO qgis_test.check_constraints VALUES (
1, -- id
4, -- a
3 -- b
);

--------------------------------------
-- Temporary table for testing renaming fields
Expand Down

0 comments on commit 3c3c00a

Please sign in to comment.