Skip to content

Commit

Permalink
Always mark features as valid when added to memory provider
Browse files Browse the repository at this point in the history 
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
nyalldawson committed Jun 30, 2016
1 parent 5d0f628 commit a66f1f8
Showing 1 changed file with 57 additions and 12 deletions.
69 changes: 57 additions & 12 deletions tests/src/python/providertestbase.py
View file
Expand Up @@ -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]
Expand Down Expand Up @@ -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]))])
Expand Down Expand Up @@ -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)
Expand All @@ -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"""
Expand Down Expand Up @@ -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]])
Expand All @@ -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"""
Expand All @@ -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))
Expand Down Expand Up @@ -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.