Skip to content
Permalink
Browse files

Always mark features as valid when added to memory provider

For other providers features will automatically be made valid
through the proces of adding to the provider's storage itself
and then later retrieving via iterators. But the memory
provider uses a direct copy of the feature, so if we don't
explicitly mark features as valid the provider may be
returning features incorrectly marked as invalid.

...and add tests to ensure all providers always return valid
features

(cherry-picked from f2b70cf)
  • Loading branch information
nyalldawson committed Jun 30, 2016
1 parent 5d0f628 commit a66f1f8906fec86281207f0edf808ba6d2ca243f
Showing with 57 additions and 12 deletions.
  1. +57 −12 tests/src/python/providertestbase.py
@@ -33,6 +33,8 @@ def testGetFeatures(self):
attributes = {}
geometries = {}
while it.nextFeature(f):
# expect feature to be valid
self.assertTrue(f.isValid())
# split off the first 5 attributes only - some provider test datasets will include
# additional attributes which we ignore
attrs = f.attributes()[0:5]
@@ -62,8 +64,10 @@ def testGetFeatures(self):
self.assertFalse(geometries[pk], 'Expected null geometry for {}'.format(pk))

def assert_query(self, provider, expression, expected):
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression).setFlags(QgsFeatureRequest.NoGeometry))])
request = QgsFeatureRequest().setFilterExpression(expression).setFlags(QgsFeatureRequest.NoGeometry)
result = set([f['pk'] for f in provider.getFeatures(request)])
assert set(expected) == result, 'Expected {} and got {} when testing expression "{}"'.format(set(expected), result, expression)
self.assertTrue(all(f.isValid() for f in provider.getFeatures(request)))

# Also check that filter works when referenced fields are not being retrieved by request
result = set([f['pk'] for f in provider.getFeatures(QgsFeatureRequest().setFilterExpression(expression).setSubsetOfAttributes([0]))])
@@ -180,18 +184,23 @@ def testSubsetString(self):
self.provider.setSubsetString(subset)
self.assertEqual(self.provider.subsetString(), subset)
result = set([f['pk'] for f in self.provider.getFeatures()])
all_valid = (all(f.isValid() for f in self.provider.getFeatures()))
self.provider.setSubsetString(None)

expected = set([2, 3, 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 rect
self.provider.setSubsetString(subset)
extent = QgsRectangle(-70, 70, -60, 75)
result = set([f['pk'] for f in self.provider.getFeatures(QgsFeatureRequest().setFilterRect(extent))])
request = QgsFeatureRequest().setFilterRect(extent)
result = set([f['pk'] for f in self.provider.getFeatures(request)])
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
self.provider.setSubsetString(None)
expected = set([2])
assert set(expected) == result, 'Expected {} and got {} when testing subset string {}'.format(set(expected), result, subset)
self.assertTrue(all_valid)

# Subset string AND filter rect, version 2
self.provider.setSubsetString(subset)
@@ -203,10 +212,13 @@ def testSubsetString(self):

# Subset string AND expression
self.provider.setSubsetString(subset)
result = set([f['pk'] for f in self.provider.getFeatures(QgsFeatureRequest().setFilterExpression('length("name")=5'))])
request = QgsFeatureRequest().setFilterExpression('length("name")=5')
result = set([f['pk'] for f in self.provider.getFeatures(request)])
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
self.provider.setSubsetString(None)
expected = set([2, 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"""
@@ -309,16 +321,31 @@ def testGetFeaturesFidTests(self):
fids = [f.id() for f in self.provider.getFeatures()]
assert len(fids) == 5, 'Expected 5 features, got {} instead'.format(len(fids))
for id in fids:
result = [f.id() for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFid(id))]
features = [f for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFid(id))]
self.assertEqual(len(features), 1)
feature = features[0]
self.assertTrue(feature.isValid())

result = [feature.id()]
expected = [id]
assert result == expected, 'Expected {} and got {} when testing for feature ID filter'.format(expected, result)

# bad features
it = self.provider.getFeatures(QgsFeatureRequest().setFilterFid(-99999999))
feature = QgsFeature(5)
feature.setValid(False)
self.assertFalse(it.nextFeature(feature))
self.assertFalse(feature.isValid())

def testGetFeaturesFidsTests(self):
fids = [f.id() for f in self.provider.getFeatures()]

result = set([f.id() for f in self.provider.getFeatures(QgsFeatureRequest().setFilterFids([fids[0], fids[2]]))])
request = QgsFeatureRequest().setFilterFids([fids[0], fids[2]])
result = set([f.id() for f in self.provider.getFeatures(request)])
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
expected = set([fids[0], fids[2]])
assert result == expected, 'Expected {} and got {} when testing for feature IDs filter'.format(expected, result)
self.assertTrue(all_valid)

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]])
@@ -330,8 +357,11 @@ def testGetFeaturesFidsTests(self):

def testGetFeaturesFilterRectTests(self):
extent = QgsRectangle(-70, 67, -60, 80)
features = [f['pk'] for f in self.provider.getFeatures(QgsFeatureRequest().setFilterRect(extent))]
request = QgsFeatureRequest().setFilterRect(extent)
features = [f['pk'] for f in self.provider.getFeatures(request)]
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
assert set(features) == set([2, 4]), 'Got {} instead'.format(features)
self.assertTrue(all_valid)

def testGetFeaturesPolyFilterRectTests(self):
""" Test fetching features from a polygon layer with filter rect"""
@@ -342,20 +372,33 @@ def testGetFeaturesPolyFilterRectTests(self):
return

extent = QgsRectangle(-73, 70, -63, 80)
features = [f['pk'] for f in self.poly_provider.getFeatures(QgsFeatureRequest().setFilterRect(extent))]
request = QgsFeatureRequest().setFilterRect(extent)
features = [f['pk'] for f in self.poly_provider.getFeatures(request)]
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
# Some providers may return the exact intersection matches (2, 3) even without the ExactIntersect flag, so we accept that too
assert set(features) == set([2, 3]) or set(features) == set([1, 2, 3]), 'Got {} instead'.format(features)
self.assertTrue(all_valid)

# Test with exact intersection
features = [f['pk'] for f in self.poly_provider.getFeatures(QgsFeatureRequest().setFilterRect(extent).setFlags(QgsFeatureRequest.ExactIntersect))]
request = QgsFeatureRequest().setFilterRect(extent).setFlags(QgsFeatureRequest.ExactIntersect)
features = [f['pk'] for f in self.poly_provider.getFeatures(request)]
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
assert set(features) == set([2, 3]), 'Got {} instead'.format(features)
self.assertTrue(all_valid)

# test with an empty rectangle
extent = QgsRectangle()
features = [f['pk'] for f in self.provider.getFeatures(QgsFeatureRequest().setFilterRect(extent))]
assert set(features) == set([1, 2, 3, 4, 5]), 'Got {} instead'.format(features)

def testRectAndExpression(self):
extent = QgsRectangle(-70, 67, -60, 80)
result = set([f['pk'] for f in self.provider.getFeatures(
QgsFeatureRequest().setFilterExpression('"cnt">200').setFilterRect(extent))])
request = QgsFeatureRequest().setFilterExpression('"cnt">200').setFilterRect(extent)
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)

def testGetFeaturesLimit(self):
it = self.provider.getFeatures(QgsFeatureRequest().setLimit(2))
@@ -465,6 +508,8 @@ def testGetFeaturesSubsetAttributes(self):
'cnt': set([-200, 300, 100, 200, 400]),
'name': set(['Pear', 'Orange', 'Apple', 'Honey', NULL]),
'name2': set(['NuLl', 'PEaR', 'oranGe', 'Apple', 'Honey'])}
for field, expected in tests.iteritems():
result = set([f[field] for f in self.provider.getFeatures(QgsFeatureRequest().setSubsetOfAttributes([field], self.provider.fields()))])
for field, expected in tests.items():
request = QgsFeatureRequest().setSubsetOfAttributes([field], self.provider.fields())
result = set([f[field] for f in self.provider.getFeatures(request)])
all_valid = (all(f.isValid() for f in self.provider.getFeatures(request)))
self.assertEqual(result, expected, 'Expected {}, got {}'.format(expected, result))

0 comments on commit a66f1f8

Please sign in to comment.
You can’t perform that action at this time.