Add method to perform invalid geometry checking in QgsFeatureRequest

Allows requests to specify how invalid geometries should be
handled. Default is to perform no geometry validity checking.

python/core/qgsfeaturerequest.sip

@@ -26,6 +26,14 @@ class QgsFeatureRequest
       FilterFids        //!< Filter using feature IDs
     };
 
+    //! Handling of features with invalid geometries
+    enum InvalidGeometryCheck
+    {
+      GeometryNoCheck,
+      GeometrySkipInvalid,
+      GeometryAbortOnInvalid,
+    };
+
     /**
      * The OrderByClause class represents an order by clause for a QgsFeatureRequest.
      *
@@ -196,6 +204,22 @@ class QgsFeatureRequest
     //! Get feature IDs that should be fetched.
     const QgsFeatureIds& filterFids() const;
 
+    /**
+     * Sets invalid geometry checking behavior.
+     * \note Invalid geometry checking is not performed when retrieving features
+     * directly from a QgsVectorDataProvider.
+     * \see invalidGeometryCheck()
+     * \since QGIS 3.0
+     */
+    QgsFeatureRequest &setInvalidGeometryCheck( InvalidGeometryCheck check );
+
+    /**
+     * Returns the invalid geometry checking behavior.
+     * \see setInvalidGeometryCheck()
+     * \since QGIS 3.0
+     */
+    InvalidGeometryCheck invalidGeometryCheck() const;
+
     /** Set the filter expression. {@see QgsExpression}
      * @param expression expression string
      * @see filterExpression

src/core/qgsfeaturerequest.cpp

@@ -76,6 +76,7 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
   {
     mFilterExpression.reset( nullptr );
   }
+  mInvalidGeometryFilter = rh.mInvalidGeometryFilter;
   mExpressionContext = rh.mExpressionContext;
   mAttrs = rh.mAttrs;
   mSimplifyMethod = rh.mSimplifyMethod;
@@ -104,6 +105,12 @@ QgsFeatureRequest &QgsFeatureRequest::setFilterFids( const QgsFeatureIds &fids )
   return *this;
 }
 
+QgsFeatureRequest &QgsFeatureRequest::setInvalidGeometryCheck( QgsFeatureRequest::InvalidGeometryCheck check )
+{
+  mInvalidGeometryFilter = check;
+  return *this;
+}
+
 QgsFeatureRequest &QgsFeatureRequest::setFilterExpression( const QString &expression )
 {
   mFilter = FilterExpression;

src/core/qgsfeaturerequest.h

@@ -85,6 +85,14 @@ class CORE_EXPORT QgsFeatureRequest
       FilterFids        //!< Filter using feature IDs
     };
 
+    //! Handling of features with invalid geometries
+    enum InvalidGeometryCheck
+    {
+      GeometryNoCheck = 0, //!< No invalid geometry checking
+      GeometrySkipInvalid = 1, //!< Skip any features with invalid geometry. This requires a slow geometry validity check for every feature.
+      GeometryAbortOnInvalid = 2, //!< Close iterator on encountering any features with invalid geometry. This requires a slow geometry validity check for every feature.
+    };
+
     /** \ingroup core
      * The OrderByClause class represents an order by clause for a QgsFeatureRequest.
      *
@@ -270,6 +278,22 @@ class CORE_EXPORT QgsFeatureRequest
     //! Get feature IDs that should be fetched.
     const QgsFeatureIds &filterFids() const { return mFilterFids; }
 
+    /**
+     * Sets invalid geometry checking behavior.
+     * \note Invalid geometry checking is not performed when retrieving features
+     * directly from a QgsVectorDataProvider.
+     * \see invalidGeometryCheck()
+     * \since QGIS 3.0
+     */
+    QgsFeatureRequest &setInvalidGeometryCheck( InvalidGeometryCheck check );
+
+    /**
+     * Returns the invalid geometry checking behavior.
+     * \see setInvalidGeometryCheck()
+     * \since QGIS 3.0
+     */
+    InvalidGeometryCheck invalidGeometryCheck() const { return mInvalidGeometryFilter; }
+
     /** Set the filter expression. {\see QgsExpression}
      * \param expression expression string
      * \see filterExpression
@@ -415,6 +439,7 @@ class CORE_EXPORT QgsFeatureRequest
     QgsSimplifyMethod mSimplifyMethod;
     long mLimit = -1;
     OrderBy mOrderBy;
+    InvalidGeometryCheck mInvalidGeometryFilter = GeometryNoCheck;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsFeatureRequest::Flags )

src/core/qgsvectorlayerfeatureiterator.cpp

@@ -24,6 +24,7 @@
 #include "qgsexpressioncontext.h"
 #include "qgsdistancearea.h"
 #include "qgsproject.h"
+#include "qgsmessagelog.h"
 
 QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( const QgsVectorLayer *layer )
 {
@@ -241,8 +242,15 @@ bool QgsVectorLayerFeatureIterator::fetchFeature( QgsFeature &f )
     if ( mFetchedFid )
       return false;
     bool res = nextFeatureFid( f );
-    mFetchedFid = true;
-    return res;
+    if ( res && checkGeometry( f ) )
+    {
+      mFetchedFid = true;
+      return res;
+    }
+    else
+    {
+      return false;
+    }
   }
 
   if ( !mRequest.filterRect().isNull() )
@@ -305,6 +313,9 @@ bool QgsVectorLayerFeatureIterator::fetchFeature( QgsFeature &f )
     if ( !( mRequest.flags() & QgsFeatureRequest::NoGeometry ) )
       updateFeatureGeometry( f );
 
+    if ( !checkGeometry( f ) )
+      continue;
+
     return true;
   }
   // no more provider features
@@ -366,6 +377,9 @@ bool QgsVectorLayerFeatureIterator::fetchNextAddedFeature( QgsFeature &f )
       // skip features which are not accepted by the filter
       continue;
 
+    if ( !checkGeometry( *mFetchAddedFeaturesIt ) )
+      continue;
+
     useAddedFeature( *mFetchAddedFeaturesIt, f );
 
     return true;
@@ -416,9 +430,12 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedGeomFeature( QgsFeature &f )
 
     useChangedAttributeFeature( fid, *mFetchChangedGeomIt, f );
 
-    // return complete feature
-    mFetchChangedGeomIt++;
-    return true;
+    if ( checkGeometry( f ) )
+    {
+      // return complete feature
+      mFetchChangedGeomIt++;
+      return true;
+    }
   }
 
   return false; // no more changed geometries
@@ -440,7 +457,7 @@ bool QgsVectorLayerFeatureIterator::fetchNextChangedAttributeFeature( QgsFeature
       addVirtualAttributes( f );
 
     mRequest.expressionContext()->setFeature( f );
-    if ( mRequest.filterExpression()->evaluate( mRequest.expressionContext() ).toBool() )
+    if ( mRequest.filterExpression()->evaluate( mRequest.expressionContext() ).toBool() && checkGeometry( f ) )
     {
       return true;
     }
@@ -658,6 +675,39 @@ void QgsVectorLayerFeatureIterator::createOrderedJoinList()
   }
 }
 
+bool QgsVectorLayerFeatureIterator::checkGeometry( const QgsFeature &feature )
+{
+  if ( !feature.hasGeometry() )
+    return true;
+
+  switch ( mRequest.invalidGeometryCheck() )
+  {
+    case QgsFeatureRequest::GeometryNoCheck:
+      return true;
+
+    case QgsFeatureRequest::GeometrySkipInvalid:
+    {
+      if ( !feature.geometry().isGeosValid() )
+      {
+        QgsMessageLog::logMessage( QObject::tr( "Geometry error: One or more input features have invalid geometry." ), QString(), QgsMessageLog::CRITICAL );
+        return false;
+      }
+      break;
+    }
+
+    case QgsFeatureRequest::GeometryAbortOnInvalid:
+      if ( !feature.geometry().isGeosValid() )
+      {
+        QgsMessageLog::logMessage( QObject::tr( "Geometry error: One or more input features have invalid geometry." ), QString(), QgsMessageLog::CRITICAL);
+        close();
+        return false;
+      }
+      break;
+  }
+
+  return true;
+}
+
 void QgsVectorLayerFeatureIterator::prepareField( int fieldIdx )
 {
   switch ( mSource->mFields.fieldOrigin( fieldIdx ) )

src/core/qgsvectorlayerfeatureiterator.h

@@ -221,6 +221,11 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
     virtual bool providerCanSimplify( QgsSimplifyMethod::MethodType methodType ) const override;
 
     void createOrderedJoinList();
+
+    /**
+     * Performs any geometry validity checking.
+     */
+    bool checkGeometry( const QgsFeature &feature );
 };
 
 #endif // QGSVECTORLAYERFEATUREITERATOR_H

tests/src/python/test_qgsfeatureiterator.py

@@ -16,7 +16,14 @@ import qgis  # NOQA
 
 import os
 
-from qgis.core import QgsVectorLayer, QgsFeatureRequest, QgsFeature, QgsField, NULL, QgsProject, QgsVectorLayerJoinInfo
+from qgis.core import (QgsVectorLayer,
+                       QgsFeatureRequest,
+                       QgsFeature,
+                       QgsField,
+                       NULL,
+                       QgsProject,
+                       QgsVectorLayerJoinInfo,
+                       QgsGeometry)
 from qgis.testing import start_app, unittest
 from qgis.PyQt.QtCore import QVariant
 
@@ -273,6 +280,33 @@ class TestQgsFeatureIterator(unittest.TestCase):
 
         QgsProject.instance().removeMapLayers([layer.id(), joinLayer.id()])
 
+    def test_invalidGeometryFilter(self):
+        layer = QgsVectorLayer(
+            "Polygon?field=x:string",
+            "joinlayer", "memory")
+
+        # add some features, one has invalid geometry
+        pr = layer.dataProvider()
+        f1 = QgsFeature()
+        f1.setAttributes(["a"])
+        f1.setGeometry(QgsGeometry.fromWkt('Polygon((0 0, 1 0, 1 1, 0 1, 0 0))')) # valid
+        f2 = QgsFeature()
+        f2.setAttributes(["b"])
+        f2.setGeometry(QgsGeometry.fromWkt('Polygon((0 0, 1 0, 0 1, 1 1, 0 0))'))  # invalid
+        f3 = QgsFeature()
+        f3.setAttributes(["c"])
+        f3.setGeometry(QgsGeometry.fromWkt('Polygon((0 0, 1 0, 1 1, 0 1, 0 0))'))  # valid
+        self.assertTrue(pr.addFeatures([f1, f2, f3]))
+
+        res = [f['x'] for f in
+               layer.getFeatures(QgsFeatureRequest().setInvalidGeometryCheck(QgsFeatureRequest.GeometryNoCheck))]
+        self.assertEqual(res, ['a', 'b', 'c'])
+        res = [f['x'] for f in
+               layer.getFeatures(QgsFeatureRequest().setInvalidGeometryCheck(QgsFeatureRequest.GeometrySkipInvalid))]
+        self.assertEqual(res, ['a', 'c'])
+        res = [f['x'] for f in
+               layer.getFeatures(QgsFeatureRequest().setInvalidGeometryCheck(QgsFeatureRequest.GeometryAbortOnInvalid))]
+        self.assertEqual(res, ['a'])
 
 if __name__ == '__main__':
     unittest.main()