Remove QgsFeatureRequest::FilterRect

This enum value has not been in use since filter rects were
separated from other filter types. Leaving it in API is confusing
and leads to incorrect use.

doc/api_break.dox

@@ -1049,6 +1049,9 @@ QgsFeatureRequest        {#qgis_api_break_3_0_QgsFeatureRequest}
 -----------------
 
 - AllAttributes was renamed to ALL_ATTRIBUTES
+- FilterRect was removed. This enum value was unused, and QgsFeatureRequest.filterRect() should be used
+instead. Check for a null rectangle returned by filterRect() to determine whether a filter rect
+is in place.
 
 QgsFieldCombobox        {#qgis_api_break_3_0_QgsFieldCombobox}
 ----------------

python/core/qgsfeaturerequest.sip

@@ -21,7 +21,6 @@ class QgsFeatureRequest
     enum FilterType
     {
       FilterNone,       //!< No filter is applied
-      FilterRect,       //!< Obsolete, will be ignored. If a filterRect is set it will be used anyway. Filter using a rectangle, no need to set NoGeometry. Instead check for request.filterRect().isNull()
       FilterFid,        //!< Filter using feature ID
       FilterExpression, //!< Filter using expression
       FilterFids        //!< Filter using feature IDs

src/core/qgscacheindexfeatureid.cpp

@@ -63,7 +63,6 @@ bool QgsCacheIndexFeatureId::getCacheIterator( QgsFeatureIterator &featureIterat
       break;
     }
     case QgsFeatureRequest::FilterNone:
-    case QgsFeatureRequest::FilterRect:
     case QgsFeatureRequest::FilterExpression:
     {
       if ( C->hasFullCache() )

src/core/qgsfeaturerequest.cpp

@@ -23,51 +23,36 @@
 const QString QgsFeatureRequest::ALL_ATTRIBUTES = QStringLiteral( "#!allattributes!#" );
 
 QgsFeatureRequest::QgsFeatureRequest()
-  : mFilter( FilterNone )
-  , mFilterFid( -1 )
-  , mFilterExpression( nullptr )
-  , mFlags( nullptr )
-  , mLimit( -1 )
+  : mFlags( nullptr )
 {
 }
 
 QgsFeatureRequest::QgsFeatureRequest( QgsFeatureId fid )
   : mFilter( FilterFid )
   , mFilterFid( fid )
-  , mFilterExpression( nullptr )
   , mFlags( nullptr )
-  , mLimit( -1 )
 {
 }
 
 QgsFeatureRequest::QgsFeatureRequest( const QgsFeatureIds &fids )
   : mFilter( FilterFids )
-  , mFilterFid( -1 )
   , mFilterFids( fids )
-  , mFilterExpression( nullptr )
   , mFlags( nullptr )
-  , mLimit( -1 )
 {
 
 }
 
 QgsFeatureRequest::QgsFeatureRequest( const QgsRectangle &rect )
-  : mFilter( FilterRect )
-  , mFilterRect( rect )
-  , mFilterFid( -1 )
-  , mFilterExpression( nullptr )
+  : mFilterRect( rect )
   , mFlags( nullptr )
-  , mLimit( -1 )
 {
 }
 
 QgsFeatureRequest::QgsFeatureRequest( const QgsExpression &expr, const QgsExpressionContext &context )
   : mFilter( FilterExpression )
-  , mFilterFid( -1 )
   , mFilterExpression( new QgsExpression( expr ) )
   , mExpressionContext( context )
   , mFlags( nullptr )
-  , mLimit( -1 )
 {
 }
 
@@ -85,11 +70,11 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
   mFilterFids = rh.mFilterFids;
   if ( rh.mFilterExpression )
   {
-    mFilterExpression = new QgsExpression( *rh.mFilterExpression );
+    mFilterExpression.reset( new QgsExpression( *rh.mFilterExpression ) );
   }
   else
   {
-    mFilterExpression = nullptr;
+    mFilterExpression.reset( nullptr );
   }
   mExpressionContext = rh.mExpressionContext;
   mAttrs = rh.mAttrs;
@@ -99,15 +84,8 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
   return *this;
 }
 
-QgsFeatureRequest::~QgsFeatureRequest()
-{
-  delete mFilterExpression;
-}
-
 QgsFeatureRequest &QgsFeatureRequest::setFilterRect( const QgsRectangle &rect )
 {
-  if ( mFilter == FilterNone )
-    mFilter = FilterRect;
   mFilterRect = rect;
   return *this;
 }
@@ -129,8 +107,7 @@ QgsFeatureRequest &QgsFeatureRequest::setFilterFids( const QgsFeatureIds &fids )
 QgsFeatureRequest &QgsFeatureRequest::setFilterExpression( const QString &expression )
 {
   mFilter = FilterExpression;
-  delete mFilterExpression;
-  mFilterExpression = new QgsExpression( expression );
+  mFilterExpression.reset( new QgsExpression( expression ) );
   return *this;
 }
 

src/core/qgsfeaturerequest.h

@@ -18,6 +18,7 @@
 #include "qgis_core.h"
 #include <QFlags>
 #include <QList>
+#include <memory>
 
 #include "qgsfeature.h"
 #include "qgsrectangle.h"
@@ -79,7 +80,6 @@ class CORE_EXPORT QgsFeatureRequest
     enum FilterType
     {
       FilterNone,       //!< No filter is applied
-      FilterRect,       //!< Obsolete, will be ignored. If a filterRect is set it will be used anyway. Filter using a rectangle, no need to set NoGeometry. Instead check for request.filterRect().isNull()
       FilterFid,        //!< Filter using feature ID
       FilterExpression, //!< Filter using expression
       FilterFids        //!< Filter using feature IDs
@@ -242,22 +242,21 @@ class CORE_EXPORT QgsFeatureRequest
     //! Assignment operator
     QgsFeatureRequest &operator=( const QgsFeatureRequest &rh );
 
-    ~QgsFeatureRequest();
-
     /**
      * Return the filter type which is currently set on this request
      *
      * @return Filter type
      */
-    FilterType filterType() const { if ( mFilter == FilterNone && !mFilterRect.isNull() ) return FilterRect; else return mFilter; }
+    FilterType filterType() const { return mFilter; }
 
     /**
      * Set rectangle from which features will be taken. Empty rectangle removes the filter.
      */
     QgsFeatureRequest &setFilterRect( const QgsRectangle &rect );
 
     /**
-     * Get the rectangle from which features will be taken.
+     * Get the rectangle from which features will be taken. If the returned
+     * rectangle is null, then no filter rectangle is set.
      */
     const QgsRectangle &filterRect() const { return mFilterRect; }
 
@@ -282,7 +281,7 @@ class CORE_EXPORT QgsFeatureRequest
      * @see setFilterExpression
      * @see expressionContext
      */
-    QgsExpression *filterExpression() const { return mFilterExpression; }
+    QgsExpression *filterExpression() const { return mFilterExpression.get(); }
 
     /** Modifies the existing filter expression to add an additional expression filter. The
      * filter expressions are combined using AND, so only features matching both
@@ -405,16 +404,16 @@ class CORE_EXPORT QgsFeatureRequest
     bool acceptFeature( const QgsFeature &feature );
 
   protected:
-    FilterType mFilter;
+    FilterType mFilter = FilterNone;
     QgsRectangle mFilterRect;
-    QgsFeatureId mFilterFid;
+    QgsFeatureId mFilterFid = -1;
     QgsFeatureIds mFilterFids;
-    QgsExpression *mFilterExpression = nullptr;
+    std::unique_ptr< QgsExpression > mFilterExpression;
     QgsExpressionContext mExpressionContext;
     Flags mFlags;
     QgsAttributeList mAttrs;
     QgsSimplifyMethod mSimplifyMethod;
-    long mLimit;
+    long mLimit = -1;
     OrderBy mOrderBy;
 };
 

src/core/qgsvectorlayercache.cpp

@@ -310,7 +310,6 @@ bool QgsVectorLayerCache::canUseCacheForRequest( const QgsFeatureRequest &featur
       break;
     }
     case QgsFeatureRequest::FilterNone:
-    case QgsFeatureRequest::FilterRect:
     case QgsFeatureRequest::FilterExpression:
     {
       if ( mFullCache )

src/core/qgsvectorlayerfeatureiterator.cpp

@@ -191,7 +191,7 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
         providerLimit += mSource->mChangedAttributeValues.size();
       }
 
-      if ( mProviderRequest.filterType() == QgsFeatureRequest::FilterExpression || mProviderRequest.filterType() == QgsFeatureRequest::FilterRect )
+      if ( mProviderRequest.filterType() == QgsFeatureRequest::FilterExpression || !mProviderRequest.filterRect().isNull() )
       {
         // geometry changes may mean some features no longer match expression or rect, so increase limit sent to provider
         providerLimit += mSource->mChangedGeometries.size();

src/providers/arcgisrest/qgsafsfeatureiterator.cpp

@@ -73,7 +73,7 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
   else
   {
     QgsRectangle filterRect = mSource->provider()->extent();
-    if ( mRequest.filterType() == QgsFeatureRequest::FilterRect )
+    if ( !mRequest.filterRect().isNull() )
       filterRect = filterRect.intersect( &mRequest.filterRect() );
     while ( mFeatureIterator < mSource->provider()->featureCount() )
     {

src/providers/oracle/qgsoraclefeatureiterator.cpp

@@ -142,9 +142,6 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource *sour
       //handled below
       break;
 
-    case QgsFeatureRequest::FilterRect:
-      // Handled in the if-statement above
-      break;
   }
 
   if ( mSource->mRequestedGeomType != QgsWkbTypes::Unknown && mSource->mRequestedGeomType != mSource->mDetectedGeomType )