Skip to content

Commit 7d3daf6

Browse files
authored
Merge pull request #8197 from signedav/fix_duplication
[Bugfix] Care about default values again on creating feature
2 parents 190f938 + 5352629 commit 7d3daf6

File tree

2 files changed

+31
-12
lines changed

2 files changed

+31
-12
lines changed

src/core/qgsvectorlayerutils.cpp

+13-12
Original file line numberDiff line numberDiff line change
@@ -384,20 +384,12 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
384384
if ( attributes.contains( idx ) )
385385
{
386386
v = attributes.value( idx );
387-
if ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique
388-
&& QgsVectorLayerUtils::valueExists( layer, idx, v ) )
389-
{
390-
// unique constraint violated
391-
QVariant uniqueValue = QgsVectorLayerUtils::createUniqueValue( layer, idx, v );
392-
if ( uniqueValue.isValid() )
393-
v = uniqueValue;
394-
}
395-
checkUnique = false;
396387
}
397388

398389
// 2. client side default expression
399390
// note - deliberately not using else if!
400-
if ( !v.isValid() && layer->defaultValueDefinition( idx ).isValid() )
391+
if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
392+
&& layer->defaultValueDefinition( idx ).isValid() )
401393
{
402394
// client side default expression set - takes precedence over all. Why? Well, this is the only default
403395
// 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
407399

408400
// 3. provider side default value clause
409401
// note - not an else if deliberately. Users may return null from a default value expression to fallback to provider defaults
410-
if ( !v.isValid() && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
402+
if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
403+
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
411404
{
412405
int providerIndex = fields.fieldOriginIndex( idx );
413406
QString providerDefault = layer->dataProvider()->defaultValueClause( providerIndex );
@@ -420,7 +413,8 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
420413

421414
// 4. provider side default literal
422415
// note - deliberately not using else if!
423-
if ( !v.isValid() && fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
416+
if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
417+
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
424418
{
425419
int providerIndex = fields.fieldOriginIndex( idx );
426420
v = layer->dataProvider()->defaultValue( providerIndex );
@@ -431,6 +425,13 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons
431425
}
432426
}
433427

428+
// 5. passed attribute value
429+
// note - deliberately not using else if!
430+
if ( !v.isValid() && attributes.contains( idx ) )
431+
{
432+
v = attributes.value( idx );
433+
}
434+
434435
// last of all... check that unique constraints are respected
435436
// we can't handle not null or expression constraints here, since there's no way to pick a sensible
436437
// value if the constraint is violated

tests/src/python/test_qgsvectorlayerutils.py

+18
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,24 @@ def testCreateFeature(self):
279279
f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
280280
self.assertEqual(f.attributes(), ['test_4', 128, NULL])
281281

282+
# test with violated unique constraints and default value expression providing unique value
283+
layer.setDefaultValueDefinition(1, QgsDefaultValue('130'))
284+
f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
285+
# since field 1 has Unique Constraint, it ignores value 123 that already has been set and adds the default value
286+
self.assertEqual(f.attributes(), ['test_4', 130, NULL])
287+
# fallback: test with violated unique constraints and default value expression providing already existing value
288+
# add the feature with the default value:
289+
self.assertTrue(layer.dataProvider().addFeatures([f]))
290+
f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 123})
291+
# since field 1 has Unique Constraint, it ignores value 123 that already has been set and adds the default value
292+
# and since the default value providing an already existing value (130) it generates a unique value (next int: 131)
293+
self.assertEqual(f.attributes(), ['test_5', 131, NULL])
294+
layer.setDefaultValueDefinition(1, QgsDefaultValue(None))
295+
296+
# test with manually correct unique constraint
297+
f = QgsVectorLayerUtils.createFeature(layer, attributes={0: 'test_1', 1: 132})
298+
self.assertEqual(f.attributes(), ['test_5', 132, NULL])
299+
282300
def testDuplicateFeature(self):
283301
""" test duplicating a feature """
284302

0 commit comments

Comments
 (0)