From 70091931ab30ed8a5143b91ee156929f65b910fe Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Fri, 24 Jan 2020 09:16:46 +0100 Subject: [PATCH 1/3] Fixes #33880 : fix backgroundcachefeatureiterator when there is a sidefilterexpression and filter fids --- .../qgsbackgroundcachedfeatureiterator.cpp | 38 ++++++++++++------- .../wfs/qgsbackgroundcachedfeatureiterator.h | 3 ++ tests/src/python/providertestbase.py | 23 +++++++++++ 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp index 9b404a6a1ba7..b5e4caa74004 100644 --- a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp +++ b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp @@ -257,25 +257,27 @@ void QgsThreadedFeatureDownloader::run() // ------------------------- -static QgsFeatureRequest addSubsetToFeatureRequest( const QgsFeatureRequest &requestIn, - const QgsBackgroundCachedSharedData *shared ) -{ - if ( shared->clientSideFilterExpression().isEmpty() ) - { - return requestIn; - } - QgsFeatureRequest requestOut( requestIn ); - requestOut.combineFilterExpression( shared->clientSideFilterExpression() ); - return requestOut; -} - QgsBackgroundCachedFeatureIterator::QgsBackgroundCachedFeatureIterator( QgsBackgroundCachedFeatureSource *source, bool ownSource, std::shared_ptr shared, const QgsFeatureRequest &request ) - : QgsAbstractFeatureIteratorFromSource( source, ownSource, addSubsetToFeatureRequest( request, shared.get() ) ) + : QgsAbstractFeatureIteratorFromSource( source, ownSource, request ) , mShared( shared ) { + if ( !shared->clientSideFilterExpression().isEmpty() ) + { + // backup current request because combine filter expression will remove the fid(s) filtering + if ( mRequest.filterType() == QgsFeatureRequest::FilterFid || mRequest.filterType() == QgsFeatureRequest::FilterFids ) + { + mAdditionalRequest = mRequest; + mRequest = QgsFeatureRequest( shared->clientSideFilterExpression() ); + } + else + { + mRequest.combineFilterExpression( shared->clientSideFilterExpression() ); + } + } + if ( mRequest.destinationCrs().isValid() && mRequest.destinationCrs() != mShared->sourceCrs() ) { mTransform = QgsCoordinateTransform( mShared->sourceCrs(), mRequest.destinationCrs(), mRequest.transformContext() ); @@ -601,6 +603,11 @@ bool QgsBackgroundCachedFeatureIterator::fetchFeature( QgsFeature &f ) continue; } + if ( !mAdditionalRequest.acceptFeature( cachedFeature ) ) + { + continue; + } + copyFeature( cachedFeature, f, true ); geometryToDestinationCrs( f, mTransform ); @@ -692,6 +699,11 @@ bool QgsBackgroundCachedFeatureIterator::fetchFeature( QgsFeature &f ) continue; } + if ( !mAdditionalRequest.acceptFeature( feat ) ) + { + continue; + } + copyFeature( feat, f, false ); return true; } diff --git a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.h b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.h index 531b399ec9df..735a8206301f 100644 --- a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.h +++ b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.h @@ -320,6 +320,9 @@ class QgsBackgroundCachedFeatureIterator final: public QObject, QgsCoordinateTransform mTransform; QgsRectangle mFilterRect; + //! typically to save a FilterFid/FilterFids request that will not be captured by mRequest + QgsFeatureRequest mAdditionalRequest; + ///////////////// METHODS //! Translate mRequest to a request compatible of the Spatialite cache diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py index 79fc655b4d93..a3e66781463f 100644 --- a/tests/src/python/providertestbase.py +++ b/tests/src/python/providertestbase.py @@ -210,6 +210,29 @@ def testSubsetString(self): result, subset) self.assertTrue(all_valid) + # Subset string AND filter fid + ids = {f['pk']: f.id() for f in self.source.getFeatures()} + self.source.setSubsetString(subset) + request = QgsFeatureRequest().setFilterFid(4) + result = set([f.id() for f in self.source.getFeatures(request)]) + all_valid = (all(f.isValid() for f in self.source.getFeatures(request))) + self.source.setSubsetString(None) + expected = set([4]) + assert set(expected) == result, 'Expected {} and got {} when testing subset string {}'.format(set(expected), + result, subset) + self.assertTrue(all_valid) + + # Subset string AND filter fids + self.source.setSubsetString(subset) + request = QgsFeatureRequest().setFilterFids([ids[2], ids[4]]) + result = set([f.id() for f in self.source.getFeatures(request)]) + all_valid = (all(f.isValid() for f in self.source.getFeatures(request))) + self.source.setSubsetString(None) + expected = set([ids[2], ids[4]]) + assert set(expected) == result, 'Expected {} and got {} when testing subset string {}'.format(set(expected), + result, subset) + self.assertTrue(all_valid) + def getSubsetString(self): """Individual providers may need to override this depending on their subset string formats""" return '"cnt" > 100 and "cnt" < 410' From 6ad3f1f5c6aaa299022d85f6d2102bd3f2245a18 Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Tue, 28 Jan 2020 09:41:24 +0100 Subject: [PATCH 2/3] fix python provider --- tests/src/python/provider_python.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/src/python/provider_python.py b/tests/src/python/provider_python.py index 804fe32167ce..e92a5cb393ab 100644 --- a/tests/src/python/provider_python.py +++ b/tests/src/python/provider_python.py @@ -78,6 +78,10 @@ def __init__(self, source, request): if self._filter_rect is not None and self._source._provider._spatialindex is not None: self._feature_id_list = self._source._provider._spatialindex.intersects(self._filter_rect) + if self._request.filterType() == QgsFeatureRequest.FilterFid or self._request.filterType() == QgsFeatureRequest.FilterFids: + fids = [self._request.filterFid()] if self._request.filterType() == QgsFeatureRequest.FilterFid else self._request.filterFids() + self._feature_id_list = list(set(self._feature_id_list).intersection(set(fids))) if self._feature_id_list else fids + def fetchFeature(self, f): """fetch next feature, return true on success""" #virtual bool nextFeature( QgsFeature &f ); From 3d3af4c143a5a8eecf976159f78a692f15c673fe Mon Sep 17 00:00:00 2001 From: Julien Cabieces Date: Fri, 28 Feb 2020 10:50:30 +0100 Subject: [PATCH 3/3] made fid request prior to expression request --- src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp index b5e4caa74004..c9bbdf3c3d3c 100644 --- a/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp +++ b/src/providers/wfs/qgsbackgroundcachedfeatureiterator.cpp @@ -269,8 +269,7 @@ QgsBackgroundCachedFeatureIterator::QgsBackgroundCachedFeatureIterator( // backup current request because combine filter expression will remove the fid(s) filtering if ( mRequest.filterType() == QgsFeatureRequest::FilterFid || mRequest.filterType() == QgsFeatureRequest::FilterFids ) { - mAdditionalRequest = mRequest; - mRequest = QgsFeatureRequest( shared->clientSideFilterExpression() ); + mAdditionalRequest = QgsFeatureRequest( shared->clientSideFilterExpression() ); } else {