[FEATURE] Improve handling of defaults (inc provider default clauses,

literal defaults, and qgis expression defaults) and automatically
handle unique value constraints on layers

Add a new method QgsVectorLayerUtils::createFeature which returns
a new feature which includes all relevant defaults. Any fields
with unique value constraints will be guaranteed to have a value
which is unique for the field.

Currently only in use by the split feature tool.

Sponsored by Canton of Zug and the QGEP project

python/core/qgsvectorlayerutils.sip

@@ -36,4 +36,16 @@ class QgsVectorLayerUtils
                                    QgsFieldConstraints::ConstraintStrength strength = QgsFieldConstraints::ConstraintStrengthNotSet,
                                    QgsFieldConstraints::ConstraintOrigin origin = QgsFieldConstraints::ConstraintOriginNotSet );
 
+    /**
+     * Creates a new feature ready for insertion into a layer. Default values and constraints
+     * (eg unique constraints) will automatically be handled. An optional attribute map can be
+     * passed for the new feature to copy as many attribute values as possible from the map,
+     * assuming that they respect the layer's constraints. Note that the created feature is not
+     * automatically inserted into the layer.
+     */
+    static QgsFeature createFeature( QgsVectorLayer* layer,
+                                     const QgsGeometry& geometry = QgsGeometry(),
+                                     const QgsAttributeMap& attributes = QgsAttributeMap(),
+                                     QgsExpressionContext* context = nullptr );
+
 };

src/core/qgsvectorlayereditutils.cpp

@@ -24,6 +24,7 @@
 #include "qgsgeometryfactory.h"
 #include "qgis.h"
 #include "qgswkbtypes.h"
+#include "qgsvectorlayerutils.h"
 
 #include <limits>
 
@@ -371,28 +372,8 @@ int QgsVectorLayerEditUtils::splitFeatures( const QList<QgsPoint>& splitLine, bo
       //insert new features
       for ( int i = 0; i < newGeometries.size(); ++i )
       {
-        QgsFeature newFeature;
-        newFeature.setGeometry( newGeometries.at( i ) );
-
-        //use default value where possible for primary key (e.g. autoincrement),
-        //and use the value from the original (split) feature if not primary key
-        QgsAttributes newAttributes = feat.attributes();
-        Q_FOREACH ( int pkIdx, L->dataProvider()->pkAttributeIndexes() )
-        {
-          const QVariant defaultValue = L->dataProvider()->defaultValueClause( pkIdx );
-          if ( !defaultValue.isNull() )
-          {
-            newAttributes[ pkIdx ] = defaultValue;
-          }
-          else //try with NULL
-          {
-            newAttributes[ pkIdx ] = QVariant();
-          }
-        }
-
-        newFeature.setAttributes( newAttributes );
-
-        newFeatures.append( newFeature );
+        QgsFeature f = QgsVectorLayerUtils::createFeature( L, newGeometries.at( i ), feat.attributes().toMap() );
+        L->editBuffer()->addFeature( f );
       }
 
       if ( topologicalEditing )
@@ -418,10 +399,6 @@ int QgsVectorLayerEditUtils::splitFeatures( const QList<QgsPoint>& splitLine, bo
     returnCode = 4;
   }
 
-
-  //now add the new features to this vectorlayer
-  L->editBuffer()->addFeatures( newFeatures );
-
   return returnCode;
 }
 

src/core/qgsvectorlayereditutils.h

@@ -19,6 +19,7 @@
 #include "qgsfeature.h"
 
 #include "qgsvectorlayer.h"
+#include "qgsgeometry.h"
 
 class QgsGeometryCache;
 class QgsCurve;

src/core/qgsvectorlayerutils.cpp

@@ -215,3 +215,95 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
   return valid;
 }
 
+QgsFeature QgsVectorLayerUtils::createFeature( QgsVectorLayer* layer, const QgsGeometry& geometry,
+    const QgsAttributeMap& attributes, QgsExpressionContext* context )
+{
+  if ( !layer )
+  {
+    return QgsFeature();
+  }
+
+  QgsExpressionContext* evalContext = context;
+  QScopedPointer< QgsExpressionContext > tempContext;
+  if ( !evalContext )
+  {
+    // no context passed, so we create a default one
+    tempContext.reset( new QgsExpressionContext() );
+    tempContext->appendScope( QgsExpressionContextUtils::globalScope() );
+    tempContext->appendScope( QgsExpressionContextUtils::projectScope() );
+    tempContext->appendScope( QgsExpressionContextUtils::layerScope( layer ) );
+    evalContext = tempContext.data();
+  }
+
+  QgsFields fields = layer->fields();
+
+  QgsFeature newFeature( fields );
+  newFeature.setValid( true );
+  newFeature.setGeometry( geometry );
+
+  // initialise attributes
+  newFeature.initAttributes( fields.count() );
+  for ( int idx = 0; idx < fields.count(); ++idx )
+  {
+    QVariant v;
+    bool checkUnique = true;
+
+    // in order of priority:
+
+    // 1. client side default expression
+    if ( !layer->defaultValueExpression( idx ).isEmpty() )
+    {
+      // client side default expression set - takes precedence over all. Why? Well, this is the only default
+      // which QGIS users have control over, so we assume that they're deliberately overriding any
+      // provider defaults for some good reason and we should respect that
+      v = layer->defaultValue( idx, newFeature, evalContext );
+    }
+
+    // 2. provider side default value clause
+    // note - not an else if deliberately. Users may return null from a default value expression to fallback to provider defaults
+    if ( !v.isValid() && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
+    {
+      int providerIndex = fields.fieldOriginIndex( idx );
+      QString providerDefault = layer->dataProvider()->defaultValueClause( providerIndex );
+      if ( !providerDefault.isEmpty() )
+      {
+        v = providerDefault;
+        checkUnique = false;
+      }
+    }
+
+    // 3. passed attribute value
+    // note - deliberately not using else if!
+    if ( !v.isValid() && attributes.contains( idx ) )
+    {
+      v = attributes.value( idx );
+    }
+
+    // 4. provider side default literal
+    // note - deliberately not using else if!
+    if ( !v.isValid() && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
+    {
+      int providerIndex = fields.fieldOriginIndex( idx );
+      v = layer->dataProvider()->defaultValue( providerIndex );
+    }
+
+    // last of all... check that unique constraints are respected
+    // we can't handle not null or expression constraints here, since there's no way to pick a sensible
+    // value if the constraint is violated
+    if ( checkUnique && fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique )
+    {
+      if ( QgsVectorLayerUtils::valueExists( layer, idx, v ) )
+      {
+        // unique constraint violated
+        QVariant uniqueValue = QgsVectorLayerUtils::createUniqueValue( layer, idx, v );
+        if ( uniqueValue.isValid() )
+          v = uniqueValue;
+      }
+    }
+
+    newFeature.setAttribute( idx, v );
+  }
+
+  return newFeature;
+}
+

src/core/qgsvectorlayerutils.h

@@ -17,6 +17,7 @@
 #define QGSVECTORLAYERUTILS_H
 
 #include "qgsvectorlayer.h"
+#include "qgsgeometry.h"
 
 /** \ingroup core
  * \class QgsVectorLayerUtils
@@ -53,6 +54,17 @@ class CORE_EXPORT QgsVectorLayerUtils
                                    QgsFieldConstraints::ConstraintStrength strength = QgsFieldConstraints::ConstraintStrengthNotSet,
                                    QgsFieldConstraints::ConstraintOrigin origin = QgsFieldConstraints::ConstraintOriginNotSet );
 
+    /**
+     * Creates a new feature ready for insertion into a layer. Default values and constraints
+     * (eg unique constraints) will automatically be handled. An optional attribute map can be
+     * passed for the new feature to copy as many attribute values as possible from the map,
+     * assuming that they respect the layer's constraints. Note that the created feature is not
+     * automatically inserted into the layer.
+     */
+    static QgsFeature createFeature( QgsVectorLayer* layer,
+                                     const QgsGeometry& geometry = QgsGeometry(),
+                                     const QgsAttributeMap& attributes = QgsAttributeMap(),
+                                     QgsExpressionContext* context = nullptr );
 
 };
 

src/providers/spatialite/qgsspatialiteprovider.cpp

@@ -762,6 +762,9 @@ void QgsSpatiaLiteProvider::loadFieldsAbstractInterface( gaiaVectorLayerPtr lyr
     }
   }
 
+  // check for constraints
+  fetchConstraints();
+
   // for views try to get the primary key from the meta table
   if ( mViewBased && mPrimaryKey.isEmpty() )
   {
@@ -862,6 +865,15 @@ void QgsSpatiaLiteProvider::fetchConstraints()
   }
   sqlite3_free_table( results );
 
+  Q_FOREACH ( int fieldIdx, mPrimaryKeyAttrs )
+  {
+    //primary keys are unique, not null
+    QgsFieldConstraints constraints = mAttributeFields.at( fieldIdx ).constraints();
+    constraints.setConstraint( QgsFieldConstraints::ConstraintUnique, QgsFieldConstraints::ConstraintOriginProvider );
+    constraints.setConstraint( QgsFieldConstraints::ConstraintNotNull, QgsFieldConstraints::ConstraintOriginProvider );
+    mAttributeFields[ fieldIdx ].setConstraints( constraints );
+  }
+
   return;
 
 error:

tests/src/python/test_provider_postgres.py

@@ -529,6 +529,21 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase):
         self.assertTrue(vl.addFeatures([f]))
         self.assertFalse(QgsVectorLayerUtils.valueExists(vl, 0, default_clause))
 
+    def testVectorLayerUtilsCreateFeatureWithProviderDefault(self):
+        vl = QgsVectorLayer('%s table="qgis_test"."someData" sql=' % (self.dbconn), "someData", "postgres")
+        default_clause = 'nextval(\'qgis_test."someData_pk_seq"\'::regclass)'
+        self.assertEqual(vl.dataProvider().defaultValueClause(0), default_clause)
+
+        # check that provider default clause takes precendence over passed attribute values
+        # this also checks that the inbuilt unique constraint handling is bypassed in the case of a provider default clause
+        f = QgsVectorLayerUtils.createFeature(vl, attributes={1: 5, 3: 'map'})
+        self.assertEqual(f.attributes(), [default_clause, 5, "'qgis'::text", "'qgis'::text", None, None])
+
+        # test take vector layer default value expression overrides postgres provider default clause
+        vl.setDefaultValueExpression(3, "'mappy'")
+        f = QgsVectorLayerUtils.createFeature(vl, attributes={1: 5, 3: 'map'})
+        self.assertEqual(f.attributes(), [default_clause, 5, "'qgis'::text", 'mappy', None, None])
+
     # See http://hub.qgis.org/issues/15188
     def testNumericPrecision(self):
         uri = 'point?field=f1:int'

tests/src/python/test_provider_spatialite.py

@@ -19,7 +19,7 @@ import sys
 import shutil
 import tempfile
 
-from qgis.core import QgsVectorLayer, QgsPoint, QgsFeature, QgsGeometry, QgsProject, QgsMapLayerRegistry, QgsField, QgsFieldConstraints
+from qgis.core import QgsVectorLayer, QgsPoint, QgsFeature, QgsGeometry, QgsProject, QgsMapLayerRegistry, QgsField, QgsFieldConstraints, QgsVectorLayerUtils
 
 from qgis.testing import start_app, unittest
 from utilities import unitTestDataPath
@@ -185,28 +185,20 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
         self.assertTrue(layer.isValid())
         self.assertTrue(layer.hasGeometryType())
         layer.startEditing()
-        self.assertEqual(layer.splitFeatures([QgsPoint(0.5, -0.5), QgsPoint(0.5, 1.5)], 0), 0)
-        self.assertEqual(layer.splitFeatures([QgsPoint(-0.5, 0.5), QgsPoint(1.5, 0.5)], 0), 0)
+        self.assertEqual(layer.splitFeatures([QgsPoint(0.75, -0.5), QgsPoint(0.75, 1.5)], 0), 0)
+        self.assertEqual(layer.splitFeatures([QgsPoint(-0.5, 0.25), QgsPoint(1.5, 0.25)], 0), 0)
         self.assertTrue(layer.commitChanges())
         self.assertEqual(layer.featureCount(), 4)
 
-    def xtest_SplitFeatureWithFailedCommit(self):
+    def test_SplitFeatureWithMultiKey(self):
         """Create spatialite database"""
         layer = QgsVectorLayer("dbname=%s table=test_pg_mk (geometry)" % self.dbname, "test_pg_mk", "spatialite")
         self.assertTrue(layer.isValid())
         self.assertTrue(layer.hasGeometryType())
         layer.startEditing()
-        self.asserEqual(layer.splitFeatures([QgsPoint(0.5, -0.5), QgsPoint(0.5, 1.5)], 0), 0)
-        self.asserEqual(layer.splitFeatures([QgsPoint(-0.5, 0.5), QgsPoint(1.5, 0.5)], 0), 0)
-        self.assertFalse(layer.commitChanges())
-        layer.rollBack()
-        feat = next(layer.getFeatures())
-        ref = [[(0, 0), (1, 0), (1, 1), (0, 1), (0, 0)]]
-        res = feat.geometry().asPolygon()
-        for ring1, ring2 in zip(ref, res):
-            for p1, p2 in zip(ring1, ring2):
-                for c1, c2 in zip(p1, p2):
-                    self.asserEqual(c1, c2)
+        self.assertEqual(layer.splitFeatures([QgsPoint(0.5, -0.5), QgsPoint(0.5, 1.5)], 0), 0)
+        self.assertEqual(layer.splitFeatures([QgsPoint(-0.5, 0.5), QgsPoint(1.5, 0.5)], 0), 0)
+        self.assertTrue(layer.commitChanges())
 
     def test_queries(self):
         """Test loading of query-based layers"""
@@ -475,6 +467,21 @@ class TestQgsSpatialiteProvider(unittest.TestCase, ProviderTestCase):
         self.assertEqual(l.dataProvider().defaultValue(3), 5.7)
         self.assertFalse(l.dataProvider().defaultValue(4))
 
+    def testVectorLayerUtilsCreateFeatureWithProviderDefaultLiteral(self):
+        vl = QgsVectorLayer("dbname=%s table='test_defaults' key='id'" % self.dbname, "test_defaults", "spatialite")
+        self.assertEqual(vl.dataProvider().defaultValue(2), 5)
+
+        f = QgsVectorLayerUtils.createFeature(vl)
+        self.assertEqual(f.attributes(), [None, "qgis 'is good", 5, 5.7, None])
+
+        # check that provider passed attribute values take precedence over default literals
+        f = QgsVectorLayerUtils.createFeature(vl, attributes={1: 'qgis is great', 0: 3})
+        self.assertEqual(f.attributes(), [3, "qgis is great", 5, 5.7, None])
+
+        # test take vector layer default value expression overrides postgres provider default clause
+        vl.setDefaultValueExpression(3, "4*3")
+        f = QgsVectorLayerUtils.createFeature(vl, attributes={1: 'qgis is great', 0: 3})
+        self.assertEqual(f.attributes(), [3, "qgis is great", 5, 12, None])
 
 if __name__ == '__main__':
     unittest.main()

tests/src/python/test_qgsvectorlayerutils.py

@@ -24,6 +24,8 @@ from qgis.core import (QgsVectorLayer,
                        QgsFieldConstraints,
                        QgsFields,
                        QgsFeature,
+                       QgsGeometry,
+                       QgsPoint,
                        NULL
                        )
 from qgis.testing import start_app, unittest
@@ -211,5 +213,64 @@ class TestQgsVectorLayerUtils(unittest.TestCase):
         self.assertEqual(QgsVectorLayerUtils.createUniqueValue(layer, 0, 'seed'), 'seed')
         self.assertEqual(QgsVectorLayerUtils.createUniqueValue(layer, 0, 'superpig'), 'superpig_1')
 
+    def testCreateFeature(self):
+        """ test creating a feature respecting defaults and constraints """
+        layer = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer&field=flddbl:double",
+                               "addfeat", "memory")
+        # add a bunch of features
+        f = QgsFeature()
+        f.setAttributes(["test", 123, 1.0])
+        f1 = QgsFeature(2)
+        f1.setAttributes(["test_1", 124, 1.1])
+        f2 = QgsFeature(3)
+        f2.setAttributes(["test_2", 125, 2.4])
+        f3 = QgsFeature(4)
+        f3.setAttributes(["test_3", 126, 1.7])
+        f4 = QgsFeature(5)
+        f4.setAttributes(["superpig", 127, 0.8])
+        self.assertTrue(layer.dataProvider().addFeatures([f, f1, f2, f3, f4]))
+
+        # no layer
+        self.assertFalse(QgsVectorLayerUtils.createFeature(None).isValid())
+
+        # basic tests
+        f = QgsVectorLayerUtils.createFeature(layer)
+        self.assertTrue(f.isValid())
+        self.assertEqual(f.fields(), layer.fields())
+        self.assertFalse(f.hasGeometry())
+        self.assertEqual(f.attributes(), [NULL, NULL, NULL])
+
+        # set geometry
+        g = QgsGeometry.fromPoint(QgsPoint(100, 200))
+        f = QgsVectorLayerUtils.createFeature(layer, g)
+        self.assertTrue(f.hasGeometry())
+        self.assertEqual(f.geometry().exportToWkt(), g.exportToWkt())
+
+        # using attribute map
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'a', 2: 6.0})
+        self.assertEqual(f.attributes(), ['a', NULL, 6.0])
+
+        # layer with default value expression
+        layer.setDefaultValueExpression(2, '3*4')
+        f = QgsVectorLayerUtils.createFeature(layer)
+        self.assertEqual(f.attributes(), [NULL, NULL, 12.0])
+        # we expect the default value expression to take precedence over the attribute map
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'a', 2: 6.0})
+        self.assertEqual(f.attributes(), ['a', NULL, 12.0])
+        # layer with default value expression based on geometry
+        layer.setDefaultValueExpression(2, '3*$x')
+        f = QgsVectorLayerUtils.createFeature(layer, g)
+        self.assertEqual(f.attributes(), [NULL, NULL, 300.0])
+        layer.setDefaultValueExpression(2, None)
+
+        # test with violated unique constraints
+        layer.setFieldConstraint(1, QgsFieldConstraints.ConstraintUnique)
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
+        self.assertEqual(f.attributes(), ['test_1', 128, NULL])
+        layer.setFieldConstraint(0, QgsFieldConstraints.ConstraintUnique)
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
+        self.assertEqual(f.attributes(), ['test_4', 128, NULL])
+
+
 if __name__ == '__main__':
     unittest.main()