Skip to content

Commit b7d2b9f

Browse files
nyalldawsonnyalldawson
authoredMar 21, 2017
Merge pull request #4283 from nyalldawson/filterrect
Remove QgsFeatureRequest::FilterRect
·
ltr-3_40final-3_0_0
2 parents 2e9f996 + f3e1772 commit b7d2b9f

File tree

10 files changed

+57
-52
lines changed

10 files changed

+57
-52
lines changed
 

‎doc/api_break.dox

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,9 @@ QgsFeatureRequest {#qgis_api_break_3_0_QgsFeatureRequest}
10621062
-----------------
10631063

10641064
- AllAttributes was renamed to ALL_ATTRIBUTES
1065+
- FilterRect was removed. This enum value was unused, and QgsFeatureRequest.filterRect() should be used
1066+
instead. Check for a null rectangle returned by filterRect() to determine whether a filter rect
1067+
is in place.
10651068

10661069
QgsFieldCombobox {#qgis_api_break_3_0_QgsFieldCombobox}
10671070
----------------

‎python/core/qgsfeaturerequest.sip

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ class QgsFeatureRequest
2121
enum FilterType
2222
{
2323
FilterNone, //!< No filter is applied
24-
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()
2524
FilterFid, //!< Filter using feature ID
2625
FilterExpression, //!< Filter using expression
2726
FilterFids //!< Filter using feature IDs

‎src/core/qgscacheindexfeatureid.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ bool QgsCacheIndexFeatureId::getCacheIterator( QgsFeatureIterator &featureIterat
6363
break;
6464
}
6565
case QgsFeatureRequest::FilterNone:
66-
case QgsFeatureRequest::FilterRect:
6766
case QgsFeatureRequest::FilterExpression:
6867
{
6968
if ( C->hasFullCache() )

‎src/core/qgsfeaturerequest.cpp

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,51 +23,36 @@
2323
const QString QgsFeatureRequest::ALL_ATTRIBUTES = QStringLiteral( "#!allattributes!#" );
2424

2525
QgsFeatureRequest::QgsFeatureRequest()
26-
: mFilter( FilterNone )
27-
, mFilterFid( -1 )
28-
, mFilterExpression( nullptr )
29-
, mFlags( nullptr )
30-
, mLimit( -1 )
26+
: mFlags( nullptr )
3127
{
3228
}
3329

3430
QgsFeatureRequest::QgsFeatureRequest( QgsFeatureId fid )
3531
: mFilter( FilterFid )
3632
, mFilterFid( fid )
37-
, mFilterExpression( nullptr )
3833
, mFlags( nullptr )
39-
, mLimit( -1 )
4034
{
4135
}
4236

4337
QgsFeatureRequest::QgsFeatureRequest( const QgsFeatureIds &fids )
4438
: mFilter( FilterFids )
45-
, mFilterFid( -1 )
4639
, mFilterFids( fids )
47-
, mFilterExpression( nullptr )
4840
, mFlags( nullptr )
49-
, mLimit( -1 )
5041
{
5142

5243
}
5344

5445
QgsFeatureRequest::QgsFeatureRequest( const QgsRectangle &rect )
55-
: mFilter( FilterRect )
56-
, mFilterRect( rect )
57-
, mFilterFid( -1 )
58-
, mFilterExpression( nullptr )
46+
: mFilterRect( rect )
5947
, mFlags( nullptr )
60-
, mLimit( -1 )
6148
{
6249
}
6350

6451
QgsFeatureRequest::QgsFeatureRequest( const QgsExpression &expr, const QgsExpressionContext &context )
6552
: mFilter( FilterExpression )
66-
, mFilterFid( -1 )
6753
, mFilterExpression( new QgsExpression( expr ) )
6854
, mExpressionContext( context )
6955
, mFlags( nullptr )
70-
, mLimit( -1 )
7156
{
7257
}
7358

@@ -85,11 +70,11 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
8570
mFilterFids = rh.mFilterFids;
8671
if ( rh.mFilterExpression )
8772
{
88-
mFilterExpression = new QgsExpression( *rh.mFilterExpression );
73+
mFilterExpression.reset( new QgsExpression( *rh.mFilterExpression ) );
8974
}
9075
else
9176
{
92-
mFilterExpression = nullptr;
77+
mFilterExpression.reset( nullptr );
9378
}
9479
mExpressionContext = rh.mExpressionContext;
9580
mAttrs = rh.mAttrs;
@@ -99,15 +84,8 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
9984
return *this;
10085
}
10186

102-
QgsFeatureRequest::~QgsFeatureRequest()
103-
{
104-
delete mFilterExpression;
105-
}
106-
10787
QgsFeatureRequest &QgsFeatureRequest::setFilterRect( const QgsRectangle &rect )
10888
{
109-
if ( mFilter == FilterNone )
110-
mFilter = FilterRect;
11189
mFilterRect = rect;
11290
return *this;
11391
}
@@ -129,8 +107,7 @@ QgsFeatureRequest &QgsFeatureRequest::setFilterFids( const QgsFeatureIds &fids )
129107
QgsFeatureRequest &QgsFeatureRequest::setFilterExpression( const QString &expression )
130108
{
131109
mFilter = FilterExpression;
132-
delete mFilterExpression;
133-
mFilterExpression = new QgsExpression( expression );
110+
mFilterExpression.reset( new QgsExpression( expression ) );
134111
return *this;
135112
}
136113

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

246223
bool QgsFeatureRequest::acceptFeature( const QgsFeature &feature )
247224
{
225+
if ( !mFilterRect.isNull() )
226+
{
227+
if ( !feature.hasGeometry() || !feature.geometry().intersects( mFilterRect ) )
228+
return false;
229+
}
230+
248231
switch ( mFilter )
249232
{
250233
case QgsFeatureRequest::FilterNone:
251234
return true;
252235

253-
case QgsFeatureRequest::FilterRect:
254-
return ( feature.hasGeometry() && feature.geometry().intersects( mFilterRect ) );
255-
256236
case QgsFeatureRequest::FilterFid:
257237
return ( feature.id() == mFilterFid );
258238

‎src/core/qgsfeaturerequest.h

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "qgis_core.h"
1919
#include <QFlags>
2020
#include <QList>
21+
#include <memory>
2122

2223
#include "qgsfeature.h"
2324
#include "qgsrectangle.h"
@@ -79,7 +80,6 @@ class CORE_EXPORT QgsFeatureRequest
7980
enum FilterType
8081
{
8182
FilterNone, //!< No filter is applied
82-
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()
8383
FilterFid, //!< Filter using feature ID
8484
FilterExpression, //!< Filter using expression
8585
FilterFids //!< Filter using feature IDs
@@ -242,22 +242,21 @@ class CORE_EXPORT QgsFeatureRequest
242242
//! Assignment operator
243243
QgsFeatureRequest &operator=( const QgsFeatureRequest &rh );
244244

245-
~QgsFeatureRequest();
246-
247245
/**
248246
* Return the filter type which is currently set on this request
249247
*
250248
* @return Filter type
251249
*/
252-
FilterType filterType() const { if ( mFilter == FilterNone && !mFilterRect.isNull() ) return FilterRect; else return mFilter; }
250+
FilterType filterType() const { return mFilter; }
253251

254252
/**
255253
* Set rectangle from which features will be taken. Empty rectangle removes the filter.
256254
*/
257255
QgsFeatureRequest &setFilterRect( const QgsRectangle &rect );
258256

259257
/**
260-
* Get the rectangle from which features will be taken.
258+
* Get the rectangle from which features will be taken. If the returned
259+
* rectangle is null, then no filter rectangle is set.
261260
*/
262261
const QgsRectangle &filterRect() const { return mFilterRect; }
263262

@@ -282,7 +281,7 @@ class CORE_EXPORT QgsFeatureRequest
282281
* @see setFilterExpression
283282
* @see expressionContext
284283
*/
285-
QgsExpression *filterExpression() const { return mFilterExpression; }
284+
QgsExpression *filterExpression() const { return mFilterExpression.get(); }
286285

287286
/** Modifies the existing filter expression to add an additional expression filter. The
288287
* filter expressions are combined using AND, so only features matching both
@@ -404,20 +403,17 @@ class CORE_EXPORT QgsFeatureRequest
404403
*/
405404
bool acceptFeature( const QgsFeature &feature );
406405

407-
// TODO: in future
408-
// void setFilterNativeExpression(con QString& expr); // using provider's SQL (if supported)
409-
410406
protected:
411-
FilterType mFilter;
407+
FilterType mFilter = FilterNone;
412408
QgsRectangle mFilterRect;
413-
QgsFeatureId mFilterFid;
409+
QgsFeatureId mFilterFid = -1;
414410
QgsFeatureIds mFilterFids;
415-
QgsExpression *mFilterExpression = nullptr;
411+
std::unique_ptr< QgsExpression > mFilterExpression;
416412
QgsExpressionContext mExpressionContext;
417413
Flags mFlags;
418414
QgsAttributeList mAttrs;
419415
QgsSimplifyMethod mSimplifyMethod;
420-
long mLimit;
416+
long mLimit = -1;
421417
OrderBy mOrderBy;
422418
};
423419

‎src/core/qgsvectorlayercache.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,6 @@ bool QgsVectorLayerCache::canUseCacheForRequest( const QgsFeatureRequest &featur
310310
break;
311311
}
312312
case QgsFeatureRequest::FilterNone:
313-
case QgsFeatureRequest::FilterRect:
314313
case QgsFeatureRequest::FilterExpression:
315314
{
316315
if ( mFullCache )

‎src/core/qgsvectorlayerfeatureiterator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
191191
providerLimit += mSource->mChangedAttributeValues.size();
192192
}
193193

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

‎src/providers/arcgisrest/qgsafsfeatureiterator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
7373
else
7474
{
7575
QgsRectangle filterRect = mSource->provider()->extent();
76-
if ( mRequest.filterType() == QgsFeatureRequest::FilterRect )
76+
if ( !mRequest.filterRect().isNull() )
7777
filterRect = filterRect.intersect( &mRequest.filterRect() );
7878
while ( mFeatureIterator < mSource->provider()->featureCount() )
7979
{

‎src/providers/oracle/qgsoraclefeatureiterator.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,6 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource *sour
142142
//handled below
143143
break;
144144

145-
case QgsFeatureRequest::FilterRect:
146-
// Handled in the if-statement above
147-
break;
148145
}
149146

150147
if ( mSource->mRequestedGeomType != QgsWkbTypes::Unknown && mSource->mRequestedGeomType != mSource->mDetectedGeomType )

‎tests/src/python/providertestbase.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ def assert_query(self, provider, expression, expected):
141141
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression).setSubsetOfAttributes([0]))])
142142
assert set(expected) == result, 'Expected {} and got {} when testing expression "{}" using empty attribute subset'.format(set(expected), result, expression)
143143

144+
# test that results match QgsFeatureRequest.acceptFeature
145+
request = QgsFeatureRequest().setFilterExpression(expression)
146+
for f in provider.getFeatures():
147+
self.assertEqual(request.acceptFeature(f), f['pk'] in expected)
148+
144149
def runGetFeatureTests(self, provider):
145150
assert len([f for f in provider.getFeatures()]) == 5
146151
self.assert_query(provider, 'name ILIKE \'QGIS\'', [])
@@ -485,6 +490,11 @@ def testGetFeaturesFidTests(self):
485490
expected = [id]
486491
assert result == expected, 'Expected {} and got {} when testing for feature ID filter'.format(expected, result)
487492

493+
# test that results match QgsFeatureRequest.acceptFeature
494+
request = QgsFeatureRequest().setFilterFid(id)
495+
for f in self.provider.getFeatures():
496+
self.assertEqual(request.acceptFeature(f), f.id() == id)
497+
488498
# bad features
489499
it = self.provider.getFeatures(QgsFeatureRequest().setFilterFid(-99999999))
490500
feature = QgsFeature(5)
@@ -503,6 +513,10 @@ def testGetFeaturesFidsTests(self):
503513
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
504514
self.assertTrue(all_valid)
505515

516+
# test that results match QgsFeatureRequest.acceptFeature
517+
for f in self.provider.getFeatures():
518+
self.assertEqual(request.acceptFeature(f), f.id() in expected)
519+
506520
result = set([f.id() for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFids([fids[1], fids[3], fids[4]]))])
507521
expected = set([fids[1], fids[3], fids[4]])
508522
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
@@ -545,6 +559,10 @@ def testGetFeaturesFilterRectTests(self):
545559
assert set(features) == set([2, 4]), 'Got {} instead'.format(features)
546560
self.assertTrue(all_valid)
547561

562+
# test that results match QgsFeatureRequest.acceptFeature
563+
for f in self.provider.getFeatures():
564+
self.assertEqual(request.acceptFeature(f), f['pk'] in set([2, 4]))
565+
548566
# test with an empty rectangle
549567
extent = QgsRectangle()
550568
request = QgsFeatureRequest().setFilterRect(extent)
@@ -590,6 +608,20 @@ def testRectAndExpression(self):
590608
assert set(expected) == result, 'Expected {} and got {} when testing for combination of filterRect and expression'.format(set(expected), result)
591609
self.assertTrue(all_valid)
592610

611+
# shouldn't matter what order this is done in
612+
request = QgsFeatureRequest().setFilterRect(extent).setFilterExpression('"cnt">200')
613+
result = set([f['pk'] for f in self.provider.getFeatures(request)])
614+
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
615+
expected = [4]
616+
assert set(
617+
expected) == result, 'Expected {} and got {} when testing for combination of filterRect and expression'.format(
618+
set(expected), result)
619+
self.assertTrue(all_valid)
620+
621+
# test that results match QgsFeatureRequest.acceptFeature
622+
for f in self.provider.getFeatures():
623+
self.assertEqual(request.acceptFeature(f), f['pk'] in expected)
624+
593625
def testGetFeaturesLimit(self):
594626
it = self.provider.getFeatures(QgsFeatureRequest().setLimit(2))
595627
features = [f['pk'] for f in it]

0 commit comments

Comments
 (0)
Please sign in to comment.