Skip to content

Commit 31cecdb

Browse files
m-kuhnm-kuhn
committedApr 19, 2013
Fix attributetable and vectorlayercache
* In the attributetable there was a mess with references and pointers, originating from 66fadee. * QgsVectorLayerCache did sometimes cache features which did not contain all information which needs to be cached and therefore corrupting the cache and leading to incomplete cache hits. * Add a unit test for the cache problem * Fix QgsCacheIndexFeatureId * QgsAbstractCacheIndex::getCacheIterator now produces a QgsFeatureIterator (instead of a list of Feature Ids). This allows to combine a mixed response, partly satisfied by the cache and partly by an additional query to the backend.
·
ltr-3_40archive/master_2
1 parent 92d24fb commit 31cecdb

10 files changed

+182
-36
lines changed
 

‎src/core/qgscacheindex.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "qgsfeature.h" // QgsFeatureIds
2020

2121
class QgsFeatureRequest;
22+
class QgsFeatureIterator;
2223

2324
/**
2425
* @brief
@@ -60,14 +61,15 @@ class QgsAbstractCacheIndex
6061
* and write the list of feature ids of cached features to cachedFeatures. If it is not able
6162
* it will return false and the cachedFeatures state is undefined.
6263
*
63-
* @param cachedFeatures A reference to {@link QgsFeatureIds}, where a list of ids is written to,
64-
* in case this index is able to answer the request.
64+
* @param featureIterator A reference to a {@link QgsFeatureIterator}. A valid featureIterator will
65+
* be assigned in case this index is able to answer the request and the return
66+
* value is true.
6567
* @param featureRequest The feature request, for which this index is queried.
6668
*
6769
* @return True, if this index holds the information to answer the request.
6870
*
6971
*/
70-
virtual bool getCachedIds( QgsFeatureIds& cachedFeatures, const QgsFeatureRequest& featureRequest ) = 0;
72+
virtual bool getCacheIterator( QgsFeatureIterator& featureIterator, const QgsFeatureRequest& featureRequest ) = 0;
7173
};
7274

7375
#endif // QGSCACHEINDEX_H

‎src/core/qgscacheindexfeatureid.cpp

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,45 @@
1414
***************************************************************************/
1515

1616
#include "qgscacheindexfeatureid.h"
17+
#include "qgsfeaturerequest.h"
18+
#include "qgscachedfeatureiterator.h"
19+
#include "qgsvectorlayercache.h"
1720

18-
QgsCacheIndexFeatureId::QgsCacheIndexFeatureId( QgsCachedVectorLayer* cachedVectorLayer )
21+
QgsCacheIndexFeatureId::QgsCacheIndexFeatureId( QgsVectorLayerCache* cachedVectorLayer )
1922
: QgsAbstractCacheIndex()
2023
, C( cachedVectorLayer )
2124
{
2225

2326
}
2427

28+
void QgsCacheIndexFeatureId::flushFeature(const QgsFeatureId fid)
29+
{
30+
Q_UNUSED( fid )
31+
}
32+
33+
void QgsCacheIndexFeatureId::flush()
34+
{
35+
}
36+
37+
void QgsCacheIndexFeatureId::requestCompleted( QgsFeatureRequest featureRequest, QgsFeatureIds fids )
38+
{
39+
Q_UNUSED( featureRequest )
40+
Q_UNUSED( fids )
41+
}
42+
43+
bool QgsCacheIndexFeatureId::getCacheIterator( QgsFeatureIterator &featureIterator, const QgsFeatureRequest &featureRequest )
44+
{
45+
if( featureRequest.filterType() == QgsFeatureRequest::FilterFid )
46+
{
47+
if ( C->isFidCached( featureRequest.filterFid() ) )
48+
{
49+
QgsFeatureIds fids;
50+
fids << featureRequest.filterFid();
51+
featureIterator = QgsFeatureIterator( new QgsCachedFeatureIterator( C, featureRequest, fids ) );
52+
return true;
53+
}
54+
}
55+
56+
return false;
57+
}
58+

‎src/core/qgscacheindexfeatureid.h

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,57 @@
1818

1919
#include "qgscacheindex.h"
2020

21-
class QgsCachedVectorLayer;
21+
class QgsVectorLayerCache;
2222

2323
class QgsCacheIndexFeatureId : public QgsAbstractCacheIndex
2424
{
2525
public:
26-
QgsCacheIndexFeatureId( QgsCachedVectorLayer* cachedVectorLayer );
26+
QgsCacheIndexFeatureId( QgsVectorLayerCache* );
27+
28+
/**
29+
* Is called, whenever a feature is removed from the cache. You should update your indexes, so
30+
* they become invalid in case this feature was required to successfuly answer a request.
31+
*/
32+
virtual void flushFeature( const QgsFeatureId fid );
33+
34+
/**
35+
* Sometimes, the whole cache changes its state and its easier to just withdraw everything.
36+
* In this case, this method is issued. Be sure to clear all cache information in here.
37+
*/
38+
virtual void flush();
39+
40+
/**
41+
* @brief
42+
* Implement this method to update the the indices, in case you need information contained by the request
43+
* to properly index. (E.g. spatial index)
44+
* Does nothing by default
45+
*
46+
* @param featureRequest The feature request that was answered
47+
* @param fids The feature ids that have been returned
48+
*/
49+
virtual void requestCompleted( QgsFeatureRequest featureRequest, QgsFeatureIds fids );
50+
51+
/**
52+
* Is called, when a feature request is issued on a cached layer.
53+
* If this cache index is able to completely answer the feature request, it will return true
54+
* and write the list of feature ids of cached features to cachedFeatures. If it is not able
55+
* it will return false and the cachedFeatures state is undefined.
56+
*
57+
* @param cachedFeatures A reference to {@link QgsFeatureIds}, where a list of ids is written to,
58+
* in case this index is able to answer the request.
59+
* @param featureRequest The feature request, for which this index is queried.
60+
*
61+
* @return True, if this index holds the information to answer the request.
62+
*
63+
*/
64+
virtual bool getCacheIterator( QgsFeatureIterator& featureIterator, const QgsFeatureRequest& featureRequest );
2765

2866
signals:
2967

3068
public slots:
3169

3270
private:
33-
QgsCachedVectorLayer* C;
71+
QgsVectorLayerCache* C;
3472
};
3573

3674
#endif // QGSCACHEINDEXFEATUREID_H

‎src/core/qgsfeaturerequest.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ QgsFeatureRequest::QgsFeatureRequest( const QgsRectangle& rect )
3838
{
3939
}
4040

41+
QgsFeatureRequest::QgsFeatureRequest( const QgsFeatureRequest &rh )
42+
{
43+
mFlags = rh.mFlags;
44+
mFilter = rh.mFilter;
45+
mFilterRect = rh.mFilterRect;
46+
mFilterFid = rh.mFilterFid;
47+
mAttrs = rh.mAttrs;
48+
}
49+
4150

4251
QgsFeatureRequest& QgsFeatureRequest::setFilterRect( const QgsRectangle& rect )
4352
{

‎src/core/qgsfeaturerequest.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ class CORE_EXPORT QgsFeatureRequest
7878
//! construct a request with rectangle filter
7979
explicit QgsFeatureRequest( const QgsRectangle& rect );
8080

81+
QgsFeatureRequest( const QgsFeatureRequest& rh );
82+
8183
FilterType filterType() const { return mFilter; }
8284

8385
//! Set rectangle from which features will be taken. Empty rectangle removes the filter.

‎src/core/qgsvectorlayercache.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ void QgsVectorLayerCache::setFullCache( bool fullCache )
105105
}
106106
}
107107

108+
void QgsVectorLayerCache::addCacheIndex( QgsAbstractCacheIndex* cacheIndex )
109+
{
110+
mCacheIndices.append( cacheIndex );
111+
}
112+
108113
void QgsVectorLayerCache::setCacheAddedAttributes( bool cacheAddedAttributes )
109114
{
110115
if ( cacheAddedAttributes )
@@ -251,11 +256,8 @@ QgsFeatureIterator QgsVectorLayerCache::getFeatures( const QgsFeatureRequest &fe
251256
// Check if an index is able to deliver the requested features
252257
foreach ( QgsAbstractCacheIndex *idx, mCacheIndices )
253258
{
254-
QgsFeatureIds featureIds;
255-
256-
if ( idx->getCachedIds( featureIds, featureRequest ) )
259+
if ( idx->getCacheIterator( it, featureRequest ) )
257260
{
258-
it = QgsFeatureIterator( new QgsCachedFeatureIterator( this, featureRequest, featureIds ) );
259261
requiresWriterIt = false;
260262
break;
261263
}
@@ -272,12 +274,27 @@ QgsFeatureIterator QgsVectorLayerCache::getFeatures( const QgsFeatureRequest &fe
272274
if ( requiresWriterIt && mLayer->dataProvider() )
273275
{
274276
// No index was able to satisfy the request
275-
it = QgsFeatureIterator( new QgsCachedFeatureWriterIterator( this, featureRequest ) );
277+
QgsFeatureRequest myRequest = QgsFeatureRequest( myRequest );
278+
279+
// Make sure if we cache the geometry, it gets fetched
280+
if ( mCacheGeometry )
281+
myRequest.setFlags( featureRequest.flags() & ~QgsFeatureRequest::NoGeometry );
282+
283+
// Make sure, all the cached attributes are requested as well
284+
QSet<int> attrs = featureRequest.subsetOfAttributes().toSet() + mCachedAttributes.toSet();
285+
myRequest.setSubsetOfAttributes( attrs.toList() );
286+
287+
it = QgsFeatureIterator( new QgsCachedFeatureWriterIterator( this, myRequest ) );
276288
}
277289

278290
return it;
279291
}
280292

293+
bool QgsVectorLayerCache::isFidCached( const QgsFeatureId fid )
294+
{
295+
return mCache.contains( fid );
296+
}
297+
281298
bool QgsVectorLayerCache::checkInformationCovered( const QgsFeatureRequest& featureRequest )
282299
{
283300
QgsAttributeList requestedAttributes;

‎src/core/qgsvectorlayercache.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,26 @@ class CORE_EXPORT QgsVectorLayerCache : public QObject
139139
*
140140
* @param cacheIndex The cache index to add.
141141
*/
142-
void addCacheIndex( const QgsAbstractCacheIndex& cacheIndex );
142+
void addCacheIndex( QgsAbstractCacheIndex *cacheIndex );
143143

144+
/**
145+
* Query this VectorLayerCache for features.
146+
* If the VectorLayerCache (and moreover any of its indices) is able to satisfy
147+
* the request, the returned {@link QgsFeatureIterator} will iterate over cached features.
148+
* If it's not possible to fully satisfy the request from the cache, part or all of the features
149+
* will be requested from the data provider.
150+
* @param featureRequest The request specifying filter and required data.
151+
* @return An iterator over the requested data.
152+
*/
144153
QgsFeatureIterator getFeatures( const QgsFeatureRequest& featureRequest );
145154

155+
/**
156+
* Check if a certain feature id is cached.
157+
* @param fid The feature id to look for
158+
* @return True if this id is in the cache
159+
*/
160+
bool isFidCached(const QgsFeatureId fid );
161+
146162
/**
147163
* Gets the feature at the given feature id. Considers the changed, added, deleted and permanent features
148164
* @param featureId The id of the feature to query

‎src/gui/attributetable/qgsattributetablemodel.cpp

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -486,31 +486,27 @@ QVariant QgsAttributeTableModel::data( const QModelIndex &index, int role ) cons
486486
return QVariant( Qt::AlignLeft );
487487
}
488488

489-
const QVariant* pVal = NULL;
489+
QVariant val;
490490

491491
// if we don't have the row in current cache, load it from layer first
492-
if ( mFeat.id() != rowId || !mFeat.isValid() )
492+
if ( mCachedField == fieldId )
493493
{
494-
if ( mCachedField == fieldId )
495-
{
496-
const QVariant& val = mFieldCache[rowId];
497-
pVal = &val;
498-
}
499-
else if ( !loadFeatureAtId( rowId ) )
500-
return QVariant( "ERROR" );
494+
val = mFieldCache[ rowId ];
501495
}
496+
else
497+
{
498+
if ( mFeat.id() != rowId || !mFeat.isValid() )
499+
{
500+
if ( !loadFeatureAtId( rowId ) )
501+
return QVariant( "ERROR" );
502502

503-
if ( pVal == NULL && mFeat.id() != rowId )
504-
return QVariant( "ERROR" );
503+
if ( mFeat.id() != rowId )
504+
return QVariant( "ERROR" );
505+
}
505506

506-
if ( !pVal )
507-
{
508-
const QVariant& val = mFeat.attribute( fieldId );
509-
pVal =&val;
507+
val = mFeat.attribute( fieldId );
510508
}
511509

512-
const QVariant& val = *pVal;
513-
514510
// For sorting return unprocessed value
515511
if ( SortRole == role )
516512
{

‎src/gui/attributetable/qgsattributetablemodel.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ class GUI_EXPORT QgsAttributeTableModel: public QAbstractTableModel
261261
/** The currently cached column */
262262
int mCachedField;
263263
/** Allows to cache one specific column (used for sorting) */
264-
QMap<QgsFeatureId, QVariant> mFieldCache;
264+
QHash<QgsFeatureId, QVariant> mFieldCache;
265265
};
266266

267267

‎tests/src/core/testqgsvectorlayercache.cpp

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#include <qgsvectordataprovider.h>
2424
#include <qgsapplication.h>
2525
#include <qgsvectorlayereditbuffer.h>
26+
#include <qgscacheindexfeatureid.h>
27+
#include <QDebug>
2628

2729
/** @ingroup UnitTests
2830
* This is a unit test for the vector layer cache
@@ -36,17 +38,19 @@ class TestVectorLayerCache: public QObject
3638
private slots:
3739
void initTestCase(); // will be called before the first testfunction is executed.
3840
void cleanupTestCase(); // will be called after the last testfunction was executed.
39-
void init() {}; // will be called before each testfunction is executed.
40-
void cleanup() {}; // will be called after every testfunction.
41+
void init(); // will be called before each testfunction is executed.
42+
void cleanup(); // will be called after every testfunction.
4143

4244
void testCacheOverflow(); // Test cache will work if too many features to cache them all are present
4345
void testCacheAttrActions(); // Test attribute add/ attribute delete
4446
void testFeatureActions(); // Test adding/removing features works
47+
void testSubsetRequest();
4548

4649
void onCommittedFeaturesAdded( QString, QgsFeatureList );
4750

4851
private:
4952
QgsVectorLayerCache* mVectorLayerCache;
53+
QgsCacheIndexFeatureId* mFeatureIdIndex;
5054
QgsVectorLayer* mPointsLayer;
5155
QgsFeatureList mAddedFeatures;
5256
QMap<QString, QString> mTmpFiles;
@@ -88,8 +92,19 @@ void TestVectorLayerCache::initTestCase()
8892
QFileInfo myPointFileInfo( myPointsFileName );
8993
mPointsLayer = new QgsVectorLayer( myPointFileInfo.filePath(),
9094
myPointFileInfo.completeBaseName(), "ogr" );
95+
}
9196

97+
void TestVectorLayerCache::init()
98+
{
9299
mVectorLayerCache = new QgsVectorLayerCache( mPointsLayer, 10 );
100+
mFeatureIdIndex = new QgsCacheIndexFeatureId( mVectorLayerCache );
101+
mVectorLayerCache->addCacheIndex( mFeatureIdIndex );
102+
}
103+
104+
void TestVectorLayerCache::cleanup()
105+
{
106+
delete mVectorLayerCache;
107+
delete mFeatureIdIndex;
93108
}
94109

95110
//runs after all tests
@@ -107,8 +122,6 @@ void TestVectorLayerCache::cleanupTestCase()
107122
mAddedFeatures.clear();
108123
}
109124

110-
delete mVectorLayerCache;
111-
mVectorLayerCache = NULL;
112125

113126
delete mPointsLayer;
114127
mPointsLayer = NULL;
@@ -190,9 +203,28 @@ void TestVectorLayerCache::testFeatureActions()
190203
// Delete feature...
191204
mPointsLayer->startEditing();
192205
QVERIFY( mPointsLayer->deleteFeature( fid ) );
193-
mPointsLayer->commitChanges();
194206

195207
QVERIFY( false == mVectorLayerCache->featureAtId( fid, f ) );
208+
mPointsLayer->rollBack();
209+
}
210+
211+
void TestVectorLayerCache::testSubsetRequest()
212+
{
213+
QgsFeature f;
214+
215+
QgsFields fields = mPointsLayer->pendingFields();
216+
QStringList requiredFields;
217+
requiredFields << "Class" << "Cabin Crew";
218+
219+
mVectorLayerCache->featureAtId( 16, f );
220+
QVariant a = f.attribute( 3 );
221+
222+
QgsFeatureIterator itSubset = mVectorLayerCache->getFeatures( QgsFeatureRequest().setSubsetOfAttributes( requiredFields, fields) );
223+
while ( itSubset.nextFeature( f ) ) {}
224+
itSubset.close();
225+
226+
mVectorLayerCache->featureAtId( 16, f );
227+
QVERIFY( a == f.attribute( 3 ) );
196228
}
197229

198230
void TestVectorLayerCache::onCommittedFeaturesAdded( QString layerId, QgsFeatureList features )

0 commit comments

Comments
 (0)
Please sign in to comment.