Merge pull request #6719 from pblottiere/bugfix_transaction_constraints_218

pblottiere · web-flow · commit 3c3c00aae77a · 2018-04-09T08:11:33.000+01:00
[backport][bugfix] Update all attributes in a single transaction
diff --git a/python/core/qgsvectorlayer.sip b/python/core/qgsvectorlayer.sip
@@ -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
      */
diff --git a/python/core/qgsvectorlayereditbuffer.sip b/python/core/qgsvectorlayereditbuffer.sip
@@ -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();
diff --git a/python/core/qgsvectorlayereditpassthrough.sip b/python/core/qgsvectorlayereditpassthrough.sip
@@ -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 );
diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -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 )
diff --git a/src/core/qgsvectorlayer.h b/src/core/qgsvectorlayer.h
@@ -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
      */
diff --git a/src/core/qgsvectorlayereditbuffer.cpp b/src/core/qgsvectorlayereditbuffer.cpp
@@ -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;
+}
diff --git a/src/core/qgsvectorlayereditbuffer.h b/src/core/qgsvectorlayereditbuffer.h
@@ -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; }
diff --git a/src/core/qgsvectorlayereditpassthrough.cpp b/src/core/qgsvectorlayereditpassthrough.cpp
@@ -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 ) )
diff --git a/src/core/qgsvectorlayereditpassthrough.h b/src/core/qgsvectorlayereditpassthrough.h
@@ -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;
diff --git a/src/gui/qgsattributeform.cpp b/src/gui/qgsattributeform.cpp
@@ -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 )
         {
@@ -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();
diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -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
@@ -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"""
@@ -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()
diff --git a/tests/testdata/provider/testdata_pg.sql b/tests/testdata/provider/testdata_pg.sql
@@ -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" (
@@ -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"
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -337,6 +337,9 @@ bool QgsAttributeForm::saveEdits()`
`337`	`337`	`{`
`338`	`338`	`mLayer->beginEditCommand( mEditCommandMessage );`
`339`	`339`
	`340`	`+ QgsAttributeMap newValues;`
	`341`	`+ QgsAttributeMap oldValues;`
	`342`	`+`
`340`	`343`	`int n = 0;`
`341`	`344`	`for ( int i = 0; i < dst.count(); ++i )`
`342`	`345`	`{`
`@@ -353,10 +356,14 @@ bool QgsAttributeForm::saveEdits()`
`353`	`356`	`QgsDebugMsg( QString( "src:'%1' (type:%2, isNull:%3, isValid:%4)" )`
`354`	`357`	`.arg( src.at( i ).toString(), src.at( i ).typeName() ).arg( src.at( i ).isNull() ).arg( src.at( i ).isValid() ) );`
`355`	`358`
`356`		`- success &= mLayer->changeAttributeValue( mFeature.id(), i, dst.at( i ), src.at( i ) );`
	`359`	`+ newValues[i] = dst.at( i );`
	`360`	`+ oldValues[i] = src.at( i );`
	`361`	`+`
`357`	`362`	`n++;`
`358`	`363`	`}`
`359`	`364`
	`365`	`+ success = mLayer->changeAttributeValues( mFeature.id(), newValues, oldValues );`
	`366`	`+`
`360`	`367`	`if ( success && n > 0 )`
`361`	`368`	`{`
`362`	`369`	`mLayer->endEditCommand();`