Merge pull request #8197 from signedav/fix_duplication

[Bugfix] Care about default values again on creating feature
qgis · Oct 16, 2018 · 7d3daf6 · 7d3daf6
2 parents 190f938 + 5352629
commit 7d3daf6
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 12 deletions.
diff --git a/src/core/qgsvectorlayerutils.cpp b/src/core/qgsvectorlayerutils.cpp
@@ -384,20 +384,12 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
     if ( attributes.contains( idx ) )
     {
       v = attributes.value( idx );
-      if ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique
-           && QgsVectorLayerUtils::valueExists( layer, idx, v ) )
-      {
-        // unique constraint violated
-        QVariant uniqueValue = QgsVectorLayerUtils::createUniqueValue( layer, idx, v );
-        if ( uniqueValue.isValid() )
-          v = uniqueValue;
-      }
-      checkUnique = false;
     }
 
     // 2. client side default expression
     // note - deliberately not using else if!
-    if ( !v.isValid() && layer->defaultValueDefinition( idx ).isValid() )
+    if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique  && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
+         && layer->defaultValueDefinition( idx ).isValid() )
     {
       // 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
@@ -407,7 +399,8 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
 
     // 3. 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 )
+    if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique  && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
+         && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
     {
       int providerIndex = fields.fieldOriginIndex( idx );
       QString providerDefault = layer->dataProvider()->defaultValueClause( providerIndex );
@@ -420,7 +413,8 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
 
     // 4. provider side default literal
     // note - deliberately not using else if!
-    if ( !v.isValid() && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
+    if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique  && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
+         && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
     {
       int providerIndex = fields.fieldOriginIndex( idx );
       v = layer->dataProvider()->defaultValue( providerIndex );
@@ -431,6 +425,13 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
       }
     }
 
+    // 5. passed attribute value
+    // note - deliberately not using else if!
+    if ( !v.isValid() && attributes.contains( idx ) )
+    {
+      v = attributes.value( idx );
+    }
+
     // 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

diff --git a/tests/src/python/test_qgsvectorlayerutils.py b/tests/src/python/test_qgsvectorlayerutils.py
@@ -279,6 +279,24 @@ def testCreateFeature(self):
         f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
         self.assertEqual(f.attributes(), ['test_4', 128, NULL])
 
+        # test with violated unique constraints and default value expression providing unique value
+        layer.setDefaultValueDefinition(1, QgsDefaultValue('130'))
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
+        # since field 1 has Unique Constraint, it ignores value 123 that already has been set and adds the default value
+        self.assertEqual(f.attributes(), ['test_4', 130, NULL])
+        # fallback: test with violated unique constraints and default value expression providing already existing value
+        # add the feature with the default value:
+        self.assertTrue(layer.dataProvider().addFeatures([f]))
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
+        # since field 1 has Unique Constraint, it ignores value 123 that already has been set and adds the default value
+        # and since the default value providing an already existing value (130) it generates a unique value (next int: 131)
+        self.assertEqual(f.attributes(), ['test_5', 131, NULL])
+        layer.setDefaultValueDefinition(1, QgsDefaultValue(None))
+
+        # test with manually correct unique constraint
+        f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 132})
+        self.assertEqual(f.attributes(), ['test_5', 132, NULL])
+
     def testDuplicateFeature(self):
         """ test duplicating a feature """