From b5864cd4327a09f2bf2b0afffa09582370c6f194 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 8 Nov 2016 16:59:07 +1000 Subject: [PATCH] [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 | 12 +++ src/core/qgsvectorlayereditutils.cpp | 29 +----- src/core/qgsvectorlayereditutils.h | 1 + src/core/qgsvectorlayerutils.cpp | 92 +++++++++++++++++++ src/core/qgsvectorlayerutils.h | 12 +++ .../spatialite/qgsspatialiteprovider.cpp | 12 +++ tests/src/python/test_provider_postgres.py | 15 +++ tests/src/python/test_provider_spatialite.py | 37 +++++--- tests/src/python/test_qgsvectorlayerutils.py | 61 ++++++++++++ 9 files changed, 230 insertions(+), 41 deletions(-) diff --git a/python/core/qgsvectorlayerutils.sip b/python/core/qgsvectorlayerutils.sip index 0c61f30a46f..1420179d669 100644 --- a/python/core/qgsvectorlayerutils.sip +++ b/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 ); + }; diff --git a/src/core/qgsvectorlayereditutils.cpp b/src/core/qgsvectorlayereditutils.cpp index 5cab0b5844e..5436bfdf296 100644 --- a/src/core/qgsvectorlayereditutils.cpp +++ b/src/core/qgsvectorlayereditutils.cpp @@ -24,6 +24,7 @@ #include "qgsgeometryfactory.h" #include "qgis.h" #include "qgswkbtypes.h" +#include "qgsvectorlayerutils.h" #include @@ -371,28 +372,8 @@ int QgsVectorLayerEditUtils::splitFeatures( const QList& 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& splitLine, bo returnCode = 4; } - - //now add the new features to this vectorlayer - L->editBuffer()->addFeatures( newFeatures ); - return returnCode; } diff --git a/src/core/qgsvectorlayereditutils.h b/src/core/qgsvectorlayereditutils.h index ed96ae97af3..7b00a460860 100644 --- a/src/core/qgsvectorlayereditutils.h +++ b/src/core/qgsvectorlayereditutils.h @@ -19,6 +19,7 @@ #include "qgsfeature.h" #include "qgsvectorlayer.h" +#include "qgsgeometry.h" class QgsGeometryCache; class QgsCurve; diff --git a/src/core/qgsvectorlayerutils.cpp b/src/core/qgsvectorlayerutils.cpp index d5845e97eae..eb54bfab874 100644 --- a/src/core/qgsvectorlayerutils.cpp +++ b/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; +} + diff --git a/src/core/qgsvectorlayerutils.h b/src/core/qgsvectorlayerutils.h index 9235de34042..64246276cd7 100644 --- a/src/core/qgsvectorlayerutils.h +++ b/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 ); }; diff --git a/src/providers/spatialite/qgsspatialiteprovider.cpp b/src/providers/spatialite/qgsspatialiteprovider.cpp index dd19b14ef65..4ff7d1c1fa4 100644 --- a/src/providers/spatialite/qgsspatialiteprovider.cpp +++ b/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: diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py index d483da905e1..3dccc073695 100644 --- a/tests/src/python/test_provider_postgres.py +++ b/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' diff --git a/tests/src/python/test_provider_spatialite.py b/tests/src/python/test_provider_spatialite.py index bf5fb4819a7..034ea3cf156 100644 --- a/tests/src/python/test_provider_spatialite.py +++ b/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() diff --git a/tests/src/python/test_qgsvectorlayerutils.py b/tests/src/python/test_qgsvectorlayerutils.py index c94154c0a23..a33f1f972c6 100644 --- a/tests/src/python/test_qgsvectorlayerutils.py +++ b/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()