Add feature source test for QgsVectorLayerCache

While it's not a QgsFeatureSource subclass (yet), it behaves just like one so when can run the feature source conformance test suite over it. Fix a few minor issues identified by the test suite, and one potential crash (requesting an invalid id from a cache iterator crashes qgis)
qgis · Jun 8, 2017 · fbad911 · fbad911
1 parent 7d9cc13
commit fbad911
Show file tree

Hide file tree

Showing 7 changed files with 202 additions and 1 deletion.
diff --git a/python/core/qgsvectorlayercache.sip b/python/core/qgsvectorlayercache.sip
@@ -195,6 +195,42 @@ class QgsVectorLayerCache : QObject
  :rtype: QgsVectorLayer
 %End
 
+    QgsCoordinateReferenceSystem sourceCrs() const;
+%Docstring
+ Returns the coordinate reference system for features in the cache.
+ :rtype: QgsCoordinateReferenceSystem
+%End
+
+    QgsFields fields() const;
+%Docstring
+ Returns the fields associated with features in the cache.
+ :rtype: QgsFields
+%End
+
+    QgsWkbTypes::Type wkbType() const;
+%Docstring
+ Returns the geometry type for features in the cache.
+ :rtype: QgsWkbTypes.Type
+%End
+
+
+    int __len__() const;
+%Docstring
+ Returns the number of features contained in the source, or -1
+ if the feature count is unknown.
+ :rtype: int
+%End
+%MethodCode
+    sipRes = sipCpp->featureCount();
+%End
+
+    long featureCount() const;
+%Docstring
+ Returns the number of features contained in the source, or -1
+ if the feature count is unknown.
+ :rtype: long
+%End
+
   protected:
 
     void requestCompleted( const QgsFeatureRequest &featureRequest, const QgsFeatureIds &fids );

diff --git a/src/core/qgscachedfeatureiterator.cpp b/src/core/qgscachedfeatureiterator.cpp
@@ -43,15 +43,26 @@ QgsCachedFeatureIterator::QgsCachedFeatureIterator( QgsVectorLayerCache *vlCache
 
 bool QgsCachedFeatureIterator::fetchFeature( QgsFeature &f )
 {
+  f.setValid( false );
+
   if ( mClosed )
     return false;
 
   while ( mFeatureIdIterator != mFeatureIds.constEnd() )
   {
+    if ( !mVectorLayerCache->mCache.contains( *mFeatureIdIterator ) )
+    {
+      ++mFeatureIdIterator;
+      continue;
+    }
+
     f = QgsFeature( *mVectorLayerCache->mCache[*mFeatureIdIterator]->feature() );
     ++mFeatureIdIterator;
     if ( mRequest.acceptFeature( f ) )
+    {
+      f.setValid( true );
       return true;
+    }
   }
   close();
   return false;
@@ -79,6 +90,11 @@ QgsCachedFeatureWriterIterator::QgsCachedFeatureWriterIterator( QgsVectorLayerCa
 
 bool QgsCachedFeatureWriterIterator::fetchFeature( QgsFeature &f )
 {
+  if ( mClosed )
+  {
+    f.setValid( false );
+    return false;
+  }
   if ( mFeatIt.nextFeature( f ) )
   {
     // As long as features can be fetched from the provider: Write them to cache

diff --git a/src/core/qgsvectorlayercache.cpp b/src/core/qgsvectorlayercache.cpp
@@ -177,6 +177,26 @@ QgsVectorLayer *QgsVectorLayerCache::layer()
   return mLayer;
 }
 
+QgsCoordinateReferenceSystem QgsVectorLayerCache::sourceCrs() const
+{
+  return mLayer->crs();
+}
+
+QgsWkbTypes::Type QgsVectorLayerCache::wkbType() const
+{
+  return mLayer->wkbType();
+}
+
+QgsFields QgsVectorLayerCache::fields() const
+{
+  return mLayer->fields();
+}
+
+long QgsVectorLayerCache::featureCount() const
+{
+  return mLayer->featureCount();
+}
+
 void QgsVectorLayerCache::requestCompleted( const QgsFeatureRequest &featureRequest, const QgsFeatureIds &fids )
 {
   // If a request is too large for the cache don't notify to prevent from indexing incomplete requests

diff --git a/src/core/qgsvectorlayercache.h b/src/core/qgsvectorlayercache.h
@@ -248,6 +248,39 @@ class CORE_EXPORT QgsVectorLayerCache : public QObject
      */
     QgsVectorLayer *layer();
 
+    /**
+     * Returns the coordinate reference system for features in the cache.
+     */
+    QgsCoordinateReferenceSystem sourceCrs() const;
+
+    /**
+     * Returns the fields associated with features in the cache.
+     */
+    QgsFields fields() const;
+
+    /**
+     * Returns the geometry type for features in the cache.
+     */
+    QgsWkbTypes::Type wkbType() const;
+
+#ifdef SIP_RUN
+
+    /**
+     * Returns the number of features contained in the source, or -1
+     * if the feature count is unknown.
+     */
+    int __len__() const;
+    % MethodCode
+    sipRes = sipCpp->featureCount();
+    % End
+#endif
+
+    /**
+     * Returns the number of features contained in the source, or -1
+     * if the feature count is unknown.
+     */
+    long featureCount() const;
+
   protected:
 
     /**

diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt
@@ -148,6 +148,7 @@ ADD_PYTHON_TEST(PyQgsVectorColorRamp test_qgsvectorcolorramp.py)
 ADD_PYTHON_TEST(PyQgsVectorFileWriter test_qgsvectorfilewriter.py)
 ADD_PYTHON_TEST(PyQgsVectorFileWriterTask test_qgsvectorfilewritertask.py)
 ADD_PYTHON_TEST(PyQgsVectorLayer test_qgsvectorlayer.py)
+ADD_PYTHON_TEST(PyQgsVectorLayerCache test_qgsvectorlayercache.py)
 ADD_PYTHON_TEST(PyQgsVectorLayerEditBuffer test_qgsvectorlayereditbuffer.py)
 ADD_PYTHON_TEST(PyQgsVectorLayerUtils test_qgsvectorlayerutils.py)
 ADD_PYTHON_TEST(PyQgsZonalStatistics test_qgszonalstatistics.py)

diff --git a/tests/src/python/featuresourcetestbase.py b/tests/src/python/featuresourcetestbase.py
@@ -139,7 +139,7 @@ def assert_query(self, source, expression, expected):
             self.assertEqual(request.acceptFeature(f), f['pk'] in expected)
 
     def runGetFeatureTests(self, source):
-        assert len([f for f in source.getFeatures()]) == 5
+        self.assertEqual(len([f for f in source.getFeatures()]), 5)
         self.assert_query(source, 'name ILIKE \'QGIS\'', [])
         self.assert_query(source, '"name" IS NULL', [5])
         self.assert_query(source, '"name" IS NOT NULL', [1, 2, 3, 4])

diff --git a/tests/src/python/test_qgsvectorlayercache.py b/tests/src/python/test_qgsvectorlayercache.py
@@ -0,0 +1,95 @@
+# -*- coding: utf-8 -*-
+"""QGIS Unit tests for QgsVectorLayerCache.
+
+.. 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__ = '08/06/2017'
+__copyright__ = 'Copyright 2017, The QGIS Project'
+# This will get replaced with a git SHA1 when you do a git archive
+__revision__ = '$Format:%H$'
+
+import qgis  # NOQA
+
+import os
+
+from qgis.PyQt.QtCore import QVariant, Qt
+from qgis.PyQt.QtGui import QPainter
+from qgis.PyQt.QtXml import QDomDocument
+
+from qgis.core import (QgsWkbTypes,
+                       QgsVectorLayer,
+                       QgsVectorLayerCache,
+                       QgsRectangle,
+                       QgsFeature,
+                       QgsFeatureRequest,
+                       QgsGeometry,
+                       QgsPointXY,
+                       QgsField,
+                       QgsFields,
+                       QgsCoordinateReferenceSystem,
+                       QgsProject,
+                       QgsPoint,
+                       NULL)
+from qgis.testing import start_app, unittest
+from featuresourcetestbase import FeatureSourceTestCase
+from utilities import unitTestDataPath
+start_app()
+
+
+class TestQgsVectorLayerCache(unittest.TestCase, FeatureSourceTestCase):
+
+    @classmethod
+    def getSource(cls):
+        cache = QgsVectorLayerCache(cls.vl, 100)
+        return cache
+
+    @classmethod
+    def setUpClass(cls):
+        """Run before all tests"""
+        # Create test layer for FeatureSourceTestCase
+        cls.vl = QgsVectorLayer(
+            'Point?crs=epsg:4326&field=pk:integer&field=cnt:integer&field=name:string(0)&field=name2:string(0)&field=num_char:string&key=pk',
+            'test', 'memory')
+        assert (cls.vl.isValid())
+
+        f1 = QgsFeature(5)
+        f1.setAttributes([5, -200, NULL, 'NuLl', '5'])
+        f1.setGeometry(QgsGeometry.fromWkt('Point (-71.123 78.23)'))
+
+        f2 = QgsFeature(3)
+        f2.setAttributes([3, 300, 'Pear', 'PEaR', '3'])
+
+        f3 = QgsFeature(1)
+        f3.setAttributes([1, 100, 'Orange', 'oranGe', '1'])
+        f3.setGeometry(QgsGeometry.fromWkt('Point (-70.332 66.33)'))
+
+        f4 = QgsFeature(2)
+        f4.setAttributes([2, 200, 'Apple', 'Apple', '2'])
+        f4.setGeometry(QgsGeometry.fromWkt('Point (-68.2 70.8)'))
+
+        f5 = QgsFeature(4)
+        f5.setAttributes([4, 400, 'Honey', 'Honey', '4'])
+        f5.setGeometry(QgsGeometry.fromWkt('Point (-65.32 78.3)'))
+
+        assert cls.vl.dataProvider().addFeatures([f1, f2, f3, f4, f5])
+        cls.source = QgsVectorLayerCache(cls.vl, 100)
+
+    def testGetFeaturesSubsetAttributes2(self):
+        """ Override and skip this QgsFeatureSource test. We are using a memory provider, and it's actually more efficient for the memory provider to return
+        its features as direct copies (due to implicit sharing of QgsFeature)
+        """
+        pass
+
+    def testGetFeaturesNoGeometry(self):
+        """ Override and skip this QgsFeatureSource test. We are using a memory provider, and it's actually more efficient for the memory provider to return
+        its features as direct copies (due to implicit sharing of QgsFeature)
+        """
+        pass
+
+
+if __name__ == '__main__':
+    unittest.main()