From 05a9c515acce62d206059333fdbe7101129cf244 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jun 2020 15:21:50 +1000 Subject: [PATCH 1/2] [layouts] Fix attribute table sort order combined with feature filter results in empty tables Fixes #36341 --- src/core/layout/qgslayoutitemattributetable.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/layout/qgslayoutitemattributetable.cpp b/src/core/layout/qgslayoutitemattributetable.cpp index 7f5ad2cd9005..d6736f5c2ea1 100644 --- a/src/core/layout/qgslayoutitemattributetable.cpp +++ b/src/core/layout/qgslayoutitemattributetable.cpp @@ -398,6 +398,7 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont context.setFields( layer->fields() ); QgsFeatureRequest req; + req.setExpressionContext( context ); //prepare filter expression std::unique_ptr filterExpression; @@ -409,7 +410,6 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont { activeFilter = true; req.setFilterExpression( mFeatureFilter ); - req.setExpressionContext( context ); } } @@ -478,7 +478,7 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont for ( const QgsLayoutTableColumn &column : qgis::as_const( mSortColumns ) ) { - req = req.addOrderBy( column.attribute(), column.sortOrder() == Qt::AscendingOrder ); + req.addOrderBy( column.attribute(), column.sortOrder() == Qt::AscendingOrder ); } QgsFeature f; From 774f6bd4d5eeb8bc03e625c3f60dd6092019b215 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 12 Jun 2020 16:24:04 +1000 Subject: [PATCH 2/2] Avoid losing expression context when accidentally assigning a context to itself --- src/core/qgsexpressioncontext.cpp | 3 + src/core/qgsfeaturerequest.cpp | 3 + tests/src/python/CMakeLists.txt | 1 + tests/src/python/test_qgsfeaturerequest.py | 253 +++++++++++++++++++++ 4 files changed, 260 insertions(+) create mode 100644 tests/src/python/test_qgsfeaturerequest.py diff --git a/src/core/qgsexpressioncontext.cpp b/src/core/qgsexpressioncontext.cpp index 2ca193563ef3..8b458072bf9b 100644 --- a/src/core/qgsexpressioncontext.cpp +++ b/src/core/qgsexpressioncontext.cpp @@ -266,6 +266,9 @@ QgsExpressionContext &QgsExpressionContext::operator=( QgsExpressionContext &&ot QgsExpressionContext &QgsExpressionContext::operator=( const QgsExpressionContext &other ) { + if ( &other == this ) + return *this; + qDeleteAll( mStack ); mStack.clear(); for ( const QgsExpressionContextScope *scope : qgis::as_const( other.mStack ) ) diff --git a/src/core/qgsfeaturerequest.cpp b/src/core/qgsfeaturerequest.cpp index a221ee473826..b8d060d64d55 100644 --- a/src/core/qgsfeaturerequest.cpp +++ b/src/core/qgsfeaturerequest.cpp @@ -63,6 +63,9 @@ QgsFeatureRequest::QgsFeatureRequest( const QgsFeatureRequest &rh ) QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh ) { + if ( &rh == this ) + return *this; + mFlags = rh.mFlags; mFilter = rh.mFilter; mFilterRect = rh.mFilterRect; diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt index e1187e504d93..0e6465f0de31 100644 --- a/tests/src/python/CMakeLists.txt +++ b/tests/src/python/CMakeLists.txt @@ -76,6 +76,7 @@ ADD_PYTHON_TEST(PyQgsProjectDisplaySettings test_qgsprojectdisplaysettings.py) ADD_PYTHON_TEST(PyQgsProjectTimeSettings test_qgsprojecttimesettings.py) ADD_PYTHON_TEST(PyQgsProjectViewSettings test_qgsprojectviewsettings.py) ADD_PYTHON_TEST(PyQgsFeatureIterator test_qgsfeatureiterator.py) +ADD_PYTHON_TEST(PyQgsFeatureRequest test_qgsfeaturerequest.py) ADD_PYTHON_TEST(PyQgsFeedback test_qgsfeedback.py) ADD_PYTHON_TEST(PyQgsFields test_qgsfields.py) ADD_PYTHON_TEST(PyQgsFieldModel test_qgsfieldmodel.py) diff --git a/tests/src/python/test_qgsfeaturerequest.py b/tests/src/python/test_qgsfeaturerequest.py new file mode 100644 index 000000000000..f691b792566e --- /dev/null +++ b/tests/src/python/test_qgsfeaturerequest.py @@ -0,0 +1,253 @@ +# -*- coding: utf-8 -*- +"""QGIS Unit tests for QgsFeatureRequest. + +.. note:: This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +""" +__author__ = 'Nyall Dawson' +__date__ = '12/06/2020' +__copyright__ = 'Copyright 2020, The QGIS Project' + +import qgis # NOQA + +from qgis.core import (QgsFeatureRequest, + QgsRectangle, + QgsExpressionContext, + QgsExpressionContextScope, + QgsFields, + QgsField, + QgsSimplifyMethod, + QgsCoordinateReferenceSystem, + QgsCoordinateTransformContext) +from qgis.PyQt.QtCore import QVariant +from qgis.testing import start_app, unittest + + +from utilities import unitTestDataPath + +start_app() +TEST_DATA_DIR = unitTestDataPath() + + +class TestQgsFeatureRequest(unittest.TestCase): + + def __init__(self, methodName): + """Run once on class initialization.""" + unittest.TestCase.__init__(self, methodName) + + def testConstructors(self): + req = QgsFeatureRequest() + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterNone) + self.assertEqual(req.filterFid(), -1) + self.assertFalse(req.filterFids()) + + req = QgsFeatureRequest(55) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFid) + self.assertEqual(req.filterFid(), 55) + self.assertFalse(req.filterFids()) + + req = QgsFeatureRequest([55, 56]) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFids) + self.assertEqual(req.filterFid(), -1) + self.assertCountEqual(req.filterFids(), [55, 56]) + + def testFilterRect(self): + req = QgsFeatureRequest().setFilterRect(QgsRectangle(1, 2, 3, 4)) + self.assertEqual(req.filterFid(), -1) + self.assertFalse(req.filterFids()) + self.assertEqual(req.filterRect(), QgsRectangle(1, 2, 3, 4)) + + def testFilterFid(self): + req = QgsFeatureRequest().setFilterFid(5) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFid) + self.assertEqual(req.filterFid(), 5) + self.assertFalse(req.filterFids()) + + # filter rect doesn't affect fid filter + req.setFilterRect(QgsRectangle(1, 2, 3, 4)) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFid) + self.assertEqual(req.filterFid(), 5) + self.assertEqual(req.filterRect(), QgsRectangle(1, 2, 3, 4)) + req.setFilterFid(6) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFid) + self.assertEqual(req.filterFid(), 6) + self.assertEqual(req.filterRect(), QgsRectangle(1, 2, 3, 4)) + + def testFilterFids(self): + req = QgsFeatureRequest().setFilterFids([5, 6]) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFids) + self.assertEqual(req.filterFid(), -1) + self.assertCountEqual(req.filterFids(), [5, 6]) + + # filter rect doesn't affect fids filter + req.setFilterRect(QgsRectangle(1, 2, 3, 4)) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFids) + self.assertEqual(req.filterFid(), -1) + self.assertCountEqual(req.filterFids(), [5, 6]) + req.setFilterFids([8, 9]) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFids) + self.assertEqual(req.filterFid(), -1) + self.assertCountEqual(req.filterFids(), [8, 9]) + self.assertEqual(req.filterRect(), QgsRectangle(1, 2, 3, 4)) + + def testInvalidGeomCheck(self): + req = QgsFeatureRequest().setFilterFids([5, 6]).setInvalidGeometryCheck(QgsFeatureRequest.GeometrySkipInvalid) + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterFids) + self.assertEqual(req.filterFid(), -1) + self.assertCountEqual(req.filterFids(), [5, 6]) + self.assertEqual(req.invalidGeometryCheck(), QgsFeatureRequest.GeometrySkipInvalid) + req.setInvalidGeometryCheck(QgsFeatureRequest.GeometryNoCheck) + self.assertEqual(req.invalidGeometryCheck(), QgsFeatureRequest.GeometryNoCheck) + + def testFilterExpression(self): + req = QgsFeatureRequest().setFilterExpression('a=5') + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterExpression) + self.assertEqual(req.filterFid(), -1) + self.assertFalse(req.filterFids()) + self.assertEqual(req.filterExpression().expression(), 'a=5') + + # filter rect doesn't affect fids filter + req.setFilterRect(QgsRectangle(1, 2, 3, 4)) + self.assertEqual(req.filterFid(), -1) + self.assertFalse(req.filterFids()) + self.assertEqual(req.filterExpression().expression(), 'a=5') + req.setFilterExpression('a=8') + self.assertEqual(req.filterFid(), -1) + self.assertFalse(req.filterFids()) + self.assertEqual(req.filterExpression().expression(), 'a=8') + self.assertEqual(req.filterRect(), QgsRectangle(1, 2, 3, 4)) + + def testCombineFilter(self): + req = QgsFeatureRequest() + req.combineFilterExpression('b=9') + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterExpression) + self.assertEqual(req.filterFid(), -1) + self.assertFalse(req.filterFids()) + self.assertEqual(req.filterExpression().expression(), 'b=9') + + req.combineFilterExpression('a=11') + self.assertEqual(req.filterExpression().expression(), '(b=9) AND (a=11)') + + def testExpressionContext(self): + req = QgsFeatureRequest() + self.assertEqual(req.expressionContext().scopeCount(), 0) + + context = QgsExpressionContext() + scope = QgsExpressionContextScope() + scope.setVariable('a', 6) + context.appendScope(scope) + req.setExpressionContext(context) + + self.assertEqual(req.expressionContext().scopeCount(), 1) + self.assertEqual(req.expressionContext().variable('a'), 6) + + def testDisableFilter(self): + req = QgsFeatureRequest().setFilterFid(5).disableFilter() + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterNone) + + req = QgsFeatureRequest().setFilterFids([5, 6]).disableFilter() + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterNone) + + req = QgsFeatureRequest().setFilterExpression('a=5').disableFilter() + self.assertEqual(req.filterType(), QgsFeatureRequest.FilterNone) + self.assertFalse(req.filterExpression()) + + def testLimit(self): + req = QgsFeatureRequest() + self.assertEqual(req.limit(), -1) + req.setLimit(6) + self.assertEqual(req.limit(), 6) + + def testFlags(self): + req = QgsFeatureRequest() + self.assertFalse(req.flags()) + req.setFlags(QgsFeatureRequest.ExactIntersect) + self.assertEqual(req.flags(), QgsFeatureRequest.ExactIntersect) + + def testSubsetAttributes(self): + req = QgsFeatureRequest() + self.assertFalse(req.subsetOfAttributes()) + self.assertFalse(req.flags() & QgsFeatureRequest.SubsetOfAttributes) + + req.setSubsetOfAttributes([1, 4]) + self.assertEqual(req.subsetOfAttributes(), [1, 4]) + self.assertTrue(req.flags() & QgsFeatureRequest.SubsetOfAttributes) + + req.setNoAttributes() + self.assertEqual(req.subsetOfAttributes(), []) + self.assertTrue(req.flags() & QgsFeatureRequest.SubsetOfAttributes) + + req.setSubsetOfAttributes([]) + self.assertFalse(req.subsetOfAttributes()) + self.assertTrue(req.flags() & QgsFeatureRequest.SubsetOfAttributes) + + req.setFlags(QgsFeatureRequest.Flags()) + f = QgsFields() + f.append(QgsField('a', QVariant.String)) + f.append(QgsField('b', QVariant.String)) + f.append(QgsField('c', QVariant.String)) + req.setSubsetOfAttributes(['a', 'c'], f) + self.assertEqual(req.subsetOfAttributes(), [0, 2]) + self.assertTrue(req.flags() & QgsFeatureRequest.SubsetOfAttributes) + + def testSimplifyMethod(self): + req = QgsFeatureRequest() + self.assertEqual(req.simplifyMethod().methodType(), QgsSimplifyMethod.NoSimplification) + method = QgsSimplifyMethod() + method.setMethodType(QgsSimplifyMethod.PreserveTopology) + req.setSimplifyMethod(method) + self.assertEqual(req.simplifyMethod().methodType(), QgsSimplifyMethod.PreserveTopology) + + def testDestinationCrs(self): + req = QgsFeatureRequest() + self.assertFalse(req.destinationCrs().isValid()) + context = QgsCoordinateTransformContext() + req.setDestinationCrs(QgsCoordinateReferenceSystem('EPSG:3857'), context) + self.assertTrue(req.destinationCrs().isValid()) + self.assertEqual(req.destinationCrs().authid(), 'EPSG:3857') + + def testTimeout(self): + req = QgsFeatureRequest() + self.assertEqual(req.timeout(), -1) + req.setTimeout(6) + self.assertEqual(req.timeout(), 6) + + def testNested(self): + req = QgsFeatureRequest() + self.assertFalse(req.requestMayBeNested()) + req.setRequestMayBeNested(True) + self.assertTrue(req.requestMayBeNested()) + + def testAssignment(self): + req = QgsFeatureRequest().setFilterFids([8, 9]).setFilterRect(QgsRectangle(1, 2, 3, 4)).setInvalidGeometryCheck(QgsFeatureRequest.GeometrySkipInvalid).setLimit(6).setFlags(QgsFeatureRequest.ExactIntersect).setSubsetOfAttributes([1, 4]).setTimeout(6).setRequestMayBeNested(True) + + context = QgsExpressionContext() + scope = QgsExpressionContextScope() + scope.setVariable('a', 6) + context.appendScope(scope) + req.setExpressionContext(context) + method = QgsSimplifyMethod() + method.setMethodType(QgsSimplifyMethod.PreserveTopology) + req.setSimplifyMethod(method) + context = QgsCoordinateTransformContext() + req.setDestinationCrs(QgsCoordinateReferenceSystem('EPSG:3857'), context) + + req2 = QgsFeatureRequest(req) + self.assertEqual(req2.limit(), 6) + self.assertCountEqual(req2.filterFids(), [8, 9]) + self.assertEqual(req2.filterRect(), QgsRectangle(1, 2, 3, 4)) + self.assertEqual(req2.invalidGeometryCheck(), QgsFeatureRequest.GeometrySkipInvalid) + self.assertEqual(req2.expressionContext().scopeCount(), 1) + self.assertEqual(req2.expressionContext().variable('a'), 6) + self.assertEqual(req2.flags(), QgsFeatureRequest.ExactIntersect | QgsFeatureRequest.SubsetOfAttributes) + self.assertEqual(req2.subsetOfAttributes(), [1, 4]) + self.assertEqual(req2.simplifyMethod().methodType(), QgsSimplifyMethod.PreserveTopology) + self.assertEqual(req2.destinationCrs().authid(), 'EPSG:3857') + self.assertEqual(req2.timeout(), 6) + self.assertTrue(req2.requestMayBeNested()) + + +if __name__ == '__main__': + unittest.main()