Skip to content

Commit 0a5ad73

Browse files
nyalldawsonnyalldawson
committedJun 13, 2016
Invalid join cache when layer is modified (fix #11140)
·
ltr-3_40archive/master_2
1 parent 1ec7ad5 commit 0a5ad73

File tree

6 files changed

+128
-2
lines changed

6 files changed

+128
-2
lines changed
 

‎python/core/qgsvectorlayer.sip

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ struct QgsVectorJoinInfo
88
%TypeHeaderCode
99
#include "qgsvectorlayer.h"
1010
%End
11+
QgsVectorJoinInfo();
1112

1213
/** Join field in the target layer*/
1314
QString targetFieldName;
@@ -17,6 +18,8 @@ struct QgsVectorJoinInfo
1718
QString joinFieldName;
1819
/** True if the join is cached in virtual memory*/
1920
bool memoryCache;
21+
/** True if the cached join attributes need to be updated*/
22+
bool cacheDirty;
2023
/** Cache for joined attributes to provide fast lookup (size is 0 if no memory caching)
2124
* @note not available in python bindings
2225
*/

‎src/core/qgsvectorlayer.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ typedef QList<QgsPointV2> QgsPointSequenceV2;
7575

7676
struct CORE_EXPORT QgsVectorJoinInfo
7777
{
78+
QgsVectorJoinInfo()
79+
: memoryCache( false )
80+
, cacheDirty( true )
81+
, targetFieldIndex( -1 )
82+
, joinFieldIndex( -1 )
83+
{}
84+
7885
/** Join field in the target layer*/
7986
QString targetFieldName;
8087
/** Source layer*/
@@ -83,6 +90,9 @@ struct CORE_EXPORT QgsVectorJoinInfo
8390
QString joinFieldName;
8491
/** True if the join is cached in virtual memory*/
8592
bool memoryCache;
93+
/** True if the cached join attributes need to be updated*/
94+
bool cacheDirty;
95+
8696
/** Cache for joined attributes to provide fast lookup (size is 0 if no memory caching)
8797
* @note not available in python bindings
8898
*/

‎src/core/qgsvectorlayerfeatureiterator.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( QgsVectorLayer *layer
3131
{
3232
mProviderFeatureSource = layer->dataProvider()->featureSource();
3333
mFields = layer->fields();
34+
35+
layer->createJoinCaches();
3436
mJoinBuffer = layer->mJoinBuffer->clone();
37+
3538
mExpressionFieldBuffer = new QgsExpressionFieldBuffer( *layer->mExpressionFieldBuffer );
3639
mCrsId = layer->crs().srsid();
3740

‎src/core/qgsvectorlayerjoinbuffer.cpp

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ bool QgsVectorLayerJoinBuffer::addJoin( const QgsVectorJoinInfo& joinInfo )
8787
if ( QgsVectorLayer* vl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( joinInfo.joinLayerId ) ) )
8888
{
8989
connect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ), Qt::UniqueConnection );
90+
connect( vl, SIGNAL( layerModified() ), this, SLOT( joinedLayerModified() ), Qt::UniqueConnection );
9091
}
9192

9293
emit joinedFieldsChanged();
@@ -118,7 +119,7 @@ bool QgsVectorLayerJoinBuffer::removeJoin( const QString& joinLayerId )
118119
void QgsVectorLayerJoinBuffer::cacheJoinLayer( QgsVectorJoinInfo& joinInfo )
119120
{
120121
//memory cache not required or already done
121-
if ( !joinInfo.memoryCache || !joinInfo.cachedAttributes.isEmpty() )
122+
if ( !joinInfo.memoryCache || !joinInfo.cacheDirty )
122123
{
123124
return;
124125
}
@@ -175,6 +176,7 @@ void QgsVectorLayerJoinBuffer::cacheJoinLayer( QgsVectorJoinInfo& joinInfo )
175176
joinInfo.cachedAttributes.insert( key, attrs2 );
176177
}
177178
}
179+
joinInfo.cacheDirty = false;
178180
}
179181
}
180182

@@ -260,11 +262,15 @@ void QgsVectorLayerJoinBuffer::createJoinCaches()
260262
QList< QgsVectorJoinInfo >::iterator joinIt = mVectorJoins.begin();
261263
for ( ; joinIt != mVectorJoins.end(); ++joinIt )
262264
{
263-
cacheJoinLayer( *joinIt );
265+
if ( joinIt->memoryCache && joinIt->cacheDirty )
Code has comments. Press enter to view.
266+
cacheJoinLayer( *joinIt );
264267

265268
// make sure we are connected to the joined layer
266269
if ( QgsVectorLayer* vl = qobject_cast<QgsVectorLayer*>( QgsMapLayerRegistry::instance()->mapLayer( joinIt->joinLayerId ) ) )
270+
{
267271
connect( vl, SIGNAL( updatedFields() ), this, SLOT( joinedLayerUpdatedFields() ), Qt::UniqueConnection );
272+
connect( vl, SIGNAL( layerModified() ), this, SLOT( joinedLayerModified() ), Qt::UniqueConnection );
273+
}
268274
}
269275
}
270276

@@ -329,6 +335,7 @@ void QgsVectorLayerJoinBuffer::readXml( const QDomNode& layer_node )
329335
info.joinLayerId = infoElem.attribute( "joinLayerId" );
330336
info.targetFieldName = infoElem.attribute( "targetFieldName" );
331337
info.memoryCache = infoElem.attribute( "memoryCache" ).toInt();
338+
info.cacheDirty = true;
332339

333340
info.joinFieldIndex = infoElem.attribute( "joinField" ).toInt(); //for compatibility with 1.x
334341
info.targetFieldIndex = infoElem.attribute( "targetField" ).toInt(); //for compatibility with 1.x
@@ -397,6 +404,9 @@ QgsVectorLayerJoinBuffer* QgsVectorLayerJoinBuffer::clone() const
397404

398405
void QgsVectorLayerJoinBuffer::joinedLayerUpdatedFields()
399406
{
407+
// TODO - check - this whole method is probably not needed anymore,
408+
// since the cache handling is covered by joinedLayerModified()
409+
400410
QgsVectorLayer* joinedLayer = qobject_cast<QgsVectorLayer*>( sender() );
401411
Q_ASSERT( joinedLayer );
402412

@@ -412,3 +422,18 @@ void QgsVectorLayerJoinBuffer::joinedLayerUpdatedFields()
412422

413423
emit joinedFieldsChanged();
414424
}
425+
426+
void QgsVectorLayerJoinBuffer::joinedLayerModified()
427+
{
428+
QgsVectorLayer* joinedLayer = qobject_cast<QgsVectorLayer*>( sender() );
429+
Q_ASSERT( joinedLayer );
430+
431+
// recache the joined layer
432+
for ( QgsVectorJoinList::iterator it = mVectorJoins.begin(); it != mVectorJoins.end(); ++it )
433+
{
434+
if ( joinedLayer->id() == it->joinLayerId )
435+
{
436+
it->cacheDirty = true;
437+
}
438+
}
439+
}

‎src/core/qgsvectorlayerjoinbuffer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ class CORE_EXPORT QgsVectorLayerJoinBuffer : public QObject
9090
private slots:
9191
void joinedLayerUpdatedFields();
9292

93+
void joinedLayerModified();
94+
9395
private:
9496

9597
QgsVectorLayer* mLayer;

‎tests/src/core/testqgsvectorlayerjoinbuffer.cpp

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ class TestVectorLayerJoinBuffer : public QObject
5757
void testJoinTwoTimes_data();
5858
void testJoinTwoTimes();
5959
void testJoinLayerDefinitionFile();
60+
void testCacheUpdate_data();
61+
void testCacheUpdate();
6062

6163
private:
6264
QList<QString> mProviders;
@@ -519,6 +521,87 @@ void TestVectorLayerJoinBuffer::testJoinLayerDefinitionFile()
519521
QVERIFY( vLayer->fieldNameIndex( joinInfo.prefix + "value" ) >= 0 );
520522
}
521523

524+
void TestVectorLayerJoinBuffer::testCacheUpdate_data()
525+
{
526+
QTest::addColumn<bool>( "useCache" );
527+
QTest::newRow( "cache" ) << true;
528+
QTest::newRow( "no cache" ) << false;
529+
}
530+
531+
void TestVectorLayerJoinBuffer::testCacheUpdate()
532+
{
533+
QFETCH( bool, useCache );
534+
535+
QgsVectorLayer* vlA = new QgsVectorLayer( "Point?field=id_a:integer", "cacheA", "memory" );
536+
QVERIFY( vlA->isValid() );
537+
QgsVectorLayer* vlB = new QgsVectorLayer( "Point?field=id_b:integer&field=value_b", "cacheB", "memory" );
538+
QVERIFY( vlB->isValid() );
539+
QgsMapLayerRegistry::instance()->addMapLayer( vlA );
540+
QgsMapLayerRegistry::instance()->addMapLayer( vlB );
541+
542+
QgsFeature fA1( vlA->dataProvider()->fields(), 1 );
543+
fA1.setAttribute( "id_a", 1 );
544+
QgsFeature fA2( vlA->dataProvider()->fields(), 2 );
545+
fA2.setAttribute( "id_a", 2 );
546+
547+
vlA->dataProvider()->addFeatures( QgsFeatureList() << fA1 << fA2 );
548+
549+
QgsFeature fB1( vlB->dataProvider()->fields(), 1 );
550+
fB1.setAttribute( "id_b", 1 );
551+
fB1.setAttribute( "value_b", 11 );
552+
QgsFeature fB2( vlB->dataProvider()->fields(), 2 );
553+
fB2.setAttribute( "id_b", 2 );
554+
fB2.setAttribute( "value_b", 12 );
555+
556+
vlB->dataProvider()->addFeatures( QgsFeatureList() << fB1 << fB2 );
557+
558+
QgsVectorJoinInfo joinInfo;
559+
joinInfo.targetFieldName = "id_a";
560+
joinInfo.joinLayerId = vlB->id();
561+
joinInfo.joinFieldName = "id_b";
562+
joinInfo.memoryCache = useCache;
563+
joinInfo.prefix = "B_";
564+
vlA->addJoin( joinInfo );
565+
566+
QgsFeatureIterator fi = vlA->getFeatures();
567+
fi.nextFeature( fA1 );
568+
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
569+
QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 11 );
570+
fi.nextFeature( fA2 );
571+
QCOMPARE( fA2.attribute( "id_a" ).toInt(), 2 );
572+
QCOMPARE( fA2.attribute( "B_value_b" ).toInt(), 12 );
573+
574+
// change value in join target layer
575+
vlB->startEditing();
576+
vlB->changeAttributeValue( 1, 1, 111 );
577+
vlB->changeAttributeValue( 2, 0, 3 );
578+
vlB->commitChanges();
579+
580+
fi = vlA->getFeatures();
581+
fi.nextFeature( fA1 );
582+
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
583+
QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 111 );
584+
fi.nextFeature( fA2 );
585+
QCOMPARE( fA2.attribute( "id_a" ).toInt(), 2 );
586+
QVERIFY( fA2.attribute( "B_value_b" ).isNull() );
587+
588+
// change value in joined layer
589+
vlA->startEditing();
590+
vlA->changeAttributeValue( 2, 0, 3 );
591+
vlA->commitChanges();
592+
593+
fi = vlA->getFeatures();
594+
fi.nextFeature( fA1 );
595+
QCOMPARE( fA1.attribute( "id_a" ).toInt(), 1 );
596+
QCOMPARE( fA1.attribute( "B_value_b" ).toInt(), 111 );
597+
fi.nextFeature( fA2 );
598+
QCOMPARE( fA2.attribute( "id_a" ).toInt(), 3 );
599+
QCOMPARE( fA2.attribute( "B_value_b" ).toInt(), 12 );
600+
601+
QgsMapLayerRegistry::instance()->removeMapLayer( vlA );
602+
QgsMapLayerRegistry::instance()->removeMapLayer( vlB );
603+
}
604+
522605

523606
QTEST_MAIN( TestVectorLayerJoinBuffer )
524607
#include "testqgsvectorlayerjoinbuffer.moc"

2 commit comments

Comments
 (2)

3nids commented on Jun 13, 2016

@3nids
3nids
on Jun 13, 2016
Member

oh yes, finally!

nirvn commented on Jun 13, 2016

@nirvn
nirvn
on Jun 13, 2016
Contributor

I second that, huge improvement for anyone dealing with joins.

Please sign in to comment.