Merge pull request #5679 from elpaso/bugfix-16935-gpkg-split-features

[bugfix] Editing a GPKG layer added by d&d leads

src/providers/ogr/qgsogrprovider.cpp

@@ -935,13 +935,20 @@ void QgsOgrProvider::loadFields()
                      fdef.GetFieldIndex( fidColumn ) < 0;
   if ( mFirstFieldIsFid )
   {
+    QgsField fidField(
+      fidColumn,
+      QVariant::LongLong,
+      QStringLiteral( "Integer64" )
+    );
+    // Set constraints for feature id
+    QgsFieldConstraints constraints = fidField.constraints();
+    constraints.setConstraint( QgsFieldConstraints::ConstraintUnique, QgsFieldConstraints::ConstraintOriginProvider );
+    constraints.setConstraint( QgsFieldConstraints::ConstraintNotNull, QgsFieldConstraints::ConstraintOriginProvider );
+    fidField.setConstraints( constraints );
     mAttributeFields.append(
-      QgsField(
-        fidColumn,
-        QVariant::LongLong,
-        QStringLiteral( "Integer64" )
-      )
+      fidField
     );
+    mDefaultValues.insert( 0, tr( "Autogenerate" ) );
   }
 
   for ( int i = 0; i < fdef.GetFieldCount(); ++i )
@@ -1195,6 +1202,26 @@ QVariant QgsOgrProvider::defaultValue( int fieldId ) const
   return resultVar;
 }
 
+QString QgsOgrProvider::defaultValueClause( int fieldIndex ) const
+{
+  return mDefaultValues.value( fieldIndex, QString() );
+}
+
+bool QgsOgrProvider::skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant &value ) const
+{
+  Q_UNUSED( constraint );
+  // If the field is a fid, skip in case it's the default value
+  if ( fieldIndex == 0 && mFirstFieldIsFid )
+  {
+    return ! mDefaultValues.value( fieldIndex ).isEmpty();
+  }
+  else
+  {
+    // stricter check
+    return mDefaultValues.contains( fieldIndex ) && mDefaultValues.value( fieldIndex ) == value.toString() && !value.isNull();
+  }
+}
+
 void QgsOgrProvider::updateExtents()
 {
   invalidateCachedExtent( true );

src/providers/ogr/qgsogrprovider.h

@@ -108,7 +108,9 @@ class QgsOgrProvider : public QgsVectorDataProvider
     virtual long featureCount() const override;
     virtual QgsFields fields() const override;
     virtual QgsRectangle extent() const override;
-    QVariant defaultValue( int fieldId ) const override;
+    virtual QVariant defaultValue( int fieldId ) const override;
+    virtual QString defaultValueClause( int fieldIndex ) const override;
+    virtual bool skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant &value = QVariant() ) const override;
     virtual void updateExtents() override;
     virtual bool addFeatures( QgsFeatureList &flist, QgsFeatureSink::Flags flags = 0 ) override;
     virtual bool deleteFeatures( const QgsFeatureIds &id ) override;

tests/src/python/test_provider_ogr_gpkg.py

@@ -12,16 +12,17 @@
 # This will get replaced with a git SHA1 when you do a git archive
 __revision__ = '$Format:%H$'
 
-import qgis  # NOQA
-
 import os
-import tempfile
 import shutil
 import sys
+import tempfile
 import time
-from osgeo import gdal, ogr
 
-from qgis.core import QgsVectorLayer, QgsVectorLayerExporter, QgsFeature, QgsGeometry, QgsRectangle, QgsSettings
+import qgis  # NOQA
+from osgeo import gdal, ogr
+from qgis.core import (QgsFeature, QgsFieldConstraints, QgsGeometry,
+                       QgsRectangle, QgsSettings, QgsVectorLayer,
+                       QgsVectorLayerExporter, QgsPointXY)
 from qgis.PyQt.QtCore import QCoreApplication
 from qgis.testing import start_app, unittest
 
@@ -632,6 +633,58 @@ def testGeopackageLargeFID(self):
         self.assertTrue(vl.deleteFeature(1234567890123))
         self.assertTrue(vl.commitChanges())
 
+    def test_AddFeatureNullFid(self):
+        """Test gpkg feature with NULL fid can be added"""
+        tmpfile = os.path.join(self.basetestpath, 'testGeopackageSplitFeatures.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('test', geom_type=ogr.wkbPolygon)
+        lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString))
+        ds = None
+
+        layer = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+
+        # Check that pk field has unique constraint
+        fields = layer.fields()
+        pkfield = fields.at(0)
+        self.assertTrue(pkfield.constraints().constraints() & QgsFieldConstraints.ConstraintUnique)
+
+        # Test add feature with default Fid (NULL)
+        layer.startEditing()
+        f = QgsFeature()
+        feat = QgsFeature(layer.fields())
+        feat.setGeometry(QgsGeometry.fromWkt('Polygon ((0 0, 0 1, 1 1, 1 0, 0 0))'))
+        feat.setAttribute(1, 'test_value')
+        layer.addFeature(feat)
+        self.assertTrue(layer.commitChanges())
+        self.assertEqual(layer.featureCount(), 1)
+
+    def test_SplitFeature(self):
+        """Test gpkg feature can be split"""
+        tmpfile = os.path.join(self.basetestpath, 'testGeopackageSplitFeatures.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('test', geom_type=ogr.wkbPolygon)
+        lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString))
+        f = ogr.Feature(lyr.GetLayerDefn())
+        f.SetGeometry(ogr.CreateGeometryFromWkt('POLYGON ((0 0,0 1,1 1,1 0,0 0))'))
+        lyr.CreateFeature(f)
+        f = None
+        ds = None
+
+        # Split features
+        layer = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+        self.assertTrue(layer.isValid())
+        self.assertTrue(layer.isSpatial())
+        self.assertEqual([f for f in layer.getFeatures()][0].geometry().asWkt(), 'Polygon ((0 0, 0 1, 1 1, 1 0, 0 0))')
+        layer.startEditing()
+        self.assertEqual(layer.splitFeatures([QgsPointXY(0.5, 0), QgsPointXY(0.5, 1)], 0), 0)
+        self.assertTrue(layer.commitChanges())
+        self.assertEqual(layer.featureCount(), 2)
+
+        layer = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+        self.assertEqual(layer.featureCount(), 2)
+        self.assertEqual([f for f in layer.getFeatures()][0].geometry().asWkt(), 'Polygon ((0.5 0, 0.5 1, 1 1, 1 0, 0.5 0))')
+        self.assertEqual([f for f in layer.getFeatures()][1].geometry().asWkt(), 'Polygon ((0.5 1, 0.5 0, 0 0, 0 1, 0.5 1))')
+
 
 if __name__ == '__main__':
     unittest.main()

tests/src/python/test_provider_ogr_sqlite.py

@@ -19,7 +19,7 @@
 import shutil
 from osgeo import gdal, ogr
 
-from qgis.core import QgsVectorLayer, QgsFeature, QgsFeatureRequest, QgsFieldConstraints, NULL
+from qgis.core import QgsVectorLayer, QgsFeature, QgsFeatureRequest, QgsFieldConstraints, QgsFieldConstraints, QgsPointXY, NULL
 from qgis.testing import start_app, unittest
 from qgis.PyQt.QtCore import QDate, QTime, QDateTime
 
@@ -147,13 +147,13 @@ def testNotNullConstraint(self):
         self.assertEqual(vl.dataProvider().fieldConstraints(-1), QgsFieldConstraints.Constraints())
         self.assertEqual(vl.dataProvider().fieldConstraints(1001), QgsFieldConstraints.Constraints())
 
-        self.assertFalse(vl.dataProvider().fieldConstraints(0) & QgsFieldConstraints.ConstraintNotNull)
+        self.assertTrue(vl.dataProvider().fieldConstraints(0) & QgsFieldConstraints.ConstraintNotNull)
         self.assertFalse(vl.dataProvider().fieldConstraints(1) & QgsFieldConstraints.ConstraintNotNull)
         self.assertTrue(vl.dataProvider().fieldConstraints(2) & QgsFieldConstraints.ConstraintNotNull)
 
         # test that constraints have been saved to fields correctly
         fields = vl.fields()
-        self.assertFalse(fields.at(0).constraints().constraints() & QgsFieldConstraints.ConstraintNotNull)
+        self.assertTrue(fields.at(0).constraints().constraints() & QgsFieldConstraints.ConstraintNotNull)
         self.assertFalse(fields.at(1).constraints().constraints() & QgsFieldConstraints.ConstraintNotNull)
         self.assertTrue(fields.at(2).constraints().constraints() & QgsFieldConstraints.ConstraintNotNull)
         self.assertEqual(fields.at(2).constraints().constraintOrigin(QgsFieldConstraints.ConstraintNotNull), QgsFieldConstraints.ConstraintOriginProvider)
@@ -270,6 +270,38 @@ def testSubsetStringFids(self):
         vl.reload()
         self.assertTrue(vl.fields().at(vl.fields().count() - 1).name() == "orig_ogc_fid")
 
+    def test_SplitFeature(self):
+        """Test sqlite feature can be split"""
+        tmpfile = os.path.join(self.basetestpath, 'testGeopackageSplitFeatures.sqlite')
+        ds = ogr.GetDriverByName('SQlite').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('test', geom_type=ogr.wkbPolygon)
+        lyr.CreateField(ogr.FieldDefn('str_field', ogr.OFTString))
+        f = ogr.Feature(lyr.GetLayerDefn())
+        f.SetGeometry(ogr.CreateGeometryFromWkt('POLYGON ((0 0,0 1,1 1,1 0,0 0))'))
+        lyr.CreateFeature(f)
+        f = None
+        ds = None
+
+        layer = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+
+        # Check that pk field has unique constraint
+        fields = layer.fields()
+        pkfield = fields.at(0)
+        self.assertTrue(pkfield.constraints().constraints() & QgsFieldConstraints.ConstraintUnique)
+
+        self.assertTrue(layer.isValid())
+        self.assertTrue(layer.isSpatial())
+        self.assertEqual([f for f in layer.getFeatures()][0].geometry().asWkt(), 'Polygon ((0 0, 0 1, 1 1, 1 0, 0 0))')
+        layer.startEditing()
+        self.assertEqual(layer.splitFeatures([QgsPointXY(0.5, 0), QgsPointXY(0.5, 1)], 0), 0)
+        self.assertTrue(layer.commitChanges())
+        self.assertEqual(layer.featureCount(), 2)
+
+        layer = QgsVectorLayer(u'{}'.format(tmpfile) + "|layername=" + "test", 'test', u'ogr')
+        self.assertEqual(layer.featureCount(), 2)
+        self.assertEqual([f for f in layer.getFeatures()][0].geometry().asWkt(), 'Polygon ((0.5 0, 0.5 1, 1 1, 1 0, 0.5 0))')
+        self.assertEqual([f for f in layer.getFeatures()][1].geometry().asWkt(), 'Polygon ((0.5 1, 0.5 0, 0 0, 0 1, 0.5 1))')
+
 
 if __name__ == '__main__':
     unittest.main()