Use cached feature iterator if cache has all needed features

Previously a cached feature iterator would only be returned if cache was either full or used a cache index On behalf of Faunalia, sponsored by ENEL (cherry-picked from 5e3d8fe)
qgis · Nov 23, 2016 · 531d63f · 531d63f
1 parent e82f8a4
commit 531d63f
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 9 deletions.
diff --git a/src/core/qgsvectorlayercache.cpp b/src/core/qgsvectorlayercache.cpp
@@ -270,6 +270,54 @@ void QgsVectorLayerCache::invalidate()
   emit invalidated();
 }
 
+bool QgsVectorLayerCache::canUseCacheForRequest( const QgsFeatureRequest &featureRequest, QgsFeatureIterator& it )
+{
+  // check first for available indices
+  Q_FOREACH ( QgsAbstractCacheIndex *idx, mCacheIndices )
+  {
+    if ( idx->getCacheIterator( it, featureRequest ) )
+    {
+      return true;
+    }
+  }
+
+  // no indexes available, but maybe we have already cached all required features anyway?
+  switch ( featureRequest.filterType() )
+  {
+    case QgsFeatureRequest::FilterFid:
+    {
+      if ( mCache.contains( featureRequest.filterFid() ) )
+      {
+        it = QgsFeatureIterator( new QgsCachedFeatureIterator( this, featureRequest ) );
+        return true;
+      }
+      break;
+    }
+    case QgsFeatureRequest::FilterFids:
+    {
+      if ( mCache.keys().toSet().contains( featureRequest.filterFids() ) )
+      {
+        it = QgsFeatureIterator( new QgsCachedFeatureIterator( this, featureRequest ) );
+        return true;
+      }
+      break;
+    }
+    case QgsFeatureRequest::FilterNone:
+    case QgsFeatureRequest::FilterRect:
+    case QgsFeatureRequest::FilterExpression:
+    {
+      if ( mFullCache )
+      {
+        it = QgsFeatureIterator( new QgsCachedFeatureIterator( this, featureRequest ) );
+        return true;
+      }
+      break;
+    }
+
+  }
+  return false;
+}
+
 QgsFeatureIterator QgsVectorLayerCache::getFeatures( const QgsFeatureRequest &featureRequest )
 {
   QgsFeatureIterator it;
@@ -285,15 +333,8 @@ QgsFeatureIterator QgsVectorLayerCache::getFeatures( const QgsFeatureRequest &fe
     }
     else
     {
-      // Check if an index is able to deliver the requested features
-      Q_FOREACH ( QgsAbstractCacheIndex *idx, mCacheIndices )
-      {
-        if ( idx->getCacheIterator( it, featureRequest ) )
-        {
-          requiresWriterIt = false;
-          break;
-        }
-      }
+      // may still be able to satisfy request using cache
+      requiresWriterIt = !canUseCacheForRequest( featureRequest, it );
     }
   }
   else

diff --git a/src/core/qgsvectorlayercache.h b/src/core/qgsvectorlayercache.h
@@ -300,5 +300,16 @@ class CORE_EXPORT QgsVectorLayerCache : public QObject
     friend class QgsCachedFeatureIterator;
     friend class QgsCachedFeatureWriterIterator;
     friend class QgsCachedFeature;
+
+    /** Returns true if the cache contains all the features required for a specified request.
+     * @param featureRequest feature request
+     * @param it will be set to iterator for matching features
+     * @returns true if cache can satisfy request
+     * @note this method only checks for available features, not whether the cache
+     * contains required attributes or geometry. For that, use checkInformationCovered()
+     */
+    bool canUseCacheForRequest( const QgsFeatureRequest& featureRequest, QgsFeatureIterator& it );
+
+    friend class TestVectorLayerCache;
 };
 #endif // QgsVectorLayerCache_H
diff --git a/tests/src/core/testqgsvectorlayercache.cpp b/tests/src/core/testqgsvectorlayercache.cpp
@@ -53,6 +53,7 @@ class TestVectorLayerCache : public QObject
     void testSubsetRequest();
     void testFullCache();
     void testFullCacheThroughRequest();
+    void testCanUseCacheForRequest();
 
     void onCommittedFeaturesAdded( const QString&, const QgsFeatureList& );
 
@@ -266,6 +267,67 @@ void TestVectorLayerCache::testFullCacheThroughRequest()
   QVERIFY( cache.hasFullCache() );
 }
 
+void TestVectorLayerCache::testCanUseCacheForRequest()
+{
+  //first get some feature ids from layer
+  QgsFeature f;
+  QgsFeatureIterator it = mPointsLayer->getFeatures();
+  it.nextFeature( f );
+  QgsFeatureId id1 = f.id();
+  it.nextFeature( f );
+  QgsFeatureId id2 = f.id();
+
+  QgsVectorLayerCache cache( mPointsLayer, 10 );
+  // initially nothing in cache, so can't use it to fulfill the request
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id1 ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id2 ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFids( QgsFeatureIds() << id1 << id2 ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterRect( QgsRectangle( 1, 2, 3, 4 ) ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterExpression( "$x<5" ), it ) );
+
+  // get just the first feature into the cache
+  it = cache.getFeatures( QgsFeatureRequest().setFilterFid( id1 ) );
+  while ( it.nextFeature( f ) ) { }
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id1 ), it ) );
+  //verify that the returned iterator was correct
+  QVERIFY( it.nextFeature( f ) );
+  QCOMPARE( f.id(), id1 );
+  QVERIFY( !it.nextFeature( f ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id2 ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFids( QgsFeatureIds() << id1 << id2 ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterRect( QgsRectangle( 1, 2, 3, 4 ) ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterExpression( "$x<5" ), it ) );
+
+  // get feature 2 into cache
+  it = cache.getFeatures( QgsFeatureRequest().setFilterFid( id2 ) );
+  while ( it.nextFeature( f ) ) { }
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id1 ), it ) );
+  QVERIFY( it.nextFeature( f ) );
+  QCOMPARE( f.id(), id1 );
+  QVERIFY( !it.nextFeature( f ) );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id2 ), it ) );
+  QVERIFY( it.nextFeature( f ) );
+  QCOMPARE( f.id(), id2 );
+  QVERIFY( !it.nextFeature( f ) );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFids( QgsFeatureIds() << id1 << id2 ), it ) );
+  QVERIFY( it.nextFeature( f ) );
+  QgsFeatureIds result;
+  result << f.id();
+  QVERIFY( it.nextFeature( f ) );
+  result << f.id();
+  QCOMPARE( result, QgsFeatureIds() << id1 << id2 );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterRect( QgsRectangle( 1, 2, 3, 4 ) ), it ) );
+  QVERIFY( !cache.canUseCacheForRequest( QgsFeatureRequest().setFilterExpression( "$x<5" ), it ) );
+
+  // can only use rect/expression requests if cache has everything
+  cache.setFullCache( true );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id1 ), it ) );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFid( id2 ), it ) );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterFids( QgsFeatureIds() << id1 << id2 ), it ) );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterRect( QgsRectangle( 1, 2, 3, 4 ) ), it ) );
+  QVERIFY( cache.canUseCacheForRequest( QgsFeatureRequest().setFilterExpression( "$x<5" ), it ) );
+}
+
 void TestVectorLayerCache::onCommittedFeaturesAdded( const QString& layerId, const QgsFeatureList& features )
 {
   Q_UNUSED( layerId )