Add provider test to ensure providers reject adding features

with geometry type mismatch
qgis · Feb 17, 2018 · 0a5d486 · 0a5d486
1 parent 7187ae3
commit 0a5d486
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 2 deletions.
diff --git a/src/core/providers/memory/qgsmemoryprovider.cpp b/src/core/providers/memory/qgsmemoryprovider.cpp
@@ -349,7 +349,7 @@ bool QgsMemoryProvider::addFeatures( QgsFeatureList &flist, Flags )
 
   int fieldCount = mFields.count();
 
-  // TODO: sanity checks of fields and geometries
+  // TODO: sanity checks of fields
   for ( QgsFeatureList::iterator it = flist.begin(); it != flist.end(); ++it )
   {
     it->setId( mNextFeatureId );
@@ -368,6 +368,16 @@ bool QgsMemoryProvider::addFeatures( QgsFeatureList &flist, Flags )
     else if ( it->attributes().count() > fieldCount )
     {
       // too many attributes
+      pushError( tr( "Feature has too many attributes (expecting %1, received %2)" ).arg( fieldCount ).arg( it->attributes().count() ) );
+      result = false;
+      continue;
+    }
+
+    if ( it->hasGeometry() && QgsWkbTypes::geometryType( it->geometry().wkbType() ) !=
+         QgsWkbTypes::geometryType( mWkbType ) )
+    {
+      pushError( tr( "Could not add feature with geometry type %1 to layer of type %2" ).arg( QgsWkbTypes::displayString( it->geometry().wkbType() ),
+                 QgsWkbTypes::displayString( mWkbType ) ) );
       result = false;
       continue;
     }

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -1328,7 +1328,10 @@ bool QgsOgrProvider::addFeaturePrivate( QgsFeature &f, Flags flags )
   {
     // don't try to set field from attribute map if it's not present in layer
     if ( ogrAttId >= fdef.GetFieldCount() )
+    {
+      pushError( tr( "Feature has too many attributes (expecting %1, received %2)" ).arg( fdef.GetFieldCount() ).arg( f.attributes().count() ) );
       return false;
+    }
 
     //if(!s.isEmpty())
     // continue;

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -2150,6 +2150,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
 
       if ( attrs.count() > mAttributeFields.count() )
       {
+        pushError( tr( "Feature has too many attributes (expecting %1, received %2)" ).arg( mAttributeFields.count() ).arg( attrs.count() ) );
         returnvalue = false;
         continue;
       }

diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -482,7 +482,7 @@ def testAddFeatureExtraAttributes(self):
         f1 = QgsFeature()
         f1.setAttributes([6, -220, NULL, 'String', '15'])
         f2 = QgsFeature()
-        f1.setAttributes([7, -230, NULL, 'String', '15', 15, 16, 17])
+        f2.setAttributes([7, -230, NULL, 'String', '15', 15, 16, 17])
 
         result, added = l.dataProvider().addFeatures([f1, f2])
         self.assertFalse(result, 'Provider returned True to addFeatures with extra attributes. Providers should reject these features.')
@@ -491,6 +491,30 @@ def testAddFeatureExtraAttributes(self):
         added = [f for f in l.dataProvider().getFeatures() if f['pk'] == 7]
         self.assertFalse(added)
 
+    def testAddFeatureWrongGeomType(self):
+        if not getattr(self, 'getEditableLayer', None):
+            return
+
+        l = self.getEditableLayer()
+        self.assertTrue(l.isValid())
+
+        if not l.dataProvider().capabilities() & QgsVectorDataProvider.AddFeatures:
+            return
+
+        # test that adding features with incorrect geometry type rejects the feature
+        # we be more tricky and also add a valid feature to stress test the provider
+        f1 = QgsFeature()
+        f1.setGeometry(QgsGeometry.fromWkt('LineString (-72.345 71.987, -80 80)'))
+        f2 = QgsFeature()
+        f2.setGeometry(QgsGeometry.fromWkt('Point (-72.345 71.987)'))
+
+        result, added = l.dataProvider().addFeatures([f1, f2])
+        self.assertFalse(result, 'Provider returned True to addFeatures with incorrect geometry type. Providers should reject these features.')
+
+        # make sure feature was not added
+        added = [f for f in l.dataProvider().getFeatures() if f['pk'] == 7]
+        self.assertFalse(added)
+
     def testAddFeaturesUpdateExtent(self):
         if not getattr(self, 'getEditableLayer', None):
             return