Skip to content
Permalink
Browse files
Merge pull request #4283 from nyalldawson/filterrect
Remove QgsFeatureRequest::FilterRect
  • Loading branch information
nyalldawson committed Mar 21, 2017
2 parents 2e9f996 + f3e1772 commit b7d2b9fd72479b68812572f2f20d87a7d176621c
@@ -1062,6 +1062,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}
----------------
@@ -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
@@ -63,7 +63,6 @@ bool QgsCacheIndexFeatureId::getCacheIterator( QgsFeatureIterator &featureIterat
break;
}
case QgsFeatureRequest::FilterNone:
case QgsFeatureRequest::FilterRect:
case QgsFeatureRequest::FilterExpression:
{
if ( C->hasFullCache() )
@@ -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;
}

@@ -245,14 +222,17 @@ QgsFeatureRequest &QgsFeatureRequest::setSimplifyMethod( const QgsSimplifyMethod

bool QgsFeatureRequest::acceptFeature( const QgsFeature &feature )
{
if ( !mFilterRect.isNull() )
{
if ( !feature.hasGeometry() || !feature.geometry().intersects( mFilterRect ) )
return false;
}

switch ( mFilter )
{
case QgsFeatureRequest::FilterNone:
return true;

case QgsFeatureRequest::FilterRect:
return ( feature.hasGeometry() && feature.geometry().intersects( mFilterRect ) );

case QgsFeatureRequest::FilterFid:
return ( feature.id() == mFilterFid );

@@ -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
@@ -404,20 +403,17 @@ class CORE_EXPORT QgsFeatureRequest
*/
bool acceptFeature( const QgsFeature &feature );

// TODO: in future
// void setFilterNativeExpression(con QString& expr); // using provider's SQL (if supported)

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;
};

@@ -310,7 +310,6 @@ bool QgsVectorLayerCache::canUseCacheForRequest( const QgsFeatureRequest &featur
break;
}
case QgsFeatureRequest::FilterNone:
case QgsFeatureRequest::FilterRect:
case QgsFeatureRequest::FilterExpression:
{
if ( mFullCache )
@@ -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();
@@ -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() )
{
@@ -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 )
@@ -141,6 +141,11 @@ def assert_query(self, provider, expression, expected):
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression).setSubsetOfAttributes([0]))])
assert set(expected) == result, 'Expected {} and got {} when testing expression "{}" using empty attribute subset'.format(set(expected), result, expression)

# test that results match QgsFeatureRequest.acceptFeature
request = QgsFeatureRequest().setFilterExpression(expression)
for f in provider.getFeatures():
self.assertEqual(request.acceptFeature(f), f['pk'] in expected)

def runGetFeatureTests(self, provider):
assert len([f for f in provider.getFeatures()]) == 5
self.assert_query(provider, 'name ILIKE \'QGIS\'', [])
@@ -485,6 +490,11 @@ def testGetFeaturesFidTests(self):
expected = [id]
assert result == expected, 'Expected {} and got {} when testing for feature ID filter'.format(expected, result)

# test that results match QgsFeatureRequest.acceptFeature
request = QgsFeatureRequest().setFilterFid(id)
for f in self.provider.getFeatures():
self.assertEqual(request.acceptFeature(f), f.id() == id)

# bad features
it = self.provider.getFeatures(QgsFeatureRequest().setFilterFid(-99999999))
feature = QgsFeature(5)
@@ -503,6 +513,10 @@ def testGetFeaturesFidsTests(self):
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
self.assertTrue(all_valid)

# test that results match QgsFeatureRequest.acceptFeature
for f in self.provider.getFeatures():
self.assertEqual(request.acceptFeature(f), f.id() in expected)

result = set([f.id() for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFids([fids[1], fids[3], fids[4]]))])
expected = set([fids[1], fids[3], fids[4]])
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
@@ -545,6 +559,10 @@ def testGetFeaturesFilterRectTests(self):
assert set(features) == set([2, 4]), 'Got {} instead'.format(features)
self.assertTrue(all_valid)

# test that results match QgsFeatureRequest.acceptFeature
for f in self.provider.getFeatures():
self.assertEqual(request.acceptFeature(f), f['pk'] in set([2, 4]))

# test with an empty rectangle
extent = QgsRectangle()
request = QgsFeatureRequest().setFilterRect(extent)
@@ -590,6 +608,20 @@ def testRectAndExpression(self):
assert set(expected) == result, 'Expected {} and got {} when testing for combination of filterRect and expression'.format(set(expected), result)
self.assertTrue(all_valid)

# shouldn't matter what order this is done in
request = QgsFeatureRequest().setFilterRect(extent).setFilterExpression('"cnt">200')
result = set([f['pk'] for f in self.provider.getFeatures(request)])
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
expected = [4]
assert set(
expected) == result, 'Expected {} and got {} when testing for combination of filterRect and expression'.format(
set(expected), result)
self.assertTrue(all_valid)

# test that results match QgsFeatureRequest.acceptFeature
for f in self.provider.getFeatures():
self.assertEqual(request.acceptFeature(f), f['pk'] in expected)

def testGetFeaturesLimit(self):
it = self.provider.getFeatures(QgsFeatureRequest().setLimit(2))
features = [f['pk'] for f in it]

0 comments on commit b7d2b9f

Please sign in to comment.